Cleanup bundle.md #210

duglin · 2015-10-02T14:56:15Z

Mainly just moved stuff around, but also tried to add some clarity around
what is required w.r.t. naming and location of files/dirs.

But to highlight some things that I want to make sure we ALL agree on:

config.json and runtime.json MUST use those exact names
name of rootfs dir is user defined - but nudge them towards rootfs
all 3 (config.json, runtime.json and rootfs dir) MUST all be in the same location/dir
that 'location' is NOT part of the bundle itself (this may matter when someone defines the on-the-wire format of a bundle)

Signed-off-by: Doug Davis dug@us.ibm.com

wking · 2015-10-02T17:44:46Z

On Fri, Oct 02, 2015 at 07:56:16AM -0700, Doug Davis wrote:

config.json and runtime.json MUST use those exact names

Agreed, although I'm fine if runtimes support extensions to select
other names (and I've added that flexibility to my proposed
command-line API spec [1,2], but we can revisit this if/when that
lands in this repo).

name of rootfs dir is user defined - but nudge them towards
rootfs

I don't see the point of requiring this 3, but yeah, I think that's
how the spec reads now.

all 3 (config.json, runtime.json and rootfs dir) MUST all be in
the same location/dir

I don't think we require that for rootfs, since the path from
config.json's root could contain directories (relative to config.json?
Relative to the runtime's current working directory?).

that 'location' is NOT part of the bundle itself (this may matter
when someone defines the on-the-wire format of a bundle)

+1.

duglin · 2015-10-02T17:50:31Z

all 3 (config.json, runtime.json and rootfs dir) MUST all be in
the same location/dir

I don't think we require that for rootfs, since the path from
config.json's root could contain directories (relative to config.json?
Relative to the runtime's current working directory?).

In bundle.md it say:

A single rootfs directory MUST be in the same directory as the config.json

I don’t personally have a preference yet - was just trying to keep the current state of things and get clarity.

-Doug

wking · 2015-10-02T18:00:41Z

On Fri, Oct 02, 2015 at 10:50:32AM -0700, Doug Davis wrote:

all 3 (config.json, runtime.json and rootfs dir) MUST all be in
the same location/dir

I don't think we require that for rootfs, since the path from
config.json's root could contain directories (relative to
config.json? Relative to the runtime's current working
directory?).

In bundle.md it say:

A single rootfs directory MUST be in the same directory as the
config.json

Well that's pretty unambiguous ;), so I'd guess language like that is
fine for now. It makes the relative-path bit in config.md a bit out
of place, but polishing for consistency is probably secondary to
figuring out why we actually need to require a root directory (and
what it means when a container doesn't create a mount namespace), or
whether it can be optional.

mrunalp · 2015-10-05T16:45:29Z

bundle.md

You mean out of scope for this section?

On Mon, Oct 05, 2015 at 09:45:31AM -0700, Mrunal Patel wrote:

+Issues such as distribution, including how to transfer a
container between runtimes, assigning names, versioning of bundle,
or discovery of bundles are out of scope of this specification.

You mean out of scope for this section?

Maybe we should just drop this line to punt on this issue, because I
think it's out of scope for this specification. Lots of back and
forth on this point in 1 ;).

Yeah, taking the line out for now may not be a bad idea.

I'm not following. These topics are out of scope of the "specification" right now, no? I know the charter discussions are happening to decide for sure, but as of now, they're out of scope. So, I don't see the issue with saying what's out of scope. What am I missing?

On Mon, Oct 05, 2015 at 10:27:00AM -0700, Doug Davis wrote:

+Issues such as distribution, including how to transfer a
container between runtimes, assigning names, versioning of bundle,
or discovery of bundles are out of scope of this specification.

I'm not following. These topics are out of scope of the
"specification" right now, no? I know the charter discussions are
happening to decide for sure, but as of now, they're out of
scope. So, I don't see the issue with saying what's out of scope.
What am I missing?

To me “out of scope” is explicitly “we will never cover this”, and I'd
use “unspecified” for “we will cover this at some point, but haven't
got around to it yet”. With that phrasing, I think we all agree that
these distribution issues are currently unspecified, but we disagree
about whether they're out of scope for this spec.

But I don't have sources to cite for that distinction, so maybe it's
just me ;).

that is the distinction. Discoverability is not explicitly out of scope.

I think what's there is more appropriate since all we can say is a reflection of the charter - and as of now its "out of scope" - we will never cover this w/o a charter change. Using a word like "unspecified" could be interpreted as "we're choosing not to define it at this time but we could later", but that's not true w/o a charter change.

