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

docs/command-line-interface: Add Runtime CLI Spec #321

Merged
merged 65 commits into from
Jan 10, 2018

Conversation

wking
Copy link
Contributor

@wking wking commented Feb 10, 2017

Transferred from opencontainers/runtime-spec#513 as discussed in yesterday's meeting. @mrunalp gave me some pointers on how he wanted this landed here and here.

I'm not squashing this down into a single commit, because it's a lot easier for me to manage revisions if I keep a branch in the stand-alone repository. I would encourage reviewers (CC @RobDolinMS and @stevvooe, who expressed an interest in reviewing this stuff) to file issues/PRs against that repository so this PR doesn't get bogged down into the same morass as the runtime-spec PR; this PR would get updated to pull those in as I land the sub-PRs in the stand-alone repo. @mrunalp thinks we might end up squashing this just before merging, and how that goes is clearly up to the runtime-tools maintainers, but squashing (if it happens at all) should be after substantive review settles.

The WIP tag in the summary is because I'm still not sure where runtime-tools maintainers want the JSON Schema from the runtime-spec PR.

@mrunalp was also open to some compliance language, but I haven't figured that out yet because the pattern set in RFC 2616 (which runtime-spec followed) wasn't even repeated by RFC 7230 (which obsoleted RFC 2616). I need some time to compare the differences in the RFC wording before I can float appropriate wording for the command line API.

julz and others added 30 commits December 2, 2015 13:27
Signed-off-by: Julian Friedman <julz.friedman@uk.ibm.com>
To match the specs convention [1].

[1]: /~https://github.com/opencontainers/specs/blob/v0.1.1/README.md#markdown-style

Signed-off-by: W. Trevor King <wking@tremily.us>
Otherwise they're rendered as a single paragraph.

Signed-off-by: W. Trevor King <wking@tremily.us>
See [1].  'sh' keyword comes from [2], and seemed more explicit to the
POSIX semantics than 'shell'.

[1]: https://help.github.com/articles/github-flavored-markdown/#syntax-highlighting
[2]: /~https://github.com/github/linguist/blob/master/lib/linguist/languages.yml

Signed-off-by: W. Trevor King <wking@tremily.us>
It's pretty clear that the header describes the rest of the file's
contents without the colon ;).

Signed-off-by: W. Trevor King <wking@tremily.us>
I think that's the possessive form of 'process' ;).

Signed-off-by: W. Trevor King <wking@tremily.us>
To give space for more detail about the streams.

I'm not sure what platform-agnostic language for this should look
like, but on POSIX I'm expecting just exec'ing the application process
and inheriting the file descriptors.

Signed-off-by: W. Trevor King <wking@tremily.us>
Signed-off-by: W. Trevor King <wking@tremily.us>
Catch up with [1].

[1]: opencontainers/runtime-spec#88

Signed-off-by: W. Trevor King <wking@tremily.us>
Use GNU-style long options to avoid ambiguous, one-character options
in the spec, while still allowing the runtime to support one-character
options with packing.

Signed-off-by: W. Trevor King <wking@tremily.us>
Let folks do whatever they want as long as they don't clobber our
command namespace.

Signed-off-by: W. Trevor King <wking@tremily.us>
Make it easy for a caller to report which runtime they're using.

Signed-off-by: W. Trevor King <wking@tremily.us>
Give the user a consistent way to pick their container ID.  For example, they may want to use:

  --id $(dirname "$PWD")

or they may want to use:

  --id $(uuidgen)

or they may want to leave ID generation to the runtime.

Signed-off-by: W. Trevor King <wking@tremily.us>
You shouldn't need these for any other operations.

Signed-off-by: W. Trevor King <wking@tremily.us>
Signed-off-by: W. Trevor King <wking@tremily.us>
Mostly for stopping them, but also useful for any number of signals
(e.g. many applications use HUP to reload their configuration).

