Skip to content

Conversation

imeoer
Copy link
Collaborator

@imeoer imeoer commented Jul 14, 2023

Some registry implementations like Docker Hub don't support the
OCI reference type (aka referrer) yet, we shouldn't enable it by
default, instead, provide a `WithReferrer` option to control it.

To fix dragonflyoss/nydus#1363

// WithReferrer associates a reference to the original OCI manifest.
// See the `subject` field description in
// https://github.com/opencontainers/image-spec/blob/main/manifest.md#image-manifest-property-descriptions
WithReferrer bool
Copy link
Member

Choose a reason for hiding this comment

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

Could help introduce why nydus image has to refer to the original OCI image?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated the comment, PTAL.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the update, looks reasonable.

Some registry implementations like Docker Hub don't support the
OCI reference type (aka referrer) yet, we shouldn't enable it by
default, instead, provide a `WithReferrer` option to control it.

With this option, we can track all nydus images associated with
an OCI image. For example, in Harbor we can cascade to show nydus
images linked to an OCI image, deleting the OCI image can also delete
the corresponding nydus images. At runtime, nydus snapshotter can also
automatically upgrade an OCI image run to nydus image.

Signed-off-by: Yan Song <imeoer@linux.alibaba.com>
Copy link
Member

@changweige changweige left a comment

Choose a reason for hiding this comment

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

LGTM

// WithReferrer associates a reference to the original OCI manifest.
// See the `subject` field description in
// https://github.com/opencontainers/image-spec/blob/main/manifest.md#image-manifest-property-descriptions
WithReferrer bool
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the update, looks reasonable.

@changweige changweige merged commit ae06355 into containerd:main Jul 17, 2023
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

Successfully merging this pull request may close these issues.

question: Failed to push image with nydusify
2 participants