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

Refactor mirror script for multiarch #27

Merged
merged 1 commit into from
Sep 15, 2020
Merged

Refactor mirror script for multiarch #27

merged 1 commit into from
Sep 15, 2020

Conversation

brandond
Copy link
Member

@brandond brandond commented Aug 28, 2020

Uses skopeo + docker buildx, with a Dapper wrapper to build the tools image. Should be a drop-in replacement for the current script.

This will push single-arch images into ${TAG}-${ARCH}${VARIANT}, and then add them to the manifest list at ${TAG}. It reads the actual image arch and variant from the manifest, which should resolve issues with upstreams incorrectly tagging amd64 images as arm64, and vice versa.

Since we're using skopeo, we don't actually have to store copies of the images locally in order to pull/push/tag. This should speed things up slightly.

I have tested this by running it against my personal org; it successfully copied all the images from the list.

Signed-off-by: Brad Davidson brad.davidson@rancher.com

@brandond brandond mentioned this pull request Aug 28, 2020
@deniseschannon deniseschannon removed their request for review August 28, 2020 23:29
Copy link
Contributor

@superseb superseb left a comment

Choose a reason for hiding this comment

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

What are the expected changes for images already present at the destination repo?

@brandond
Copy link
Member Author

brandond commented Aug 31, 2020

@superseb

  • The arch-specific tag behavior varies:
    • If the upstream tag is a manifest list, all amd64/arm64/arm images referenced will be copied as ${TAG}-${ARCH}${VARIANT}
    • If the upstream tag is a manifest (single arch) for amd64/arm64/arm, it will be copied as ${TAG}-${ARCH}${VARIANT}
  • ${TAG} will be replaced with a manifest list that points at images for whatever architectures we found upstream

For the few multi-arch manifest lists currently present in our mirror, any architectures not found upstream will not be available in ${TAG} after the script runs. ${TAG}-${ARCH} will not be affected, since we only push to that if it's found upstream.

I don't currently search around for alternative architectures in upstream ${TAG}-${ARCH} combinations - hopefully anyone with multi-arch support is pushing manifest lists to ${TAG} by now.

@superseb
Copy link
Contributor

I'm fine with this as long as there is no change on our side that causes images to be repulled in existing setups (or causes differences between nodes when new nodes are added)

@brandond
Copy link
Member Author

brandond commented Aug 31, 2020

@superseb I am not aware of a good tool to compare repo contents. I could probably hack something together to compare the manifests and see if the layers change if @cjellick is down with that.

This is all somewhat complicated by the fact that the script will push a manifest list to the base tag even if there is only a single supported architecture. This makes directly comparing tags difficult, since I have to parse the new manifest lists and figure out which architectures (and variants) to to compare to in the existing repo to confirm that nothing's changed.

@brandond
Copy link
Member Author

brandond commented Sep 1, 2020

OK, I pulled together a script to compare images, and ran it against both the current rancher org, and my personal org (created using the new script). It looks like new script should actually do a better job of mirroring, as a couple of the current rancher images have different IDs for the same layer content due to the docker pull/docker push reordering elements in the manifest JSON.

The actual resulting layers are the same across the board though; it's only the manifest checksums that are different.

The results are attached:
rancher.txt
brandond.txt

@superseb
Copy link
Contributor

superseb commented Sep 2, 2020

So layers wont change, so pulling or pushing images won't result in a difference on existing nodes from clusters or adding a new node to an existing cluster won't result in a difference between images.

And as we only use manifest when mirroring Linux and Windows images (https://github.com/rancher/rancher/releases/download/v2.4.7/rancher-load-images.sh), we can confirm that nothing will change when we start using this script.

superseb
superseb previously approved these changes Sep 2, 2020
@brandond
Copy link
Member Author

brandond commented Sep 12, 2020

I made a couple modifications:

  • Added a PR template to clarify that new tags should be added to the list, rather than replacing existing entries
  • Modified the script so that it does not copy images between repos if they haven't changed.

I believe this should address the concerns from our meeting last week.

Copy link
Contributor

@Oats87 Oats87 left a comment

Choose a reason for hiding this comment

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

What is this script going to do with images that exist in our mirrored repos today via the old version of this script, i.e. no manifest list was pushed to docker hub even though there is one at the source? Is it going to then try and push over all of the prod images today that don't match the manifest list source?

@cjellick
Copy link
Contributor

cjellick commented Sep 15, 2020

I think this script is the way to move forward, but I just want to be able to confidently say "we will not overwrite any images when we merge this." If I can say that, we can greatly shrink the risk and qa impact here.

Maybe there is a super simple fix for this? What if we just commented out all the images that are in the image list thus far and add a comment above them that says something like

 # These images were pushed with an older version of the script that did not support multi-arch. They are being left in the list for an easy visual historical record, but are being commented out to ensure they won't be repushed by the script

We would of course have to support skipping comment lines in the processing script.

@brandond
Copy link
Member Author

brandond commented Sep 15, 2020

The current behavior is that it will convert the tags to manifest lists. The images themselves will not change (as confirmed by the compare output I shared earlier) but the tags will be updated to point at manifest lists that refer to the existing images.

If we want to make a clean break, I'll update this PR to include comment support, and start with a clean list so that none of the existing images are touched.

@cjellick
Copy link
Contributor

For the purpose of getting this out the door (hopefully today so that it can be used to push the multi-arch busybox image), I'd be fine if you just moved all the current images to another file called images-list-legacy. Comment support could wait.

cjellick
cjellick previously approved these changes Sep 15, 2020
@cjellick
Copy link
Contributor

giving the go-ahead to merge with just my approval

Oats87
Oats87 previously approved these changes Sep 15, 2020
.drone.yml Outdated Show resolved Hide resolved
Dockerfile.dapper Outdated Show resolved Hide resolved
Uses skopeo + docker buildx, with a Dapper wrapper to build the tools
image. Should be a drop-in replacement for the current script.

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
@brandond brandond merged commit feffb23 into rancher:master Sep 15, 2020
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.

4 participants