-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
extract the release image on a machine with internet #17569
Conversation
The preview will be available shortly at: |
I am not sure @umohnani8's issue, but pls make sure really follow the steps in our official doc. https://docs.openshift.com/container-platform/4.2/installing/installing_restricted_networks/installing-restricted-networks-preparations.html#cli-installing-cli_installing-restricted-networks-preparations
|
Thank you for reminding me @jianlinliu, @umohnani8, do you have another suggestion for getting your customer back on track? |
@kalexand-rh yeah we are just working on confirming the detail about the ENV variable. I will comment back here in a bit. But the |
Using the official installer without alteration is what we want to have work. Testing in openshift/release#5567. But until we get that green, extracting from the mirrored release like the master docs have is certainly more official than using the environment variable override. |
@wking, let me know when that merges so that we can account for it in doc. @umohnani8, I'm not going to merge this change as-is, but if there's additional information to present instead, please let me know so I can amend this PR. |
I'm going to close this PR. @umohnani8, please let me know if you have more information that you'd like me to incorporate. |
@kalexand-rh can we reopen this. We confirmed that this works as expected. We need to remove the |
@umohnani8, is it supported now? I hesitate to bring a change in that both Abhinav and Jianlin have declined. |
Yeah, we don't want to add the |
9616132
to
63ffe9d
Compare
63ffe9d
to
aa741a0
Compare
LGTM |
Trevor says that this looks ok to him. @jianlinliu, are you ok with this change? |
Actually I still can not figure out why user have to extract installer on a machine that has access to the internet. I agree |
@jianlinliu, that's about where I am with this PR - I think that the existing direction to perform the steps on the bastion host was enough, but adding a reminder in a different location is harmless. |
@@ -74,4 +74,6 @@ $ oc adm release extract --command=openshift-install "${LOCAL_REGISTRY}/${LOCAL_ | |||
To ensure that you use the correct images for the version of {product-title} | |||
that you selected, you must extract the installation program from the mirrored | |||
content. | |||
|
|||
You must perform this step on a machine with an active internet connection. |
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.
Consider combining this with the previous sentence into a single admonition: "...you must extract the installation program from the mirrored content using a machine with an active internet connection."
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.
If the major point of the PR wasn't "people are missing that you need to do this on a machine with internet," I'd take that improvement.
One small comment; otherwise, LGTM! |
/cherrypick enterprise-4.2 |
@kalexand-rh: new pull request created: #18340 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. |
/cherrypick enterprise-4.3 |
@kalexand-rh: new pull request created: #18341 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. |
Today someone reported similar issue - https://bugzilla.redhat.com/show_bug.cgi?id=1777890. I reproduced it, refer to that bug for more details. So now this PR sound reasonable for me now. |
Per @umohnani8 in Slack.
(@wking, FYI)
@jianlinliu, will you PTAL?