copy: add OmitPrimaryManifestUpdateIfUnchanged option - #1033
Open
satwiksps wants to merge 1 commit into
Open
Conversation
Author
|
Hi @mtrmac ,I added optimization check for top-level manifests but skipping overhead for child images (pushed by digest), just as you suggested. |
mtrmac
reviewed
Jul 28, 2026
mtrmac
left a comment
mtrmac
left a comment
Contributor
There was a problem hiding this comment.
Thanks!
- I think this would be rather cleaner as a separate
copier.putManifesthelper, or something like that. E.g. themanifestList = attemptedManifestListflow should not be duplicated. - That helper should document the performance rationale of not always doing this (per #918)
| // is slightly pessimistic if the destination image doesn't exist, or is not equivalent. | ||
| OptimizeDestinationImageAlreadyExists bool | ||
|
|
||
| // OmitManifestUpdateIfUnchanged, if set, skips the manifest update (PutManifest) if the |
Contributor
There was a problem hiding this comment.
Callers don’t know / don’t care what PutManifest is.
| // OmitManifestUpdateIfUnchanged, if set, skips the manifest update (PutManifest) if the | ||
| // destination already contains exactly the same manifest. This is useful for registries | ||
| // which reject writes to existing tags (tag immutability). | ||
| OmitManifestUpdateIfUnchanged bool |
Contributor
There was a problem hiding this comment.
This option name does not match what the implementation does, nor what the user actually wants.
Copying an image that is already at the destination still writes the image's primary manifest unconditionally. Registries which enforce tag immutability reject that write, so an otherwise complete no-op copy fails against them. Add an option that makes the copy read the manifest currently stored at the destination and skip the write when it is byte-for-byte identical. The check lives in a new (*copier).putManifest helper used by both the single-image and the manifest-list paths, so the two cannot drift apart. The helper only does this for the primary manifest: instances of a manifest list are written by digest, which such registries do not restrict, and checking each of them would cost one extra read per instance for nothing. The option name says so, since the behaviour is not "omit any unchanged manifest update". Fixes: podman-container-tools#918 Signed-off-by: Satwik Sai Prakash Sahoo <sahoospsatwik@gmail.com>
satwiksps
force-pushed
the
issue-918-omit-manifest-update
branch
from
August 4, 2026 00:56
a097491 to
e7f6b11
Compare
satwiksps
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Copying an image that is already at the destination still writes the image's primary manifest unconditionally. registries that enforce tag immutability (Harbor, Artifactory, Quay) reject that write, so a copy that changes nothing fails at the very last step, even though every child manifest was correctly skipped
This adds
OmitPrimaryManifestUpdateIfUnchangedtocopy.Options. when set, the copy reads the manifest currently stored at the destination and skips the write if it is byte-for-byte identical. check lives in a new(*copier).putManifesthelper that bothsingle.goandmultiple.gocall, so the two paths can't drift apart and themanifestList = attemptedManifestListflow isn't duplicated. deliberately only applies to the primary manifest. Instances of a manifest list are written by digest, which those registries don't restrict, so checking each one would cost an extra read per instance for nothing, the helper documents that rationale. The option name says "Primary" for the same reason: "omit any unchanged manifest update" wouldn't be accuratecopy_test.gocovers the option on and off, changed/unchanged/absent destination manifests, and that instances are never skipped