@vbatts : what's in or out of scope is still TBD. As much as I think the current scope of what's in this doc & PR says is too limited, I'm trying to go with what's agreeable for "in scope" for now and then will let the charter discussions change this doc later.

not. the charter currently says

d. Decentralized. Discovery of container images should be simple and facilitate a federated namespace and distributed retrieval.

So there should not be wording that something like this is "out of scope"

To be honest, I have no idea what a section titled "OCI Values" even means and I've been pushing to fix that because some people interpret it as just fluff filler text that's non-normative, while others (as you've hinted) view it as part of our "in scope list of work items". Because if its our list of work items then when 'c' includes "image auditing" and "cryptographic primitives", where is that in our work?

But I was actually hoping to avoid charter discussions as part of this PR. I'm ok with removing this sentence for now but immediately after the charter is finalized I think we need to add something like it back in so people have the right level of expectations w.r.t. what our goals are.

ya, i would remove for now until the charter is finished, otherwise this pr LGTM

duglin · 2015-10-05T20:03:28Z

questionable text removed - all comments addressed - I think

crosbymichael · 2015-10-05T20:03:54Z

LGTM

wking · 2015-10-05T20:11:43Z

bundle.md

“security permissions” seems vague. Do you mean linux.capabilities? I'm not sure how that separates out from cgroups which are also sometimes about permissions (e.g. the device cgroup is just about permissions), but I'd probably drop that entry from your list of examples. However, after dropping it, the only remaining entries are about process, so I think we should maybe just stop after “application specific” ;).

Not sure who wrote that phrase originally. @vbatts @crosbymichael any thoughts? Remove examples or is there another example we should use?

it's just an example, being vague is ok.

On Mon, Oct 05, 2015 at 03:40:11PM -0700, Michael Crosby wrote:

+This file, which MUST be named config.json, contains settings
that are host independent and application specific such as
security permissions, environment variables and arguments.

it's just an example, being vague is ok.

I think it's misleading, because almost all of the security stuff is
going to be handled in runtime.json. If we mean Linux capabilities,
it seems easy enough to just say that.

But whatever, there's a link to the config specs right after each
bullet point, so folks can see what's actually in the files ;).

@duglin, ‘git log -p -G 'security permissions'’ points to 7232e4b
(specs: introduce the concept of a runtime.json, 2015-07-30, #88).
The only inline comment on anything in its general viscinity was the
conversation I had with @philips 1 that lead to me filing #107 with
a definition for “application” (since removed from that PR 2).

mrunalp · 2015-10-07T17:10:16Z

bundle.md

s/will/MUST/ ?

Mainly just moved stuff around, but also tried to add some clarity around what is required w.r.t. naming and location of files/dirs. Signed-off-by: Doug Davis <dug@us.ibm.com>

mrunalp · 2015-10-07T19:13:39Z

LGTM

duglin force-pushed the modBundle branch from 1abc017 to 1e9bd82 Compare October 2, 2015 17:23

mrunalp reviewed Oct 5, 2015
View reviewed changes

duglin force-pushed the modBundle branch from 1e9bd82 to 60f47b5 Compare October 5, 2015 18:40

wking reviewed Oct 5, 2015
View reviewed changes

duglin force-pushed the modBundle branch from 60f47b5 to 4c6494c Compare October 5, 2015 22:37

mrunalp reviewed Oct 7, 2015
View reviewed changes

bundle.md Outdated

Copy link

Contributor

mrunalp Oct 7, 2015

Choose a reason for hiding this comment

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

s/will/MUST/ ?

Cleanup bundle.md

106ec2d

Mainly just moved stuff around, but also tried to add some clarity around what is required w.r.t. naming and location of files/dirs. Signed-off-by: Doug Davis <dug@us.ibm.com>

duglin force-pushed the modBundle branch from 4c6494c to 106ec2d Compare October 7, 2015 17:22

mrunalp merged commit cf8dd12 into opencontainers:master Oct 7, 2015

duglin deleted the modBundle branch October 13, 2015 13:35

wking mentioned this pull request Dec 9, 2015

start: Change --config and --runtime to --bundle wking/oci-command-line-api#11

Merged

wking mentioned this pull request Mar 16, 2017

config: Lead off with the purpose of the config #737

Merged

wking mentioned this pull request Apr 5, 2017

config: Drop the redundant config.json filename requirement #756

Merged

Cleanup bundle.md #210

Cleanup bundle.md #210

Uh oh!

Conversation

duglin commented Oct 2, 2015

Uh oh!

wking commented Oct 2, 2015

Uh oh!

duglin commented Oct 2, 2015

Uh oh!

wking commented Oct 2, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

duglin commented Oct 5, 2015

Uh oh!

crosbymichael commented Oct 5, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrunalp commented Oct 7, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants