config: Explicitly list 'hooks' as optional #427

wking · 2016-05-06T16:00:33Z

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).

This may be unnecessary if #384 kills hooks, although:

I'm not sure if the run command will keep hooks, and
The current Split create and start #384 tip still has these hook docs.

crosbymichael · 2016-05-06T17:36:27Z

config.md

+    Entries in the array contain the following properties:
+    * **`path`** (string, required) with the same semantics as Go's [`Cmd.Path`][go-cmd].
+      Hook paths are absolute and are executed from the host's filesystem in the [runtime namespace][runtime-namespace].
+    * **`args`** (array of strings, optional) with the same semantics as Go's [`Cmd.Args`][go-cmd].


Don't make this go specific, its args and env like every args and env ever

On Fri, May 06, 2016 at 10:36:29AM -0700, Michael Crosby wrote:

* path (string, required) with the same semantics as Go's [Cmd.Path][go-cmd].

Hook paths are absolute and are executed from the host's filesystem in the [runtime namespace][runtime-namespace].

* args (array of strings, optional) with the same semantics as Go's [Cmd.Args][go-cmd].

Don't make this go specific, its args and env like every args and env ever

The Go reference is inherited from 48049d2 (Clarify the semantics of
hook elements, 2015-11-25, #255). I'm fine changing it to an
execve(2) reference or some such if you'd rather, but folks might
consider that platform-specific. POSIX also has execve(3p) 1, but
it's in unistd.h, which may be POSIX-specific. Is there a generic C
spec that covers this idea?

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, opencontainers#255) with POSIX references (following the new process docs) to address pushback against referencing Go [1,2] in favor of punting to platforms with POSIX links for POSIX systems [3]. [1]: opencontainers#427 (comment) [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-54 Signed-off-by: W. Trevor King <wking@tremily.us>

wking · 2016-05-20T06:44:50Z

This got some airtime in Wendesday's meeting, with the consensus being:

Don't reference Go (fine with me).
Do reference POSIX (also fine with me), but phrase it so the POSIX reference only applies to POSIX systems (I thought this would be ok, but no longer like it).

I've just pushed cb4cfd8 → a77a35a where I try to stick to the letter of that consensus (also updating the docs for process.args and process.env for consistency). However, now that I see it in action, I'd much rather make remove the “platform-appropriate way” wiggle words and point to POSIX for all platforms.

Attempting to dig into Windows support (@RobDolinMS, please correct me if I'm getting this wrong), it seems like Visual Studio 2015 does not support POSIX's execv and similar but does support C++ analogs in _execv and similar. Those look like drop-in replacements to me (although the docs are very sparse). And the compatibility notes discuss the leading underscore and say:

Only the default POSIX names are deprecated, not the functions

More generally, the compatibility notes say:

The UCRT also implements a large subset of the POSIX.1 (ISO/IEC 9945-1:1996, the POSIX System Application Program Interface) C library, but is not fully conformant to any specific POSIX standard.

which sounds like enough grounds for POSIX links here. If that sounds plausible, I can try to dig up a linkable version of the 1996 standard (although it's been withdrawn). Or we can keep linking to the 2001/2004 version and assume there haven't been serious changes at the basic level we're referencing (seems plausible based on the history, although I haven't tracked down the relevant System V docs).

wking · 2016-05-20T06:48:32Z

Short form of the previous comment:

I'd like to drop the “platform-appropriate” wiggle and just require POSIX. I think that's ok based on the age of the execv interface and apparent Windows (and cross-platform Go) support. I'm also ok keeping the old Go links, since Go covers the obvious platforms. Can I do either of those, or does everyone else really want the wordy “platform-appropriate” wiggle?

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, opencontainers#255) with POSIX references (following the new process docs) to address pushback against referencing Go [1,2] in favor of punting to platforms with 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]: opencontainers#427 (comment) [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 <wking@tremily.us>

wking · 2016-05-20T20:51:56Z

With a77a35a → b0f0096 I just went ahead and dropped the “platform-appropriate” wiggle. Details in each commit message with links to Windows' Visual Studio 2015 supporting the POSIX semantics.

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, opencontainers#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]: opencontainers#427 (comment) [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 <wking@tremily.us>

I'd added some omitempties in 5c2193f (specs-go/config: Make Linux and Solaris omitempty, 2016-05-06, opencontainers#431), but it turns out to not have the intended effect unless the field is also a pointer type. Before this commit: $ ./ocitools generate --template <(echo '{}') $ jq . config.json { "ociVersion": "1.0.0-rc1-dev", "platform": { "os": "linux", "arch": "amd64" }, "process": { "user": { "uid": 0, "gid": 0 }, "args": [], "cwd": "/" }, "root": { "path": "rootfs" }, "hooks": {}, "linux": { "cgroupsPath": "" }, "solaris": { "cappedCPU": {}, "cappedMemory": {} } } And after this commit: $ ./ocitools generate --template <(echo '{}') $ jq . config.json { "ociVersion": "1.0.0-rc1-dev", "platform": { "os": "linux", "arch": "amd64" }, "process": { "user": { "uid": 0, "gid": 0 }, "args": [], "cwd": "/" }, "root": { "path": "rootfs" }, "hooks": {}, } The remaining useless properties are addressed by other in-flight pull requests: * 5ca74df (config: Make 'process.args' optional, 2016-06-04, opencontainers#489) * ad33f9c (config: Explicitly list 'hooks' as optional, 2016-05-06, opencontainers#427) So I've left them alone here. Signed-off-by: W. Trevor King <wking@tremily.us>

I'd added some omitempties in 5c2193f (specs-go/config: Make Linux and Solaris omitempty, 2016-05-06, opencontainers#431), but it turns out to not have the intended effect unless the field is also a pointer type (even after I shifted the 'omitempty' from the platform tag to the json tag $ ./ocitools generate --template <(echo '{}') $ jq . config.json { "ociVersion": "1.0.0-rc1-dev", "platform": { "os": "linux", "arch": "amd64" }, "process": { "user": { "uid": 0, "gid": 0 }, "args": [], "cwd": "/" }, "root": { "path": "rootfs" }, "hooks": {}, "linux": { "cgroupsPath": "" }, "solaris": { "cappedCPU": {}, "cappedMemory": {} } } And after this commit: $ ./ocitools generate --template <(echo '{}') $ jq . config.json { "ociVersion": "1.0.0-rc1-dev", "platform": { "os": "linux", "arch": "amd64" }, "process": { "user": { "uid": 0, "gid": 0 }, "args": [], "cwd": "/" }, "root": { "path": "rootfs" }, "hooks": {}, } The remaining useless properties are addressed by other in-flight pull requests: * 5ca74df (config: Make 'process.args' optional, 2016-06-04, opencontainers#489) * ad33f9c (config: Explicitly list 'hooks' as optional, 2016-05-06, opencontainers#427) So I've left them alone here. Signed-off-by: W. Trevor King <wking@tremily.us>

I'd added some omitempties in 5c2193f (specs-go/config: Make Linux and Solaris omitempty, 2016-05-06, opencontainers#431), but it turns out to not have the intended effect unless the field is also a pointer type (even after I shifted the 'omitempty' from the platform tag to the json tag). Before this commit: $ ./ocitools generate --template <(echo '{}') $ jq . config.json { "ociVersion": "1.0.0-rc1-dev", "platform": { "os": "linux", "arch": "amd64" }, "process": { "user": { "uid": 0, "gid": 0 }, "args": [], "cwd": "/" }, "root": { "path": "rootfs" }, "hooks": {}, "linux": { "cgroupsPath": "" }, "solaris": { "cappedCPU": {}, "cappedMemory": {} } } And after this commit: $ ./ocitools generate --template <(echo '{}') $ jq . config.json { "ociVersion": "1.0.0-rc1-dev", "platform": { "os": "linux", "arch": "amd64" }, "process": { "user": { "uid": 0, "gid": 0 }, "args": [], "cwd": "/" }, "root": { "path": "rootfs" }, "hooks": {}, } The remaining useless properties are addressed by other in-flight pull requests: * 5ca74df (config: Make 'process.args' optional, 2016-06-04, opencontainers#489) * ad33f9c (config: Explicitly list 'hooks' as optional, 2016-05-06, opencontainers#427) So I've left them alone here. Signed-off-by: W. Trevor King <wking@tremily.us>

Using jq to format the output so we don't have to worry about ocitools' default tab indents or lack of trailing newlines, neither of which play nicely with <<-EOF here documents. The tests are known failures, because runtime-spec v1.0.0-rc1 lacks good omitempty handling. $ ocitools generate --template <(echo '{}') … "hooks": {}, "linux": {}, "solaris": { "cappedCPU": {}, "cappedMemory": {} } } These issues are addressed by the in-flight [1,2], although their late landings in runtime-spec mean we'll never be able to address them in the v1.0.0.rc1 branch of ocitools. [1]: opencontainers/runtime-spec#427 Subject: config: Explicitly list 'hooks' as optional [2]: opencontainers#112 Subject: generate: Adjust to Spec.Linux being a pointer Signed-off-by: W. Trevor King <wking@tremily.us>

The new wording isn't particularly close to either of the old wordings, but it reads more clearly to me. I've also added our usual: (<type>, <required|optional>) to the Markdown so folks can see that this is an optional object (although see [1] for a more complete version). [1]: opencontainers#427 Subject: config: Explicitly list 'hooks' as optional Signed-off-by: W. Trevor King <wking@tremily.us>

wking · 2016-08-17T22:39:20Z

Now that we've closed #483 and are aiming for a certifiable/testable hook specification, I think we want to land this PR or something like it.

I'd added some omitempties in 5c2193f (specs-go/config: Make Linux and Solaris omitempty, 2016-05-06, opencontainers#431), but it turns out to not have the intended effect unless the field is also a pointer type (even after I shifted the 'omitempty' from the platform tag to the json tag). Before this commit: $ ./ocitools generate --template <(echo '{}') $ jq . config.json { "ociVersion": "1.0.0-rc1-dev", "platform": { "os": "linux", "arch": "amd64" }, "process": { "user": { "uid": 0, "gid": 0 }, "args": [], "cwd": "/" }, "root": { "path": "rootfs" }, "hooks": {}, "linux": { "cgroupsPath": "" }, "solaris": { "cappedCPU": {}, "cappedMemory": {} } } And after this commit: $ ./ocitools generate --template <(echo '{}') $ jq . config.json { "ociVersion": "1.0.0-rc1-dev", "platform": { "os": "linux", "arch": "amd64" }, "process": { "user": { "uid": 0, "gid": 0 }, "args": [], "cwd": "/" }, "root": { "path": "rootfs" }, "hooks": {}, } The remaining useless properties are addressed by other in-flight pull requests: * 5ca74df (config: Make 'process.args' optional, 2016-06-04, opencontainers#489) * ad33f9c (config: Explicitly list 'hooks' as optional, 2016-05-06, opencontainers#427) So I've left them alone here. Signed-off-by: W. Trevor King <wking@tremily.us>

The new wording isn't particularly close to either of the old wordings, but it reads more clearly to me. I've also added our usual: (<type>, <required|optional>) to the Markdown so folks can see that this is an optional object (although see [1] for a more complete version). [1]: opencontainers#427 Subject: config: Explicitly list 'hooks' as optional Signed-off-by: W. Trevor King <wking@tremily.us>

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, opencontainers#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]: opencontainers#427 (comment) [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 <wking@tremily.us>

vbatts · 2016-09-16T12:22:51Z

On 15/09/16 20:37 -0700, W. Trevor King wrote:

Anything I can do to help this along? It had an LGTM and an SGTM two
weeks back before a tiny rebase ;).

