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.

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

Make path to bundle optional #447

Closed
wants to merge 1 commit into from

Conversation

hqhq
Copy link
Contributor

@hqhq hqhq commented May 23, 2016

Make it optional looks reasonable to me, and it's what
we do in runc now.

Signed-off-by: Qiang Huang h.huangqiang@huawei.com

Make it optional looks reasonable to me, and it's what
we do in runc now.

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
@wking
Copy link
Contributor

wking commented May 23, 2016

On Mon, May 23, 2016 at 02:31:56AM -0700, Qiang Huang wrote:

Make it optional looks reasonable to me, and it's what we do in runc
now.

These sketches are not intended to be a command-line API 1. I think
this change would be useful in a command-line API document, but is
probably too detailed for the operation overview here (where “the
runtime needs to know which config to use” is all you need to
express).

@crosbymichael
Copy link
Member

@hqhq i think this is just an error in the docs, it is a flag on the runtime and has always been a flag. I think its safe to just remove this for now as it was never the case.

@wking
Copy link
Contributor

wking commented May 23, 2016

On Mon, May 23, 2016 at 12:57:55PM -0700, Michael Crosby wrote:

I think its safe to just remove this for now as it was never the
case.

What was never the case? The runtime does need to know which
bundle/config it's starting. For runC, it can get a reasonable
default by using getcwd(3), but that won't be a reasonable default for
all runtimes (e.g. if your runtime API is via HTTP web requests, the
web-server's CWD is probably not interesting). The ‘start’ operation
needs more information here than, say, the ‘state’ operation, so I
think it makes sense to list that extra information here.

@crosbymichael
Copy link
Member

Oh, I see. This is not specifying a CLI but just specifying the inputs to the start action. If we look at it that way then bundle path is required even though most runtimes will imply the bundle path is the cwd but for the start action it is a required input.

@wking
Copy link
Contributor

wking commented May 23, 2016

On Mon, May 23, 2016 at 01:15:23PM -0700, Michael Crosby wrote:

This is not specifying a CLI but just specifying the inputs to the
start action. If we look at it that way…

That's how I've been looking at it based on 1, but a few folks have
interpreted these as a CLI API now, so it's probably worth explicitly
saying that they aren't in the spec. I can file a separate PR to that
effect tonight.

hqhq added a commit to hqhq/runtime-spec that referenced this pull request May 24, 2016
Replace: opencontainers#447

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
@hqhq
Copy link
Contributor Author

hqhq commented May 24, 2016

@wking It make sense if it's input for general operation, I already opened a PR #450 to clarify this, PTAL, thanks.

@hqhq hqhq closed this May 24, 2016
Mashimiao pushed a commit to Mashimiao/specs that referenced this pull request Aug 19, 2016
Replace: opencontainers#447

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants