[release-4.9] Bug 2071689: lib/resourcemerge: handle container env var deletions #3057
Conversation
The logic in the resourcemerge functions only iterate through required variables, meaning any removed variable is not handled. As a fix to bug 1981549, this adds removal check for env vars, which ensures that e.g. a removed proxy will correctly propagate to the daemonset definition, which needs the proxy injected as an environment variable to allow MCD to pull os image updates. This of course is also a problem for everything else being synced via these lib functions, but for now I only added the fix to EnvVar for proxy, as it is the most likely issue for users to hit. If we want to fix all other variables, we should probably also consider reworking the resource* libraries in general, since they are outdated and error prone. To test, you can pause the pool, add a cluster proxy and then remove it, checking the MCD daemonset after both steps to see the proxy being added/removed. Interestingly, the adding of the proxy is almost instant, whereas the removal can take up to 10 minutes due to the MCO seemingly not resyncing (no action from proxy informer?). I am investigating that separately. Signed-off-by: Yu Qi Zhang <jerzhang@redhat.com>
This aims to add some initial tests for EnvVar handling specifically, to make sure we don't regress proxy. Other unit tests should be added as we clean up the code. Signed-off-by: Yu Qi Zhang <jerzhang@redhat.com>
|
@openshift-cherrypick-robot: Bugzilla bug 1981549 has been cloned as Bugzilla bug 2071689. Retitling PR to link against new bug. In response to this:
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-ci
bot
changed the title
openshift-ci
bot
added
bugzilla/severity-high
|
@openshift-cherrypick-robot: This pull request references Bugzilla bug 2071689, 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. 6 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
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. |
There was a problem hiding this comment.
Came up again via https://bugzilla.redhat.com/show_bug.cgi?id=2070930
Should probably cherry pick to 4.8 at least.
/label backport-risk-assessed
openshift-ci
bot
added
backport-risk-assessed
|
/retest |
|
/retest-required |
|
/retest |
1 similar comment
|
/retest |
|
@palonsoro the additional test failures are just flakes and the PR isn't blocked on them, so no retest needed. This would need to get cherry-pick-approved (QE) and lgtm'ed (another team member), as well as wait for CI stability in general (probably will wait until next week before we lgtm it). /cherry-pick release-4.8 for when this merges |
|
@yuqi-zhang: once the present PR merges, I will cherry-pick it on top of release-4.8 in a new PR and assign it to you. In response to this:
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. |
|
@yuqi-zhang thanks for letting me know :-) |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: openshift-cherrypick-robot, sinnykumari, yuqi-zhang 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-required Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/label cherry-pick-approved |
openshift-ci
bot
added
the
cherry-pick-approved
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
12 similar comments
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
@openshift-cherrypick-robot: The following tests failed, say
Full PR test history. Your PR dashboard. 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. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
@openshift-cherrypick-robot: All pull requests linked via external trackers have merged: Bugzilla bug 2071689 has been moved to the MODIFIED state. In response to this:
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. |
|
@yuqi-zhang: new pull request created: #3161 In response to this:
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. |
This is an automated cherry-pick of #2800
/assign palonsoro