-
Notifications
You must be signed in to change notification settings - Fork 456
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
Label nodes with expected OperatingSystemConfig name #9757
Label nodes with expected OperatingSystemConfig name #9757
Conversation
The only place that creates operatingsystemconfig.Data objects always sets SecretName to a pointer. Thus, convert the string pointer to a plain string.
Nodes with an outdated operating system config key are skipped while checking whether a shoot is healthy. Currently, this requires botanist to know how to calculate the OSCkey based on a node object, which leaks implementation details. Instead, each node is now just labeled with the expected config key. This allows for future extensions of the OSCkey calculation without having to introduce additional labels on the node objects. By including the keyName in the operatingsystemconfig.Data struct, no other component has to know how the keyName is calculated.
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.
/ok-to-test
/retest |
/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.
Nice PR, thanks!
pkg/component/extensions/operatingsystemconfig/operatingsystemconfig.go
Outdated
Show resolved
Hide resolved
This reverts commit bf5342c.
I've addressed the review comments. PTAL |
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 @MichaelEischer
Let's go
/lgtm
/approve
LGTM label has been added. Git tree hash: e04bd5a18567d0980a6fd953bc6726a74dcaf87a
|
[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 |
/retest |
How to categorize this PR?
/area control-plane
/kind enhancement
What this PR does / why we need it:
Label all workers with the name of their expected operating system config.
Nodes with an outdated operating system config key are skipped while checking whether a shoot is healthy. Currently, this requires botanist to know how to calculate the OSCkey based on a node object, which leaks implementation details. Instead, each node is now just labeled with the expected config key. This allows for future extensions of the OSCkey calculation without having to introduce additional labels on the node objects.
By including the KeyName in the operatingsystemconfig.Data struct, no other component has to know how the KeyName is calculated. The existing
oscName
field is not really suitable as it also includes a keySuffix which the external components shouldn't know about.Which issue(s) this PR fixes:
Part of #9699 . For
nodeToBeDeleted
I've kept the fallback tonodeOSCKeyDifferentFromSecretOSCKey(node, secretOSCKey)
for nodes that are not labeled yet. Otherwise the healthcheck would be inaccurate until the next successful reconcile of the Worker extension object.Special notes for your reviewer:
Reworked implementation of #9465 .
Release note: