From ca2fda108d67c0a57b320c136b5d82437b8d9c8a Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Thu, 19 May 2016 22:04:52 -0700 Subject: [PATCH 1/3] config: Adjust process.env to immediately punt to POSIX The uppercase letter / digit / underscore restriction is just for "variables used by the utilities in the Shell and Utilities volume of IEEE Std 1003.1-2001". Copying over some POSIX wording and then linking to POSIX didn't seem like much gain. Just point people at POSIX and let them read about the name=value definition, charset suggestions, etc. there. Also link specifically to chapter 8 section 1 (instead of just chapter 8). Rob Dolin had suggested "platform-appropriate" wording [1], but it seems like Visual Studio 2015 supports an environment-variable array with the same semantics [2], and providing an explicit "platform-appropriate" wiggle seems like it's adding useless complication. [1]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-05-18-17.01.log.html#l-54 [2]: https://msdn.microsoft.com/en-us/library/431x4c1w.aspx Signed-off-by: W. Trevor King --- config.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/config.md b/config.md index 42a4701e6..c32dc32a0 100644 --- a/config.md +++ b/config.md @@ -125,9 +125,7 @@ See links for details about [mountvol](http://ss64.com/nt/mountvol.html) and [Se * **`width`** (uint, REQUIRED) * **`cwd`** (string, REQUIRED) is the working directory that will be set for the executable. This value MUST be an absolute path. -* **`env`** (array of strings, OPTIONAL) contains a list of variables that will be set in the process's environment prior to execution. - Elements in the array are specified as Strings in the form "KEY=value". - The left hand side MUST consist solely of letters, digits, and underscores `_` as outlined in [IEEE Std 1003.1-2001](http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html). +* **`env`** (array of strings, OPTIONAL) with the same semantics as [IEEE Std 1003.1-2001's `environ`][ieee-1003.1-2001-xbd-c8.1]. * **`args`** (array of strings, REQUIRED) executable to launch and any flags as an array. The executable is the first element and MUST be available at the given path inside of the rootfs. If the executable path is not an absolute path then the search $PATH is interpreted to find the executable. @@ -762,6 +760,7 @@ Here is a full example `config.json` for reference. [container-namespace]: glossary.md#container-namespace [go-environment]: https://golang.org/doc/install/source#environment +[ieee-1003.1-2001-xbd-c8.1]: http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html#tag_08_01 [runtime-namespace]: glossary.md#runtime-namespace [uts-namespace]: http://man7.org/linux/man-pages/man7/namespaces.7.html [mount.8-filesystem-independent]: http://man7.org/linux/man-pages/man8/mount.8.html#FILESYSTEM-INDEPENDENT_MOUNT%20OPTIONS From 70858bc49904a0c28a24fe64dc025bbd299d62e3 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Thu, 19 May 2016 22:32:56 -0700 Subject: [PATCH 2/3] config: Adjust process.args to cite POSIX's execvp This punts the awkward-to-enforce "MUST be available at the given path inside of the rootfs" to the kernel, which will do a much better job of enforcing that constraint than runtime code or a static validator. It also punts most of the semantics to POSIX, which does a better job than we'll do at specifying this. The extension is necessary because POSIX allows argv to be empty. In the DESCRIPTION: The argument arg0 should point to a filename that is associated with the process being started by one of the exec functions. And in RATIONALE: Early proposals required that the value of argc passed to main() be "one or greater". This was driven by the same requirement in drafts of the ISO C standard. In fact, historical implementations have passed a value of zero when no arguments are supplied to the caller of the exec functions. This requirement was removed from the ISO C standard and subsequently removed from this volume of IEEE Std 1003.1-2001 as well. The wording, in particular the use of the word should, requires a Strictly Conforming POSIX Application to pass at least one argument to the exec function, thus guaranteeing that argc be one or greater when invoked by such an application. In fact, this is good practice, since many existing applications reference argv[0] without first checking the value of argc. But with an empty 'args' we will have no process to call (since process lacks an explicit 'file' analog). I chose the 2001/2004 POSIX spec for consistency with the existing reference (which landed in 7ac41c69, config.md: reformat into a standard style, 2015-06-30, which did not motivate it's use of an older standard). For 2001 vs. 2004, [1] has: Abstract: The 2004 edition incorporates Technical Corrigendum Number 1 and Technical Corrigendum 2 addressing problems discovered since the approval of the 2001 edition. These are mainly due to resolving integration issues raised by the merger of the Base documents. and the text in the linked pages uses "IEEE Std 1003.1-2001" for internal linking. Rob Dolin had suggested "platform-appropriate" wording [2], but it seems like Visual Studio 2015 supports execvp [3], and providing an explicit "platform-appropriate" wiggle seems like it's adding useless complication. [1]: http://pubs.opengroup.org/onlinepubs/009695399/mindex.html [2]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-05-18-17.01.log.html#l-54 [3]: https://msdn.microsoft.com/en-us/library/3xw6zy53.aspx Signed-off-by: W. Trevor King --- config.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config.md b/config.md index c32dc32a0..aec437ae7 100644 --- a/config.md +++ b/config.md @@ -126,9 +126,8 @@ See links for details about [mountvol](http://ss64.com/nt/mountvol.html) and [Se * **`cwd`** (string, REQUIRED) is the working directory that will be set for the executable. This value MUST be an absolute path. * **`env`** (array of strings, OPTIONAL) with the same semantics as [IEEE Std 1003.1-2001's `environ`][ieee-1003.1-2001-xbd-c8.1]. -* **`args`** (array of strings, REQUIRED) executable to launch and any flags as an array. - The executable is the first element and MUST be available at the given path inside of the rootfs. - If the executable path is not an absolute path then the search $PATH is interpreted to find the executable. +* **`args`** (array of strings, REQUIRED) with similar semantics to [IEEE Std 1003.1-2001 `execvp`'s *argv*][ieee-1003.1-2001-xsh-exec]. + This specification extends the IEEE standard in that at least one entry is REQUIRED, and that entry is used with the same semantics as `execvp`'s *file*. For Linux-based systems the process structure supports the following process specific fields: @@ -761,6 +760,7 @@ Here is a full example `config.json` for reference. [container-namespace]: glossary.md#container-namespace [go-environment]: https://golang.org/doc/install/source#environment [ieee-1003.1-2001-xbd-c8.1]: http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html#tag_08_01 +[ieee-1003.1-2001-xsh-exec]: http://pubs.opengroup.org/onlinepubs/009695399/functions/exec.html [runtime-namespace]: glossary.md#runtime-namespace [uts-namespace]: http://man7.org/linux/man-pages/man7/namespaces.7.html [mount.8-filesystem-independent]: http://man7.org/linux/man-pages/man8/mount.8.html#FILESYSTEM-INDEPENDENT_MOUNT%20OPTIONS From a78f2559829c30d76d3f5cf3ae8c94d9bc4cdf27 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Fri, 6 May 2016 08:48:36 -0700 Subject: [PATCH 3/3] config: Explicitly list 'hooks' as optional And make it omitempty, otherwise: $ ocitools generate --template <(echo '{}') $ cat config.json | jq -S . { "hooks": {}, ... } To provide space for the type information and 'optional', I've shuffled the hook docs to follow our usual: * **`{property}`** ({type}, {when-needed}) {notes} format. I've kept the separate event-trigger sections (e.g. "### Prestart") since they go into more detail on the timing, purpose, and exit handling for the different events (and that seemed like too much information to put into the nested lists). I've replaced the Go reference from 48049d2 (Clarify the semantics of hook elements, 2015-11-25, #255) with POSIX references (following the new process docs) to address pushback against referencing Go [1,2] in favor of POSIX links [3]. Rob Dolin had suggested "platform-appropriate" wording [4], but it seems like Visual Studio 2015 supports execv [5], and providing an explicit "platform-appropriate" wiggle seems like it's adding useless complication. [1]: https://github.com/opencontainers/runtime-spec/pull/427#discussion_r62362761 [2]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-05-18-17.01.log.html#l-46 [3]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-05-18-17.01.log.html#l-52 [4]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-05-18-17.01.log.html#l-54 [5]: https://msdn.microsoft.com/en-us/library/886kc0as.aspx Signed-off-by: W. Trevor King --- config.md | 26 +++++++++++++------------- schema/config-schema.json | 3 +-- specs-go/config.go | 2 +- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/config.md b/config.md index aec437ae7..85c43c1ec 100644 --- a/config.md +++ b/config.md @@ -310,19 +310,24 @@ For Windows based systems the user structure has the following fields: ## Hooks -**`hooks`** (object, OPTIONAL) configures callbacks for container lifecycle events. Lifecycle hooks allow custom events for different points in a container's runtime. -Presently there are `Prestart`, `Poststart` and `Poststop`. -* [`Prestart`](#prestart) is a list of hooks to be run before the container process is executed -* [`Poststart`](#poststart) is a list of hooks to be run immediately after the container process is started -* [`Poststop`](#poststop) is a list of hooks to be run after the container process exits +* **`hooks`** (object, OPTIONAL) MAY contain any of the following properties: + * **`prestart`** (array, OPTIONAL) is an array of [pre-start hooks](#prestart). + Entries in the array contain the following properties: + * **`path`** (string, REQUIRED) with similar semantics to [IEEE Std 1003.1-2001 `execv`'s *path*][ieee-1003.1-2001-xsh-exec]. + This specification extends the IEEE standard in that **`path`** MUST be absolute. + * **`args`** (array of strings, REQUIRED) with the same semantics as [IEEE Std 1003.1-2001 `execv`'s *argv*][ieee-1003.1-2001-xsh-exec]. + * **`env`** (array of strings, OPTIONAL) with the same semantics as [IEEE Std 1003.1-2001's `environ`][ieee-1003.1-2001-xbd-c8.1]. + * **`timeout`** (int, OPTIONAL) is the number of seconds before aborting the hook. + * **`poststart`** (array, OPTIONAL) is an array of [post-start hooks](#poststart). + Entries in the array have the same schema as pre-start entries. + * **`poststop`** (array, OPTIONAL) is an array of [post-stop hooks](#poststop). + Entries in the array have the same schema as pre-start entries. Hooks allow one to run programs before/after various lifecycle events of the container. Hooks MUST be called in the listed order. -The state of the container is passed to the hooks over stdin, so the hooks could get the information they need to do their work. - -Hook paths are absolute and are executed from the host's filesystem in the [runtime namespace][runtime-namespace]. +The [state](runtime.md#state) of the container is passed to the hooks over stdin, so the hooks could get the information they need to do their work. ### Prestart @@ -374,11 +379,6 @@ If a hook returns a non-zero exit code, then an error is logged and the remainin } ``` -`path` is REQUIRED for a hook. -`args` and `env` are OPTIONAL. -`timeout` is the number of seconds before aborting the hook. -The semantics are the same as `Path`, `Args` and `Env` in [golang Cmd](https://golang.org/pkg/os/exec/#Cmd). - ## Annotations **`annotations`** (object, OPTIONAL) contains arbitrary metadata for the container. diff --git a/schema/config-schema.json b/schema/config-schema.json index 95113a800..9bae9ad18 100644 --- a/schema/config-schema.json +++ b/schema/config-schema.json @@ -171,7 +171,6 @@ "platform", "process", "root", - "mounts", - "hooks" + "mounts" ] } diff --git a/specs-go/config.go b/specs-go/config.go index 0166f46b3..34fecf5f6 100644 --- a/specs-go/config.go +++ b/specs-go/config.go @@ -17,7 +17,7 @@ type Spec struct { // Mounts configures additional mounts (on top of Root). Mounts []Mount `json:"mounts,omitempty"` // Hooks configures callbacks for container lifecycle events. - Hooks Hooks `json:"hooks"` + Hooks *Hooks `json:"hooks,omitempty"` // Annotations contains arbitrary metadata for the container. Annotations map[string]string `json:"annotations,omitempty"`