Re-reading the patches, one thing I don't like about it is that it
doesn't directly convey the form. Just simply linking out to IEEE is a
punt, and if an implementer only has this document then it's not quite
helpful.

It would be more helpful to convey more of the form, even if quoting
from the IEEE link and including the link for more reference.

wking · 2016-09-16T15:03:25Z

On Fri, Sep 16, 2016 at 05:22:52AM -0700, Vincent Batts wrote:

Re-reading the patches, one thing I don't like about it is that it
doesn't directly convey the form. Just simply linking out to IEEE is a
punt, and if an implementer only has this document then it's not quite
helpful.

Punting was the plan ;). When would an implementer have this document
handy but not be able to click on the links to POSIX?

It would be more helpful to convey more of the form, even if quoting
from the IEEE link and including the link for more reference.

We don't quote when we punt MUST, MAY, etc. to RFC 2119, or when we
punt the code of conduct to the TOB, or when we punt UTF-8 to its
spec, or when we punt SemVer to its spec, or when we punt the GOOS
listing to Go's spec, or when we punt mount options to mount(8), ….
So I'm fine not quoting here. It would make our spec wordier and have
the usual not-DRY exposure to deviations.

But those are small downsides, so if for whatever reason you really
want me to inline some POSIX here I can do that. How much would you
like? Do you want me to follow the environment variable definition
1 down into the Portable Character Set 2? Do you want me to keep
going as far as “It is unwise to conflict…” 1? From the exec docs
3, do you want me to quote the whole definition for argv, file, and
path? Should these excerpts go inline in hooks, or be pushed down to
an appendix or some such?

vbatts · 2016-09-16T19:30:09Z

Quite literally, having the example of "NAME=VALUE" demonstration with referring the particulars to IEEE.

wking · 2016-09-16T19:46:36Z

On Fri, Sep 16, 2016 at 12:30:11PM -0700, Vincent Batts wrote:

Quite literally, having the example of "NAME=VALUE" demonstration
with referring the particulars to IEEE.

Don't we already have that name=value example down here 1?

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, opencontainers#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]: opencontainers#427 (comment) [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 <wking@tremily.us>

wking · 2016-09-21T23:58:33Z

Changed Spec.Hooks to a pointer in abc704a → 9bd270e to avoid the issue from #569.

wking · 2016-10-15T04:11:27Z

Anything I can do to help this along?

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, opencontainers#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]: opencontainers#427 (comment) [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 <wking@tremily.us>

wking · 2016-10-15T04:19:09Z

Rebased onto master resolving a few conflicts (mostly with #574 but also shifting some wording that had landed in #525).

hqhq · 2016-12-30T06:01:53Z

LGTM

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 <wking@tremily.us>

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 7ac41c6, 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 <wking@tremily.us>

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, opencontainers#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]: opencontainers#427 (comment) [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 <wking@tremily.us>

wking · 2017-01-04T22:17:13Z

I've just pushed b281671 → a78f255 rebasing onto master (no conflicts) to avoid the golint compilation failure. @hqhq, can you re-LGTM?

hqhq · 2017-01-05T01:28:59Z

LGTM, thanks.

mrunalp · 2017-01-06T00:00:28Z

LGTM

These references had been using IEEE Std 1003.1, 2004 Edition, but: $ curl -s http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap08.html | grep -B2 'newer edition' <center>The Open Group Base Specifications Issue 6 IEEE Std 1003.1, 2004 Edition Copyright © 2001-2004 The IEEE and The Open Group, All Rights reserved.</center><center>A newer edition of this document exists <a href="http://pubs.opengroup.org/onlinepubs/9699919799/" target="_parent">here</a></center> Shifting to 2016 also syncs us with the 'file' reference in config-linux.md. The initial reasoning for the 2004 edition is unclear to me (more on that in 70858bc, config: Adjust process.args to cite POSIX's execvp, 2016-05-19, opencontainers#427), The change-log for the exec page [1] doesn't list any relevant changes, and skimming a diff of the two HTML pages didn't turn up anything significant. Diffing the two HTML pages for environment variables also turned up no significant changes. Both definition reference the Portable Character Set for uppercase letters, digits, and underscore, but the U#### values for those have not changed [2,3]. [1]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html#tag_16_111_14 [2]: http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap06.html#tag_06_01 [3]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap06.html#tag_06_01 Signed-off-by: W. Trevor King <wking@tremily.us>

crosbymichael reviewed May 6, 2016
View reviewed changes

wking force-pushed the optional-hooks branch from cb4cfd8 to 9226ae4 Compare May 20, 2016 06:02

wking force-pushed the optional-hooks branch from 9226ae4 to 05f1e2d Compare May 20, 2016 06:38

wking force-pushed the optional-hooks branch from 05f1e2d to a77a35a Compare May 20, 2016 06:39

wking force-pushed the optional-hooks branch from a77a35a to b0f0096 Compare May 20, 2016 20:50

wking force-pushed the optional-hooks branch from b0f0096 to ad33f9c Compare May 20, 2016 20:53

This was referenced May 26, 2016

validate/env: add env name begins with digit check opencontainers/runtime-tools#80

Merged

[Config | Hooks] Add "if supported by the platform" #472

Closed

*: Remove hooks #483

Closed

wking mentioned this pull request Jun 18, 2016

specs-go/config: Make Linux and Solaris omitempty (again) #502

Merged

wking mentioned this pull request Jun 20, 2016

validation: add args validation for process opencontainers/runtime-tools#116

Closed

wking mentioned this pull request Aug 3, 2016

config: Synchronize comments between Markdown and Go #525

Merged

wking force-pushed the optional-hooks branch from ad33f9c to d40e9b1 Compare August 26, 2016 06:59

wking force-pushed the optional-hooks branch from abc704a to 9bd270e Compare September 21, 2016 23:57

wking force-pushed the optional-hooks branch from 9bd270e to b281671 Compare October 15, 2016 04:17

wking mentioned this pull request Oct 28, 2016

config.md: clarify timeout of hooks is OPTIONAL #604

Closed

wking mentioned this pull request Dec 28, 2016

Define hook structure clearly and explicitly #631

Closed

mrunalp added this to the 1.0.0 milestone Jan 4, 2017

wking added 3 commits January 4, 2017 14:12

wking force-pushed the optional-hooks branch from b281671 to a78f255 Compare January 4, 2017 22:13

wking mentioned this pull request Jan 5, 2017

Perfect json content #646

Merged

mrunalp merged commit 7dce97b into opencontainers:master Jan 6, 2017

wking deleted the optional-hooks branch January 6, 2017 05:44

vbatts mentioned this pull request Jan 27, 2017

Bump version for 1.0.0-rc4 #672

Merged

wking mentioned this pull request Mar 5, 2018

config: Clarify execution environment for hooks #953

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config: Explicitly list 'hooks' as optional #427

config: Explicitly list 'hooks' as optional #427

wking commented May 6, 2016

crosbymichael May 6, 2016

wking May 6, 2016

wking commented May 20, 2016

wking commented May 20, 2016

wking commented May 20, 2016

wking commented Aug 17, 2016

vbatts commented Sep 16, 2016

wking commented Sep 16, 2016

vbatts commented Sep 16, 2016 via email

wking commented Sep 16, 2016

wking commented Sep 21, 2016

wking commented Oct 15, 2016

wking commented Oct 15, 2016

hqhq commented Dec 30, 2016 •

edited by caniszczyk

Loading

wking commented Jan 4, 2017

hqhq commented Jan 5, 2017 •

edited by caniszczyk

Loading

mrunalp commented Jan 6, 2017 •

edited by caniszczyk

Loading

config: Explicitly list 'hooks' as optional #427

config: Explicitly list 'hooks' as optional #427

Conversation

wking commented May 6, 2016

crosbymichael May 6, 2016

Choose a reason for hiding this comment

wking May 6, 2016

Choose a reason for hiding this comment

wking commented May 20, 2016

wking commented May 20, 2016

wking commented May 20, 2016

wking commented Aug 17, 2016

vbatts commented Sep 16, 2016

wking commented Sep 16, 2016

vbatts commented Sep 16, 2016 via email

wking commented Sep 16, 2016

wking commented Sep 21, 2016

wking commented Oct 15, 2016

wking commented Oct 15, 2016

hqhq commented Dec 30, 2016 • edited by caniszczyk Loading

wking commented Jan 4, 2017

hqhq commented Jan 5, 2017 • edited by caniszczyk Loading

mrunalp commented Jan 6, 2017 • edited by caniszczyk Loading

hqhq commented Dec 30, 2016 •

edited by caniszczyk

Loading

hqhq commented Jan 5, 2017 •

edited by caniszczyk

Loading

mrunalp commented Jan 6, 2017 •

edited by caniszczyk

Loading