-
Notifications
You must be signed in to change notification settings - Fork 1.5k
baremetal: remove redundant proxy setting #5598
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
baremetal: remove redundant proxy setting #5598
Conversation
|
Good catch! /approve |
…man in the container
|
lgtm, I'll test it later. |
|
/retest |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ardaguclu, elfosardo, zhouhao3 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
I did wonder why we needed the explicit proxy settings in #5569 - thanks @andfasano for pointing out how the global variables are defined. This lgtm, but we need test confirmation from @zhouhao3 given this effectively reverts #5569 I'm unclear atm why that was needed, https://bugzilla.redhat.com/show_bug.cgi?id=2045927 says this change in behavior was related to the day-1 networking changes, but I'm unclear why the icc started via startironic.sh won't just use the global environment variables mentioned? @andfasano if we merge openshift-metal3/dev-scripts#1341 ASAP can we get a new CI job to prove this definitely works (and stays working)? |
I think the main problem was captured in openshift/image-customization-controller#33, since ICC didn't use the proxy vars for the
Yes, I've got a pending PR ready for that (openshift/release#25912) - and planned also to open a new one for the installer repo as well |
Ah Ok so we only actually needed the icc fix - @zhouhao3 please can you confirm if you're happy with this change based on your testing, thanks! |
|
This looks correct, podman passes these vars automagically: https://docs.podman.io/en/latest/markdown/podman-run.1.html#http-proxy |
|
/unhold |
|
/retest |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
5 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. |
13 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. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
@andfasano: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
7 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. |
During the bootstrap, proxy related variables should be already injected in the service by https://github.com/openshift/installer/blob/master/data/data/bootstrap/files/etc/systemd/system.conf.d/10-default-env.conf.template.
In addition, proxy environment variable should be already passed to the container by podman