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

update documentation for content-flow #10203

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

deitch
Copy link
Contributor

@deitch deitch commented May 10, 2024

Per the discussion in slack with @Kern-- ; once he shared the info, let's capture it and make it broadly available.

@k8s-ci-robot
Copy link

Hi @deitch. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link
Contributor

@Kern-- Kern-- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overally these are good clarifications 👍

@@ -352,7 +353,8 @@ they aren't there, they won't be removed.
### Snapshots

The content in the content store is immutable, but also in formats that often are unusable. For example,
most container layers are in a tar-gzip format. One cannot simply mount a tar-gzip file. Even if one could,
most container layers are in a tar-gzip format. One cannot simply mount a tar-gzip file. In addition, they might not only be new
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OCI Image Spec does explicitly say that for the layer media type, modifications and additions are represented the same way (https://github.com/opencontainers/image-spec/blob/v1.1.0/layer.md#change-types). So while they can be modifications, they're represented as the entire new file to replace the previous file.

Is this clarification in preparation for a new media type that actually represents changes rather than the full replacement content? If not, does this accidentally make that less clear?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while they can be modifications, they're represented as the entire new file to replace the previous file.

That is a valid point. What I wrote can confuse people. I will change that.

Is this clarification in preparation for a new media type that actually represents changes rather than the full replacement content? If not, does this accidentally make that less clear?

No, it isn't. I have interest in a new media type, but it is for my own specific use case, not something that I will attempt to upstream. That has nothing to do with this paragraph.

The entire purpose of this paragraph is to explain why we need those committed and then active snapshots at all. Someone might come to this fresh and think, "I have all of the content in the containerd content store as blobs. Why does it not just create the overlay right on top of them?"

This explains that these layers are the content, but possibly unusable directly (due to immutability and their format). This is more of a "storage format" and less of a "mount and use format."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reworked this paragraph. Have a look?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks good to me

Signed-off-by: Avi Deitcher <avi@deitcher.net>
@deitch deitch force-pushed the content-flow-with-snapshot-updates branch from a2f287e to 8ca7b11 Compare May 12, 2024 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants