[gardenlet] Switch `Shoot` main controller to `controller-runtime` #7082

timebertt · 2022-11-25T13:47:38Z

How to categorize this PR?

/area dev-productivity scalability
/kind enhancement

What this PR does / why we need it:
Refactor the Shoot main controller to controller-runtime.
Co-Authored-By @rfranzke

Which issue(s) this PR fixes:
Part of #4251

Special notes for your reviewer:

This PR comes with a few behavioral changes:

the dedicated queue in the shoot controller for shoots that are registered as managed seeds is dropped
status.UID is always written when the operation starts, previously this was only done on the start of a reconciliation (i.e., not on deletion or migration)
the logic for skipping reconciliations during migration in the BackupEntry reconciler is changed and harmonized with the shoot controller (cc @plkokanov)

✅ In draft, until I have performed some manual tests for the special cases of shoot reconciliation (see docs)

@rfranzke and I are working on a follow-up PR for refactoring the corresponding logic of these special cases to make it more readable and testable.

Generally, we want to follow this cookbook while refactoring existing controllers:

Add documentation
Add integration test based on envtest (if not already present)
Switch controller to controller-runtime

Integration tests are skipped for this controller, as they can't be achieved with low effort while still providing meaningful benefits.

Release note:

NONE

- Precisely determine responsibility for next reconciliation - Move central logic from `reconciler_migrate.go` to `reconciler.go` - Move common preparation work (Cluster resource sync, operation initialization, early exits, ...) to the `prepareOperation` function Greetings to all reviewers: Do not even try to review this. Otherwise, lay down to the floor, and ..., good luck.

gardener-prow · 2022-11-25T13:47:47Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

timebertt · 2022-11-25T14:14:02Z

/test all

rfranzke · 2022-11-28T08:31:18Z

/assign

Previously, this was done to prevent duplicated reconciliations. With predicate.GenerationChangedPredicate, this is no longer necessary as we only enqueue when metadata.generation changes.

timebertt · 2022-11-28T08:59:10Z

/test all

timebertt · 2022-11-28T09:50:03Z

https://prow.gardener.cloud/view/gs/gardener-prow/pr-logs/pull/gardener_gardener/7082/pull-gardener-e2e-kind-ha-single-zone/1597153155513061376 looks like another occurrence of #7064

timebertt · 2022-11-28T09:53:31Z

While testing locally, I found the following problem:

gardenlet is unable to record events on shoots:

