Eliminate the `Worker` State Reconciler #8559

rfranzke · 2023-09-26T10:58:14Z

How to categorize this PR?

/area scalability cost
/kind enhancement

What this PR does / why we need it:
This PR drops the Worker state reconciler that currently is auto-added to all Worker controllers. Please see GEP-22 for information about the motivation.

With this PR, the migration flow is adapted as follows:

gardenlet collects machine state and persists it in the ShootState under .spec.gardener[].type=machine-state.
gardenlet shallow-deletes the machine-related resources (this was previously done by the generic Worker actuator).
For a few releases, gardenlet no longer persists the state stored in .status.state of Worker resources (to not duplicate the machine state in the ShootState).

The restoration flow is adapted as follows:

The generic Worker actuator tries to read the machine state directly from the ShootState in the garden cluster (which is now possible since Garden Cluster Access For Extensions In Seed Clusters #8001). This alleviates the necessity to let gardenlet read it and populate it somehow to the destination seed for the provider extensions to pick up - they can do it directly.
gardenlet fetches the machine state from the ShootState's .spec.gardener[] field (previously, the machine state was fetched from .spec.extensions[].kind=Worker) and still populates it to the Worker's .status.state field. This is just to preserve backwards-compatibility, because registered extensions might not yet have vendored the newest library, hence they might still rely on this field during restoration. This will be dropped in a future version.

The PR also addresses a TODO leftover task from #8082 about only persisting the ShootState after all extension resources have been migrated.

Which issue(s) this PR fixes:
Fixes #8488

Special notes for your reviewer:

The version skew policy for extensions was clarified as part of this PR.
github.com/gardener/machine-controller-manager was revendored to 0.50.0 according to [ci:component:github.com/gardener/machine-controller-manager:v0.49.3->v0.50.0] #8539. In this version, the legacy provider-specific API types are cleaned up. The revendoring was needed to make the machine.sapcloud.io/v1alpha1 CRDs available for the integration tests.
Accordingly, the no longer needed MachineClass-related methods were dropped from the generic Worker actuator interface.

/cc @plkokanov

Release note:

The `Worker` state reconciler has been dropped, i.e., updated provider extensions will no longer populate the machine state to the `.status.state` field of `Worker` resources. For a few releases, `gardenlet` will no longer persist any still existing data in the `.status.state` field of `Worker` resources during a control plane migration of a `Shoot`, and it will set `.status.state` to `nil` after a successful reconciliation or restore operation.

Provider extensions must now pass the `cluster.Cluster` object for the garden cluster to the `genericactuator.NewActuator` function. See [this](https://github.com/gardener/gardener/blob/8d2f116aa606e5181cd430e5063dd798629bdc78/cmd/gardener-extension-provider-local/app/app.go#L228-L246) for an example how to create such a `cluster.Cluster` object.

The `MachineClassKind()`, `MachineClass()`, and `MachineClassList()` methods have been dropped from the generic `Worker` actuator's interface and do not need to be implemented anymore.

During the `Migrate` phase of a control plane migration of a `Shoot`, the state is now only persisted after all extension resources have been migrated. Consequently, make sure that you have added all state to the `.status.state` field of the respective extension object when running `Migrate()`.

plkokanov · 2023-09-26T11:06:10Z

/assign

timuthy · 2023-09-28T09:57:34Z

/assign

timuthy

Nice PR and as usual very understandable 👏 😄
Some minor points, rest looks good to me.

pkg/utils/gardener/machines.go

pkg/utils/gardener/shootstate/machines.go

pkg/utils/gardener/shootstate/shootstate.go

pkg/component/extensions/worker/worker.go

docs/extensions/migration.md

extensions/pkg/controller/worker/genericactuator/actuator_restore_test.go

timuthy

/lgtm

Now `gardenlet` persists the machine state as part of `shootstate.Deploy`. This function is executed after all extension resources were migrated.

Will be needed in `botanist/migration.go` in a subsequent commit.

Now that the `gardenlet` persists the machine state explicitly, we do not need to duplicate it via the `Worker` state.

- For backwards-compatibility, we have to keep this flow since the generic `Worker` actuator's `Restore` function expects to find the state in the `Worker`'s `.status.state` field: https://github.com/gardener/gardener/blob/422e2bbedd23351383154bb733838a416f39f2b6/extensions/pkg/controller/worker/genericactuator/actuator_restore.go#L121C1-L141 - This is somewhat dirty for now, but probably acceptable given that this was the flow for the past years. - A subsequent commit will adapt the generic `Worker` actuator to fetch the state from elsewhere, however we have to wait until all provider extensions have been re-vendored with the new logic before we change this here.

…luster This is to prevent `gardenlet` from duplicating the machine state into the destination seed cluster.

…tore

timuthy

/lgtm

gardener-prow · 2023-10-04T11:53:58Z

LGTM label has been added.

Git tree hash: 9a306655be5d3e65493773d9f178233a223eb630

plkokanov · 2023-10-05T07:14:02Z

/lgtm

timuthy

/approve

gardener-prow · 2023-10-05T07:15:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: timuthy

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~~ [timuthy]

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

gardener-prow · 2023-10-05T08:11:16Z

@rfranzke: 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	`5388995`	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.

shafeeqes · 2023-10-05T08:15:18Z

/test pull-gardener-unit

follow-up of gardener#8559, released with `v1.82.0`

* Remove MCM legacy CRD deletion follow-up of #8559, released with `v1.82.0` * Remove legacy `shoot-node-logging` MR cleanup follow-up of #8501, released with `v1.80.0` * Remove MCM legacy resources cleanup in generic `Worker` actuator follow-up of #8596, released with `v1.82.0` * Restrict GRM's token requestor to secrets with `class=shoot` follow-up of #8152, released with `v1.74.0` * Remove support for deprecated `NetworkPolicy` annotations follow-up of #7907, released with `v1.71.0`

gardener-prow bot requested a review from plkokanov September 26, 2023 10:58

gardener-prow bot assigned plkokanov Sep 26, 2023

rfranzke force-pushed the worker-state branch from 60d2686 to ea160fc Compare September 26, 2023 11:19

rfranzke mentioned this pull request Sep 26, 2023

Generate remaining CRDs based on vendored APIs #8560

Merged

rfranzke force-pushed the worker-state branch from ea160fc to f212fe3 Compare September 26, 2023 13:45

gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 26, 2023

rfranzke force-pushed the worker-state branch from f212fe3 to 0cd3c33 Compare September 26, 2023 13:47

gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 26, 2023

rfranzke force-pushed the worker-state branch from 0cd3c33 to 47b5c35 Compare September 26, 2023 14:02

gardener-prow bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 28, 2023

gardener-prow bot assigned timuthy Sep 28, 2023

rfranzke force-pushed the worker-state branch from 47b5c35 to aa3bd89 Compare September 29, 2023 06:46

gardener-prow bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 29, 2023

rfranzke force-pushed the worker-state branch from aa3bd89 to 8f15db7 Compare September 29, 2023 13:32

gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 29, 2023

rfranzke force-pushed the worker-state branch from 8f15db7 to bb5d1d1 Compare September 29, 2023 13:34

timuthy reviewed Sep 29, 2023

View reviewed changes

rfranzke requested a review from timuthy October 1, 2023 14:05

timuthy reviewed Oct 4, 2023

View reviewed changes

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Oct 4, 2023

rfranzke added 8 commits October 4, 2023 11:06

Drop worker state reconciler

d1c5625

Now `gardenlet` persists the machine state as part of `shootstate.Deploy`. This function is executed after all extension resources were migrated.

Move machineclass purpose constant to constants package

dcc9606

Will be needed in `botanist/migration.go` in a subsequent commit.

Move machine migration logic from generic Worker actuator to botanist

2385e6c

Drop Worker state when persisting ShootState

9e0e1f8

Now that the `gardenlet` persists the machine state explicitly, we do not need to duplicate it via the `Worker` state.

Extensions fetch machine state directly from ShootState in garden c…

a298cd4

…luster This is to prevent `gardenlet` from duplicating the machine state into the destination seed cluster.

Clean Worker's .status.state field after successful reconcile/res…

7691e6c

…tore

Address PR review feedback

5388995

rfranzke force-pushed the worker-state branch from 89efdc6 to 5388995 Compare October 4, 2023 09:08

gardener-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 4, 2023

gardener-prow bot requested a review from timuthy October 4, 2023 09:08

gardener-prow bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 4, 2023

timuthy reviewed Oct 4, 2023

View reviewed changes

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Oct 4, 2023

timuthy reviewed Oct 5, 2023

View reviewed changes

gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 5, 2023

gardener-prow bot merged commit 25ecc27 into gardener:master Oct 5, 2023

rfranzke deleted the worker-state branch October 5, 2023 09:25

rfranzke mentioned this pull request Oct 6, 2023

Compress machine state in ShootState resource #8618

Merged

rfranzke mentioned this pull request Oct 19, 2023

Make machine state computation backwards-compatible #8673

Merged

rfranzke added a commit to rfranzke/gardener that referenced this pull request Nov 30, 2023

Remove MCM legacy CRD deletion

b234466

follow-up of gardener#8559, released with `v1.82.0`

rfranzke mentioned this pull request Nov 30, 2023

🧹 Cleanup a few TODOs #8883

Merged

ialidzhikov mentioned this pull request Dec 12, 2023

Fix CPM for hibernated Shoots #8943

Merged

rfranzke mentioned this pull request Mar 26, 2024

🧹 Cleanup a few TODOs #9479

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eliminate the `Worker` State Reconciler #8559

Eliminate the `Worker` State Reconciler #8559

rfranzke commented Sep 26, 2023

plkokanov commented Sep 26, 2023

timuthy commented Sep 28, 2023

timuthy left a comment

timuthy left a comment

timuthy left a comment

gardener-prow bot commented Oct 4, 2023

plkokanov commented Oct 5, 2023

timuthy left a comment

gardener-prow bot commented Oct 5, 2023

gardener-prow bot commented Oct 5, 2023 •

edited

Loading

shafeeqes commented Oct 5, 2023

Eliminate the Worker State Reconciler #8559

Eliminate the Worker State Reconciler #8559

Conversation

rfranzke commented Sep 26, 2023

plkokanov commented Sep 26, 2023

timuthy commented Sep 28, 2023

timuthy left a comment

Choose a reason for hiding this comment

timuthy left a comment

Choose a reason for hiding this comment

timuthy left a comment

Choose a reason for hiding this comment

gardener-prow bot commented Oct 4, 2023

plkokanov commented Oct 5, 2023

timuthy left a comment

Choose a reason for hiding this comment

gardener-prow bot commented Oct 5, 2023

gardener-prow bot commented Oct 5, 2023 • edited Loading

shafeeqes commented Oct 5, 2023

Eliminate the `Worker` State Reconciler #8559

Eliminate the `Worker` State Reconciler #8559

gardener-prow bot commented Oct 5, 2023 •

edited

Loading