Signed-off-by: W. Trevor King <wking@tremily.us>
On IRC just now:

10:56 < crosbymichael> if the main process dies in the container, all other process are killed
...
10:58 < julz> crosbymichael: Im assuming what you mean is you kill everything in the cgroup -> everything in the container dies?
10:58 < crosbymichael> julz: yes, that is how its implemented
...
10:59 < crosbymichael> julz: we actually freeze first, send the KILL, then unfreeze so we don't have races

We don't have lifecycle docs for that yet, but once we do, we can
update this to link to them.

Signed-off-by: W. Trevor King <wking@tremily.us>
These are useful for checkpointing, since getting a consistent
checkpoint of a running container is hard ;).  This doesn't handle the
checkpoints themselves though, which are currently not specified.
Checkpoint behavior will look something like:

  $ funC pause --wait container-id
  $ checkpoint ...
  $ funC resume container-id

Signed-off-by: W. Trevor King <wking@tremily.us>
This landed in runC with [1], but the bundle-author <-> runtime specs
explicitly avoid talking about how this is set (since the
bundle-author doesn't care about the runtime-caller <-> runtime
interface) [2].  However, *this* spec is about the runtime-caller <->
runtime interface, so we need to document it here.

I've left LISTEN_PID [3,4] out, since I don't see how the
runtime-caller would choose anything other than 1 for its value.  It
seems like something that a process would have to set for itself
(because guessing the PID of a child before spawning it seems racy ;).
In any event, the runC implementation seems to set this to 1
regardless of what systemd passes to it [4].

I've borrowed Shishir's wording for the example [4].

[1]: opencontainers/runc#231
[2]: opencontainers/runtime-spec#113 (comment)
[3]: http://www.freedesktop.org/software/systemd/man/sd_listen_fds.html
[4]: opencontainers/runc#231 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>

Cherry-picked from [1].

[1]: opencontainers/runtime-spec@84707b0

Signed-off-by: W. Trevor King <wking@tremily.us>
See /~https://github.com/blog/1184-contributing-guidelines

Also strip some trailing whitespace.

Signed-off-by: W. Trevor King <wking@tremily.us>
Signed-off-by: W. Trevor King <wking@tremily.us>
Without this, "may contain any Unicode characters" seemed too
ambiguous.

I wish there were cleaner references for the {language}.{encoding}
locales like en_US.UTF-8 and UTF-8.  But [1,2] seems too glib, and I
can't find a more targetted UTF-8 link than just dropping folks into a
Unicode chapter (which is what [1] does):

  The Unicode Standard, Version 6.0, §3.9 D92, §3.10 D95 (2011)

With the current v8.0 (2015-06-17), it's still §3.9 D92 and §3.10 D95.

The TR35 link is for:

  In addition, POSIX locales may also specify the character encoding,
  which requires the data to be transformed into that target encoding.

and the POSIX §6.2 link is for:

  In other locales, the presence, meaning, and representation of any
  additional characters are locale-specific.

[1]: https://en.wikipedia.org/wiki/UTF-8
[2]: https://en.wikipedia.org/wiki/Locale#POSIX_platforms

Signed-off-by: W. Trevor King <wking@tremily.us>
Reviewed-by: Jesse Butler <jeeves.butler@gmail.com>
The other commands have the following layout:

  ### {command name}

  {one-line description}

  * *Options:* ...
  ...
  * *Exit code:* ...

  {additional notes}

  {example}

so its good to follow that pattern for 'version'.

This commit also removes a dangling "The version" which snuck in with
ffa124c (Add a 'version' command, 2015-09-14).

Signed-off-by: W. Trevor King <wking@tremily.us>
Reviewed-by: Mike Brown <brownwm@us.ibm.com>
With:

  $ sed -i 's/[[:space:]]*$//' *

Signed-off-by: W. Trevor King <wking@tremily.us>
Reviewed-by: Mike Brown <brownwm@us.ibm.com>
All of these require state information to be shared between funC
invocations (to map from a container ID to the
cgroups/namespaces/etc.), and after today's meeting we may be backing
away from that [1,2].  Even if we keep a requirement for sharing state
between funC invocations, we don't want to specify these IPC-requiring
commands until we have more clarity on that requirement in the spec.

On systems like Solaris, the kernel maintains a registry of container
IDs directly, so they don't need an external registry [3].  But
without a consensus around the minimal amount of inter-process state
sharing, we don't want to require container ID → state lookups in the
command-line spec.  Once we have more clarity on a minimal required
mechanism (e.g. Julz's --state-file [4,5,6]), we can add them back in
with an API that all runtimes can easily support (although runtimes
are of course free to provide more convenient APIs as additional
extensions).

Pause, resume, and signal are still in the current lifecycle pull
request [7], but I've requested they be removed until we have more
clarity around the basic lifecycle [8].

[1]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2015/opencontainers.2015-12-02-18.01.html
[2]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2015/opencontainers.2015-12-02-18.01.log.html#l-79
[3]: wking/oci-command-line-api#3 (comment)
[4]: wking/oci-command-line-api#3 (comment)
[5]: wking/oci-command-line-api#3 (comment)
[6]: wking/oci-command-line-api#3 (comment)
[7]: mrunalp/specs@bd549a2#diff-b84a8d65d8ed53f4794cd2db7e8ea731R48
[8]: /~https://github.com/opencontainers/specs/pull/231/files#r45532412

Signed-off-by: W. Trevor King <wking@tremily.us>
Reviewed-by: Mike Brown <brownwm@us.ibm.com>
Reviewed-by: Jesse Butler <jeeves.butler@gmail.com>
Reviewed-by: Julian Friedman <julz.friedman@uk.ibm.com>
This makes everything consistent with the version command's:

  Print the runtime version and exit.

And follows the practice recommended by Python's PEP 257 [1]:

  The docstring is a phrase ending in a period. It prescribes the
  function or method's effect as a command ("Do this", "Return that"),
  not as a description; e.g. don't write "Returns the pathname ...".

[1]: https://www.python.org/dev/peps/pep-0257/#one-line-docstrings

Signed-off-by: W. Trevor King <wking@tremily.us>
Reviewed-by: Mike Brown <brownwm@us.ibm.com>
The reference landed in the initial 3f0aafb (Add initial command line
spec, 2015-09-02) in the operations section.  In f3a1c08 (Shift
config.json and runtime.json into the 'start' section, 2015-09-15)
they moved to the 'start' command section, because other commands
don't need them.  But we talk about them in start's options section
since 9d97d39 (Add --config and --runtime options, 2015-09-14), so
there's no need for this command-scoped reference.

Signed-off-by: W. Trevor King <wking@tremily.us>
Reviewed-by: Mike Brown <brownwm@us.ibm.com>
While reviewing the `global options` section in `runtime.md`, it seemed additional clarity
was needed for the `command` and `global options` requirements.

Discussed and worked on wording with @wking via private IRC.

Also adds uppercase RFC 2119 words for this section.

With regard to the statement `Command names MUST not start with hyphens,` the rationale
behind this decision is to "distinguish unrecognized commands from unrecognized options,
and because we are "requiring runtimes to fail-fast for unrecognized commands" [1,2].

`[1]` /~https://github.com/wking/oci-command-line-api/pull/8/files#r46898167
`[2]` wking/oci-command-line-api@527f3c6#commitcomment-14835617

Signed-off-by: Mike Brown <brownwm@us.ibm.com>
Reviewed-by: W. Trevor King <wking@tremily.us>
Personally I prefer a single config file [1].  I want folks to be able
to pipe their config into the 'funC start' command (e.g. via a
/dev/fd/3 pseudo-filesystem path) [2], and I have a working example
that supports this without difficulty [3].  But since [4] landed on
2015-11-16, runC has replaced their --config-file and --runtime-file
flags with --bundle, and the current goal of this repository is
"keeping as much similarity with the existing runC command-line as
possible", not "makes sense to Trevor" ;).

[1]: https://groups.google.com/a/opencontainers.org/forum/#!topic/dev/0QbyJDM9fWY
     Subject: Single, unified config file (i.e. rolling back specs#88)
     Date: Wed, 4 Nov 2015 09:53:20 -0800
     Message-ID: <20151104175320.GC24652@odin.tremily.us>
[2]: opencontainers/runc#202
[3]: /~https://github.com/wking/ccon/tree/8ab5b535b5eca1a62e12b5e865735e24f1e1666d#configuration
[4]: opencontainers/runc#373

Signed-off-by: W. Trevor King <wking@tremily.us>
Reviewed-by: Mike Brown <brownwm@us.ibm.com>
@wking
Copy link
Contributor Author

wking commented Feb 15, 2017

I've taken a stab at compliance language with afdf0596d8cfe2 (with the compliance language coming in with be7de4f).

@wking wking mentioned this pull request Mar 9, 2017
@crosbymichael
Copy link
Member

crosbymichael commented Mar 15, 2017

LGTM

Approved with PullApprove

@mrunalp
Copy link
Contributor

mrunalp commented Mar 15, 2017

@wking Can you squash the commits before merge?

@wking wking force-pushed the command-line-api branch from 6d8cfe2 to 7eaa0b8 Compare March 15, 2017 23:23
@wking wking changed the title WIP: docs/command-line-interface: Add Runtime CLI Spec docs/command-line-interface: Add Runtime CLI Spec Mar 15, 2017
@wking
Copy link
Contributor Author

wking commented Mar 15, 2017 via email

@wking wking force-pushed the command-line-api branch from 7eaa0b8 to 841fcac Compare March 15, 2017 23:35
@wking
Copy link
Contributor Author

wking commented Mar 15, 2017

Pushed 7eaa0b8841fcac fixing a broken link into schema/README.md.

@@ -0,0 +1,24 @@
{
"description": "Open Container Runtime Socket Teminal Request Schema",
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: Teminal -> Terminal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks :). Fixed with 841fcacebae480.

…ket messages

Even though there aren't many new types, these are fairly different
from the rest of the runtime-tools code, and a separate package lets
folks pull in only the code they need (assuming they are sophisticated
enough to grab only a subset of the Git repository).

Mrunal approved the JSON Schema landing under schema/ [1].

[1]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2017/opencontainers.2017-03-15-21.02.log.html#l-122

Signed-off-by: W. Trevor King <wking@tremily.us>
Catching up with erratum 499 [1].

[1]: https://www.rfc-editor.org/errata_search.php?rfc=2119&eid=499

Signed-off-by: W. Trevor King <wking@tremily.us>
@zhouhao3
Copy link

zhouhao3 commented Jan 10, 2018

@liangchenye
Copy link
Member

liangchenye commented Jan 10, 2018

LGTM and I'll squash the commits while merging it.

Approved with PullApprove

@Mashimiao Mashimiao merged commit 7a4cb36 into opencontainers:master Jan 10, 2018
wking added a commit to wking/ocitools-v2 that referenced this pull request Jan 11, 2018
Make docs/, which landed in 7a4cb36 (docs/command-line-interface: Add
Runtime CLI Spec, 2018-01-09, opencontainers#321), more discoverable.  Also document
the 'RUNTIME' option.
wking added a commit to wking/ocitools-v2 that referenced this pull request Jan 11, 2018
Make docs/, which landed in 7a4cb36 (docs/command-line-interface: Add
Runtime CLI Spec, 2018-01-09, opencontainers#321), more discoverable.  Also document
the 'RUNTIME' option.

Signed-off-by: W. Trevor King <wking@tremily.us>
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.

8 participants