Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions 02_configure_host.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,11 @@ fi

export ANSIBLE_FORCE_COLOR=true


# 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=")}}}
Copy link

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?

Copy link
Member Author

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.

Copy link
Member Author

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.

Copy link

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 :)

Copy link
Member Author

@andfasano andfasano Feb 9, 2021

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)

Copy link

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


if [[ ! -z "${ENABLE_LOCAL_REGISTRY}" ]]; then
setup_local_registry
fi

Expand Down Expand Up @@ -212,7 +215,7 @@ echo "${PROVISIONING_HOST_EXTERNAL_IP} ${LOCAL_REGISTRY_DNS_NAME}" | sudo tee -a
# Remove any previous file, or podman login panics when reading the
# blank authfile with a "assignment to entry in nil map" error
rm -f ${REGISTRY_CREDS}
if [[ ! -z "${MIRROR_IMAGES}" || $(env | grep "_LOCAL_IMAGE=") || ! -z "${ENABLE_CBO_TEST}" ]]; then
if [[ ! -z "${ENABLE_LOCAL_REGISTRY}" ]]; then
# create authfile for local registry
sudo podman login --authfile ${REGISTRY_CREDS} \
-u ${REGISTRY_USER} -p ${REGISTRY_PASS} \
Expand Down
3 changes: 3 additions & 0 deletions config_example.sh
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ set -x
# for an IPv4 install.
#export MIRROR_IMAGES=true

# Ensure that the local registry will be available
#export ENABLE_LOCAL_REGISTRY=true

# Switch to upstream metal3-io ironic images instead of openshift ones.
#export UPSTREAM_IRONIC=true

Expand Down