-
Notifications
You must be signed in to change notification settings - Fork 645
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
manifest: layer federation #169
Conversation
@@ -191,6 +191,10 @@ Unlike the [Manifest List](#manifest-list), which contains information about a s | |||
|
|||
This REQUIRED property is the digest of the content, as defined by the [Descriptor](descriptor.md) digest format. | |||
|
|||
- **`urls`** *array* | |||
|
|||
For an ordinary layer, this is empty, and the layer contents can be retrieved directly from the registry. For a layer with mediatype of application/vnd.docker.image.rootfs.foreign.diff.tar.gzip, this contains a non-empty list of URLs from which this object can be downloaded. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The application/vnd.docker.image.rootfs.foreign.diff.tar.gzip
media type is not defined in this spec, so we probably want to either define it or remove the restriction. Is there a reason for the media-type restriction or for making this layer-specific? I think “when I wrote this, you could get the blob from {urls}” is useful information for descriptor-targets in general (in which case this field should go here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a copy-paste error :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can see urls
is there https://github.com/opencontainers/image-spec/blob/master/descriptor.md#reserved I just wanted to bring up the discussion here for a future version of the spec probably
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Wed, Jul 06, 2016 at 10:41:39AM -0700, W. Trevor King wrote:
For an ordinary layer, this is empty, and the layer contents can be retrieved directly from the registry. For a layer with mediatype of application/vnd.docker.image.rootfs.foreign.diff.tar.gzip, this contains a non-empty list of URLs from which this object can be downloaded.
The
application/vnd.docker.image.rootfs.foreign.diff.tar.gzip
media type is not defined in this spec…
Now that I have a clearer idea of what this media type is for 1, I
think a better approach is to make ‘foreign’ (or some such) an
explicit property of wherever the mutable mirror data is stored 2.
Then the publisher is saying “you need these blobs to build my image”,
but not telling you how to get them. And the distributor (managing
the mutable mirror data) is saying “you can get that blob from the
usual place and also these mirrors” or “you can't get that blob from
the usual place and have to get it from one of these mirrors”.
Clients who choose not to hit the mutable mirror engine and go to the
usual place for foreign blobs will get the usual key-not-found error
(and potentially a hint pointing at a particular mutable mirror
engine), but that's an acceptable workflow to my eyes.
On Wed, Jul 06, 2016 at 10:37:41AM -0700, Antonio Murdaca wrote:
Putting this on or next to the descriptor means that only the |
makes a lot of sense, but I'm not sure ppl other than the publisher needs to do edit mirrors |
On Wed, Jul 06, 2016 at 10:47:29AM -0700, Antonio Murdaca wrote:
Why not? For example, Gentoo mirrors lots of upstream package source |
I'm all for mirrors and being able for clients to specify their own :O I just said I'm not sure and want more people to chime in :) |
On Wed, Jul 06, 2016 at 11:04:36AM -0700, Antonio Murdaca wrote:
I think this is different from clients specifying their own (they
This would be good too ;). |
The plan was to make sure this field worked well from a UX perspective before bringing into OCI. Docker is going to be the guinea pig here. Also, this needs to go to the descriptor specification. |
I'm interested into defining what we call layer federation also which could or could not be strictly tied to the urls field (like additional data as wking highlighted above) |
Having a URL-based mirror system would also allow folks to use IPFS The only downside is a rehashing step as you confirm both the native
|
I'm not sure if we should consider this a good place to make that transition. The use case of this field is for assets that must come from a specific source. I would think IPFS would work directly with an image layout. |
On Wed, Jul 06, 2016 at 01:29:02PM -0700, Stephen Day wrote:
Who cares where the blob comes from? You have the digest/size so you
Possibly. I'm guessing that choice will come down to how easy it will |
You're missing my point: the field was designed for a particular purpose. It allows distributors of software to host a layer outside of the main distribution flow. If these layers are in IPFS, they're already distributed in that channel and the use of the urls field is pointless. |
On Wed, Jul 06, 2016 at 01:49:54PM -0700, Stephen Day wrote:
Ah, searching around turned up distribution/distribution#1725, where the new
This assumes that “the usual place” is IPFS, and with the current |
A foreign media type only signals to the implementation how to process the Windows layer. The presence of
If it works over HTTP/S, then fine. We will not, however, require clients to implement IPFS. This is probably not the right model and will lead to unnecessary complexity. |
On Wed, Jul 06, 2016 at 02:30:08PM -0700, Stephen Day wrote:
Yup, I think we're on the same page here 1.
And I think we're on the same page here 2.
Maybe. Whether I agree here depends on how the ecosystem evolves 3. |
I'm all for this. Especially given the nature of the descriptor, it has the mimetype to be expected. And the URL(s) could just be a redirector to mirrored content. It seems the change would not be exclusive to this single spot, rather a change to descriptors in general. |
+1 on adding this feature. I am confused on why we need the new mime type though, that is just a hint/optimization? It seems like the language would just say that you SHOULD try to fetch from the URL instead if it exists and use that mimetype in the fetch. It is super confusing having the mimetype be "foreign" when the object you are getting back from the URL is really a "non-foreign". |
On Wed, Aug 03, 2016 at 01:16:51PM -0700, Brandon Philips wrote:
I'm not sure what you mean by “non-foreign”, but the idea behind the On the other hand, if we're ok with failed attempts, I don't see a |
The mimetype is incorrect. That is to mark whether or not the implementation should push or pull the image regularly. It is out of scope for |
@runcom Do you want to clean this up and remove the new mime-type? Where do you want to go with this? |
@runcom bump |
The discussion in the OCI f2f is that this is important for Solaris and Windows support. And both Microsoft and Oracle would like to see it in by v1.0.0. @JLB13 is taking point and if it makes it in in the next two weeks it will make 1.0. |
|
||
The following are field keys that MUST NOT be used in descriptors specified in other OCI specifications: | ||
This OPTIONAL property specifies a list of URLs from which this object can be downloaded. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“object” → “blob” (see this and later).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we use object here as well https://github.com/opencontainers/image-spec/pull/169/files#diff-d1f5919405acb9727b345dc7b1c68785R19
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, we are referring to a specific object, since we have type and size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you saying it's fine as object
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Thu, Aug 25, 2016 at 07:48:50AM -0700, Stephen Day wrote:
-The following are field keys that MUST NOT be used in descriptors specified in other OCI specifications:
- This OPTIONAL property specifies a list of URLs from which this object can be downloaded.
In this case, we are referring to a specific object, since we have type and size.
The caller may know the type and size, but it's only downloading the
blob. And that only meets your “free-form binary data” criterion 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: "may be downloaded."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use may instead of can
|
||
The following are field keys that MUST NOT be used in descriptors specified in other OCI specifications: | ||
This OPTIONAL property specifies a list of URLs from which this object can be downloaded. | ||
For an ordinary layer, this property is empty and the layer contents can be retrieved directly from the registry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we drop this line? I have a few concerns with it:
- This is the descriptor spec and “layer” (i.e. application/vnd.oci.image.serialization.rootfs.tar.gzip) is a higher level idea. You could rename “layer” → “blob”, but…
- Who knows if you can get the content from “the registry”? What registry are we talking about anyway? The descriptor spec doesn't currently give you a way to say “and you can fetch this blob from this registry” (that's sort of what
urls
is for).
The previous line clearly declares urls
as OPTIONAL, so I think it will be pretty clear to implementers that in its absence, they should be asking for the blob in any CAS engines they know about, as configured by the caller. I don't think we gain any additional clarity by waving our hands about “the registry” here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
@runcom Do you also want to handle specification of the "un-pushable" layer media type? |
@stevvooe yes, I think we should say something like "a not empty list of urls means the object isn't supposed to be uploaded to a registry". Note I'm not sure about the language there because we shouldn't talk about registries. Any advice? |
That is not the condition we use to make that decision. The existence of The media type of the target dictates whether or not the layer should not be distributed by an end user. This means it won't be pushed or added to an image layout file. Does that make sense? |
@stevvooe opened #216 to take care of the un-pushable media type (as per #169 (comment)). |
@runcom Please address https://github.com/opencontainers/image-spec/pull/169/files#r77850606. Need to use "may" instead of "can". |
Yup, currently afk though. |
|
||
The following field keys MUST NOT be used in descriptors specified in other OCI specifications: | ||
This OPTIONAL property specifies a list of URLs from which this object may be downloaded. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
“may” → “MAY”? This would allow a compliant implementation to ignore urls
if it didn't want to support them (or didn't want to follow a particular URL, or whatever). In that case the “OPTIONAL” would mean “descriptor authors don't have to set this field” and the “MAY” would mean “implementations don't have to use this field”.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Signed-off-by: Antonio Murdaca <runcom@redhat.com>
Bringing up discussion on layer federation..
The docker v2s2 schema also has them https://github.com/docker/distribution/blob/master/docs/spec/manifest-v2-2.md#image-manifest-field-descriptions and docker itself it's already using this feature for Windows layer.
This will allow people to implement layer federation functionality in their own registry. ISVs who want to host their layers somewhere else are now free to do so. It will be up to whatever client/server implementation to fully support them. The basic idea still remains and OCI would be fine by having this standardized.
Signed-off-by: Antonio Murdaca runcom@redhat.com