Docker hub mirror switch #1900
Docker hub mirror switch #1900
Conversation
wainersm
added
test_e2e_libvirt
There was a problem hiding this comment.
yeah, it's 2024 and docker.io is still hurting us :(
Thanks @stevenhorsman !
|
Re-triggered CI to see if it passes this time. |
| @@ -472,10 +472,10 @@ func DoTestPodToServiceCommunication(t *testing.T, e env.Environment, assert Clo | |||
| func DoTestPodsMTLSCommunication(t *testing.T, e env.Environment, assert CloudAssert) { | |||
| clientPodName := "curl" | |||
| clientContainerName := "curl" | |||
| clientImageName := "docker.io/curlimages/curl:8.4.0" | |||
| clientImageName := "quay.io/curl/curl:8.4.0" | |||
There was a problem hiding this comment.
In some other places in the code we use quay.io/curl/curl:latest. Can we use the same curl image ? Better is to use a version tag than latest to avoid issues with change of command line options etc.
Also can all these images be moved to separate file as constants ? It'll help to override the images if needed.
I understand the scope of this PR is to just switch out of docker hub mirror, but since you are anyway touching these files thought of suggesting it :-)
This can be done later on as well.
There was a problem hiding this comment.
Sure, that makes sense. I was trying to keep versions the same incase that was important, but happy to pin them and separating out as constants is good too. Let me mark this as draft until I get a chance to get around to this.
There was a problem hiding this comment.
@stevenhorsman are you thinking in add the version in versions.yaml or just a simple constant in code?
I'm okay with a constant in code, but if you want the other approach then you want to use 5169aec#diff-a3bd85dcf59fe8d1cf225b0edbbd31d0390a9588a53b44941ad49d14d9f562eb
There was a problem hiding this comment.
I'm on the fence with this. I can see the merit of having the versions defined in a single place, but it also doesn't feel quite right to me to mix up test dependencies with the built dependencies in the versions file (which is weird as lots of toolchains do it!).
There was a problem hiding this comment.
May be have a separate versions file for test deps and placed inside the test folder ?
There was a problem hiding this comment.
Yeah, that sounds like a reasonable plan - I'm just sticking to a constant in the test go code for now as Wainer's change that we can piggyback on top of hasn't been merged yet.
There was a problem hiding this comment.
yup, I also don't like the mix up as in Kata. Good idea @bpradipt !
There was a problem hiding this comment.
So I've defrosted this PR and updated it to add a test versions.yaml with the test images and code to read from there. It feels like quite a bit of extra code, for maybe not a tons better user experience.
The main issue I have with this is that reading from yaml seems to inherently require the need to throw errors and some of our code in common.go for specifying the busybox image doesn't have a testing.T object or any existing errors thrown, so I don't know if you have any suggestions of how to handle this apart from added return errors and error handling to a whole bunch of methods?
stevenhorsman
force-pushed
the
docker-hub-mirror-switch
branch
3 times, most recently
from
020ee6e
to
02dc02b
Compare
|
The failed test: has been unstable? I looked the last 4 nightly jobs that failed, that test passed. So no conclusion on that matter ;) |
Maybe the problem is with the image change then? When I get around to it I'll try with the docker hub and quay versions of the nginx image locally to see if there is a difference |
stevenhorsman
force-pushed
the
docker-hub-mirror-switch
branch
from
02dc02b
to
279b0b4
Compare
stevenhorsman
force-pushed
the
docker-hub-mirror-switch
branch
4 times, most recently
from
a691b89
to
ae8f765
Compare
| registry: "quay.io/prometheus/busybox" | ||
| tag: "latest" | ||
| curl: | ||
| registry: "quay.io/curl/curl" |
There was a problem hiding this comment.
You can use the following for nginx - quay.io/confidential-containers/test/nginx:latest
I created the multi-arch manifest including amd64 and s390x. We can keep it updated as needed for tests.
For unprivileged nginx you can use - "quay.io/nginx/nginx-unprivileged:latest"
There was a problem hiding this comment.
Updated. Thanks, sjeninng's nginx is getting old now!
stevenhorsman
force-pushed
the
docker-hub-mirror-switch
branch
from
ae8f765
to
3ce6b6f
Compare
Add a central versions file for all test dependencies and initially add our test container image versions and the code to process it. In some e2e tests, particularly the docker-provider ones, when you do multiple runs we are likely to hit the rate limits of docker hub, so try switching to the coco version of nginx hosted on quay.io, [quay mirror](https://hub.docker.com/r/curlimages/curl) of the curl container image instead to reduce our usage of docker.io. Signed-off-by: stevenhorsman <[email protected]>
stevenhorsman
force-pushed
the
docker-hub-mirror-switch
branch
from
3ce6b6f
to
9ac2489
Compare
In some e2e tests, particularly the docker-provider ones, when you do multiple
runs we are likely to hit the rate limits of docker hub, so try switching to the
official mirrors of nginx and curl container images instead to reduce our usage of docker.io.