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.

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

crd: Refactor RegisterCRDsCell to be extensible #25590

Conversation

pippolo84
Copy link
Member

Current implementation of RegisterCRDsCell is strictly tied to the registration of the CRDs belonging to the "cilium.io" group.

This PR refactors the cell to make it easily extendable if other groups needs to register their CRDs at operator startup.

@pippolo84 pippolo84 added area/operator Impacts the cilium-operator component release-note/misc This PR makes changes that have no direct user impact. labels May 22, 2023
@pippolo84 pippolo84 force-pushed the pr/pippolo84/extensible-crd-registration-cell branch from 65e00e4 to f81da27 Compare May 22, 2023 16:02
@pippolo84 pippolo84 marked this pull request as ready for review May 22, 2023 16:10
@pippolo84 pippolo84 requested review from a team as code owners May 22, 2023 16:10
@pippolo84 pippolo84 requested review from brlbil and joamaki May 22, 2023 16:10
Copy link
Contributor

@joamaki joamaki left a comment

Choose a reason for hiding this comment

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

LGTM. At some point we could also consider making the CRDs we want to register a group value.

@pippolo84
Copy link
Member Author

LGTM. At some point we could also consider making the CRDs we want to register a group value.

Actually, I wasn't completely satisfied by the package level variable and tried the group approach.
I think the group is better, so I'm gonna push-force the new commit.

@pippolo84 pippolo84 force-pushed the pr/pippolo84/extensible-crd-registration-cell branch from f81da27 to c235bba Compare May 23, 2023 09:22
@pippolo84 pippolo84 requested a review from joamaki May 23, 2023 09:22
@pippolo84 pippolo84 force-pushed the pr/pippolo84/extensible-crd-registration-cell branch from c235bba to 1f90e40 Compare May 23, 2023 09:57
Copy link
Contributor

@brlbil brlbil left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/k8s/apis/cell.go Outdated Show resolved Hide resolved
Current implementation of RegisterCRDsCell is strictly tied to the
registration of the CRDs belonging to the "cilium.io" group.

This commit refactors the cell to make it easily extendable if other
groups needs to register their CRDs at operator startup.

Signed-off-by: Fabio Falzoi <fabio.falzoi@isovalent.com>
@pippolo84 pippolo84 force-pushed the pr/pippolo84/extensible-crd-registration-cell branch from 1f90e40 to 1b61b53 Compare May 24, 2023 14:40
@pippolo84
Copy link
Member Author

/test

@pippolo84
Copy link
Member Author

External workloads flake tracked in #25736, going to rerun.

@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 29, 2023
@julianwiedmann julianwiedmann merged commit 1d580c3 into cilium:main May 29, 2023
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/operator Impacts the cilium-operator component 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants