Migrate away from docker.io #7534

edsantiago · 2020-09-02T17:57:12Z

CI and system tests currently pull some images from docker.io.
Eliminate that, by:

building a custom image containing much of what we need
for testing; and
copying other needed images to quay.io

The principal change is to create a new quay.io/libpod/testimage,
using the new test/system/build-testimage script, instead of
relying on quay.io/libpod/alpine_labels. We also switch to
using a hardcoded :YYYYMMDD tag, instead of :latest, in an
attempt to futureproof our CI. This image includes 'httpd'
from busybox-extras, which we use in our networking test
(previously we had to pull and run busybox from docker.io).

The testimage can and should be extended as needed for future
tests, e.g. adding test file content or other useful tools.

For the '--pull' tests which require actually pulling from
the registry, I've created an image with the same name but
tagged :00000000 so it will never be pulled by default.
Since this image is only used minimally, it's just busybox.

Unfortunately there remain two cases we cannot solve in
this tiny alpine-based image:

docker registry
systemd

For those, I've (manually) run:

podman pull [ docker.io/library/registry:2.7 | registry.fedoraproject.org/fedora:31 ]
podman tag !$ quay.io/...
podman push !$

...and amended the calling tests accordingly.

I've tried to make the the smallest reasonable diff, not the
smallest possible one. I hope it's a reasonable tradeoff.

Signed-off-by: Ed Santiago [email protected]

edsantiago · 2020-09-02T17:59:09Z

Marked WIP because I'd like feedback on the image name: testimage seems too undescriptive.

Until naming is resolved, images live in my personal quay.io namespace instead of quay.io/libpod.

edsantiago · 2020-09-02T18:23:34Z

test/system/500-networking.bats

@@ -12,7 +12,7 @@ load helpers
    random_2=$(random_string 30)

    HOST_PORT=8080
-    SERVER=http://localhost:$HOST_PORT
+    SERVER=http://127.0.0.1:$HOST_PORT


This is completely weird. When running podman as root, 'localhost' no longer seems to work, at least with alpine: wget barfs with ECONNREFUSED. It works fine with rootless. I gave up trying to track it down, and explicit IPv4 home seems like a better idea anyway.

Weird, but localhost is hard coded within container/image and podman to represent locally built images.

edsantiago · 2020-09-02T18:44:22Z

test/system/helpers.bash

-PODMAN_TEST_IMAGE_NAME=${PODMAN_TEST_IMAGE_NAME:-"alpine_labels"}
-PODMAN_TEST_IMAGE_TAG=${PODMAN_TEST_IMAGE_TAG:-"latest"}
+PODMAN_TEST_IMAGE_USER=${PODMAN_TEST_IMAGE_USER:-"edsantiago"}
+PODMAN_TEST_IMAGE_NAME=${PODMAN_TEST_IMAGE_NAME:-"testimage"}


Please offer suggestions here.

I am fine with testimage?

Okay, going once...

rhatdan

A few questions, and need to get rid of the edsantiago references
but overall LGTM

rhatdan · 2020-09-03T10:52:08Z

test/system/260-sdnotify.bats

