Bug 1993922: fixes 1 to 1 kubelet config mapping #2752

rphillips · 2021-09-02T18:34:46Z

- What I did
The bug reported a problem with the 1-to-1 mapping to MCPs and Kubelet Configs. I figured out the getManageKubeletConfigKey function did not include a LabelSelector to filter out the KubeletConfig for the Pool, thus including all the KubeletConfigs in the system for the limit validation.

- How to verify it
The patch adds a test to validate multiple pools and their 1:1 relationship with KubeletConfigs.

- Description for the changelog

Fix KubeletConfig 1-to-1 relationship to Pools.

openshift-ci · 2021-09-02T18:35:48Z

@rphillips: This pull request references Bugzilla bug 1993922, which is invalid:

expected the bug to target the "4.9.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 1993922: fixes 1 to 1 kubelet config mapping

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.

rphillips · 2021-09-02T18:40:21Z

/bugzilla refresh

openshift-ci · 2021-09-02T18:40:29Z

@rphillips: This pull request references Bugzilla bug 1993922, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.9.0) matches configured target release for branch (4.9.0)
bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

No GitHub users were found matching the public email listed for the QA contact in Bugzilla ([email protected]), skipping review request.

Details

In response to this:

/bugzilla refresh

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.

kikisdeliveryservice · 2021-09-02T18:47:19Z

PTAL

/assign @umohnani8 @QiWang19

kikisdeliveryservice · 2021-09-02T19:41:54Z

@rphillips might need to double check whether this change affects e2e bc it's failing in the kc section.

QiWang19 · 2021-09-02T21:08:41Z

Launched a cluster of this pr, failed to create the kubeletconfig

Status:
  Conditions:
    Last Transition Time:  2021-09-02T20:56:07Z
    Message:               could not get kubelet config key: error listing kubelet configs: unable to parse requirement: <nil>: Invalid value: "&LabelSelector{MatchLabels:map[string]string{machineconfiguration.openshift.io/role:": prefix part a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*'); name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')

rphillips · 2021-09-02T21:41:27Z

@QiWang19 thanks... I had to add a filter. Looks like we do this in most of the other areas of the code.

QiWang19 · 2021-09-02T22:41:48Z

@QiWang19 thanks... I had to add a filter. Looks like we do this in most of the other areas of the code.

Yes, good to know. the suffix is incremented within each pool now.

QiWang19

/lgtm

kikisdeliveryservice

just one small thing bc the comment is now not accurate

pkg/controller/kubelet-config/helpers.go

kikisdeliveryservice

Adding approval and hold. If you prefer to fix in a followup feel free to remove.

/hold

Co-authored-by: Kirsten <[email protected]>

kikisdeliveryservice · 2021-09-03T00:29:12Z

removing hold and re-adding lgtm since this was a comment tweak

/hold cancel
/lgtm

openshift-bot · 2021-09-03T06:29:06Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T06:41:33Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T06:54:34Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T07:07:33Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T07:19:34Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T07:31:34Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T07:55:34Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T08:07:33Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T08:19:33Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T08:31:56Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T08:43:33Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T09:07:33Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2021-09-03T09:14:01Z

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

Test name	Commit	Details	Rerun command
ci/prow/okd-e2e-aws	`b0139ea`	link	`/test okd-e2e-aws`
ci/prow/e2e-aws-workers-rhel8	`b0139ea`	link	`/test e2e-aws-workers-rhel8`
ci/prow/e2e-aws-single-node	`b0139ea`	link	`/test e2e-aws-single-node`
ci/prow/e2e-aws-workers-rhel7	`b0139ea`	link	`/test e2e-aws-workers-rhel7`
ci/prow/e2e-metal-ipi	`b0139ea`	link	`/test e2e-metal-ipi`
ci/prow/e2e-aws-serial	`b0139ea`	link	`/test e2e-aws-serial`
ci/prow/e2e-aws-disruptive	`b0139ea`	link	`/test e2e-aws-disruptive`
ci/prow/e2e-aws-upgrade-single-node	`b0139ea`	link	`/test e2e-aws-upgrade-single-node`

Full PR test history. Your PR dashboard.

Details

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.

openshift-bot · 2021-09-03T09:43:33Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T09:55:35Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T10:07:36Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T10:20:12Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2021-09-03T10:32:47Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-ci · 2021-09-03T11:55:00Z

@rphillips: All pull requests linked via external trackers have merged:

openshift/machine-config-operator#2752

Bugzilla bug 1993922 has been moved to the MODIFIED state.

Details

In response to this:

Bug 1993922: fixes 1 to 1 kubelet config mapping

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.

openshift-cherrypick-robot · 2021-09-03T11:55:38Z

@rphillips: new pull request created: #2753

Details

In response to this:

/cherry-pick release-4.8

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.

QiWang19 · 2021-09-09T15:05:56Z

/cherry-pick release-4.7

openshift-cherrypick-robot · 2021-09-09T15:06:35Z

@QiWang19: #2752 failed to apply on top of branch "release-4.7":

Applying: fix 1 to 1 relation to kubelet configs and machine config pools
Using index info to reconstruct a base tree...
M	pkg/controller/kubelet-config/helpers.go
M	pkg/controller/kubelet-config/kubelet_config_controller_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/controller/kubelet-config/kubelet_config_controller_test.go
CONFLICT (content): Merge conflict in pkg/controller/kubelet-config/kubelet_config_controller_test.go
Auto-merging pkg/controller/kubelet-config/helpers.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix 1 to 1 relation to kubelet configs and machine config pools
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherry-pick release-4.7

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.

openshift#2752 fixed the 1-1 mapping for kubeletconfig, this PR fix the machineconfig name and pool name 1-1 mapping for containerruntime config. Signed-off-by: Qi Wang <[email protected]>

openshift-ci bot requested review from kikisdeliveryservice and yuqi-zhang September 2, 2021 18:35

rphillips changed the title ~~Bug 1993922 : fixes 1 to 1 kubelet config mapping~~ Bug 1993922: fixes 1 to 1 kubelet config mapping Sep 2, 2021

openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Sep 2, 2021

openshift-ci bot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Sep 2, 2021

openshift-ci bot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Sep 2, 2021

openshift-ci bot assigned QiWang19 and umohnani8 Sep 2, 2021

rphillips added 2 commits September 2, 2021 16:40

fix 1 to 1 relation to kubelet configs and machine config pools

6e1c4f9

add more debug functionality to the test suite

c1fc3db

rphillips force-pushed the fixes/1_to_1_kubelet_config_mapping branch from f0ac207 to c1fc3db Compare September 2, 2021 21:40

QiWang19 reviewed Sep 2, 2021

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 2, 2021

kikisdeliveryservice reviewed Sep 2, 2021

View reviewed changes

pkg/controller/kubelet-config/helpers.go Outdated Show resolved Hide resolved

kikisdeliveryservice approved these changes Sep 2, 2021

View reviewed changes

openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 2, 2021

fix comment in kubelet-config/helpers.go

b0139ea

Co-authored-by: Kirsten <[email protected]>

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2021

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 3, 2021

openshift-merge-robot merged commit 2ec816a into openshift:master Sep 3, 2021

openshift-cherrypick-robot mentioned this pull request Sep 3, 2021

[release-4.8] Bug 2000958: fixes 1 to 1 kubelet config mapping #2753

Merged

QiWang19 mentioned this pull request Nov 30, 2021

Bug 2028731: fixes 1 to 1 containerruntime config mapping #2846

Merged

QiWang19 mentioned this pull request Dec 22, 2021

[release-4.9] Bug 2032985: fixes 1 to 1 containerruntime config mapping #2876

Merged

Bug 1993922: fixes 1 to 1 kubelet config mapping #2752

Bug 1993922: fixes 1 to 1 kubelet config mapping #2752

Uh oh!

Conversation

rphillips commented Sep 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Sep 2, 2021

Uh oh!

rphillips commented Sep 2, 2021

Uh oh!

openshift-ci bot commented Sep 2, 2021

Uh oh!

kikisdeliveryservice commented Sep 2, 2021

Uh oh!

kikisdeliveryservice commented Sep 2, 2021

Uh oh!

QiWang19 commented Sep 2, 2021

Uh oh!

rphillips commented Sep 2, 2021

Uh oh!

QiWang19 commented Sep 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

QiWang19 left a comment

Choose a reason for hiding this comment

Uh oh!

kikisdeliveryservice left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kikisdeliveryservice left a comment

Choose a reason for hiding this comment

Uh oh!

kikisdeliveryservice commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-ci bot commented Sep 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-bot commented Sep 3, 2021

Uh oh!

openshift-ci bot commented Sep 3, 2021

Uh oh!

openshift-cherrypick-robot commented Sep 3, 2021

Uh oh!

QiWang19 commented Sep 9, 2021

Uh oh!

openshift-cherrypick-robot commented Sep 9, 2021

rphillips commented Sep 2, 2021 •

edited

Loading

QiWang19 commented Sep 2, 2021 •

edited

Loading

openshift-ci bot commented Sep 3, 2021 •

edited

Loading