Bug 1857162: daemon: inject proxy vars into MCD container #2037

yuqi-zhang · 2020-08-28T20:00:39Z

With the introduction of extensions and reworking of OS updates in
#1941,
we now run oc/podman directly in the MCD to fetch the OS image.
Thus we need to inject the proxy env vars into the container via the
daemonset definition, otherwise image pulls will fail.

openshift-ci-robot · 2020-08-28T20:00:48Z

@yuqi-zhang: This pull request references Bugzilla bug 1857162, 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.6.0) matches configured target release for branch (4.6.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1857162: daemon: inject proxy vars into MCD container

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 · 2020-08-28T20:02:00Z

Tested that the env vars are injected correctly, and that with the env vars proxy should be able to pull the image. Tested the image pull manually (successful) on a failing cluster but was not able to do a full test on a failing proxy cluster, will try to set it up if possible but I'm pretty confident this should work.

Also added a comment to the OS extraction function so we can keep this in mind if we ever rework it

With the introduction of extensions and reworking of OS updates in openshift#1941, we now run `oc/podman` directly in the MCD to fetch the OS image. Thus we need to inject the proxy env vars into the container via the daemonset definition, otherwise image pulls will fail. Signed-off-by: Yu Qi Zhang <[email protected]>

openshift-ci-robot · 2020-08-28T20:09:18Z

@yuqi-zhang: This pull request references Bugzilla bug 1857162, which is valid.

3 validation(s) were run on this bug

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

In response to this:

Bug 1857162: daemon: inject proxy vars into MCD container

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.

runcom · 2020-08-30T16:50:57Z

/approve

yuqi-zhang · 2020-08-31T13:29:11Z

/hold

did not pass on an upgrade in proxy, investigating

Sync env var changes in the daemonset definition, so injected proxy env var updates will get applied upon an upgrade. Signed-off-by: Yu Qi Zhang <[email protected]>

yuqi-zhang · 2020-09-01T19:16:58Z

Ive tested this to work on AWS again, and it properly synced the env var on an upgrade. Will test on Azure before removing the hold. Also note that we should eventually look to either update or remove our lib code but that's not in scope of this PR

openshift-ci-robot · 2020-09-01T19:17:02Z

@yuqi-zhang: This pull request references Bugzilla bug 1857162. The bug has been updated to no longer refer to the pull request using the external bug tracker.

In response to this:

Bug 1857162: daemon: inject proxy vars into MCD container

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-robot · 2020-09-01T19:17:06Z

@yuqi-zhang: This pull request references Bugzilla bug 1857162, which is valid. 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.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1857162: daemon: inject proxy vars into MCD container

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 · 2020-09-02T13:53:10Z

/hold cancel
/retest

Can confirm this successfully passed an upgrade on an azure proxy env

sinnykumari

Looks great, thanks Jerry for fixing it!

/lgtm

sinnykumari · 2020-09-03T14:21:27Z

@yuqi-zhang Curious if we have any public documentation somewhere to run OCP cluster in proxy environment, this can help us in future to do proxy related testing.

openshift-bot · 2020-09-03T14:23:39Z

/retest

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

cgwalters

Overall LGTM but does seem like we should take on switching to library-go after branching for 4.6.

cgwalters · 2020-09-03T14:47:26Z

lib/resourcemerge/core.go

+			existingCurr = &existing.Env[len(existing.Env)-1]
+		}
+		ensureEnvVar(modified, existingCurr, required)
+	}


This doesn't support removing variables right? Which is probably OK short term but seems like a trap if e.g. someone wants to remove a proxy.

Hmm yeah I just copied the Ports section below but now you mentioned it we don't check for removals. I think that's a problem for all changes applied to the resource. Maybe merging with upstream library-go will fix this

openshift-ci-robot · 2020-09-03T14:48:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, runcom, 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:

~~OWNERS~~ [cgwalters,runcom,sinnykumari,yuqi-zhang]

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

yuqi-zhang · 2020-09-03T14:55:03Z

Curious if we have any public documentation somewhere to run OCP cluster in proxy environment, this can help us in future to do proxy related testing.

Unfortunately not really. The AWS cluster-bot proxy env is the closest we have but it doesn't use a real proxy today. I talked to Eric Wolinetz and he is looking to get it updated. For this PR specifically I requested some environments from QE.

openshift-bot · 2020-09-03T16:59:49Z

/retest

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

openshift-bot · 2020-09-03T17:13:27Z