@@ -116,8 +116,7 @@ function _assert_mainpid_is_conmon() {
 @test "sdnotify : container" {
    # Sigh... we need to pull a humongous image because it has systemd-notify.
    # FIXME: is there a smaller image we could use?
-    _FEDORA=registry.fedoraproject.org/fedora:31
-
+    local _FEDORA="$PODMAN_TEST_IMAGE_REGISTRY/$PODMAN_TEST_IMAGE_USER/fedora:31"


Should we update this to 32? Or eliminate the version altogether?

We need explicit :31 because fedora:latest removed systemd-notify, causing an uninterruptible hang in the tests. Shame on me for not including a code comment. I will fix that.

rhatdan · 2020-09-03T10:54:03Z

test/system/helpers.bash

-PODMAN_TEST_IMAGE_NAME=${PODMAN_TEST_IMAGE_NAME:-"alpine_labels"}
-PODMAN_TEST_IMAGE_TAG=${PODMAN_TEST_IMAGE_TAG:-"latest"}
+PODMAN_TEST_IMAGE_USER=${PODMAN_TEST_IMAGE_USER:-"edsantiago"}
+PODMAN_TEST_IMAGE_NAME=${PODMAN_TEST_IMAGE_NAME:-"testimage"}


I am fine with testimage?

rhatdan · 2020-09-03T10:54:52Z

test/system/build-testimage

+#    busybox-extras  : provides httpd needed in 500-networking.bats
+#
+podman rmi -f testimage &> /dev/null || true
+podman build --squash-all -t testimage - <<EOF


Would we want to switch this to --timestamp=0 or some other timestamp, to keep it consistent once this is available to Podman?

I considered that but couldn't think of a reason - it'll be useful to have a number of tools built-in, and it would be super useful to figure out a way some day to include registry and systemd, but if/when that happens we'll change the YMD tag. Do you see value in reproducible/comparable images?

openshift-ci-robot · 2020-09-03T10:55:32Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, 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~~ [rhatdan]

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

vrothberg · 2020-09-07T13:07:24Z

@edsantiago, what's the motivation to migrate away? I assume the latest policy changes of Docker Hub but it would be nice to mention it in the commit message for historical reasons.

vrothberg

Other than Dan's comments, LGTM

CI and system tests currently pull some images from docker.io. Eliminate that, by: - building a custom image containing much of what we need for testing; and - copying other needed images to quay.io (Reason: effective 2020-11-01 docker.io will limit the number of image pulls). The principal change is to create a new quay.io/libpod/testimage, using the new test/system/build-testimage script, instead of relying on quay.io/libpod/alpine_labels. We also switch to using a hardcoded :YYYYMMDD tag, instead of :latest, in an attempt to futureproof our CI. This image includes 'httpd' from busybox-extras, which we use in our networking test (previously we had to pull and run busybox from docker.io). The testimage can and should be extended as needed for future tests, e.g. adding test file content or other useful tools. For the '--pull' tests which require actually pulling from the registry, I've created an image with the same name but tagged :00000000 so it will never be pulled by default. Since this image is only used minimally, it's just busybox. Unfortunately there remain two cases we cannot solve in this tiny alpine-based image: 1) docker registry 2) systemd For those, I've (manually) run: podman pull [ docker.io/library/registry:2.7 | registry.fedoraproject.org/fedora:31 ] podman tag !$ quay.io/... podman push !$ ...and amended the calling tests accordingly. I've tried to make the the smallest reasonable diff, not the smallest possible one. I hope it's a reasonable tradeoff. Signed-off-by: Ed Santiago <[email protected]>

edsantiago · 2020-09-08T14:58:16Z

what's the motivation to migrate away? I assume the latest policy changes of Docker Hub but it would be nice to mention it in the commit message for historical reasons.

Yes, Docker Hub pull limit. I updated commit message to reflect that.

Since testimage seems OK, I've pushed those images to quay.io/libpod, made them public, updated the reference in helpers.bash, and pushed.

rhatdan · 2020-09-08T17:18:39Z

/lgtm

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 2, 2020

openshift-ci-robot requested review from baude and jwhonce September 2, 2020 17:57

edsantiago commented Sep 2, 2020

View reviewed changes

edsantiago mentioned this pull request Sep 2, 2020

APIv2: DELETE libpod/containers/foo?force=true: unlinkat: directory not empty #7535

Closed

rhatdan approved these changes Sep 3, 2020

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 3, 2020

vrothberg reviewed Sep 7, 2020

View reviewed changes

edsantiago force-pushed the bats branch from 5bb6bff to a9dbd2b Compare September 8, 2020 12:06

edsantiago changed the title ~~WIP: Migrate away from docker.io~~ Migrate away from docker.io Sep 8, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 8, 2020

openshift-ci-robot assigned rhatdan Sep 8, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2020

openshift-merge-robot merged commit 54a61e3 into containers:master Sep 8, 2020

edsantiago mentioned this pull request Feb 3, 2021

[v2.2.1-rhel] Backport fix for CVE-2021-20199 #9221

Merged

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate away from docker.io #7534

Migrate away from docker.io #7534

edsantiago commented Sep 2, 2020

edsantiago commented Sep 2, 2020

edsantiago Sep 2, 2020

rhatdan Sep 3, 2020

edsantiago Sep 2, 2020

rhatdan Sep 3, 2020

edsantiago Sep 3, 2020

rhatdan left a comment

rhatdan Sep 3, 2020

edsantiago Sep 3, 2020

rhatdan Sep 3, 2020

rhatdan Sep 3, 2020

edsantiago Sep 3, 2020

openshift-ci-robot commented Sep 3, 2020

vrothberg commented Sep 7, 2020

vrothberg left a comment

edsantiago commented Sep 8, 2020

rhatdan commented Sep 8, 2020

Migrate away from docker.io #7534

Migrate away from docker.io #7534

Conversation

edsantiago commented Sep 2, 2020

edsantiago commented Sep 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhatdan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Sep 3, 2020

vrothberg commented Sep 7, 2020

vrothberg left a comment

Choose a reason for hiding this comment

edsantiago commented Sep 8, 2020

rhatdan commented Sep 8, 2020