-
Notifications
You must be signed in to change notification settings - Fork 474
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
Introduce secret to track hash version used by OperatingSystemConfig #9846
Introduce secret to track hash version used by OperatingSystemConfig #9846
Conversation
Hi @MichaelEischer. Thanks for your PR. I'm waiting for a gardener member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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-sigs/prow repository. |
/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.
Thank you for the nice PR 🎉
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
eca1aeb
to
51c87c2
Compare
@rfranzke I've addressed the review comment. However, some E2E tests still fail, and I don't have a clue why. |
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.
Minor nits left :)
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
51c87c2
to
c622ea2
Compare
Cool, looks good @MichaelEischer. Please fix the tests so that we can get this PR merged. |
I forgot to run /retest |
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.
/lgtm
/approve
LGTM label has been added. Git tree hash: 0502dcf3985fa2ab99400799bf0747093bbd382d
|
[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 |
(part of gardener#9846, released with `v1.97.0`)
(part of gardener#9846, released with `v1.97.0`)
(part of gardener#9846, released with `v1.97.0`)
* Drop deletion of deprecated `allow-to-shoot-networks` `NetworkPolicy` (part of #9752, released with `v1.96.0`) * Drop fetching extension observability configs with deprecated/legacy method (part of #9695, released with `v1.95.0`) * Drop Prometheus/Alertmanager migration coding (part of #9695, released with `v1.95.0`) * Drop deprecated `.spec.pools[].userData` from `extensions.gardener.cloud/v1alpha1.Worker` API (part of #9722, released with `v1.95.0`) * Drop OSC hash migration `Secret` creation (part of #9846, released with `v1.97.0`) * Drop OSC hash assertion from upgrade tests (part of #9865, released with `v1.98.0`) * Drop removal code of `HVPA` resources (part of #9698, released with `v1.95.0`) * Address PR review feedback
How to categorize this PR?
/area control-plane
/kind enhancement
What this PR does / why we need it:
This PR implements the
pool-hashes
secret, proposed in #9699, that is stored in the shoot namespaces on each seed cluster. The secret contains information on which hash version is used for the OperatingSystemConfig as well as the calculated hash values. If the calculated and stored hash values differ, then the hash is upgraded to the latest version.Currently, only hash version 1 exists. Version 2 will be introduced in a follow-up PR.
The PR includes migration code, that creates a placeholder secret in each shoot namespace. As a small difference to the proposal in #9699, that secret only contains a "migrated" field. This field causes the normal reconcile for the secret to bootstrap the secret with version 1 instead of the latest version (which currently is still 1, but version 2 will be added in a follow-up PR). This minimizes the complexity of the migration code, but might require keeping a part of it around for some time.
Which issue(s) this PR fixes:
Part of #9699
Special notes for your reviewer:
I'm somewhat unsure what the best approach is to check that the
pool-hashes
secret properly survives a control plane migration. I've extended the corresponding e2e test to add a special label to the secret, which will be checked by the existing test code.Release note: