containers / podman

Podman: A tool for managing OCI containers and pods.
https://podman.io
Apache License 2.0
23.6k stars 2.4k forks source link

podman pull zstd:chunked-image, with vfs: not supported #24308

Open edsantiago opened 1 week ago

edsantiago commented 1 week ago

WHEW! It's vfs:

# bin/podman --storage-driver vfs pull quay.io/libpod/testimage:20241010   <<< this was pushed to quay with zstd:chunked
Trying to pull quay.io/libpod/testimage:20241010...
Getting image source signatures
Copying blob a3ed95caeb02 done   | 
Error: copying system image from manifest list: reading blob sha256:d005489f1e59bed02315c1c34bb95b3f46a970a5fd20cdc73c93c6e279689dde: not supported

This is a highly hacked podman with all of @mtrmac's changes to c-s c-i c-c c. Full patch history available on request, but I suspect it will not be needed so I'm saving myself that time.

mtrmac commented 1 week ago

@edsantiago Thanks for tracking this down! A detailed reproducer is indeed unnecessary.

After https://github.com/containers/storage/pull/2118 , we now default to not falling back if a chunked pull fails.

Here, c/storage/pkg/chunked.GetDiffer finds a valid chunked input, and returns a success, regardless of the current graph driver. Afterwards, c/image calls PrepareStagedLayer; that fails with the VFS driver with ErrNotSupported because it does not implement DriverWithDiffer. And because ErrNotSupported is not ErrFallbackToOrdinaryLayerDownload, this is a hard failure.

One way to handle this would be to return ErrFallbackToOrdinaryLayerDownload from GetDiffer if the graph driver does not support chunked pulls.

@giuseppe How will the planned options to always enforce composefs affect this? Would we just fail in GetDiffer? Or, maybe, should storage.GetStore immediately fail with an invalid composefs + graph driver combination?

mtrmac commented 1 week ago

https://github.com/containers/image/pull/2607 should improve the error message, but it does not actually allow things to work.

mtrmac commented 1 week ago

Confirmed by manual testing that this is indeed the code path from https://github.com/containers/image/pull/2607 .

mtrmac commented 1 week ago

RFC fix, at least to start the conversation: https://github.com/containers/storage/pull/2140 .