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

use skopeo to preserve multi-arch lists during the tag operation #4592

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Not-Code
Copy link

Description

This is an attempt to resolve #4591 using skopeo instead of docker: skopeo has the capability to clone list of images.

ubuntu-latest runner that Kimai is actually using comes with skopeo already preinstalled and skopeo should fallback to credentials created in step Login to DockerHub.

Before approving this PR, @tobybatch or @kevinpapst should verifiy that everything is fine because I'm not able to test a complete workflow.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I verified that my code applies to the guidelines (composer code-check)
  • I updated the documentation (see here)
  • I agree that this code is used in Kimai (see license)

Copy link

codecov bot commented Jan 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (12ef19d) 88.16% compared to head (21b63a4) 88.15%.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #4592      +/-   ##
============================================
- Coverage     88.16%   88.15%   -0.01%     
  Complexity     8781     8781              
============================================
  Files           784      784              
  Lines         29361    29361              
============================================
- Hits          25885    25883       -2     
- Misses         3476     3478       +2     

see 2 files with indirect coverage changes

@kevinpapst
Copy link
Member

kevinpapst commented Jan 27, 2024

Ok, I did some reading (as I never heard about skopeo), but it sounds like a good idea.
Usually I would prefer to stick to the most native option (here the docker commands), but from what I understand docker-cli is rather limited in its feature set.

The --all flag copies all architectures: https://github.com/containers/skopeo/blob/main/docs/skopeo-copy.1.md
And skopeo doesn't need a local clone of the image, so it should also be much faster.

I give my approval. @tobybatch any input / objections from your end?

@Not-Code
Copy link
Author

Yes, if it can help this is a question about this topic on stackoverflow.

With the current runner/actions I think that the only available options are skopeo or docker buildx.

In any case before approving this change I think that it's better that @tobybatch tests it locally, otherwise in case of any issues this will break the release (it wasn't possible for me to test the E2E so I'm trusting the documentation of skopeo and the sourcecode of the docker login action, for the fact that skopeo will be able to do the push).

@Not-Code
Copy link
Author

And skopeo doesn't need a local clone of the image, so it should also be much faster.

We will see about this. But after a good result with the flag --all we can try to substitute that with --multi-arch index-only. If the index-only works, that for sure should not clone the image.

@kevinpapst
Copy link
Member

@tobybatch I can only look as such stuff from a bird view. Can you review please?

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing ARM64/AArch64 docker images
3 participants