[release-5.27] Preparing 5.27 backport #2075
Merged
Conversation
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
This was referenced Aug 4, 2023
5d4fd76 to
c04b0f7
Compare
|
Rebased with updates from #2071. |
|
@mtrmac Could you point this exact PR to a newer |
`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>
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>
Signed-off-by: Aditya R <arajan@redhat.com> Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Signed-off-by: Miloslav Trmač <mitr@redhat.com>
c04b0f7 to
22f70e8
Compare
|
Redirected to the 5.27 branch, rebased to be unambiguously after the 5.27.0-dev version bump. (Contra #2071, |
flouthoc
approved these changes
Aug 6, 2023
|
@rhatdan Could you force merge this please. |
|
I think that’s just a warning… but Cc: @cevich — this introduces a new Force-merging now, hopefully I didn’t screw this up. |
|
@mtrmac Thanks. |
cevich
reviewed
Aug 7, 2023
| @@ -6,7 +6,7 @@ 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.
@mtrmac shouldn't this be release-5.27?
There was a problem hiding this comment.
This may not be important, IIRC (though my memory is really fuzzy), we don't actually use this value anywhere in any CI scripts.
There was a problem hiding this comment.
@mtrmac shouldn't this be
release-5.27?
Totally. #2084
Thanks!
we don't actually use this value anywhere in any CI scripts.
It’s used for the git-validation command run in validate_task. In this case, where release-5.27 is a continuation of release-5.26, it is just a small optimization; in the general case it’s a bit more complex (but it should only matter in rare cases, e.g. if git-validation rules were tightened.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Looking at #2074, CI is not running on the newrelease-5.27branch yet.This:
Includes [release-5.27] Test the release-5.27 branch against the 1.13 branch of Skopeo #2074 , to hopefully make tests pass with this older codebasecopyandlistcomponents from upstream. #2071 backportversion.goto 5.27.0I.e. this should be all that is necessary to tag a tested 5.27.0 release — except that this PR is filed against the 5.26 branch, to trigger a test run.
If the test passes, this can be merged into the 5.27 branch, and (the right commit) tagged as c/image 5.27.0. (Alternatively, if the move to 5.27 were unreasonable, a variant of #2074 can be used on the
release-5.26branch.