storage
storage copied to clipboard
`pull_options.convert_images` does not guarantee chunked/composefs usage
#2115 + #2118 worked to give the c/storage code the option, if convert_images is set and a partial pull (or creation of a composefs-based layer) is not possible, to fail GetDiffer in a way which terminates the c/image pull process entirely, disabling the fallback to non-chunked pulls.
https://github.com/containers/storage/pull/2118#issuecomment-2385165798 discussed that, but, to be explicit and to list the various conditions:
- If some variant of the layer exists locally,
PutBlobPartial/GetDifferwill not be called; instead, the existing layer will be directly reused (probably fine), or (if the parent or ID differs) exported to a tar file and reimported the traditional way, without invokingGetDiffer - If the source of the pull is a transport which just doesn’t support partial pulls (i.e. anything but a registry),
PutBlobPartialwill not be called; the traditional path will be used - If the pull operation also includes a conversion from schema1 to a different format (not during ordinary
podman pullbut possible with Skopeo),PutBlobPartialwill not be called; the traditional path will be used - If the pull operation is decrypting an encrypted image (or, uh, encrypting one, in theory),
PutBlobPartialwill not be called; the traditional path will be used FUTURE: If the source image is schema1,PutBlobPartialwill trigger a fallback without callingGetDiffer
So, enforcing the chunked-only code path would require one of:
- Some kind of more explicit coordination with c/image, telling it to not attempt any of the other code paths (… and reimplementing the affected features some other way???)
- Also enforcing
convert_imagesaroundlayerStore.applyDiffWithOptions, triggering a failure (… and losing the affected features entirely; unacceptable) - Refactoring the code so that
layerStore.applyDiffWithOptionscan also fully implement theconvert_imagescode path (probably best overall) - (Independently), possibly special-casing the “reuse of a local layer with a different ID/parent” situation in c/storage, so that this happens in some more efficient way.
Again, #2115/#2118 were clear enough that fully enforcing the conversions had not yet been a goal at this time; I’m filing this issue
- to be fairly explicit about that situation
- to have a place to discuss the tradeoffs or implementation approaches
because the current zstd:chunked layer ID work means that we should not even attempt to use partial pulls for schema1 images (because they don’t commit to a DiffID value, the only way to avoid a layer view ambiguity is to only ever use one view); and I wanted to have an issue to point to for the design discussion of that.
Cc/RFC: @giuseppe