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

Hooks to linux,solaris and doc clarification #855

Merged
merged 1 commit into from
May 31, 2017

Conversation

lowenna
Copy link
Contributor

@lowenna lowenna commented May 24, 2017

Signed-off-by: John Howard jhoward@microsoft.com

Made hooks Linux and solaris. Windows does not currently support hooks. While we might at a later date, have clarified that in the .go and .md file.

Also, as I mentioned in the description of #854, platform-specific things are generally in the .md file, not the .go file. Hence moved the Linux comment.

Signed-off-by: John Howard <jhoward@microsoft.com>
@@ -356,7 +356,8 @@ Runtime implementations MAY support any valid values for platform-specific field

## <a name="configHooks" />Hooks

Hooks allow for the configuration of custom actions related to the [lifecycle](runtime.md#lifecycle) of the container.
Hooks allow for the configuration of custom actions related to the [lifecycle](runtime.md#lifecycle) of the container if supported by the platform.
On Linux, they are run after the container namespaces are created.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not all hooks are run after creation. In fact, there are currently no postcreate hooks (prestart hooks are called “after the start operation is called but before the user-specified program command is executed”. I suggest not adding this line (and removing it from the Go comment).

Copy link
Member

Choose a reason for hiding this comment

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

correct.

@@ -356,7 +356,8 @@ Runtime implementations MAY support any valid values for platform-specific field

## <a name="configHooks" />Hooks

Hooks allow for the configuration of custom actions related to the [lifecycle](runtime.md#lifecycle) of the container.
Hooks allow for the configuration of custom actions related to the [lifecycle](runtime.md#lifecycle) of the container if supported by the platform.
Copy link
Contributor

@wking wking May 24, 2017

Choose a reason for hiding this comment

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

I'd rather follow the example set for process.user.uid and friends and make this:

## Linux and Solaris Hooks

For Linux- and Solaris-based systems, the configuration structure supports `hooks` for configuring custom actions related to the [lifecycle](runtime.md#lifecycle) of the container.

Although I'd rather have this ABNF so we could say:

  • hooks (object, OPTIONAL, linux solaris) …

to protect readers who skipped over this lead-in paragraph and went straight to the hooks list entry.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't clearly say which platforms can rely on hooks, we're back to the rejected #472.

@vbatts
Copy link
Member

vbatts commented May 31, 2017

SGTM

@crosbymichael
Copy link
Member

crosbymichael commented May 31, 2017

LGTM

Approved with PullApprove

1 similar comment
@tianon
Copy link
Member

tianon commented May 31, 2017

LGTM

Approved with PullApprove

@tianon tianon merged commit ec44a7e into opencontainers:master May 31, 2017
@wking
Copy link
Contributor

wking commented May 31, 2017

On Wed, May 31, 2017 at 03:44:08PM -0700, Tianon Gravi wrote:

Merged #855.

Hmm. I'll file follow-up PRs for at least this point and something about this one.

wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request May 31, 2017
Not all hooks are run after creation.  In fact, there are currently no
postcreate hooks (prestart hooks are called "after the start operation
is called but before the user-specified program command is executed".

Fixing 28e8f68 (Hooks to linux,solaris and doc clarification,
2017-05-24, opencontainers#855), which pulled the broken line in from a Go comment
[1].

[1]: opencontainers#855 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request May 31, 2017
Clarifying the language from 28e8f68 (Hooks to linux,solaris and doc
clarification, 2017-05-24, opencontainers#855), which did not say which platforms
support hooks and which don't.  Without something like this commit,
there's no clear way for config authors to know if their runtime will
support hooks or not, and there was previous agreement that that sort
of ambiguity was not helpful [1].  This also gives normative Markdown
grounding for the Go platform tag added in 28e8f68.

[1]: opencontainers#472 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
@vbatts vbatts mentioned this pull request Jul 5, 2017
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.

5 participants