An image may have empty layers array #313

coolljt0725 · 2016-09-14T09:32:00Z

Signed-off-by: Lei Jitang [email protected]
from https://github.com/opencontainers/image-spec/pull/305/files/17c1f00097a716ca369d089159dd9569e7a0e0d0#r78707107

make this clear in manifest.md

wking · 2016-09-14T09:37:28Z

manifest.md

@@ -139,7 +139,7 @@ Unlike the [Manifest List](#manifest-list), which contains information about a s

 - **`layers`** *array*

-    Each item in the array MUST be a [descriptor](descriptor.md).
+    This property MAY be empty, an image can have an empty layers array. Each item in the array MUST be a [descriptor](descriptor.md).


I think this should be “This OPTIONAL property”. And we should stick to the one sentence per line rule.

To make this change clear, I think we also want the validation fix in #308, omitempty in the Go property, and dropping layers from required in schema/image-manifest-schema.json.

runcom · 2016-09-14T09:39:34Z

Wait, I didn't state that Layers is optional and the other stuff in #313 (comment) - I just opened this discussion in #308 because there are some discrepancies around the repo.

I would wait for @vbatts @philips @stevvooe and others to chime in here before making any other fix to this PR

wking · 2016-09-14T09:41:13Z

manifest.md

@@ -139,7 +139,7 @@ Unlike the [Manifest List](#manifest-list), which contains information about a s

 - **`layers`** *array*

-    Each item in the array MUST be a [descriptor](descriptor.md).
+    This property MAY be empty, an image can have an empty layers array. Each item in the array MUST be a [descriptor](descriptor.md).
    The array MUST have the base image at index 0.
    Subsequent layers MUST then follow in the order in which they are to be layered on top of each other.


These two lines should be restructured to avoid requiring a zero-index layer. Something like:

The root filesystem MUST match an empty directory onto which the layers have been [applied](layer.md#applying) in the listed order.

With “match” instead of “be” to allow folks to apply via a union filesystem or whatever. As long as the finished product is right.

Signed-off-by: Lei Jitang <[email protected]>

wking · 2016-09-14T09:45:59Z

On Wed, Sep 14, 2016 at 02:39:35AM -0700, Antonio Murdaca wrote:

Wait, I didn't state that Layers is optional and the other stuff in
#313 (comment)

I just opened this discussion in
image: manifest.go: remove empty layers check #308 because there
are some discrepancies around the repo.

I would wait for @vbatts @philips @stevvooe and others to chime in
here before making any other fix to this PR

Waiting for feedback is good. My motivation for OPTIONAL is that
there's no need to require folks to set an empty array when they could
express the same thing more compactly by leaving the property out of
their JSON.

stevvooe

What is the purpose of a layerless manifest? I don't think makes any sense at all.

stevvooe · 2016-09-14T19:44:10Z

manifest.md

@@ -139,7 +139,7 @@ Unlike the [Manifest List](#manifest-list), which contains information about a s

 - **`layers`** *array*

-    Each item in the array MUST be a [descriptor](descriptor.md).
+    This property MAY be empty, an image can have an empty layers array. Each item in the array MUST be a [descriptor](descriptor.md).


Let's not mangle this line. The fact that these are descriptors is way more important than whether or not it can be empty.

runcom · 2016-09-14T19:47:55Z

What is the purpose of a layerless manifest? I don't think makes any sense at all.

Indeed mine was just a question (for which I just removed 3 lines of code and happy to put them back)

stevvooe

Didn't mean to approve anything here: I think we should close this.

runcom · 2016-09-14T20:04:20Z

Damn new UI - @stevvooe this has been started from #308 - I can re-put the layers check there if this is a non-sense

stevvooe · 2016-09-14T20:15:20Z

Damn new UI -

I've already tweeted in anger. 😸

@stevvooe this has been started from #308 - I can re-put the layers check there if this is a non-sense

I'll be honest: I am confused here. What is the purpose of an image with no layers? The only thing I can imagine is a common distributed command that would run against a default rootfs (perhaps the host). Very weird and I am not sure if the use case is valid or viable.

runcom · 2016-09-14T20:18:33Z

Damn new UI -
I've already tweeted in anger.

I did the same 😸

wking · 2016-09-14T20:42:54Z

On Wed, Sep 14, 2016 at 01:15:20PM -0700, Stephen Day wrote:

I'll be honest: I am confused here. What is the purpose of an image
with no layers?

You might be mounting in the meat from somewhere outside the bundle
directory. Or you might be using the initially-empty-rootfs container
to hold open a mount namespace for subcontainers 1.

stevvooe · 2016-09-14T20:55:47Z

@wking How is that a valid use case for a distributable image? What are you distributing at that point? A command?

wking · 2016-09-14T21:08:52Z

On Wed, Sep 14, 2016 at 01:55:48PM -0700, Stephen Day wrote:

@wking How is that a valid use case for a distributable image? What
are you distributing at that point? A command?

You're distributing the config, which has more than the command (but
not all that much more ;).

jonboulle · 2016-09-16T15:04:58Z

While the use case might seem particularly uncommon, I don't think it's necessarily invalid, nor have a strong reason to disallow it (I don't see it as putting any real burden on implementers for example). So I'm okay with this change in principle.

stevvooe · 2016-09-16T20:15:14Z

@jonboulle What does it mean, though?

jonboulle · 2016-09-16T20:27:45Z

@stevooe similar to the previous suggestions - basically a distributable runtime/exec config, where the root filesystem might be supplied from the host, or a volume mount, or ...

stevvooe · 2016-09-16T20:29:26Z

@jonboulle Ok, but I think this needs to be thought through further. Wouldn't it be better for the spec to reserve the layerless image and then we can think it through?

wking · 2016-09-16T20:30:02Z

On Fri, Sep 16, 2016 at 01:15:15PM -0700, Stephen Day wrote:

@jonboulle What does it mean, though?

With the wording of #318 making it explicit, it means that the
unpacked rootfs will be an empty directory with unspecified
attributes.

stevvooe · 2016-09-16T20:32:25Z

With the wording of #318 making it explicit, it means that the
unpacked rootfs will be an empty directory with unspecified
attributes.

Ok, I agree. Could we merge the intent of this PR with #318 and close this one?

wking · 2016-09-16T20:33:56Z

On Fri, Sep 16, 2016 at 01:29:27PM -0700, Stephen Day wrote:

@jonboulle Ok, but I think this needs to be thought through
further. Wouldn't it be better for the spec to reserve the layerless
image and then we can think it through?

No need to reserve it, just 1:

layers MUST contain at least one entry.

for now and drop that line once you're comfortable doing so.

Most folks will distribute images containing layers, but the specified behavior applies cleanly to the layer-less case too. The unpacked rootfs will just be an empty directory with unspecified attributes. Folks might want to use that sort of thing for namespace pinning [1], distributing configs [2], setting up containers that mount the meat from the host [2], etc. [1]: opencontainers/runtime-spec#395 (comment) [2]: opencontainers#313 (comment) Signed-off-by: W. Trevor King <[email protected]>

jonboulle · 2016-09-16T20:59:24Z

Let's resolve this via #318.

Most folks will distribute images containing layers, but the specified behavior applies cleanly to the layer-less case too. The unpacked rootfs will just be an empty directory with unspecified attributes. Folks might want to use that sort of thing for namespace pinning [1], distributing configs [2], setting up containers that mount the meat from the host [2], etc. [1]: opencontainers/runtime-spec#395 (comment) [2]: opencontainers#313 (comment) Signed-off-by: W. Trevor King <[email protected]>

wking · 2016-10-25T18:45:36Z

On Fri, Sep 16, 2016 at 01:59:25PM -0700, Jonathan Boulle wrote:

Let's resolve this via #318.

I've closed #318 because there seemed to be a consensus around part of
what it was doing. Part of what I proposed in #318 has landed via
#349. Of the remaining bits, #408 has the wording that appears
uncontentious and #407 continues the discussion started by this PR
(which seems to still be unsettled 1).

coolljt0725 mentioned this pull request Sep 14, 2016

*: rename serialization.md -> config.md #305

Merged

wking reviewed Sep 14, 2016
View reviewed changes

An image may have emtpy layers array

4492138

Signed-off-by: Lei Jitang <[email protected]>

stevvooe requested changes Sep 14, 2016

View reviewed changes

stevvooe approved these changes Sep 14, 2016

View reviewed changes

stevvooe requested changes Sep 14, 2016

View reviewed changes

runcom mentioned this pull request Sep 14, 2016

image: manifest.go: remove empty layers check #308

Closed

wking mentioned this pull request Sep 15, 2016

Replace 'base image' with 'base layer' #312

Closed

jonboulle changed the title ~~An image may have emtpy layers array~~ An image may have empty layers array Sep 16, 2016

wking mentioned this pull request Sep 16, 2016

manifest: Explicitly base manifest.layers on an empty directory #318

Closed

jonboulle closed this Sep 16, 2016

wking mentioned this pull request Oct 20, 2016

manifest: Explicitly make 'layers' optional #407

Closed

coolljt0725 deleted the layer_could_be_empty branch October 26, 2016 01:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

An image may have empty layers array #313

An image may have empty layers array #313

coolljt0725 commented Sep 14, 2016

wking Sep 14, 2016

runcom commented Sep 14, 2016

wking Sep 14, 2016

wking commented Sep 14, 2016

stevvooe left a comment

stevvooe Sep 14, 2016

runcom commented Sep 14, 2016

stevvooe left a comment

runcom commented Sep 14, 2016

stevvooe commented Sep 14, 2016

runcom commented Sep 14, 2016

wking commented Sep 14, 2016

stevvooe commented Sep 14, 2016

wking commented Sep 14, 2016

jonboulle commented Sep 16, 2016

stevvooe commented Sep 16, 2016

jonboulle commented Sep 16, 2016

stevvooe commented Sep 16, 2016

wking commented Sep 16, 2016

stevvooe commented Sep 16, 2016

wking commented Sep 16, 2016

jonboulle commented Sep 16, 2016

wking commented Oct 25, 2016

An image may have empty layers array #313

An image may have empty layers array #313

Conversation

coolljt0725 commented Sep 14, 2016

wking Sep 14, 2016

Choose a reason for hiding this comment

runcom commented Sep 14, 2016

wking Sep 14, 2016

Choose a reason for hiding this comment

wking commented Sep 14, 2016

stevvooe left a comment

Choose a reason for hiding this comment

stevvooe Sep 14, 2016

Choose a reason for hiding this comment

runcom commented Sep 14, 2016

stevvooe left a comment

Choose a reason for hiding this comment

runcom commented Sep 14, 2016

stevvooe commented Sep 14, 2016

runcom commented Sep 14, 2016

wking commented Sep 14, 2016

stevvooe commented Sep 14, 2016

wking commented Sep 14, 2016

jonboulle commented Sep 16, 2016

stevvooe commented Sep 16, 2016

jonboulle commented Sep 16, 2016

stevvooe commented Sep 16, 2016

wking commented Sep 16, 2016

stevvooe commented Sep 16, 2016

wking commented Sep 16, 2016

jonboulle commented Sep 16, 2016

wking commented Oct 25, 2016