Skip to content

Conversation

joamaki
Copy link
Contributor

@joamaki joamaki commented Oct 28, 2024

This implements the CiliumEnvoyConfig handling against the experimental load-balancing control-plane (pkg/loadbalancer/experimental).

The exp_cec.go defines the Table[CEC] containing the envoy configs. The CEC object defines the full desired envoy Resources, including backends. Changes to the object are reconciled against the XDS server.

The main moving parts around Table[CEC] are:

  • Reflector: reflects the CiliumEnvoyConfig and CiliumClusterwideEnvoyConfig into Table[CEC]. The backends are filled in on the fly.
  • Backend controller: Updates backends in the CEC object when they change
  • Node label controller: Recomputes which CECs are selected when node labels change
  • Proxy redirect controller: Watches Table[CEC] and sets the service L7 redirects accordingly.
  • Reconciler: calls UpdateEnvoyResources/DeleteEnvoyResources to reconcile changes in the resources.

The implementation is tested with hive/script and builds on top of the test commands provided by the load-balancer control-plane:

  • testdata/namespaced.txtar: Tests CiliumEnvoyConfig and validates calls towards Envoy and the LB BPF map contents
  • testdata/clusterwide.txtar: Similar to namespaced.txtar but testing CiliumClusterwideEnvoyConfig
  • testdata/anyport.txtar: Tests CiliumEnvoyConfig without port filtering
  • testdata/labels.txtar: Tests selection of CiliumEnvoyConfigs based on node labels

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 28, 2024
@joamaki joamaki force-pushed the pr/joamaki/experimental-ciliumenvoyconfig branch from 01f253b to 0727129 Compare October 29, 2024 09:35
@maintainer-s-little-helper

This comment was marked as resolved.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Oct 29, 2024
@joamaki joamaki force-pushed the pr/joamaki/experimental-ciliumenvoyconfig branch from 0727129 to c50b945 Compare October 29, 2024 17:36
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Oct 29, 2024
@joamaki joamaki force-pushed the pr/joamaki/experimental-ciliumenvoyconfig branch from c50b945 to 05320ac Compare October 30, 2024 13:30
@joamaki joamaki added the release-note/misc This PR makes changes that have no direct user impact. label Oct 30, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 30, 2024
@joamaki joamaki marked this pull request as ready for review October 30, 2024 13:38
@joamaki joamaki requested review from a team as code owners October 30, 2024 13:38
@joamaki joamaki force-pushed the pr/joamaki/experimental-ciliumenvoyconfig branch from 05320ac to ad1751b Compare October 30, 2024 13:42
@squeed
Copy link
Contributor

squeed commented Oct 30, 2024

What does "experimental" mean :-)? Is this expected to merge? Be used? Or should this be a draft PR?

@joamaki
Copy link
Contributor Author

joamaki commented Oct 31, 2024

What does "experimental" mean :-)? Is this expected to merge? Be used? Or should this be a draft PR?

It is meant to be merged. Experimental as in it's meant to eventually replace the existing implementation and we're slowly building it up towards it. And yes hopefully people try this out in v1.17 (loadBalancer.experimental: true). As this is such a large undertaking it's unfeasible to do this work fully in separate branch and thus we're building this incrementally as a new implementation side-by-side with the existing one. See #32185 for more context.

Copy link
Member

@jrajahalme jrajahalme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked the non-experimental changes only at this point, LGTM :-)

Copy link
Contributor

@ovidiutirla ovidiutirla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for my code owners

We could potentially drop the hive bump after #35707 is merged.

Copy link
Contributor

@learnitall learnitall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@ovidiutirla
Copy link
Contributor

We should now be able to drop the hive bump commit 🚀

To allow filling in data from other tables during reflection, pass
the current transaction to the transform functions.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
Add ServicePort to Frontend to allow proxy redirection to match against
NodePort frontends based on the service port (ClusterIP port).

Add ports to the ProxyRedirect in Service to allow applying the redirect
to only frontends matching the port.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
Implement CiliumEnvoyConfig handling and backend syncing for the
experimental LB control-plane.

Signed-off-by: Jussi Maki <jussi@isovalent.com>
@joamaki joamaki force-pushed the pr/joamaki/experimental-ciliumenvoyconfig branch from cde6d22 to e27b9b8 Compare November 21, 2024 13:01
@joamaki joamaki requested a review from squeed November 21, 2024 13:02
@joamaki
Copy link
Contributor Author

joamaki commented Nov 21, 2024

/test

@joamaki joamaki added this pull request to the merge queue Nov 21, 2024
Merged via the queue into cilium:main with commit 21ad137 Nov 21, 2024
66 checks passed
@joamaki joamaki deleted the pr/joamaki/experimental-ciliumenvoyconfig branch November 21, 2024 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants