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.

Sign up for GitHub

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

Jump to bottom

Clean up language around tag listing and repository naming #50

Merged
merged 2 commits into from
Mar 26, 2018
Merged

Clean up language around tag listing and repository naming #50

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4b530c
Clean up language around tag listing and repository naming
dmcgowan Mar 14, 2018
2b4308e
proposals/distribution: remove non-sense about image indexes
stevvooe Mar 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions proposals/distribution.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,19 +103,16 @@ The following entries should be added to the [scope table][scope]:
* “Specifying a distribution method”.
This entry replaces part of the previous “Creating Reference spec for optional DNS based naming & distribution” and “Standardizing on a particular Distribution method” entries.

Retrieving image indexes covers the current “tag listing” (e.g. “what named manifests are in `library/busybox`?”), because tags are entries in the image format's [`manifests` array][manifests].
Other tag-listing endpoints needed for backwards-compatibility are therefore in scope as well.
Tag-listing (e.g. “what named manifests are in `library/busybox`?”) endpoints are in scope and required for backwards compatibility with clients.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not totally convinced that tag listing should be included in distribution.

Does "backwards compatibility with clients" here refer to just docker pull -a?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not totally convinced that tag listing should be included in distribution.

Specifically tag listing or tag operations in general?

Does "backwards compatibility with clients" here refer to just docker pull -a?

Also skopeo inspect

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd argue tags in general but I doubt that would get much traction :)

Reading through the distribution spec, it seems to me that tags are an implicitly-defined mutable layer on top of the CAS with some missing features (e.g. deleting a tag). It would be nice if tags were first class. Merging distribution/distribution#2169 would help with some of my concerns, but I'm not sure what the plan is for the PRs under Related GitHub Issues.

Happy to punt on this for now if it can be improved later, but the registry is currently a mix of a DAG/CAS/CMS and I'd like to scope down 'distribution' to be as narrow as we can get away with.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with everything you just said 😄

There is where we are with tags right now (what we need to include so clients can continue to work just using the OCI specification) and where we want to be with tagging, which I believe is an explicit tags endpoint like you listed. Ideally the CAS and tagging elements are distinct and the registry API can be used as CAS only. I am trying to communicate that here so we remain open to extension, backwards compatible, and still support just CAS with tagging elsewhere. I think maybe this isn't fully accomplishing that, other than dropping do you have any recommendations for making the point more clear?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jonjohnsonjr The intent of "Related Github Issues" was that these would be merged as part of the specification process. I am not sure at all how my comment about "These should be included" became "Related Github Issues".

In general, for the registry to be useful, you need CAS plus name pinning, so I don't think we can go full CAS here.

An endpoint for listing image repository names within a registry is considered out of scope for this specification.

Grouping image indexes in repositories is considered part of distribution policy or content management, which are out of scope for this entry's per-image action.
For example, “what images are under `library/`?” is out of scope for this project.
Managing the grouping of image repository names is considered part of distribution policy or content management, which are out of scope.
For example, “which image repositories are under `library/`?” is out of scope for this project.

* What: Specifying a distribution method
* In/Out/Future: In scope
* Status: In progress (see opencontainers/distribution-spec)
* Description: Define a protocol for creating, retrieving, updating, and deleting objects defined in the [image specification][image-spec].
Listing repositories (like [`/v2/_catalog`][catalog]) is a multi-[image-index][] action, which is out of scope for this entry.
Managing groups of image indexes requires multi-[image-index][] actions, which are out of scope for this entry.
Listing image indexes within a group is a multi-[image-index][] action, which is out of scope for this entry.
* Description: Define a protocol for creating, retrieving, updating, and deleting content-addressable objects, such as those defined in the [image specification][image-spec].
Copy link
Contributor

@wking wking Mar 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

The distribution mandate needs to be broader than CAS. I don't think this sentence is broad enough to justify including the mutable, name-addressable tag-listing and tag -> manifest endpoints, which you list as in-scope above.

Copy link
Member Author

Choose a reason for hiding this comment

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

This sentence is describing the protocol which is based on content-addressable objects. Naming just maps to these objects, the objects themselves are not mutable. It is clearly listed up above that these mapping endpoints are in scope and will be supported. The protocol however can be used without this tagging.

Copy link
Contributor

Choose a reason for hiding this comment

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

This sentence is describing the protocol which is based on content-addressable objects.

The protocol uses content-addressable objects with a mutable, name-addressable layer on top (like Git's refs). Of the endpoints listed here, only /v2/<name>/blobs/<digest> is strictly content addressable. The stuff under /v2/<name>/blobs/uploads/ is how you feed data into CAS, so that's fine too. But /v2/<name>/manifests/<reference> and /v2/<name>/tags/list are mutable and name-addressable, so you need some other wording to justify them.

Naming just maps to these objects, the objects themselves are not mutable.

So add a sentence that includes this naming in the scope? As long as any mentions of “naming” are clearly distinct from the out-of-scope “… DNS based naming…” entry.

It is clearly listed up above that these mapping endpoints are in scope and will be supported.

Right, but that text is just informative background that won't end up in the scope table. I think we want normative language that will end up in the scope table to explain why tag listing and name-addressable references are in scope.

Copy link
Member Author

Choose a reason for hiding this comment

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

So add a sentence that includes this naming in the scope?

Can you propose a sentence to add and we can discuss. I don't want to go back and forth on the correctness of the description. I am not sure how the scope table is generated, sounds like the previous sentences are just there for the proposal but unrelated to the scope table at https://www.opencontainers.org/about/oci-scope-table

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you propose a sentence to add and we can discuss.

My previous attempt is what you're removing here ;). I was trying to lean on image-spec indexes and org.opencontainers.image.ref.name to show why tag-listing and tag -> manifest-descriptor resolution were in scope. It sounds like that's the basic idea, but @stevvooe wants phrasing that doesn't mention the image-spec primitives, and I don't have a rewording I like better yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure how the scope table is generated...

I think @caniszczyk just updates WordPress based on the instructions here after the TOB approves the proposal.

Copy link
Member Author

@dmcgowan dmcgowan Mar 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I was trying to lean on image-spec indexes and org.opencontainers.image.ref.name to show why tag-listing and tag -> manifest-descriptor resolution were in scope.

I get that, the problem is that image-spec indexes and that annotation have nothing to do with the distribution specification, so it was confusing. The annotation may be the same as what the registry has, but there is no relationship in the API. The reason the current tag endpoints are included are for backwards compatibility and there are discussions which need to be had about updating that interface and capabilities. However the point of this proposal is to take the specification as is and keep the scope narrowed down. The CAS part of the specification has already been widely agreed on and is good to have in the scope table, we can always broaden later as the discussion matures.

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason the current tag endpoints are included are for backwards compatibility and there are discussions which need to be had about updating that interface and capabilities.

So add a sentence saying which endpoints are needed for backwards compat? And making it clear that backwards-compat with those endpoints is in-scope while backwards-compat with /v2/_catalog is out-of-scope?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is why I was asking about the format of these entries, these sentences are already there. Are you saying we should just move the sentences from before the What to under Description? The previous version also had in/out of scope language in two places, it is really not clear what this section is trying to accomplish with the formatting.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is why I was asking about the format of these entries, these sentences are already there. Are you saying we should just move the sentences from before the What to under Description? The previous version also had in/out of scope language in two places, it is really not clear what this section is trying to accomplish with the formatting.

My intention with the formatting was:

* “{What entry}”.
    {Detailed informative discussion, intended for the proposal only}.

    * What: {What entry}
    * In/Out/Future: {Scope entry}
    * Status: {Status entry}
    * Description: {Normative definiton}
    * Why: {High-level informative motivation}

If you want to drop the detailed-informative-discussion section entirely because you find it not-very-informative, that's fine with me. For the “Description” entry, how about something like:

    * Description: Define a protocol for creating, retrieving, updating, and deleting content-addressable objects, such as those defined in the [image specification][image-spec].
        Also define, as an optional layer, the `/v2/<name>/tags/list` and `/v2/<name>/manifests/<reference>` endpoints needed for backwards compatibility with existing clients.

* Why: This specification will provide one (of possibly many) distribution specifications.
Alternative distribution specifications may be developed for uses cases not covered by this specification, but defining them is currently out of scope for the OCI.

Expand Down