-
Notifications
You must be signed in to change notification settings - Fork 200
Add ENABLE_LOCAL_REGISTRY config var to setup the local registry #1183
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
Add ENABLE_LOCAL_REGISTRY config var to setup the local registry #1183
Conversation
|
|
||
| # Setup the registry for mirroring images | ||
| if [[ ! -z "${MIRROR_IMAGES}" || $(env | grep "_LOCAL_IMAGE=") || ! -z "${ENABLE_CBO_TEST}" ]]; then | ||
| export ENABLE_LOCAL_REGISTRY=${ENABLE_LOCAL_REGISTRY:-${MIRROR_IMAGES:-${ENABLE_CBO_TEST:-$(env | grep "_LOCAL_IMAGE=")}}} |
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.
Why do we need to add this new variable, when MIRROR_IMAGES already provides an interface to enable the local registry?
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.
The fact is that MIRROR_IMAGES in dev-scripts is enabled by default for IPv6 and dualstack configuration, but not for IPv4.
Since we need to enable the disconnected run mode for the CI tests in all the configurations, this approach would allow to have the registry available also in such case - without the need of mirroring the release images.
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.
Note: we could also enable MIRROR_IMAGES always for the CI tests, but this means paying additional 15 minutes also when not strictly required.
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.
But you can set MIRROR_IMAGES in the IPv4 case, it's just not enabled by default, so why do we need a new variable that does basically the same thing? Sorry, perhaps I'm missing something obvious here :)
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.
Yes, setting MIRROR_IMAGES for IPv4 case technically speaking will let this change from #15639 work properly, but it will also fill the private registry with the release images - something that is not really required for the IPv4 case.
The real feature from dev-scripts that we're really interested into it's just the local private registry, to be consumed by the openshift-tests runner for some of the e2e tests (which requires ie images like "k8s.gcr.io/pause:3.2 that cannot be fetched by a Packet server)
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.
Ah yes I see now, thanks for clarifying, so we can enable the registry, but skip the release mirroring that happens in 04_setup_ironic.sh
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hardys 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 |
The introduction of ENABLE_LOCAL_REGISTRY will allow then to setup the local registry both explicitly - by setting such variable - or in all the other previous situations where it was required).
This approach will be used to expand the OpenShift CI tests coverage, since it's required to run the e2e suites in disconnected mode (see [1]). To do that, the test workflow will mirror the required images to the local dev-scripts private registry.
[1] openshift/origin#24887