Explicitly ref. quay images for CI #4828

cevich · 2023-05-30T17:59:07Z

What type of PR is this?

/kind other

What this PR does / why we need it:

Commit cevich@beae564 updated the conformance test references to a
deprecated repository. However, by pointing at the docker-hub it
inadvertantly introduce a significant possibility for flakes. This is
because anonymous docker hub access is rate-limited by IP. We cannot
predict the IP used for CI VMs & Containers, any of which could be at or
close to the limit. Fix this by pointing explicitly at a
quay.io/libpod/centos repo. which is excluesively for use by CI, with
static images.

How to verify it

Conformance testing in CI will pass

Which issue(s) this PR fixes:

Special notes for your reviewer:

Ref: #4819

Does this PR introduce a user-facing change?

None

flouthoc

LGTM

edsantiago · 2023-05-30T18:07:28Z

LGTM at first glance, but might as well fix the instances in from.bats also?

cevich · 2023-05-30T18:10:59Z

LGTM at first glance, but might as well fix the instances in from.bats also?

Yes. I was in a bit of a panic to open this but found in contrib/cirrus/setup.sh that we are setting up a proxy to redirect to quay. That will cover this case, so there's really not a huge rush to get this PR in.

I'll add a commit to fix up references in bats. Thanks for the suggestion.

nalind · 2023-05-30T18:20:10Z

How does this overlap with mirroring in tests/registries.conf?

cevich · 2023-05-30T18:30:18Z

How does this overlap with mirroring in tests/registries.conf?

That file is put in place from contrib/cirrus/setup.sh. So that is what makes the current docker-hub references work fine in CI (including for conformance test). However, this is mostly due to luck: Where "library/centos/centos" in the docker hub happens to translate properly to the "libpod/centos" namespace in quay. I think it's desirable to update all the references to point at the quay images so we're not relying on luck.

Edit: proxy is to quay.io/libpod not quay.io/centos

cevich · 2023-05-30T18:49:06Z

Hmmm...thinking about this more, we maintain quay.io/libpod/centos:7 whereas somebody else runs centos/centos:7. There's a risk the later could change unexpectedly, but the former is treated (by practice) as "untouchable" for use in CI. So probably I should instead update all the references to quay.io/libpod/centos:7 to maintain the current behavior but reduce the "magic"?

nalind · 2023-05-30T19:08:37Z

That file is put in place from contrib/cirrus/setup.sh. So that is what makes the current docker-hub references work fine in CI (including for conformance test).

That file exists in the repository, so we shouldn't need to be overwriting it for CI. If CI is changing it, we need to update the copy in the repository, so that integration tests that aim to be usable outside of CI don't behave differently in hard-to-find ways when they're run outside of CI.

There's also a case for setting CONTAINERS_REGISTRIES_CONF to point to the same file for unit and conformance tests, which I don't think we currently do, but it's worth remembering that when the conformance tests build an image using dockerd, it's still almost certainly going to be ignored.

cevich · 2023-05-30T19:34:50Z

don't behave differently in hard-to-find ways when they're run outside of CI

Yes, I agree. CI doesn't change the file. I looked through the scripts and Makefile and it looks like for the most part we reference the tests/registries.conf file explicitly. Except...

conformance tests build an image using dockerd, it's still almost certainly going to be ignored

Right, excellent point - conformance tests use docker, not podman. So we do need the the conformance tests pointing explicitly at quay.io/libpod/centos:7 in all those Dockerfiles on main and in all the release branches where CI is running. Ugg 😞

I'm guessing there's no docker-equivalent for tests/registries.conf, is there?

nalind · 2023-05-30T21:31:14Z

I'm guessing there's no docker-equivalent for tests/registries.conf, is there?

Not that I'm aware of. The --registry-mirror option and its daemon.json equivalent only allow for substituting hostnames, and not more general rewriting of namespaces (from "library" to "libpod", for example).

cevich · 2023-05-31T15:14:20Z