{"level":"error","ts":"2022-11-28T08:49:54.605Z","msg":"Server rejected event '&v1.Event{TypeMeta:v1.TypeMeta{Kind:\"\", APIVersion:\"\"}, ObjectMeta:v1.ObjectMeta{Name:\"e2e-hibernated.172bb3734939828b\", GenerateName:\"\", Namespace:\"garden-local\", SelfLink:\"\", UID:\"\", ResourceVersion:\"\", Generation:0, CreationTimestamp:time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), DeletionTimestamp:<nil>, DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string(nil), Annotations:map[string]string(nil), OwnerReferences:[]v1.OwnerReference(nil), Finalizers:[]string(nil), ManagedFields:[]v1.ManagedFieldsEntry(nil)}, InvolvedObject:v1.ObjectReference{Kind:\"Shoot\", Namespace:\"garden-local\", Name:\"e2e-hibernated\", UID:\"65ef8916-cd35-409c-8d08-cb0a2d57861f\", APIVersion:\"core.gardener.cloud/v1beta1\", ResourceVersion:\"324\", FieldPath:\"\"}, Reason:\"Reconciled\", Message:\"Reconciled Shoot cluster\", Source:v1.EventSource{Component:\"shoot-controller\", Host:\"\"}, FirstTimestamp:time.Date(2022, time.November, 28, 8, 49, 54, 602017419, time.Local), LastTimestamp:time.Date(2022, time.November, 28, 8, 49, 54, 602017419, time.Local), Count:1, Type:\"Normal\", EventTime:time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), Series:(*v1.EventSeries)(nil), Action:\"\", Related:(*v1.ObjectReference)(nil), ReportingController:\"\", ReportingInstance:\"\"}': 'events is forbidden: User \"system:serviceaccount:garden:gardenlet\" cannot create resource \"events\" in API group \"\" in the namespace \"garden-local\"' (will not retry!)\n","stacktrace":"k8s.io/client-go/tools/record.recordEvent\n\tk8s.io/client-go@v11.0.1-0.20190409021438-1a26190bd76a+incompatible/tools/record/event.go:267\nk8s.io/client-go/tools/record.recordToSink\n\tk8s.io/client-go@v11.0.1-0.20190409021438-1a26190bd76a+incompatible/tools/record/event.go:216\nk8s.io/client-go/tools/record.(*eventBroadcasterImpl).StartRecordingToSink.func1\n\tk8s.io/client-go@v11.0.1-0.20190409021438-1a26190bd76a+incompatible/tools/record/event.go:194\nk8s.io/client-go/tools/record.(*eventBroadcasterImpl).StartEventWatcher.func1\n\tk8s.io/client-go@v11.0.1-0.20190409021438-1a26190bd76a+incompatible/tools/record/event.go:314"}
...
{"level":"error","ts":"2022-11-28T08:53:52.724Z","msg":"Server rejected event '&v1.Event{TypeMeta:v1.TypeMeta{Kind:\"\", APIVersion:\"\"}, ObjectMeta:v1.ObjectMeta{Name:\"e2e-hibernated.172bb3aab99578a4\", GenerateName:\"\", Namespace:\"garden-local\", SelfLink:\"\", UID:\"\", ResourceVersion:\"\", Generation:0, CreationTimestamp:time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), DeletionTimestamp:<nil>, DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string(nil), Annotations:map[string]string(nil), OwnerReferences:[]v1.OwnerReference(nil), Finalizers:[]string(nil), ManagedFields:[]v1.ManagedFieldsEntry(nil)}, InvolvedObject:v1.ObjectReference{Kind:\"Shoot\", Namespace:\"garden-local\", Name:\"e2e-hibernated\", UID:\"65ef8916-cd35-409c-8d08-cb0a2d57861f\", APIVersion:\"core.gardener.cloud/v1beta1\", ResourceVersion:\"439\", FieldPath:\"\"}, Reason:\"Deleted\", Message:\"Deleted Shoot cluster\", Source:v1.EventSource{Component:\"shoot-controller\", Host:\"\"}, FirstTimestamp:time.Date(2022, time.November, 28, 8, 53, 52, 710293668, time.Local), LastTimestamp:time.Date(2022, time.November, 28, 8, 53, 52, 710293668, time.Local), Count:1, Type:\"Normal\", EventTime:time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), Series:(*v1.EventSeries)(nil), Action:\"\", Related:(*v1.ObjectReference)(nil), ReportingController:\"\", ReportingInstance:\"\"}': 'events is forbidden: User \"system:serviceaccount:garden:gardenlet\" cannot create resource \"events\" in API group \"\" in the namespace \"garden-local\"' (will not retry!)\n","stacktrace":"k8s.io/client-go/tools/record.recordEvent\n\tk8s.io/client-go@v11.0.1-0.20190409021438-1a26190bd76a+incompatible/tools/record/event.go:267\nk8s.io/client-go/tools/record.recordToSink\n\tk8s.io/client-go@v11.0.1-0.20190409021438-1a26190bd76a+incompatible/tools/record/event.go:216\nk8s.io/client-go/tools/record.(*eventBroadcasterImpl).StartRecordingToSink.func1\n\tk8s.io/client-go@v11.0.1-0.20190409021438-1a26190bd76a+incompatible/tools/record/event.go:194\nk8s.io/client-go/tools/record.(*eventBroadcasterImpl).StartEventWatcher.func1\n\tk8s.io/client-go@v11.0.1-0.20190409021438-1a26190bd76a+incompatible/tools/record/event.go:314"}

Looking into it now.

timebertt · 2022-11-28T11:17:24Z

This PR is ready. I have performed several manual tests for the special cases mentioned in the docs and fixed a few things accordingly.

timebertt · 2022-11-28T12:11:37Z

https://prow.gardener.cloud/view/gs/gardener-prow/pr-logs/pull/gardener_gardener/7082/pull-gardener-e2e-kind/1597187779509358592 is another occurrence of #7064

shafeeqes · 2022-11-29T08:58:42Z

Should pass now, #7064 is fixed.
/test pull-gardener-e2e-kind

shafeeqes

Thanks a lot for the PR.
Had a combined look with @oliver-goetz and @ary1992.
Looks good in general. Just left some questions.
/assign @oliver-goetz @ary1992 @shafeeqes

pkg/gardenlet/controller/shoot/shoot/add_test.go

pkg/utils/gardener/migration_test.go

pkg/gardenlet/controller/shoot/shoot/reconciler.go

shafeeqes · 2022-12-02T04:54:38Z

/lgtm

rfranzke

Minor comment, I can fix it myself and then let's get this in. Thanks for making this ready @timebertt!

docs/concepts/gardenlet.md

rfranzke · 2022-12-02T10:10:47Z

/test pull-gardener-integration
/lgtm
/approve

gardener-prow · 2022-12-02T10:10:55Z

