Try zstd:chunked in local cache registry #388
Conversation
|
Cirrus CI build successful. Found built image names and IDs:
|
|
| @@ -82,6 +82,7 @@ declare -a IMAGELIST=( | |||
| testimage:00000004 | |||
| testimage:20221018 | |||
| testimage:20240123 | |||
| testimage:20241011 | |||
There was a problem hiding this comment.
Do we need some form of process to delete the old ones? Otherwise we just keep adding new ones. I guess for testimage not a big concern as it is small enough for now.
There was a problem hiding this comment.
I have a find-unused-images script but I don't quite trust it yet. Am planning to fine-tune it then submit it as part of the repo one day. It's just a oneliner:
for i in $(sed -ne '/IMAGELIST=/,/^[^ ]/p' <cache_images/local-cache-registry | sed -ne 's/^ *//p');do grep -q -R $i ../libpod/test ../buildah/tests || echo "unused $i";done...and its current output is:
unused fedora/python-311:latest
unused podman_python:latest
unused testimage:20240123 <<< yay nice catch
unused testimage:multiimage <<< wrong and hard to test for and maybe I can find a way to not need it
unused podman/stable:v4.3.1 <<< obviously wrong, but hard to check for
unused podman/stable:v4.8.0 <<< ibid
The python ones I think are used in libpod compose tests, but have never taken the time to investigate why
|
@Luap99 low-priority question: should I add |
No we do not use this image in our CI images at all, AFAIK all the cirrus container tasks run in their community cluster so it will be pulled there automatically |
edsantiago
force-pushed
the
zstd-cache
branch
from
67242c7
to
7ccf5e6
Compare
|
Cirrus CI build successful. Found built image names and IDs:
|
|
The local cache registry now has images pushed with zstd:chunked compression. Built in: containers/automation_images#388 Signed-off-by: Ed Santiago <[email protected]>
| @@ -219,6 +220,7 @@ function cache_image() { | |||
| for retry in 1 2 3 0;do | |||
| skopeo --registries-conf /dev/null \ | |||
| copy --all --dest-tls-verify=false \ | |||
| --dest-compress-format zstd:chunked \ | |||
There was a problem hiding this comment.
This changes image digests, which are hard-coded in tests (e.g. ALPINELISTDIGEST).
That’s fine in principle — and I’ll update the test digests in the integration PR — I’m just noting that it’s expected that some tests will start failing with this.
Outside of the current zstd:chunked testing effort, consider using --preserve-digests in the command to make any accidental digest changes (e.g. if an unusual image weren’t supported by the cache registry and triggered a format conversion) an error, to make sure this creates a bit-for-bit-identical copy.
There was a problem hiding this comment.
Thanks for this. The new images built just now use --preserve-digests. Would you care to try with c20241022t120000z-f40f39d13 ?
There was a problem hiding this comment.
I’m sorry, with --preserve-digest + --dest-compress-format, --dest-compress-format is, AFAICS, ignored (because that requires a digest change). I’ll inspect the images to confirm.
Arguably it’s a bug that this option combination is not rejected.
There was a problem hiding this comment.
Ack, thanks, I misunderstood.
So, back to your initial comment: "This changes image digests". Are these changes reliably consistent? When we make new VM images next week/year, different version of skopeo, different kernel/filesystem/tools, different phase of the moon, will the image digests be the same?
If so, then it might be okay to change the digests in the tests, but it's important to remember that e2e tests need to be able to run using local cache and also run using quay. Tests can determine the environment via UsingCacheRegistry() (returns bool). This is ugly but it's a potential way to get tests to pass.
If digests are not consistently reproducible, all bets are off and the tests will need to be completely reimagined.
There was a problem hiding this comment.
Are these changes reliably consistent?
No; there is no guarantee that compression produces consistent results even with one version, and the output can (and, from time to time, does) change when the implementation changes.
For the purpose of the current experiment, I’ll hard-code the digests as they exist in the local cache. But, other than that, we’ll indeed need to specifically maintain zstd:chunked images on quay.io — or, alternatively, not hard-code the expected digests in tests, and obtain them at runtime, or maybe at the cache build time, using something like Skopeo.
There was a problem hiding this comment.
I think the digest always depends on the compression, so as long as the compression is reproducible it should produce the same digest?
In any case is there any need to hard code the digests? Skopeo inspect can give us that
skopeo inspect --no-tags --format {{.Digest}} docker://quay.io/libpod/testimage:20241011
(The command seems quite slow for some reason, so it isn't something I would like to run in every test but only once and cache it)
There was a problem hiding this comment.
Compression isn’t reproducible.
Using skopeo inspect is definitely an option — noting that it decreases the test coverage a bit: if c/image were broken, we’d see consistent incorrect data from both Skopeo and the Podman binary being tested.
There was a problem hiding this comment.
Using skopeo inspect is definitely an option — noting that it decreases the test coverage a bit: if c/image were broken, we’d see consistent incorrect data from both Skopeo and the Podman binary being tested.
Fair but one thing to consider is we would be using skopeo from the rpm build and podman from main so if there is a breaking change only podman on main should be different until we have a new skopeo + c/image build in our VMs so it seem less likely that we hit a corner case there?
There was a problem hiding this comment.
Sure — and the risk is not that high.
Maybe, first, we need some thought about the final test plan: do we actually want to add a gzip / zstd:chunked dimension to the testing matrix, to make this a variable? Maybe the necessary testing coverage can be achieved by a few dedicated tests/images. That’s very unclear to me.
There was a problem hiding this comment.
Exactly. This is one of the questions on my radar, that I don't yet know enough to answer. My hope is that we can do an integration, mixed set of images, but as of this writing my recollection is that some images have worked (with zstd:chunked) and some have not, and that makes me uncomfortable. It's possible that the reason for this is that some tests use different images and that's what triggers the failures, but I haven't spent the time to investigate. Thank you for bringing this up.
There's a strong push to use zstd:chunked by default, but very little testing of it. This forces zstd:chunked on images in the local mirror cache. Also, add testimage:20241010 which was actually pushed to quay with zstd. Signed-off-by: Ed Santiago <[email protected]>
edsantiago
force-pushed
the
zstd-cache
branch
from
7ccf5e6
to
09d272d
Compare
DO NOT USE THIS BUILDCirrus CI build successful. Found built image names and IDs:
|
DO NOT USE THIS BUILD
|
There's a strong push to use zstd:chunked by default, but
very little testing of it. This forces zstd:chunked on
images in the local mirror cache.
Also, add new testimage:20241011 (podman#24238)
Signed-off-by: Ed Santiago [email protected]