/retest

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

openshift-bot · 2020-09-03T18:16:18Z

/retest

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

openshift-bot · 2020-09-03T18:55:17Z

/retest

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

openshift-bot · 2020-09-03T19:47:18Z

/retest

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

openshift-bot · 2020-09-03T20:52:19Z

/retest

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

openshift-ci-robot · 2020-09-03T21:07:57Z

@yuqi-zhang: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/e2e-ovn-step-registry	`197db25`	link	`/test e2e-ovn-step-registry`
ci/prow/okd-e2e-aws	`197db25`	link	`/test okd-e2e-aws`

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.

openshift-ci-robot · 2020-09-03T21:36:44Z

@yuqi-zhang: All pull requests linked via external trackers have merged:

openshift/machine-config-operator#2037

Bugzilla bug 1857162 has been moved to the MODIFIED state.

In response to this:

Bug 1857162: daemon: inject proxy vars into MCD container

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-robot added bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Aug 28, 2020

openshift-ci-robot requested review from cgwalters and ericavonb August 28, 2020 20:00

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 28, 2020

yuqi-zhang requested review from runcom and sinnykumari August 28, 2020 20:06

yuqi-zhang force-pushed the inject-proxy-into-mcd branch from 6413150 to ea0f893 Compare August 28, 2020 20:09

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 31, 2020

lib/resourcemerge: sync daemonset container env vars

197db25

Sync env var changes in the daemonset definition, so injected proxy env var updates will get applied upon an upgrade. Signed-off-by: Yu Qi Zhang <[email protected]>

yuqi-zhang closed this Sep 1, 2020

yuqi-zhang reopened this Sep 1, 2020

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

sinnykumari approved these changes Sep 3, 2020

View reviewed changes

openshift-ci-robot assigned sinnykumari Sep 3, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2020

cgwalters approved these changes Sep 3, 2020

View reviewed changes

openshift-merge-robot merged commit 1e336ef into openshift:master Sep 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1857162: daemon: inject proxy vars into MCD container #2037

Bug 1857162: daemon: inject proxy vars into MCD container #2037

yuqi-zhang commented Aug 28, 2020 •

edited

Loading

openshift-ci-robot commented Aug 28, 2020

yuqi-zhang commented Aug 28, 2020 •

edited

Loading

openshift-ci-robot commented Aug 28, 2020

runcom commented Aug 30, 2020

yuqi-zhang commented Aug 31, 2020

yuqi-zhang commented Sep 1, 2020

openshift-ci-robot commented Sep 1, 2020

openshift-ci-robot commented Sep 1, 2020

yuqi-zhang commented Sep 2, 2020

sinnykumari left a comment

sinnykumari commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

cgwalters left a comment

cgwalters Sep 3, 2020

yuqi-zhang Sep 3, 2020

openshift-ci-robot commented Sep 3, 2020

yuqi-zhang commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-ci-robot commented Sep 3, 2020 •

edited

Loading

openshift-ci-robot commented Sep 3, 2020

Bug 1857162: daemon: inject proxy vars into MCD container #2037

Bug 1857162: daemon: inject proxy vars into MCD container #2037

Conversation

yuqi-zhang commented Aug 28, 2020 • edited Loading

openshift-ci-robot commented Aug 28, 2020

yuqi-zhang commented Aug 28, 2020 • edited Loading

openshift-ci-robot commented Aug 28, 2020

runcom commented Aug 30, 2020

yuqi-zhang commented Aug 31, 2020

yuqi-zhang commented Sep 1, 2020

openshift-ci-robot commented Sep 1, 2020

openshift-ci-robot commented Sep 1, 2020

yuqi-zhang commented Sep 2, 2020

sinnykumari left a comment

Choose a reason for hiding this comment

sinnykumari commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

cgwalters left a comment

Choose a reason for hiding this comment

cgwalters Sep 3, 2020

Choose a reason for hiding this comment

yuqi-zhang Sep 3, 2020

Choose a reason for hiding this comment

openshift-ci-robot commented Sep 3, 2020

yuqi-zhang commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-ci-robot commented Sep 3, 2020 • edited Loading

openshift-ci-robot commented Sep 3, 2020

yuqi-zhang commented Aug 28, 2020 •

edited

Loading

yuqi-zhang commented Aug 28, 2020 •

edited

Loading

openshift-ci-robot commented Sep 3, 2020 •

edited

Loading