-
Notifications
You must be signed in to change notification settings - Fork 602
refactor: Group setup by feature flag #3647
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
refactor: Group setup by feature flag #3647
Conversation
@dlipovetsky: This issue is currently awaiting triage. If CAPA/CAPI contributors determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Test failed:
I've repeated (not using cached results) this test 10 times locally but still haven't seen a failure. |
/retest |
retest passed, weird that it failed as this issue was fixed long back. lets see if we face this again, although little difficult to debug if not failing locally. |
/test ? |
@richardcase: The following commands are available to trigger required jobs:
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/test pull-cluster-api-provider-aws-e2e |
I like this change @dlipovetsky , its a lot cleaner 👍 From myside: /lgtm (assuming the e2e pass) |
/test pull-cluster-api-provider-aws-e2e |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sedefsavas The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
While reviewing #3633, I noticed that we had set up reconcilers and webhooks based on feature flags in
main
itself, although we put some code in a separateenableGates
function. I also noticed that we had two identical conditional blocks to setup EKS-only reconcilers and webhooks, respectively.To make the setup code easier to follow, I grouped it by feature flag, because that seems like the most useful grouping. (This means we interleave the setup of reconcilers and webhooks; I think this is safe, but hope to get feedback).
I also moved the EKS-only setup into a separate function.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
Checklist: