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

Hooks to linux,solaris and doc clarification #855

Merged
merged 1 commit into from
May 31, 2017
+3 −3
Merged

Hooks to linux,solaris and doc clarification #855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 •
edited
Loading

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.

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.


* **`hooks`** (object, OPTIONAL) MAY contain any of the following properties:
* **`prestart`** (array of objects, OPTIONAL) is an array of [pre-start hooks](#prestart).
Expand Down
3 changes: 1 addition & 2 deletions specs-go/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,omitempty"`
Hooks *Hooks `json:"hooks,omitempty" platform:"linux,solaris"`
// Annotations contains arbitrary metadata for the container.
Annotations map[string]string `json:"annotations,omitempty"`

Expand Down Expand Up @@ -134,7 +134,6 @@ type Hook struct {
// Hooks for container setup and teardown
type Hooks struct {
// Prestart is a list of hooks to be run before the container process is executed.
// On Linux, they are run after the container namespaces are created.
Prestart []Hook `json:"prestart,omitempty"`
// Poststart is a list of hooks to be run after the container process is started.
Poststart []Hook `json:"poststart,omitempty"`
Expand Down