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.

Sign up for GitHub

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

Jump to bottom

Add initial pass at a cmd line spec #511

Closed
wants to merge 1 commit into from
Closed

Add initial pass at a cmd line spec #511

wants to merge 1 commit into from

Conversation

duglin
Copy link
Contributor

@duglin duglin commented Jul 5, 2016

Gives us something to shoot arrows at :-)

Signed-off-by: Doug Davis [email protected]

@wking
Copy link
Contributor

wking commented Jul 5, 2016

On Tue, Jul 05, 2016 at 03:10:31PM -0700, Doug Davis wrote:

Gives us something to shoot arrows at :-)

There's also existing work in 1, linked from 2.

 Subject: Re: Specifying the runtime's command-line interface
 Date: Wed, 2 Dec 2015 12:40:09 -0800
 Message-ID: <[email protected]>

cyphar
cyphar reviewed Jul 6, 2016
View reviewed changes
cmd.md

Options:
* `-b <dir>`, `--bundle <dir>` The path to the root of the bundle directory. If not specified the default value MUST be the current working directory.
* `--console <path>` The PTY slave path for the newly created container.
Copy link
Member

Choose a reason for hiding this comment

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

We still need to figure out how we should deal with consoles in the spec. I'm personally still wondering what the semantics of consoles should be.

/cc @wking

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that kind of thing would be in the runtime.md doc no? I tried really hard to keep this doc focused just on the syntax and not semantics, where possible.

Copy link
Member

@cyphar cyphar Jul 6, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I'm including whether or not the console options should even be a command-line option. Right now it feels wrong to me, especially since terminal is in the config. Once we fix how we do path resolution, --console will be intimately related to the mountpoints of the container.

Copy link
Contributor

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 07:16:56AM -0700, Doug Davis wrote:

+* --console <path> The PTY slave path for the newly created container.

I think that kind of thing would be in the runtime.md doc no?

I agree, and think it's not worth worrying about how terminal
information fits into the API before we have a clearer non-API spec
around what the setting means or have punted it from the non-API spec
[1](which is why wking/oci-command-line-api does not talk about
process.terminal-related stuff).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now I tried to make this doc align with what runc does so at least we're consistent. If we decide to change runc then that PR can cover both the code and this doc.

@wking wking mentioned this pull request Jul 6, 2016
Closed
wking
wking reviewed Jul 11, 2016
View reviewed changes
cmd.md

Options: None

This action MUST display the state of the specific container to stdout.
Copy link
Contributor

Choose a reason for hiding this comment

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

#513 has some wording around the expected encoding for the output from these commands. Background on this in wking/oci-command-line-api#2 and wking/oci-command-line-api#16.

@wking wking mentioned this pull request Jul 12, 2016
Closed
@runcom
Copy link
Member

runcom commented Jul 19, 2016

@duglin what about --version? and its output? I found runc has a 3 line output from --version. Should it be part of this? Should the output follow some convention? (sorry if I'm out of mind saying this)

@duglin
Copy link
Contributor Author

duglin commented Jul 19, 2016

@runcom I would actually prefer for this to be left out of the spec for now. I like the idea of our cmd line spec focusing on just the corresponding features mentioned in the runtime spec.

While getting the version info is interesting, I don't think its critical for interop right now. If we want it then, yes as you said, we'd need to include the definition of the output (in machine readable format too), and should probably be in the runtime spec.

@runcom
Copy link
Member

runcom commented Jul 19, 2016

Yeah, I didn't mean to have that included in this PR, I'll open another issue to track my question/issue

@vbatts
Copy link
Member

vbatts commented Jul 19, 2016

/me on my phone
Is there presently a way to determine version of the spec implemented by a
cli?

On Tue, Jul 19, 2016, 17:41 Antonio Murdaca [email protected]
wrote:

Yeah, I didn't mean to have that included in this PR, I'll open another
issue to track my question/issue

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#511 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEF6Z8Smb2k8WQFoG1KfLXKfDaz4xtvks5qXURagaJpZM4JFlJW
.

@wking
Copy link
Contributor

wking commented Jul 19, 2016

On Tue, Jul 19, 2016 at 02:46:10PM -0700, Vincent Batts wrote:

Is there presently a way to determine version of the spec
implemented by a cli?

No, and there's a general lack of clarity in how ociVersion should be
applied. More details in opencontainers/tob#16 and discussion in 1.
@duglin was going to file something about this 2.

@duglin duglin closed this Jul 20, 2016
wking added a commit to wking/oci-command-line-api that referenced this pull request Feb 9, 2017
@wking
runtime: Add --pid-file to 'create'
fdd157c 
You can get the PID by calling 'state' [1], and container PIDs may not
be portable [2].  But it's a common way for interfacing with init
systems like systemd [3], and for this first pass at the command line
API folks are ok with some Linux-centrism [4].

[1]: opencontainers/runtime-spec#511 (comment)
     Subject: Add initial pass at a cmd line spec
[2]: opencontainers/runtime-spec#459
     Subject: [ Runtime ] Allow for excluding pid from state
[3]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-07-20-21.03.log.html#l-69
[4]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-07-13-17.03.log.html#l-39

Signed-off-by: W. Trevor King <[email protected]>
wking added a commit to wking/oci-command-line-api that referenced this pull request Feb 9, 2017
@wking
runtime: Add --pid-file to 'create'
6f5d033 
You can get the PID by calling 'state' [1], and container PIDs may not
be portable [2].  But it's a common way for interfacing with init
systems like systemd [3], and for this first pass at the command line
API folks are ok with some Linux-centrism [4].

[1]: opencontainers/runtime-spec#511 (comment)
     Subject: Add initial pass at a cmd line spec
[2]: opencontainers/runtime-spec#459
     Subject: [ Runtime ] Allow for excluding pid from state
[3]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-07-20-21.03.log.html#l-69
[4]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-07-13-17.03.log.html#l-39

Signed-off-by: W. Trevor King <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@duglin @wking @runcom @vbatts @cyphar