[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:

~~OWNERS~~ [rfranzke]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

gardener-prow · 2022-12-02T10:14:49Z

@timebertt: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-gardener-apidiff	`9fb6b6e`	link	false	`/test pull-gardener-apidiff`

Full PR test history. Your PR dashboard. Command help for this repository.
Please help us cut down on flakes by linking this test failure to an open flake report or filing a new flake report if you can't find an existing one. Also see our testing guideline for how to avoid and hunt flakes.

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. I understand the commands that are listed here.

rfranzke · 2022-12-02T10:17:02Z

/test pull-gardener-integration

timebertt and others added 6 commits November 25, 2022 14:43

Add documentation for shoot controller

3f4f516

Move Get*SeedNames functions to utils/gardener package

48ed78f

Switch main Shoot controller to native controller-runtime

185aae1

Reorder method order based on sophisticated criteria

209ab70

Move reconciliation due tracker to dedicated file

efcc0eb

gardener-prow bot added the cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. label Nov 25, 2022

gardener-prow bot requested review from ary1992 and timuthy November 25, 2022 13:47

gardener-prow bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 25, 2022

timebertt mentioned this pull request Nov 25, 2022

Refactor Gardener components to native controller-runtime components #4251

Closed

76 tasks

gardener-prow bot assigned rfranzke Nov 28, 2022

Stop returning after adding finalizer

6cb3281

Previously, this was done to prevent duplicated reconciliations. With predicate.GenerationChangedPredicate, this is no longer necessary as we only enqueue when metadata.generation changes.

timebertt added 4 commits November 28, 2022 11:04

Fix: use garden cluster for event recorder

e75db3d

Rename event recorder of shoot-scheduler

66eb6dd

Fix: stop reconciling failed or ignored shoot

a6e84f5

Doc nits

f7ffa33

timebertt marked this pull request as ready for review November 28, 2022 11:16

gardener-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 28, 2022

gardener-prow bot requested a review from acumino November 28, 2022 11:16

gardener-prow bot requested a review from shafeeqes November 28, 2022 11:16

rfranzke assigned ary1992, shafeeqes and oliver-goetz Nov 29, 2022

shafeeqes reviewed Nov 29, 2022

View reviewed changes

pkg/gardenlet/controller/shoot/shoot/add_test.go Show resolved Hide resolved

pkg/gardenlet/controller/shoot/shoot/add_test.go Show resolved Hide resolved

pkg/utils/gardener/migration_test.go Show resolved Hide resolved

pkg/gardenlet/controller/shoot/shoot/reconciler.go Show resolved Hide resolved

rfranzke requested a review from shafeeqes December 1, 2022 20:14

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Dec 2, 2022

rfranzke reviewed Dec 2, 2022

View reviewed changes

docs/concepts/gardenlet.md Outdated Show resolved Hide resolved

Update docs/concepts/gardenlet.md

9fb6b6e

gardener-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 2, 2022

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Dec 2, 2022

gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 2, 2022

gardener-prow bot merged commit c11c86a into gardener:master Dec 2, 2022

timebertt deleted the refactor/shoot-ctrl-2 branch December 5, 2022 07:26

timebertt mentioned this pull request Dec 12, 2022

[gardenlet] Refactor handling of Shoot main controller special cases #7189

Merged

rfranzke changed the title ~~[gardenlet] Switch Shoot main controller to controller-runtime~~ [gardenlet] Switch Shoot main controller to controller-runtime Dec 14, 2022

rfranzke mentioned this pull request Dec 14, 2022

[gardenlet] Switch Shoot care controller to controller-runtime #7206

Merged

rfranzke mentioned this pull request Jan 16, 2023

Fix bug preventing shoot deletion when never reconciled at least once #7326

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[gardenlet] Switch `Shoot` main controller to `controller-runtime` #7082

[gardenlet] Switch `Shoot` main controller to `controller-runtime` #7082

timebertt commented Nov 25, 2022 •

edited

Loading

gardener-prow bot commented Nov 25, 2022

timebertt commented Nov 25, 2022

rfranzke commented Nov 28, 2022

timebertt commented Nov 28, 2022

timebertt commented Nov 28, 2022

timebertt commented Nov 28, 2022 •

edited

Loading

timebertt commented Nov 28, 2022

timebertt commented Nov 28, 2022

shafeeqes commented Nov 29, 2022

shafeeqes left a comment

shafeeqes commented Dec 2, 2022

rfranzke left a comment

rfranzke commented Dec 2, 2022

gardener-prow bot commented Dec 2, 2022

gardener-prow bot commented Dec 2, 2022 •

edited

Loading

rfranzke commented Dec 2, 2022

[gardenlet] Switch Shoot main controller to controller-runtime #7082

[gardenlet] Switch Shoot main controller to controller-runtime #7082

Conversation

timebertt commented Nov 25, 2022 • edited Loading

gardener-prow bot commented Nov 25, 2022

timebertt commented Nov 25, 2022

rfranzke commented Nov 28, 2022

timebertt commented Nov 28, 2022

timebertt commented Nov 28, 2022

timebertt commented Nov 28, 2022 • edited Loading

timebertt commented Nov 28, 2022

timebertt commented Nov 28, 2022

shafeeqes commented Nov 29, 2022

shafeeqes left a comment

Choose a reason for hiding this comment

shafeeqes commented Dec 2, 2022

rfranzke left a comment

Choose a reason for hiding this comment

rfranzke commented Dec 2, 2022

gardener-prow bot commented Dec 2, 2022

gardener-prow bot commented Dec 2, 2022 • edited Loading

rfranzke commented Dec 2, 2022

[gardenlet] Switch `Shoot` main controller to `controller-runtime` #7082

[gardenlet] Switch `Shoot` main controller to `controller-runtime` #7082

timebertt commented Nov 25, 2022 •

edited

Loading

timebertt commented Nov 28, 2022 •

edited

Loading

gardener-prow bot commented Dec 2, 2022 •

edited

Loading