-
Notifications
You must be signed in to change notification settings - Fork 402
[release-5.26] backport API changes copy
and list
components from upstream.
#2071
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
[release-5.26] backport API changes copy
and list
components from upstream.
#2071
Conversation
`c/image` uses `Instance(` API to get the required details of an instance while performing replication `Platform` of instance is needed hence `ListUpdate` must include platform. Needed by: containers#1987 Signed-off-by: Aditya R <arajan@redhat.com>
TryReusingBlob now contains a new option `RequiredCompression` which filters the blob by checking against a compression of the blob which is considerd to be resued, in case `RequiredCompression` is set and `info` of the blob being reused does not matches, no blob is returned. Signed-off-by: Aditya R <arajan@redhat.com>
Fixes: containers#2023 (comment). `assert.Equal` expects `assert.Equal(t, expected, actual)` instead of `assert.Equal(t, actual, expected)`. Ref:https://pkg.go.dev/github.com/stretchr/testify/assert#Assertions.Equal Signed-off-by: Aditya R <arajan@redhat.com>
Setting SystemContext.ArchitectureChoice to "" does not mean "match any/the first platform"; it's the default behavior of SystemContext, and it means "choose for the current runtime architecture". (Originally discussed in containers#1789 (comment) ) I.e. on amd64 these two test cases are redundant with the first two instances above, and on other architectures (notably ARM) they cause failures. So just drop them. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…gorithmNames There is a need to read annotations of a particular instance to get its compression. Expose `Annotations` as a read-only field. Needed By: containers#1987 Signed-off-by: Aditya R <arajan@redhat.com>
- Generally I discourage unsing named return values, it's easy to miss that it wasn't set - I have no idea what the "in the middle of a multi-streamed copy" paragraph refers to. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... so that we don't have to carry it around in extra parameters. Migrating individual functions will follow. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... so that we can eliminate three parameters. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... so that the code looks the same here and in possible callees. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Use options.Progress and ProgressInterval directly. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Use c.options.OciDecryptConfig directly. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Use c.options.OciEncryptConfig directly. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Use c.options.DownloadForeignLayers directly. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Use ic.c.options.OciEncryptLayers directly. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
So that we don't need to pass it around in a parameter. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Use copier.unparsedToplevel now that it exists. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... so that we don't need to carry it around in parameters. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
It is no longer used. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... so that we don't have so many unnamed return values, and we can manage the return values as a batch. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
…sult on a match ... so that the caller doesn't have to assemble it. Using a pointer-or-nil eliminates a separate boolean. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
... instead of three separate ones. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
If `UpdateCompressionAlgorithms` is set then `Annotations` map must be initialized if its not set. Signed-off-by: Aditya R <arajan@redhat.com>
Modifies `copy/single` to correctly use the feature from containers#2023, that is if compression is changed blob must be resued only if it matches required compression. Signed-off-by: Aditya R <arajan@redhat.com>
Create a wrapper around arguments of `copySingleImage` Signed-off-by: Aditya R <arajan@redhat.com>
After containers#2048 there is no room for copy/multiple to pass custom compressionFormat to copySingleImage while processing each instance, following PR introduces that functionality again and wraps options to simpler struct. Signed-off-by: Aditya R <arajan@redhat.com>
If its a regular copy callers might not set `compressionFormat` and `compressionLevel` in such case still honor compression which is set in DestinationCtx from copier. Signed-off-by: Aditya R <arajan@redhat.com>
* copy.Options now contains a new field `EnsureCompressionVariantExists map[string]int` which allows users to specify if they want to clone images with specified `compression`. Signed-off-by: Aditya R <arajan@redhat.com>
Signed-off-by: Aditya R <arajan@redhat.com>
Implement `instanceCopyClone` for multiple compression. Signed-off-by: Aditya R <arajan@redhat.com>
While performing copy, set a custom compression passed generated from prepareInstanceCopies. Signed-off-by: Aditya R <arajan@redhat.com>
Following option does not provides a way to detect and exclude compression if it already exists, this feature may be implemented in future. See: containers#1987 (comment) Signed-off-by: Aditya R <arajan@redhat.com>
After implementing `instanceCopyClone` now instance to be copied can exceed the original number of instances in the source, so amend report message to make it more meaningful. Signed-off-by: Aditya R <arajan@redhat.com>
Signed-off-by: Aditya R <arajan@redhat.com>
Signed-off-by: Aditya R <arajan@redhat.com>
* test multiple copy requests of same compression Signed-off-by: Aditya R <arajan@redhat.com>
Signed-off-by: Aditya R <arajan@redhat.com>
EnsureCompressionVariantsExist is only valid when working with a manifest list. Signed-off-by: Aditya R <arajan@redhat.com>
When copying multiple images i.e `instanceCopyClone` and no image exists in registry in such case if clones are prepared and copied first then original copies will reuse blobs from the clone which is unexpected since argument `requireCompressionFormatMatch` is by default false for `instanceCopyCopy` case. Problem described in following PR is easily reproduceable when working with tools such as buildah, because without this PR buildah will push `clones` first instead of originals and later on `originals` will reuse blobs from `clones`. Signed-off-by: Aditya R <arajan@redhat.com>
@mtrmac PTAL, I will create a bump version PR after this. |
Huh skopeo tests are failing I wonder why ? is it because I missed to backport some commit ? Edit: Looks like skopeo test on this branch is using skopeo from upstream, which has updated deps. Not sure if we can update those deps in this backport. @mtrmac Any suggestions ? |
@flouthoc On a stable branch, set Ideally we should have done that at time of branching … I’m afraid that rarely happens. |
Code LGTM… but per the other conversation, this should be a 5.27.0. Let me work on that… |
Signed-off-by: Aditya R <arajan@redhat.com>
Okay should I close this PR and create similar PR against |
@@ -6,9 +6,9 @@ env: | |||
#### Global variables used for all tasks | |||
#### | |||
# Name of the ultimate destination branch for this CI run | |||
DEST_BRANCH: "main" | |||
DEST_BRANCH: "release-5.26" |
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.
Great catch!
PR's which are backported
TryReusingBlobWithOptions
considerRequiredCompression
if set #2023ListUpdate
addimgspecv1.Platform
field #2029annotations
andCompressionAlgorithmNames
#2040UpdateCompressionAlgorithms
#2047*Options
and wrap arguments incopySingleImageOptions
#2050instanceCopyCopy
must be higher thaninstanceCopyClone
#2059