-
Notifications
You must be signed in to change notification settings - Fork 464
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
[controller-manager] Switch Shoot
controller to controller-runtime
#6620
Conversation
Shoot
controller to controller-runtime
and introduce common indexersShoot
controller to controller-runtime
23883aa
to
a860422
Compare
a860422
to
4259ef1
Compare
This failed because of
which is unrelated to the PR. There are no logs for the
Probably the extension was shortly unavailable due to scaling/restart. /test pull-gardener-e2e-kind |
/assign |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a joint review with @acumino (made it until commit Update documentation for quota reconciler
).
test/integration/controllermanager/shoot/conditions/conditions_suite_test.go
Outdated
Show resolved
Hide resolved
test/integration/controllermanager/shoot/conditions/conditions_suite_test.go
Outdated
Show resolved
Hide resolved
test/integration/controllermanager/shoot/conditions/conditions_test.go
Outdated
Show resolved
Hide resolved
test/integration/controllermanager/shoot/hibernation/hibernation_suite_test.go
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Humongous but nice PR 😄
test/integration/controllermanager/shoot/quota/quota_suite_test.go
Outdated
Show resolved
Hide resolved
test/integration/controllermanager/shoot/conditions/conditions_suite_test.go
Outdated
Show resolved
Hide resolved
test/integration/controllermanager/shoot/quota/quota_suite_test.go
Outdated
Show resolved
Hide resolved
test/integration/controllermanager/shoot/quota/quota_suite_test.go
Outdated
Show resolved
Hide resolved
test/integration/controllermanager/shoot/reference/reference_suite_test.go
Outdated
Show resolved
Hide resolved
test/integration/controllermanager/shoot/statuslabel/statuslabel_suite_test.go
Outdated
Show resolved
Hide resolved
test/integration/controllermanager/shoot/statuslabel/statuslabel_test.go
Outdated
Show resolved
Hide resolved
4259ef1
to
2392176
Compare
Awesome, thanks @timebertt, highly appreciated! I have rebased the PR and addressed all your comments (except for one which is still unresolved) - please have another look :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, one remaining comment :)
2392176
to
f346f5e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing all my suggestions!
/lgtm
f346f5e
to
1d13cde
Compare
New changes are detected. LGTM label has been removed. |
rebased now, adding the labels to make Prow merge this :) /lgtm |
@rfranzke: you cannot LGTM your own PR. 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. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rfranzke 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 |
Shoot
controller to controller-runtime
Shoot
controller to controller-runtime
How to categorize this PR?
/area dev-productivity scalability
/kind enhancement
What this PR does / why we need it:
Refactor the
Shoot
controller tocontroller-runtime
. It consists of several reconcilers, so there are some empty separator commits to allow easy navigation through the PR.Which issue(s) this PR fixes:
Part of #4251
Special notes for your reviewer:
Generally, we want to follow this cookbook while refactoring existing controllers:
envtest
controller-runtime
Release note: