-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
ci: mirror ubuntu:22.04 to ghcr #135530
ci: mirror ubuntu:22.04 to ghcr #135530
Conversation
3a93e51
to
4b10bae
Compare
.github/workflows/ghcr.yml
Outdated
# Needed to write to the ghcr.io registry | ||
packages: write | ||
steps: | ||
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used pinned github action, because it's a good security practice. Let me know if you prefer @v4
.
I assume we will enable renovate or dependabot to update our GH Actions at some point 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some random action I'd probably also pin it, but for the official checkout
action, I'd just use @v4
, tbh. We already use it elsewhere in the repo like this, so it should be consistent.
fe05051
to
9d59f44
Compare
.github/workflows/ghcr.yml
Outdated
# Needed to write to the ghcr.io registry | ||
packages: write | ||
steps: | ||
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some random action I'd probably also pin it, but for the official checkout
action, I'd just use @v4
, tbh. We already use it elsewhere in the repo like this, so it should be consistent.
@@ -0,0 +1,75 @@ | |||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally I'm a fan of rewriting complex shell logic into Python or Rust, but in this case, it's literally curl github > crane && crane ...
, right? I would just include this in the YAML file directly, this file seems unnecessarily complex for the logic that it does. Since we just have a single image now, we don't even need to write a bash for loop :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll close this PR and open a new one so that we don't loose the python code in case we need it another time.
Anyway my opinion is that every bash script started with the thought "oh, it's simple, I'll just write it in bash", and than slowly the complexity increased, so I thought to start in python directly and try to do better error reporting, comments and using a temporary directory for the download so that the current directory isn't full of other useless files.
Plus I thought that the code could become complex in the case where:
- we might need to mirror more than one image.
- we might need to mirror images for arm. Not sure if crane allows to do this from an x86 machine.
This looks complex, but for example we could extract the functions to common modules that could be shared among other CI scripts and so in the end the code will read like plain english (similar to the main function)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I definitely agree that once it becomes complex, it might be a good idea to rewrite. But if it's very straightforward in Bash, I would keep it there, until the complexity balloons. Tbh, I would probably do just
docker pull <image>
docker pugh ghcr.io/<image>
if it works :) But maybe crane manages some extra things on top.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I haven't checked the crane source code 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to docker pull and push. The difference is that crane syncs all available architectures.
For example, compare
- https://github.com/marcoieni/dockerhub-mirror/pkgs/container/ubuntu/337169018?tag=25.04 (crane)
- https://github.com/marcoieni/dockerhub-mirror/pkgs/container/ubuntu/337792099?tag=18.04 (created with simple pull and push at this commit)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, thanks for trying! Well, for now we only need x64, so I'd be fine with just staying with the simplest option and just doing pull/push, which is easy to understand (vs using an external tool). We can switch in the future if we also need to mirror ARM images.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's still useful to mirror all architectures: https://github.com/rust-lang/rust/pull/135574/files#r1918185315
b38aba0
to
04295f6
Compare
opened #135574 |
This PR mirrors the
ubuntu:22.04
image from DockerHub to ghcr.io.Tested here. In my repository, I tried pushing the ubuntu 25.04 image, which you can see it was pushed here.
Why is this needed
The PR job mingw-check-tidy always downloads the base layer from DockerHub, while the other jobs rely on the cache to download the base layer.
To avoid the risk of being rate limited by DockerHub, we want to download the base layer from ghcr.io, which doesn't have rate limits on pulls.
Agreed in zulip.
Next steps
Once this workflow runs, we can edit the
mingw-check-tidy
Dockerfile to use it.Related to rust-lang/infra-team#176
r? @Kobzol