@edsantiago PTAL (careful) at the second commit that touches all non-conformance test files. My hope is that it's optional in terms of needing to backport to all release branches. Though if you prefer I can squash everything down into a single commit.

cevich · 2023-05-31T15:19:08Z

Woops, force-push: Fixed missed conformance test update.

cevich · 2023-05-31T17:12:17Z

tests/helpers.bash

-            echo -n docker.io/library/"${img##*/}"
+            echo -n quay.io/libpod/"$img"
+        elif [[ quay.io/libpod/"${img##*/}" == "$img" ]] ; then
+            echo -n quay.io/libpod"${img##*/}"


@edsantiago I think maybe these test-failures are caused by my changes here. Should I have done:

Suggested change

echo -n quay.io/libpod"${img##*/}"

echo -n quay.io/libpod/"${img##*/}"

?

If I'm reading it right, can't this clause be dropped?

Ed seems busy, let me just try removing it...

No, I just have nothing to add. This is not my code, I don't even understand what it does or is supposed to do. I stand by anything @nalind says about it.

Oh okay, thanks for the update Ed.

cevich · 2023-06-01T15:04:37Z

force-push: Rebased and tried removing second clause in normalize_image_name().

TomSweeneyRedHat · 2023-06-01T19:52:31Z

LGTM
but the tests are not at all happy anywhere.

cevich · 2023-06-05T14:54:47Z

Sigh...I think maybe I need to break this PR up since the first commit (conformance test) fixes a problem occurring on CI for basically every release branch.

Commit beae564 updated the conformance test references to a deprecated repository. However, by pointing at the docker-hub it inadvertantly introduce a significant possibility for flakes. This is because anonymous docker hub access is rate-limited by IP. We cannot predict the IP used for CI VMs & Containers, any of which could be at or close to the limit. Fix this by pointing explicitly at a `quay.io/libpod/centos` repo. which is excluesively for use by CI, with static images. Signed-off-by: Chris Evich <cevich@redhat.com>

edsantiago

LGTM with two nits: one, I'd be happier if the commit title explicitly included "conformance tests" to make it clear that this doesn't touch bats tests; and second, below. Neither of them is worth blocking for, I'm just commenting in the unlikely event that you need to repush.

edsantiago · 2023-06-05T16:54:30Z

tests/conformance/testdata/Dockerfile.edgecases

@@ -1,3 +1,4 @@
+# Note: a registries.conf alias should redirect this to quay.io/libpod/busybox


The word "should" is inherently ambiguous and should not be used in documentation (see what I did there?) In this instance, it's not clear whether you mean "we hope this happens", or "we know this will happen because there is an existing registries.conf mapping", or "we should some day set up a registries.conf mapping". This is fine to fix in the followup PR.

rhatdan · 2023-06-05T19:19:15Z

/approve
/lgtm

openshift-ci · 2023-06-05T19:19:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, edsantiago, flouthoc, rhatdan

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~~ [cevich,flouthoc,rhatdan]

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

cevich requested a review from edsantiago May 30, 2023 17:59

openshift-ci bot added the approved label May 30, 2023

flouthoc approved these changes May 30, 2023

View reviewed changes

cevich mentioned this pull request May 30, 2023

[release-1.24] remove registry.centos.org #4827

Merged

cevich marked this pull request as draft May 30, 2023 18:11

openshift-ci bot added the do-not-merge/work-in-progress label May 30, 2023

cevich changed the title ~~Fix pulling from docker-hub in CI~~ [WIP] Fix pulling from docker-hub in CI May 30, 2023

cevich mentioned this pull request May 30, 2023

remove registry.centos.org since its decommissioned #4819

Merged

cevich changed the title ~~[WIP] Fix pulling from docker-hub in CI~~ [WIP] Explicitly ref. quay images for CI May 30, 2023

edsantiago mentioned this pull request May 30, 2023

[release-1.27] remove registry.centos.org #4826

Merged

cevich force-pushed the fix_ci_using_docker_hub branch from 9a9036c to c72f4ef Compare May 31, 2023 15:03

cevich force-pushed the fix_ci_using_docker_hub branch from c72f4ef to 6155bae Compare May 31, 2023 15:18

cevich commented May 31, 2023

View reviewed changes

cevich marked this pull request as ready for review June 1, 2023 15:00

cevich force-pushed the fix_ci_using_docker_hub branch from 6155bae to 190b8dc Compare June 1, 2023 15:03

cevich changed the title ~~[WIP] Explicitly ref. quay images for CI~~ Explicitly ref. quay images for CI Jun 1, 2023

openshift-ci bot removed the do-not-merge/work-in-progress label Jun 1, 2023

cevich force-pushed the fix_ci_using_docker_hub branch from 190b8dc to 5a83012 Compare June 5, 2023 14:56

cevich mentioned this pull request Jun 5, 2023

[WIP] Replace all docker hub references with quay #4847

Closed

edsantiago approved these changes Jun 5, 2023

View reviewed changes

openshift-ci bot assigned rhatdan Jun 5, 2023

openshift-ci bot added the lgtm label Jun 5, 2023

openshift-merge-robot merged commit c4af80e into containers:main Jun 5, 2023
32 checks passed

github-actions bot added the locked - please file new issue/PR label Sep 4, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explicitly ref. quay images for CI #4828

Explicitly ref. quay images for CI #4828

cevich commented May 30, 2023 •

edited

flouthoc left a comment

edsantiago commented May 30, 2023

cevich commented May 30, 2023

nalind commented May 30, 2023

cevich commented May 30, 2023 •

edited

cevich commented May 30, 2023

nalind commented May 30, 2023

cevich commented May 30, 2023

nalind commented May 30, 2023

cevich commented May 31, 2023

cevich commented May 31, 2023

cevich May 31, 2023

nalind May 31, 2023

cevich Jun 1, 2023

cevich Jun 1, 2023

edsantiago Jun 1, 2023

cevich Jun 1, 2023

cevich commented Jun 1, 2023

TomSweeneyRedHat commented Jun 1, 2023

cevich commented Jun 5, 2023

edsantiago left a comment

edsantiago Jun 5, 2023

rhatdan commented Jun 5, 2023

openshift-ci bot commented Jun 5, 2023

	echo -n quay.io/libpod"${img##*/}"
	echo -n quay.io/libpod/"${img##*/}"

		@@ -1,3 +1,4 @@
		# Note: a registries.conf alias should redirect this to quay.io/libpod/busybox

Explicitly ref. quay images for CI #4828

Explicitly ref. quay images for CI #4828

Conversation

cevich commented May 30, 2023 • edited

What type of PR is this?

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

flouthoc left a comment

Choose a reason for hiding this comment

edsantiago commented May 30, 2023

cevich commented May 30, 2023

nalind commented May 30, 2023

cevich commented May 30, 2023 • edited

cevich commented May 30, 2023

nalind commented May 30, 2023

cevich commented May 30, 2023

nalind commented May 30, 2023

cevich commented May 31, 2023

cevich commented May 31, 2023

cevich May 31, 2023

Choose a reason for hiding this comment

nalind May 31, 2023

Choose a reason for hiding this comment

cevich Jun 1, 2023

Choose a reason for hiding this comment

cevich Jun 1, 2023

Choose a reason for hiding this comment

edsantiago Jun 1, 2023

Choose a reason for hiding this comment

cevich Jun 1, 2023

Choose a reason for hiding this comment

cevich commented Jun 1, 2023

TomSweeneyRedHat commented Jun 1, 2023

cevich commented Jun 5, 2023

edsantiago left a comment

Choose a reason for hiding this comment

edsantiago Jun 5, 2023

Choose a reason for hiding this comment

rhatdan commented Jun 5, 2023

openshift-ci bot commented Jun 5, 2023

cevich commented May 30, 2023 •

edited

cevich commented May 30, 2023 •

edited