Skip to content
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 RD_USE_GHCR_IMAGES option to BATS to pull images from ghcr.io #4917

Merged
merged 1 commit into from
Jun 29, 2023

Conversation

jandubois
Copy link
Member

This avoids hitting the pull rate limit when running multiple full BATS runs in sequence, especially if not logged into Docker Hub.

Pull rate limit for docker.io is 100 pulls / 6 hours, or twice that when authenticated.

@jandubois jandubois force-pushed the bats-ghcr branch 2 times, most recently from 5f76b53 to 3db7552 Compare June 9, 2023 02:12
@jandubois jandubois changed the title Add RD_USE_GHCR option to BATS to pull images from ghcr.io Add RD_USE_GHCR_IMAGES option to BATS to pull images from ghcr.io Jun 9, 2023
@jandubois jandubois force-pushed the bats-ghcr branch 2 times, most recently from 9ad513a to 92b741d Compare June 11, 2023 20:04
Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Needs an entry in the main README file
  • Needs an upfront check that skopeo has been installed when set
  • Needs to be reviewed by someone with ghcr.io credentials

@jandubois
Copy link
Member Author

jandubois commented Jun 29, 2023

  • Needs an entry in the main README file

What do you expect to be documented in the README? All the variables are kind-of documented in tests/helpers/defaults.bash, and the helper script has comments inside on how to run it.

  • Needs an upfront check that skopeo has been installed when set

Why? The script runs with set -o errexit, so you will get an error like

-bash: skopeo: command not found

and then the script exits. What would you do differently in the check?

Note that the mirror script only needs to be run when we add another image in the tests/helpers/images.bash file. The normal BATS user never has to run this.

I think I misunderstood the comment, and you are talking about the BATS tests. They don't use skopeo; it is only used by the mirror script.

  • Needs to be reviewed by someone with ghcr.io credentials

Or you could just trust that it works by checking https://github.com/orgs/rancher-sandbox/packages?tab=packages&q=bats to see that all the images have been copied. I did run the script to make those copies.

You'll notice that I've modified the PR to put all images under ghcr.io/rancher-sandbox/bats to make them easier to find. I've deleted the previous copies that were polluting the top-level namespace.

I made additional changes to the "allowed images" logic, so you may want to re-review those parts of the PR.

@ericpromislow
Copy link
Contributor

Fails on Linux:

$ RD_LOCATION=npm RD_USE_GHCR_IMAGES=true bats bats/tests/{containers,registry}
allowed-images.bats
   ===== containers/allowed-images =====
 ✓ start
   Install location:         npm
   Resources path:           /home/ericp/workspace/rancher/desktop/resources
   
   Container engine:         containerd
   Mount type:               reverse-sshfs
   Using image allow list:   true
   
   Capturing logs:           false
   Taking screenshots:       false
   Using ghcr.io images:     true
 ✓ update the list of patterns first time
 ✗ verify pull nginx succeeds
   (from function `nerdctl' in file bats/tests/helpers/commands.bash, line 74,
    from function `ctrctl' in file bats/tests/helpers/commands.bash, line 50,
    in test file bats/tests/containers/allowed-images.bats, line 17)
     `ctrctl pull --quiet "$IMAGE_NGINX"' failed
   time="2023-06-29T19:33:21Z" level=info msg="trying next host" error="failed to do request: Head \"https://ghcr.io/v2/rancher-sandbox/bats/nginx/manifests/latest\": proxyconnect tcp: dial tcp 127.0.0.1:3128: connect: connection refused" host=ghcr.io
   time="2023-06-29T19:33:21Z" level=error msg="server \"ghcr.io\" does not seem to support HTTPS" error="failed to resolve reference \"ghcr.io/rancher-sandbox/bats/nginx:latest\": failed to do request: Head \"https://ghcr.io/v2/rancher-sandbox/bats/nginx/manifests/latest\": proxyconnect tcp: dial tcp 127.0.0.1:3128: connect: connection refused"
   time="2023-06-29T19:33:21Z" level=info msg="Hint: you may want to try --insecure-registry to allow plain HTTP (if you are in a trusted network)"
   time="2023-06-29T19:33:21Z" level=fatal msg="failed to resolve reference \"ghcr.io/rancher-sandbox/bats/nginx:latest\": failed to do request: Head \"https://ghcr.io/v2/rancher-sandbox/bats/nginx/manifests/latest\": proxyconnect tcp: dial tcp 127.0.0.1:3128: connect: connection refused"
   Error: exit status 1
...

Another failure:

 ✗ can run kubectl
   (from function `kubectl_exe' in file bats/tests/helpers/commands.bash, line 67,
    from function `kubectl' in file bats/tests/helpers/commands.bash, line 64,
    in test file bats/tests/containers/allowed-images.bats, line 58)
     `kubectl run nginx --image="${IMAGE_NGINX}:latest" --port=8080' failed
   Error from server (Forbidden): pods "nginx" is forbidden: error looking up service account default/default: serviceaccount "default" not found

ericpromislow
ericpromislow previously approved these changes Jun 29, 2023
Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up to you to double-quote the expressions in ghcr-mirror.sh . Everything else looks good, and tests finally passed.

This avoids hitting the pull rate limit when running multiple full
BATS runs in sequence, especially if not logged into Docker Hub.

Pull rate limit for docker.io is 100 pulls / 6 hours, or twice that
when authenticated.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>
Copy link
Contributor

@ericpromislow ericpromislow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jandubois jandubois enabled auto-merge June 29, 2023 21:24
@jandubois jandubois merged commit 1db0e9c into rancher-sandbox:main Jun 29, 2023
@jandubois jandubois deleted the bats-ghcr branch June 29, 2023 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants