image-layout: describe an OPTIONAL index #438

vbatts · 2016-11-03T14:41:34Z

Depending on where and how an OCI image-layout is provided (tar, zip,
HTTP, etc) the mechanism to "walk" the refs can vary in complexity and
expense. For HTTP in example, getting a directory listing is often
disabled, but even would require multiple fetches to build the map of refs.

Signed-off-by: Vincent Batts vbatts@hashbangbash.com

Closes #374

wking · 2016-11-03T15:00:33Z

image-layout.md

@@ -115,4 +116,42 @@ $ cat ./blobs/sha256/e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7f
 [tar stream]
 ```

+## Index 


Trailing whitespace.

wking · 2016-11-03T15:00:46Z

image-layout.md

+  Each object has the following properties:
+
+  - **ref** *string*, REQUIRED
+


Trailing whitespace.

wking · 2016-11-03T15:02:26Z

image-layout.md

@@ -115,4 +116,42 @@ $ cat ./blobs/sha256/e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7f
 [tar stream]
 ```

+## Index 
+
+Index is intended to be generated from the content in an image-layout after any updates have been made.


“any updates” → “any ref additions or removals”. You shouldn't need to update it after other changes (e.g. blob addition or removal, or changing a ref's descriptor payload).

Those are both are updates of one kind or another.

But there are updates that are not included in “ref additions or removals” (e.g. blob addition, or changing a ref's descriptor payload) which would not require index regeneration. I'm suggesting we make our regeneration suggestions as narrowly-scoped as possible, because conflicts around regenerating this index are one of the major pain-points with this approach.

wking · 2016-11-03T15:05:08Z

image-layout.md

+## Index 
+
+Index is intended to be generated from the content in an image-layout after any updates have been made.
+Allows for mapping random access to the references in the image-layout, and needing only to read from the `oci-layout` once to build this map of references.


I'd rather replace this line with your HTTP use-case. Maybe just:

For example, HTTP servers often disable directory listing.

As it stands, the line is not a sentence (that would be “The index allows…” or some such). And you already have random access with refs/. The think the index adds is single-file list support when you can't walk refs/ with readdir(3) or similar. And the HTTP sentence makes that clear with a concrete example.

wking · 2016-11-03T15:09:14Z

image-layout.md

+
+**index** *array of objects*, OPTIONAL
+
+  Provides a single access to top-level information about the layout.


Four-space indents nside list entries where we'll be using sub-lists (opencontainers/runtime-spec#495)?

And for the wording, maybe just “Provides a list of available refs.”

This is the same indentation used in the config.md and manifest.md for its field descriptions.

If you want to punt for now, that's fine with me (I'm not a consumer of the compiled HTML or PDF output). But runtime-spec has had both opencontainers/runtime-spec#495 and opencontainers/runtime-spec#608 to avoid issues with two-space indents for sub-lists. It seems like we should be sticking to that pattern here for new additions, and eventually someone will be bothered enough by:

<ul> <li><code>mediaType</code> string</li> </ul> This REQUIRED property contains the MIME type of the referenced content. The OCI image specification defines <a href="media-types.md">several of its \ own MIME types</a> for resources defined in the specification.

That they'll go through and fix the existing Markdown ;).

wking · 2016-11-03T15:13:02Z

image-layout.md

+Index is intended to be generated from the content in an image-layout after any updates have been made.
+Allows for mapping random access to the references in the image-layout, and needing only to read from the `oci-layout` once to build this map of references.
+
+**index** *array of objects*, OPTIONAL


The ref list could get large, and I'd rather not overload the “do I understand how to parse this?“ oci-layout. Can we put this index in refs.json instead? You could just dive in with the array:

$ cat refs.json ["latest", "v1"]

Since the filename + the version from oci-layout sufficiently namespace it.

it has the potential for being long, no matter where it lives. And as the oci-layout is required file, it will already be stat'ed or fetched. Deferring to another file means another read, that could be a ENOENT or a 404. Plus this truncated list you mention does not allow for having tagged metadata.

With a separate file, a long index only impacts ref-listing. By putting it in oci-layout, a long index impacts any image-layout activity (because everyone should be checking to see “is this an image-layout version I understand” before trying to use the bits they need).

Plus this truncated list you mention does not allow for having tagged metadata.

If annotating refs is important, I'd rather have annotations added to the descriptor model. Do you have an example use case that shows that annotations are useful for refs but not for descriptors in general?

That still means that a read per ref.
The whole notion of an index will be for those sharing it in their own use-case being able to bubble up the important pieces of metadata per ref. Adding it to the descriptor might be fine as well, but as RPM, debian, and other repos have shown, there is a vast amount of metadata provided as early as possible. It is not my job to limit to these unknown users what information they're allowed to bubble up. Again, I am not just thinking through my own personal use-case of what is needed in my home directory of my personal machine.

Adding annotations to descriptors is not directly coupled to removing it from an index. It just means that folks who find the ref payload via refs/ will still have access to it (and can skip loading the potentially large index JSON). Once you have descriptor annotations, you can certainly cache them in the index as a separate performance optimization. But only supporting ref annotations via the index (and not via refs/) seems like a strange position.

wking · 2016-11-03T15:15:29Z

image-layout.md

+
+  - **annotations** *string-string map*, OPTIONAL
+
+    This field is under the same constaints as [manifest `annotations`](manifest.md#image-manifest-property-descriptions)


I'd rather not add this field to the ref index, since it wouldn't be supported by ref/-walking listers. Can you motivate a use-case for it? Folks who wanted to annotate a ref can already do so by adding a new ref (e.g. v1.0.0.annotations) pointing at a CAS blob with their desired payload.

Because there is tooling that may publish or fetch from an OCI image-layout, that needs top-level views. This is arbitrary metadata.

I've spun this off into #440. Can we drop it from this PR and focus on the ref-listing use case? We can come back and add it to the index if the consensus after #440 is “yes we need these, and no we can't add annotations to the descriptor model” (that's not the consensus I expect, but I'm not a maintainer ;).

really spreading out the topics. there is hardly a consensus reachable when the topic is so diluted.

Folks have suggested that I keep too many points in a single PR and end up distracting from that PR. If you feel like the annotations discussion is fundamental to this PR, I'm happy to keep it here (and you should probably close #440 with “Discussion in #438” or some such).

oh, it's just here, there and everywhere. The further dilution of topics never seems to have the intention to arrive at a conclusion, only to further add items to come to agreement on.

I expect we will more quickly arrive at a consensus for a ref index to support our existing ref listing over HTTP (when the HTTP server disables directory listing), since we both agree that's important. I'm suggesting we decouple the orthogonal ref-annotation issue so that it doesn't bog down the ref-indexing discussion, which should help us reach a consensus on ref-indexing more quickly.

wking · 2016-11-03T16:41:25Z

On Thu, Nov 03, 2016 at 07:41:35AM -0700, Vincent Batts wrote:

Depending on where and how an OCI image-layout is provided (tar,
zip, HTTP, etc) the mechanism to "walk" the refs can vary in
complexity and expense. For HTTP in example, getting a directory
listing is often disabled, but even would require multiple fetches
to build the map of refs.

Ref payloads are just descriptors, so they usually won't be very big.
For cases where a JSON listing of all refs is feasible, we can
probably inline the descriptor value in the ref-listing JSON file.
For example:

$ cat refs.json
{
"v1.0" {
"size": 4096,
"digest": "sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f",
"mediaType": "application/vnd.oci.image.manifest.list.v1+json"}
},
…
}

Once you have that, maybe you no longer need /refs? There are pros
and cons to either approach:

A refs.json works well over HTTP, where listing refs/ may not be
enabled. It may provide slow access to a known ref, since you have
to unmarshal a potentially large JSON document before you can lookup
the ref you need.
A refs/ directory avoids racy conflicts around reading/writing a
single file. Multiple consumers can read from and atomically update
entries in the directory without problems. It also provides fast
access to a known ref.

My feeling is that in situations where you're using a ref-listing JSON
(e.g. over HTTP):

You don't really care about speed, because it's dwarfed by network
latency.
You don't really care about update conflicts, because if you are
running a service supporting concurrent modifications, you probably
have something more sophisticated than direct access to refs.json.

So I'd argue for refs.json as a complete replacement for refs/ which
should live under a 2.x imageLayoutVersion series. Refs and CAS are
completely orthogonal, so we might want to replace imageLayoutVersion
with separate version numbers for refs and CAS. Then you could say:

$ cat oci-layout
{
"refsLayoutVersion": "1.0.0",
"casLayoutVersion": "1.0.0"
}

for refs/ and:

$ cat oci-layout
{
"refsLayoutVersion": "2.0.0",
"casLayoutVersion": "1.0.0"
}

for refs.json.

wking · 2016-11-03T18:38:59Z

On Thu, Nov 03, 2016 at 09:41:26AM -0700, W. Trevor King wrote:

Refs and CAS are completely orthogonal, so we might want to replace
imageLayoutVersion with separate version numbers for refs and CAS.

Spun off into #441 to avoid distracting here.

coolljt0725 · 2016-11-04T03:31:35Z

image-layout.md

+
+  - **annotations** *string-string map*, OPTIONAL
+
+    This field is under the same constaints as [manifest `annotations`](manifest.md#image-manifest-property-descriptions)


constaints a typo here :)

vbatts · 2016-11-29T22:21:45Z

updated. PTAL

philips · 2016-11-30T22:13:23Z

image-layout.md

@@ -118,4 +119,40 @@ $ cat ./blobs/sha256/e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7f
 [tar stream]
 ```

+## Index
+
+Index is intended to be generated from the content in an image-layout after any udates have been made.


nit: udates

stevvooe · 2016-11-30T22:57:52Z

This is a great change, but the structure is nearly identical to manifest-list. Let's do the following:

Remove the refs concept. It overlaps too much and isn't very useful.
Add an annotations field to descriptor.
Modify manifest-list type to clarify that it can be used in this role.
Have the image layout specify and index.json as a manifest-list.

In general, this addresses concerns I've had with refs. Thanks for taking this problem on!

philips · 2016-11-30T23:59:06Z

@stevvooe I think I largely agree but what replaces the concept of a "ref name" in the manifest list? Any arbitrary annotation?

vbatts · 2016-12-01T00:31:14Z

Perhaps a specific annotation. "org.opencontainer.ref.name" or similar?

…

On Wed, Nov 30, 2016, 18:59 Brandon Philips ***@***.***> wrote: @stevvooe <https://github.com/stevvooe> I think I largely agree but what replaces the concept of a "ref name" in the manifest list <https://github.com/opencontainers/image-spec/blob/master/manifest-list.md>? Any arbitrary annotation? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#438 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEF6S19anozd4BT6gyWmjT7Ni54-xgyks5rDg3LgaJpZM4KocFV> .

stevvooe · 2016-12-02T03:23:37Z

@philips @vbatts I was leaving that as arbitrary for now. We could have tools on import pick up various fields then work on conventions from there.

Perhaps we work through this change and address this in a follow up.

I can already imagine a tool that dumps the index and annotations:

$ oci-image inspect foo.tar.gz
sha256:abcdef
  org.opencontainer.ref.name=foo
  org.opencontainer.morejunk=bar

Maybe integrate such that the consumer doesn't need to have a convention:

$ docker load -t $(oci-image inspect -f '{.Annotations["org.opencontainer.ref.name"}' foo.tar.gz) foo.tar.gz

stevvooe · 2016-12-07T22:25:03Z

@vbatts Any movement on updating this approach to use the manifest list type?

vbatts · 2016-12-13T16:45:38Z

i'm still mulling on this @stevvooe . I realize this is very overlapped with the manifest-list, though the manifest list is meant for pointing to only manifests with an election based on platform, though the act of indexing could allow for pointing to any mediatypes (even arbitrary types). I feel like this is different enough.

It would take some restructuring of the manifest-list that would make it generic.

wking · 2016-12-13T17:04:18Z

On Tue, Dec 13, 2016 at 08:45:39AM -0800, Vincent Batts wrote: It would take some restructuring of the manifest-list that would make it generic.

With a new Descriptor.annotations, I think manifest-list is *already* generic enough (although it would be good to provide more context around these dual use-cases, e.g. near [1]). What else are you missing? [1]: https://github.com/opencontainers/image-spec/blame/v1.0.0-rc3/manifest-list.md#L35

vbatts · 2016-12-13T17:07:32Z

@wking besides the manifests field name with the sentence This REQUIRED property contains a list of manifests for specific platforms.

wking · 2016-12-13T17:51:30Z

On Tue, Dec 13, 2016 at 09:07:32AM -0800, Vincent Batts wrote: @wking besides the `manifests` field name with the sentence `This REQUIRED property contains a list of manifests for specific platforms.`

The ‘manifests’ name could be made more generic. Are you suggesting lifting the requirement to allow for indexes that contain no entries? I'm ok with that. The “list of manifests” context could be made more generic too, but it's currently not blocking you putting non-manifests in because the descriptor types are flexible [1]. We'd probably also want to move the currently-REQUIRED platform stuff [2] into Descriptor.annotations. [1]: https://github.com/opencontainers/image-spec/blame/v1.0.0-rc3/manifest-list.md#L35 [2]: https://github.com/opencontainers/image-spec/blame/v1.0.0-rc3/manifest-list.md#L39

vbatts · 2016-12-13T18:11:38Z

i do agree all the platform values would be better as annotations. Especially given their inconsistent naming convention (to speak of, we use under_score, camelCase, and field.name 😬 )

stevvooe · 2016-12-13T21:34:53Z

@vbatts The manifest-list is already generic enough. I'd prefer not to see a secondary data type that is the same but not.

though the manifest list is meant for pointing to only manifests with an election based on platform

The manifest-list was never meant to be limited to this particular use case. It got de-featured during the design process, since no one could understand its usefulness in other cases (like this one), as I had argued back at that time. Subsequently, this was the use case that got it in, but the limitation is arbitrary.

Adding annotations is all it would take. No other movements are necessary.

alexlarsson · 2016-12-14T08:34:34Z

Dropping platform from the manifest list and having a generic annotations with some common standard fields seems like a great improvement to me. The current set of fields has a bunch of essentially free-form fields with zero guidance to how you would use them anyway.

For the manifest list digest it would be quite nice if you could extend it with other types. For instance, when using OCI registries for flatpak I would like to put in an index in the registry that points to an appstream[1] xml file which could be used by a graphical app-store frontent. I.e. it would get me metadata like title, description, icons, screenshots, etc for all the apps in the registry. But I agree with @wking in that it seems the spec already allows this.

[1] https://www.freedesktop.org/wiki/Distributions/AppStream/

Depending on where and how an OCI image-layout is provided (tar, zip, HTTP, etc) the mechanism to "walk" the refs can vary in complexity and expense. For HTTP in example, getting a directory listing is often disabled, but even would require multiple fetches to build the map of refs. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts · 2016-12-15T21:12:59Z

i've just rebased this, but haven't had a chance to merge it with @stevvooe thoughts on an index.json that is a manifest-list. Nor updates to the manifest-list.md that would make it more generic to be a descriptor-list. 😬

RobDolinMS · 2017-01-18T22:37:49Z

@vbatts has some hesitation on this potentially being too generic.
@stevvooe Can you comment on this?

stevvooe · 2017-01-19T00:23:45Z

@vbatts @RobDolinMS I thought we had agreed that these were structurally identical to manifest lists.

That would entail closing this PR and fixing it to take that into account.

xiekeyang · 2017-01-23T02:30:37Z

@vbatts
If we should restrain that objects in Index field MUST be manifest (maybe or layer) type? These might be all possible types for Index.

And if we should NOTE the different of Index and manifest-list in this section? Currently manifest-list is mainly for pointing to one manifest under specific platform. While Index is for including many manifests, with some sequential dependency.

vbatts · 2017-01-25T22:26:25Z

Next steps here to move forward and satisfy the requirements would be:

keeping manifest-list as "the entry point" ( it was a lot work to get it there for schema 2)
loosen up required fields
optional /index.json
add to manifest-list a field like "channels" or "descriptors"

Last thing for me to do is clarify the scope/context of annotations and expectations around conveying them.

From the 2017-01-25 call we agreed to keep the manifest list as the entry point for accessing objects. The index concept is only a more generic use of the manifest-list. opencontainers#438 (comment) This change uses the manifest-list as an index that allows implementations to work around needing to walk/traverse the `./refs/` or `./blobs/` directories. Obsoletes opencontainers#438 Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

From the 2017-01-25 call we agreed to keep the manifest list as the entry point for accessing objects. The index concept is only a more generic use of the manifest-list. opencontainers#438 (comment) This change uses the manifest-list as an index that allows implementations to work around needing to walk/traverse the `./refs/` or `./blobs/` directories. This includes validating the image-layout example. This also makes `validate-examples` automatically update `schema-fs` if there have been changes to the JSON schema. Obsoletes opencontainers#438 Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

From the 2017-01-25 call we agreed to keep the manifest list as the entry point for accessing objects. The index concept is only a more generic use of the manifest-list. opencontainers#438 (comment) This change uses the manifest-list as an index that allows implementations to work around needing to walk/traverse the `./refs/` directory. As the `./refs/` directory was REQUIRED to exist, with its removal this `index.json` is REQUIRED to be present. This includes validating the image-layout example. This also makes `validate-examples` automatically update `schema-fs` if there have been changes to the JSON schema. Obsoletes opencontainers#438 Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts · 2017-02-08T13:43:14Z

#533 merged

From the 2017-01-25 call we agreed to keep the manifest list as the entry point for accessing objects. The index concept is only a more generic use of the manifest-list. opencontainers/image-spec#438 (comment) This change uses the manifest-list as an index that allows implementations to work around needing to walk/traverse the `./refs/` directory. As the `./refs/` directory was REQUIRED to exist, with its removal this `index.json` is REQUIRED to be present. This includes validating the image-layout example. This also makes `validate-examples` automatically update `schema-fs` if there have been changes to the JSON schema. Obsoletes #438 Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

wking reviewed Nov 3, 2016

View reviewed changes

vbatts force-pushed the image-layout_index branch from 3c8b9ef to 7cffab1 Compare November 3, 2016 17:00

This was referenced Nov 3, 2016

Annotations for descriptors? #440

Closed

Separate versioning for refs and CAS portions of image-layout? #441

Closed

wking mentioned this pull request Nov 3, 2016

image-layout: clarification of oci-layout #434

Merged

coolljt0725 reviewed Nov 4, 2016

View reviewed changes

vbatts force-pushed the image-layout_index branch 2 times, most recently from 88eca7e to eb8a764 Compare November 29, 2016 22:21

philips reviewed Nov 30, 2016

View reviewed changes

vbatts force-pushed the image-layout_index branch from eb8a764 to 9b6dd3d Compare November 30, 2016 23:44

wking mentioned this pull request Dec 8, 2016

Suggestion: refer to label-schema.org for examples of label usage #485

Closed

vbatts force-pushed the image-layout_index branch from 9b6dd3d to 04404f0 Compare December 15, 2016 21:11

wking mentioned this pull request Dec 19, 2016

descriptor: add a license for each descriptor #506

Closed

vbatts added this to the v1.0.0-rc4 milestone Jan 18, 2017

stevvooe mentioned this pull request Jan 25, 2017

reference management in image layout #374

Closed

stevvooe modified the milestones: v1.0.0, v1.0.0-rc4 Jan 25, 2017

vbatts mentioned this pull request Jan 26, 2017

image-layout: ./refs/ -> index.json #533

Merged

vbatts modified the milestones: v1.0.0-rc5, v1.0.0 Feb 1, 2017

wking mentioned this pull request Feb 7, 2017

spec: Standardize language for actions ((un)packing, etc.) #554

Closed

vbatts closed this Feb 8, 2017

vbatts deleted the image-layout_index branch February 8, 2017 13:58

wking mentioned this pull request Nov 9, 2017

[rfc] cas: cmpxchg interface? opencontainers/umoci#216

Open

		Each object has the following properties:

		- ref string, REQUIRED


		index array of objects, OPTIONAL

		Provides a single access to top-level information about the layout.


		- annotations string-string map, OPTIONAL

		This field is under the same constaints as [manifest `annotations`](manifest.md#image-manifest-property-descriptions)

image-layout: describe an OPTIONAL index #438

image-layout: describe an OPTIONAL index #438

Conversation

vbatts commented Nov 3, 2016 • edited by stevvooe Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wking commented Nov 3, 2016

wking commented Nov 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vbatts commented Nov 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevvooe commented Nov 30, 2016

philips commented Nov 30, 2016

vbatts commented Dec 1, 2016 via email

stevvooe commented Dec 2, 2016

stevvooe commented Dec 7, 2016

vbatts commented Dec 13, 2016

wking commented Dec 13, 2016 via email

vbatts commented Dec 13, 2016

wking commented Dec 13, 2016 via email

vbatts commented Dec 13, 2016

stevvooe commented Dec 13, 2016

alexlarsson commented Dec 14, 2016

vbatts commented Dec 15, 2016

RobDolinMS commented Jan 18, 2017

stevvooe commented Jan 19, 2017

xiekeyang commented Jan 23, 2017

vbatts commented Jan 25, 2017

vbatts commented Feb 8, 2017

vbatts commented Nov 3, 2016 •

edited by stevvooe

Loading