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

Jump to bottom

runtime-config: Require serial hook execution #265

Closed
wants to merge 1 commit into from
Closed

runtime-config: Require serial hook execution #265

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
2 changes: 1 addition & 1 deletion runtime-config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Presently there are `Prestart`, `Poststart` and `Poststop`.
* [`Poststop`](#poststop) is a list of hooks to be run after the container process exits

Hooks allow one to run code before/after various lifecycle events of the container.
Hooks MUST be called in the listed order.
Hooks MUST be called in the listed order, with each hook process being reaped before the next hook is executed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this cause confusion if "each hook process" includes the child processes it creates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Thu, Dec 10, 2015 at 10:55:18PM -0800, Qiang Huang wrote:

-Hooks MUST be called in the listed order.
+Hooks MUST be called in the listed order, with each hook process being reaped before the next hook is executed.

Wouldn't this cause confusion if "each hook process" includes the
child processes it creates?

What sort of confusion? If the hook wants to explicitly launch a
daemon or some such, I'm fine with:
—
hook 1: spawn ------> reaped
daemon------------------------------------------->
hook 2: spawn --> reaped
hook 3: spawn --> reaped

If folks are worried about hook-children surviving an early hook exit:
—
hook 1: spawn ------(died)-> reaped
child------------------------------------------->
hook 2: spawn --> reaped
hook 3: spawn --> reaped

we can require the runtime to be more shell-like and make a new
process group to contain the hooks (but this will start getting pretty
complicated).

Without some sort of rule about reaping, I don't see much gain to the
“Hooks MUST be called in the listed order” requirement. If they're
all spawned one after the other, you don't have any guarantees that
earlier hooks will have accomplished their task before later hooks are
run.

It might be easier to just separate ‘create’ (most of the container
setup) from ‘exec’ (process launch and possible PID namespace
creation) and ‘destroy’ (remove anything left-over from ‘create’) and
get out of the hook business altogether [1,2,3].

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should take out reaped from the suggested change.

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.
Expand Down