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.

Sign up for GitHub

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

Jump to bottom

Multi-arch images cannot be published to a repository with immutable tags #2299

Open
1 task
c0d1ngm0nk3y opened this issue Dec 11, 2024 · 5 comments · May be fixed by #2314
Open
1 task

Multi-arch images cannot be published to a repository with immutable tags #2299

c0d1ngm0nk3y opened this issue Dec 11, 2024 · 5 comments · May be fixed by #2314
Assignees
@jjbustamante
Labels
help wanted Need some extra hands to get this done. status/ready Issue ready to be worked on. type/enhancement Issue that requests a new feature or improvement.

Comments

@c0d1ngm0nk3y
Copy link
Contributor

Description

When the repository does not allow to overwrite existing tags, it is not possible to publish an image for multiple architectures.

Proposed solution

In case of of a multi-arch image, the actual manifest must not be written to the repository. Only the the layer. The manifest containing all digest will be created later.

Describe alternatives you've considered

The target repository needs to allow overwriting of published tags which might not always be desired / possible.

Additional context

  • This feature should be documented somewhere

Code in question:

pack/pkg/client/package_buildpack.go

Lines 83 to 99 in c5fadcd

targets = dist.ExpandTargetsDistributions(targets...)
for _, target := range targets {
digest, err := c.packageBuildpackTarget(ctx, opts, target, multiArch)
if err != nil {
return err
}
digests = append(digests, digest)
}
if opts.Publish && len(digests) > 1 {
// Image Index must be created only when we pushed to registry
return c.CreateManifest(ctx, CreateManifestOptions{
IndexRepoName: opts.Name,
RepoNames: digests,
Publish: true,
})
}

The tag will be created for each architecture, just to be overwritten later with the combined manifest.
@c0d1ngm0nk3y c0d1ngm0nk3y added status/triage Issue or PR that requires contributor attention. type/enhancement Issue that requests a new feature or improvement. labels Dec 11, 2024
@jjbustamante jjbustamante added status/ready Issue ready to be worked on. help wanted Need some extra hands to get this done. and removed status/triage Issue or PR that requires contributor attention. labels Dec 12, 2024
@jjbustamante jjbustamante self-assigned this Dec 16, 2024
@loewenstein-sap
Copy link

@jjbustamante Happy new year. Did you have a chance to look into this?

@jjbustamante
Copy link
Member

Hi @loewenstein-sap, I am working on it, but I still don't have the fix

@jjbustamante
Copy link
Member

@c0d1ngm0nk3y , @loewenstein-sap , @pbusko

I've been trying to find a way to fix this without making many changes.

If I am not wrong the problem is the following:
When we run pack build my-repo/my-image:my-tag for a multi-arch buildpack or builder, what pack is doing is:

  • For each target
    • pack will create an OCI image and push it to the registry as my-repo/my-image:my-tag, creating a manifest for each architecture.
  • At the end, pack will create an Image Index to combine all the previous manifest created

I think that process is not wrong, the problem appears when we uses the same image name my-repo/my-image:my-tag for each architecture, right? and when the registry doesn't allow overwriting an existent tag then an error is thrown?

Proposal

what if pack, maybe with a flag, adds a suffix os-architecture at the end given tag, something like:

  • my-repo/my-image:my-tag-linux-amd64 (manifest-1)
  • my-repo/my-image:my-tag-linux-arm (manifest-2)
  • my-repo/my-image:my-tag (image index)

ociimage (1)

Does this solution make sense to you? I think, I implemented it in this way during the PoC but I can't remember why I changed it, maybe because I tested against a mutable registry.

image source: go-containerregistry

@jjbustamante jjbustamante linked a pull request Jan 12, 2025 that will close this issue
Draft
2 tasks
@pbusko
Copy link
Contributor

pbusko commented Jan 13, 2025

I think that process is not wrong, the problem appears when we uses the same image name my-repo/my-image:my-tag for each architecture, right? and when the registry doesn't allow overwriting an existent tag then an error is thrown?

Correct. For comparison, if you run crane copy <src> <dst> for a multi-arch image, it writes a tag only once at the end. Ideally pack should follow the same principle and not create tags for individual images at all, but only during the CreateManifest call:

pack/pkg/client/package_buildpack.go

Line 94 in 2f1af85

return c.CreateManifest(ctx, CreateManifestOptions{

@jjbustamante
Copy link
Member

Thanks @pbusko !

Yeah, when we developed the multi-arch feature on pack, we did it with some constraints in mind, one of them was to keep working as it was to avoid breaking workflows for end-users. At that moment, the best way we found was to keep doing what we were doing (creating a manifest for each image) and just add an Image Index to combine them. Because the responsibility of writing the actual image to the Registry is done by imgutil and it is a critical code used by lifecycle too, I do not want to change the behavior there, I will move forward with the workaround I mentioned above, but, maybe we can create an improvement issue later to revisit the way multi-arch images are created in the registry 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jjbustamante jjbustamante

Labels
help wanted Need some extra hands to get this done. status/ready Issue ready to be worked on. type/enhancement Issue that requests a new feature or improvement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

WIP - adding a new flag --append-image-name-suffix to append [os]-[ar… buildpacks/pack
4 participants
@jjbustamante @c0d1ngm0nk3y @pbusko @loewenstein-sap