storage: enable partial images by default

[giuseppe]

by default enable pulling a partial image, it is still possible to disable the feature through the configuration file.

@rhatdan @mtrmac

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe

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:

• OWNERS [giuseppe]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mtrmac]

Code LGTM; but doing this is ~blocked on #1826 and c/image using that API, otherwise power loss can lead to broken stores.

---

As discussed in chat, having the check for enable_partial_images at the very first entry point (chunked.GetDiffer) would isolate more of the risk.

[mtrmac]

FWIW tests are failing

[giuseppe]

fixed now

[mtrmac]

Code LGTM again… still #1826.

[rhatdan]

This looks wrong options is true or falsenot true or true.

[giuseppe]

fixed

[giuseppe]

Code LGTM again… still #1826.

unblocked now

[mtrmac]

/lgtm

[cgwalters]

Likely fallout in containers/bootc#509 (comment)

[mtrmac]

Likely fallout in containers/bootc#509 (comment)

@cgwalters almost 2 months later … is the cause clear?

I see “corrupted blob, expecting …” from Skopeo proxy in that ticket, reporting that some kind of GetBlob seems to be working incorrectly, which worries me quite a bit.

But the total sequence of operations is not trivially clear from that report, and I’m naively thinking that if we broke podman pull && podman push, there would have been really many reports by now (and, hopefully, CI failures); so is this something much more specific to bootc?

[cgwalters]

Thanks for following up. Still to this day, we haven't really dug into zstd:chunked images and ostree because containers/bootc#20 means it's basically not useful to create them right now - in theory it should be pullable (and most of the time was), but without the actual delta effect.

But the total sequence of operations is not trivially clear from that report, and I’m naively thinking that if we broke podman pull && podman push,

Right but remember "podman pull" is exercising the combination of c/image and c/storage which are wired together for zstd:chunked.

What ostree is doing is more like skopeo copy docker://quay.io/some-zstd-chunked-image oci:ocidir - and actually I guess not even that because that path goes through the whole "copy machinery" where c/image is driving things instead of being pull based. But it's at least a lot closer to "just fetch the blobs for an zstd:chunked images".

[cgwalters]

I guess we could try stress testing this path with just raw code talking to the proxy and calling GetBlob (https://github.com/containers/containers-image-proxy-rs/blob/main/examples/client.rs is a trivial client that doesn't involve anything ostree, etc.)

[mtrmac]

Hum… skopeo copy docker://… non-c/storage:… would not trigger the chunked path at all. That is less likely to be exercised in existing tests, but it’s also simple enough in implementation that I don’t see anything that could obviously be wrong when creating +pushing, or consuming zstd:chunked layers on registries.