-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Update system tests to handle zstd:chunked images #24286
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mtrmac The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Ephemeral COPR build failed. @containers/packit-build please check. |
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.
Yeah, this is nowhere close to working, ignore for now.
Also [255] podman-kube@.service template with rollback
#| FAIL: global auto-update policy gets applied
#| expected: '.*podman-kube@-tmp-podman_bats.roXH3U-test.yaml.service,.* (test_pod-a),quay.io/libpod/testimage:20241010,false,registry.*' (using expr)
#| actual: 'podman-kube@-tmp-podman_bats.roXH3U-test.yaml.service,c104ff383fd7 (test_pod-a),quay.io/libpod/testimage:20241010,pending,registry'
#| > 'podman-kube@-tmp-podman_bats.roXH3U-test.yaml.service,aa204347c263 (test_pod-b),localhost/image:Kl8NYi3jxL,false,local'
51edef6
to
a644fe8
Compare
@edsantiago With this, Could you take a preliminary look to make sure I’m not missing/breaking the point of these tests, please? See individual commit messages for details. |
@mtrmac I've been peeking periodically and have seen no major cause for alarm. I will not be able to give this a thorough review until Monday. Apologies. |
@edsantiago Thanks! The peeks are reassuring already, and I need to make this pass first. There’s no rush at all. |
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.
I think the 120 tests need a complete revamp: my initial decision to use $IMAGE
has backfired. Next week I will look into a different safer approach.
The test got the stores RW status backwards. Before zstd:chunked, both image IDs should be the same, so this used to make no difference. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
When looking up the current-store image ID, do that from the same output where we verify that the ID is from the current store, instead of listing images twice. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
a644fe8
to
2832b1b
Compare
… methods Historically, non-schema1 images had a deterministic image ID == config digest. With zstd:chunked, we don't want to deduplicate layers pulled by consuming the full tarball and layers partially pulled based on TOC, because we can't cheaply ensure equivalence; so, image IDs for images where a TOC was used differ. To accommodate that, compare images using their configs digests, not using image IDs. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
The additional image store feature assumes that images / layers in the additional store never go away, while we do remove it after this test. Try to repair the store. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
Don't assume that the loaded image will be deduplicated with the server image. Signed-off-by: Miloslav Trmač <mitr@redhat.com>
2832b1b
to
4db012e
Compare
Ready for review: |
Working on test failures as discussed in containers/storage#2130 .
At this point absolutely untested, andI don’t really know what I’m doing.Cc: @edsantiago — perhaps read this only later after this is at least minimally shown to be working.
Does this PR introduce a user-facing change?