Skip to content
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

pull_options.convert_images does not guarantee chunked/composefs usage #2180

Open
mtrmac opened this issue Nov 26, 2024 · 0 comments
Open

pull_options.convert_images does not guarantee chunked/composefs usage #2180

mtrmac opened this issue Nov 26, 2024 · 0 comments

Comments

@mtrmac
Copy link
Collaborator

mtrmac commented Nov 26, 2024

#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.

#2118 (comment) discussed that, but, to be explicit and to list the various conditions:

  • If some variant of the layer exists locally, PutBlobPartial/GetDiffer will 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 invoking GetDiffer
  • If the source of the pull is a transport which just doesn’t support partial pulls (i.e. anything but a registry), PutBlobPartial will 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 pull but possible with Skopeo), PutBlobPartial will not be called; the traditional path will be used
  • If the pull operation is decrypting an encrypted image (or, uh, encrypting one, in theory), PutBlobPartial will not be called; the traditional path will be used
  • FUTURE: If the source image is schema1, PutBlobPartial will trigger a fallback without calling GetDiffer

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_images around layerStore.applyDiffWithOptions, triggering a failure (… and losing the affected features entirely; unacceptable)
  • Refactoring the code so that layerStore.applyDiffWithOptions can also fully implement the convert_images code 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant