Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Decouple CiliumEndpointSlice controller from operator Clientset #32353

Merged
merged 2 commits into from
May 10, 2024
Merged

Decouple CiliumEndpointSlice controller from operator Clientset #32353

merged 2 commits into from
May 10, 2024

Conversation

thorn3r
Copy link
Contributor

@thorn3r thorn3r commented May 4, 2024 •
edited

Decouple the CiliumEndpointSlice controller from the main operator Clientset to prevent it from saturating the rate limit and affecting other controllers.

The 1st commit introduces a new Hive Cell to provide a ClientBuilderFunc, which can be executed by a controller to retrieve a new Clientset. The 2nd commit utilizes the ClientBuilderFunc in the CiliumEndpointSlice controller to create a new Clientset

Grant the CiliumEndpointSlice controller a new Clientset to decouple it from the other Cilium Operator controllers.

@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 May 4, 2024
@thorn3r thorn3r force-pushed the decoupleOperatorClients branch 2 times, most recently from 25f65e7 to 51db531 Compare May 6, 2024 14:31
@thorn3r
Copy link
Contributor Author

thorn3r commented May 6, 2024

/test

@thorn3r thorn3r marked this pull request as ready for review May 6, 2024 15:07
@thorn3r thorn3r requested review from a team as code owners May 6, 2024 15:07
@thorn3r thorn3r added the release-note/misc This PR makes changes that have no direct user impact. label May 6, 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 May 6, 2024
@thorn3r thorn3r requested a review from marseel May 6, 2024 15:17
@thorn3r thorn3r marked this pull request as draft May 6, 2024 16:49
@thorn3r
Copy link
Contributor Author

thorn3r commented May 6, 2024

found a real CI failure, putting back into draft until I apply a fix

@thorn3r
Copy link
Contributor Author

thorn3r commented May 6, 2024

/test

@thorn3r
Copy link
Contributor Author

thorn3r commented May 7, 2024

/test

@thorn3r thorn3r mentioned this pull request May 7, 2024
Closed
@thorn3r thorn3r marked this pull request as ready for review May 7, 2024 15:10
@thorn3r thorn3r requested a review from a team as a code owner May 7, 2024 15:10
@thorn3r thorn3r requested a review from christarazi May 7, 2024 15:11
@christarazi christarazi added kind/enhancement This would improve or streamline existing functionality. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. area/operator Impacts the cilium-operator component labels May 7, 2024
pippolo84
pippolo84 approved these changes May 8, 2024
View reviewed changes
Copy link
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

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

Nice! 🚀
I've just left a non-blocking suggestion inline, feel free to ignore it if you prefer the current approach for the error handling.

pkg/k8s/client/cell.go Outdated Show resolved Hide resolved
marseel
marseel reviewed May 9, 2024
View reviewed changes
pkg/k8s/client/cell.go Outdated Show resolved Hide resolved
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 9, 2024
marseel
marseel approved these changes May 9, 2024
View reviewed changes
Copy link
Contributor

@marseel marseel left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@ldelossa ldelossa enabled auto-merge May 9, 2024 14:35
@ldelossa ldelossa disabled auto-merge May 9, 2024 14:53
@ldelossa ldelossa enabled auto-merge May 9, 2024 14:55
@thorn3r
operator: add Hive cell for creating new Clientsets
d41012f 
Add ClientBuilderCell to Cilium Operator's client package, which
provides a ClientBuilderFunc that can be used to create a new Clientset
for a controller. This allows decoupling the various controllers from
the main Clientset used by the operator.

Additionally, add support for optionally specifying a user agent when
creating a new Clientset.

Signed-off-by: Tim Horner <timothy.horner@isovalent.com>
@thorn3r thorn3r requested a review from marseel May 9, 2024 17:33
@thorn3r
operator: decouple CES controller from main Clientset
88933c2 
Create a new Clientset for the CiliumEndpointSlice controller using the
ClientBuilderFunc to decouple it from the other controllers.

Signed-off-by: Tim Horner <timothy.horner@isovalent.com>
@thorn3r
Copy link
Contributor Author

thorn3r commented May 9, 2024

/test

@thorn3r thorn3r mentioned this pull request May 9, 2024
Closed
@ldelossa ldelossa added this pull request to the merge queue May 10, 2024
Merged via the queue into cilium:main with commit b7e2ec9 May 10, 2024
64 checks passed
This was referenced May 16, 2024
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pippolo84 pippolo84 pippolo84 approved these changes

@marseel marseel marseel approved these changes

@christarazi christarazi christarazi approved these changes

@youngnick youngnick youngnick approved these changes

Assignees
No one assigned
Labels
area/operator Impacts the cilium-operator component kind/enhancement This would improve or streamline existing functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@thorn3r @pippolo84 @marseel @christarazi @youngnick @ldelossa