Add timeout field to hooks

[crosbymichael]

So that runtimes don't lock up forever waiting on a hook to return I
added a timeout field. This allows the author to specify the timeout in
seconds that they want the hook to run before aborting the hook and
container execution.

I think the hook is the proper place to specify a timeouot like this
because any type of overall timeout in the runtime will probably not
work for all hooks and will not be flexable enough.

Let me know that you all think.

Signed-off-by: Michael Crosby crosbymichael@gmail.com

[vishh]

This change makes sense. But adding a create step feels like a better solution :)

[wking]

On Wed, Mar 16, 2016 at 04:54:22PM -0700, Michael Crosby wrote:

I think the hook is the proper place to specify a timeouot like this
because any type of overall timeout in the runtime will probably not
work for all hooks and will not be flexable enough.

I'd rather folks used ‘timeout’ 1 or similar in the hook itself.

[crosbymichael]

@wking i don't think that is a good general solution for everyone. having this enforced by the runtime seems better and we don't have to rely on an external binary.

[marcosnils]

Agree with @crosbymichael. It helps when developing a spec implementation for multiple platforms.

[vbatts]

adding that line will require a comma on the end of this line

[vbatts]

While we have hooks, this LGTM,
though like @vishh said, if things were broken into create sandbox and start then it could be left up to the runtime

[wking]

On Wed, Mar 16, 2016 at 05:02:38PM -0700, Michael Crosby wrote:

@wking i don't think that is a good general solution for everyone.
having this enforced by the runtime seems better and we don't have
to rely on an external binary.

I don't think this argument scales particularly well. The GNU Core
Utilities provide timeout(1) and a few other wrappers 1. I don't
have any systems without coreutils installed, and a timeout wrapper
seems like a lightweight dependency on systems that don't have
coreutils. What happens when somebody wants to set nice on a hook?
Do we add a nice setting, or point them at [2,3]?

And hooks are setup / audited by the runtime-caller right 4? They
should know if they have a local timeout(1) or equivalent, and know
how to get it installed if they don't have it.

[wking]

On Wed, Mar 16, 2016 at 06:42:33PM -0700, Vincent Batts wrote:

though like @vishh said, if things were broken into create sandbox
and start then it could be left up to the runtime

Breaking create / (exec|start) doesn't get you out of hooks entirely,
since anything that depends on the container PID will still need to
happen in an exec|start hook [1,2].

[vishh]

This PR is not the place to talk about hooks. @wking it will be helpful if
you can enumerate the use cases for hooks that cannot be satisfied by the
split.

On Wed, Mar 16, 2016 at 9:00 PM, W. Trevor King notifications@github.com
wrote:

On Wed, Mar 16, 2016 at 06:42:33PM -0700, Vincent Batts wrote:

though like @vishh said, if things were broken into create sandbox
and start then it could be left up to the runtime

Breaking create / (exec|start) doesn't get you out of hooks entirely,
since anything that depends on the container PID will still need to
happen in an exec|start hook [1,2].

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#346 (comment)

[hqhq]

It makes sense for me, LGTM.

[mrunalp]

LGTM