*: Remove hooks #483
*: Remove hooks #483
Conversation
|
I'm not sure about this and have started a thread on the mailing list. |
|
On Thu, Jun 02, 2016 at 08:38:24AM -0700, Mrunal Patel wrote:
Link to thread 1. |
|
This isn't strictly relevant to the OCI spec, but hooks are currently used by docker to configure the container network. If they go away, it's unclear to me how networking will be handled, so this change could well break docker. Maybe hooks should be retained until there is some sort of OCI network spec since, like annotations, they are a useful facility to handle arbitrary container scenarios. |
|
On Fri, Jun 17, 2016 at 01:53:59AM -0700, James Hunt wrote:
We should keep discussion in @mrunalp's thread 1 to avoid |
This drops all instances (outside the ChangeLog) turned up by case-insensitive searches for 'hook'. The desire to drop hooks has been around for a while now [1], but it took a while to land a create/start split. By the time that split did land, dropping hooks was still part of its motivation [2]. Hooks are hard to support robustly [3], and after the create/start split, callers can do whatever they like at those times without having to go through the runtime [4]. There is still a use-case for folks who prefer the old all-in-one UX, but we can support those folks with a higher-level wrapper built on hookless create/start primatives [5]. There was some last-minute discussion of pre-pivot mount propagation needing pre-start hooks [6], but that use case can be addressed by manipulating the mounts array [7]. With those arguments in place, the consensus at today's meeting seemed in favor of removing hooks from the spec [8]. And after some second-guessing [9], we're now back in favor of removing hooks from the spec [10]. [1]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2015/opencontainers.2015-10-28-17.02.log.html#l-71 <wking> if you have a distinct create operation, you can drop the pre-start hooks, which makes a simpler spec [2]: opencontainers#384 Subject: Split create and start In the topic post: > # Motivating usecases: > > * to simplify the flows/interaction patterns by removing hooks, > but still allow for the same functionality [3]: opencontainers#265 Subject: runtime-config: Require serial hook execution [4]: http://ircbot.wl.linuxfoundation.org/eavesdrop/%23opencontainers/%23opencontainers.2016-03-24.log.html#t2016-03-24T18:56:15 <vishh> mrunalp: How do I run http hooks? I need a binary wrapper to execute http hooks [5]: https://groups.google.com/a/opencontainers.org/d/msg/dev/Y7p6YW8zr4s/OVaAI_WDBAAJ Subject: Re: Hooks and all-in-one operation Date: Wed, 1 Jun 2016 11:49:07 -0700 Message-ID: <20160601184907.GF4149@odin.tremily.us> [6]: opencontainers#384 (comment) Subject: Split create and start [7]: opencontainers#384 (comment) Subject: Split create and start [8]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-06-01-17.01.log.html#l-83 [9]: https://groups.google.com/a/opencontainers.org/forum/#!topic/dev/Y7p6YW8zr4s Subject: Hooks and all-in-one operation Date: Wed, 1 Jun 2016 11:38:27 -0700 Message-ID: <CANEZBD69Q4VV4UoBodWjtAR1PrJ1OysaMUHgA5cW_aGj2ZEhLQ@mail.gmail.com> [10]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-08-03-21.01.log.html#l-21 Signed-off-by: W. Trevor King <wking@tremily.us>
|
On Fri, Jun 17, 2016 at 09:09:31AM -0700, W. Trevor King wrote:
I posted to the thread on Tuesday asking for guidance now that the |
|
So consensus on today's meeting was to keep hooks 1. To square a. The create/start split lets you do any hook-like things you like Possible approaches: i. Include both hooks and the create/start split in this spec. The DockerCon decision seemed to revolve around (a) and (f), arguing Today's decision seemed to revolve around (b), (d), and (e). Because The distinction between these three choices is small because of (d), |
…k-lifecycle
Restore hook language removed by create/start split with some tweaks
to adjust to the new environment, for example:
* Put the pre-start hooks after the 'start' call, but before the meat
of the start call (the container-process exec trigger). Folks who
want a post-create hook can add one with that name. I'd like to
have renamed poststop to post-delete to avoid confusion like [1].
But the motivation for keeping hooks was backwards compatibility [2]
so I've left the name alone.
* Put each "...command is invoked..." lifecycle entry in its own list
entry, to match the 'create' list entry.
* Move the rules about what happens on hook failure into the
lifecycle. This matches pre-split entries like:
If any prestart hook fails, then the container MUST be stopped and
the lifecycle continues at step 7.
and avoids respecifying that information in a second location
(config.md).
* I added the warning section to try and follow post-split's generic
"generates an error" approach while respecting the pre-split desire
to see what failed (we had "then an error including the exit code
and the stderr is returned to the caller" and "then an error is
logged").
[1]: opencontainers#395
Subject: Run post-stop hooks before the container sandbox is deleted.
[2]: opencontainers#483 (comment)
Subject: *: Remove hooks
* duglin/SplitCreate:
Split create and start
Signed-off-by: W. Trevor King <wking@tremily.us>
I expect the lifecycle information was removed accidentally in be59415 (Split create and start, 2016-04-01, opencontainers#384), because for a time it seemed like that PR would also be removing hooks. Putting the lifecycle information back in, I made some tweaks to adjust to the new environment, for example: * Put the pre-start hooks after the 'start' call, but before the meat of the start call (the container-process exec trigger). Folks who want a post-create hook can add one with that name. I'd like to have renamed poststop to post-delete to avoid confusion like [1]. But the motivation for keeping hooks was backwards compatibility [2] so I've left the name alone. * Put each "...command is invoked..." lifecycle entry in its own list entry, to match the 'create' list entry. * Move the rules about what happens on hook failure into the lifecycle. This matches pre-split entries like: If any prestart hook fails, then the container MUST be stopped and the lifecycle continues at step 7. and avoids respecifying that information in a second location (config.md). * I added the warning section to try and follow post-split's generic "generates an error" approach while respecting the pre-split desire to see what failed (we had "then an error including the exit code and the stderr is returned to the caller" and "then an error is logged"). * I left the state 'id' context out, since Michael didn't want it [3]. [1]: opencontainers#395 Subject: Run post-stop hooks before the container sandbox is deleted. [2]: opencontainers#483 (comment) Subject: *: Remove hooks [3]: opencontainers#532 (comment) Subject: Restore hook language removed by create/start split Signed-off-by: W. Trevor King <wking@tremily.us>
I expect the lifecycle information was removed accidentally in be59415 (Split create and start, 2016-04-01, opencontainers#384), because for a time it seemed like that PR would also be removing hooks. Putting the lifecycle information back in, I made some tweaks to adjust to the new environment, for example: * Put the pre-start hooks after the 'start' call, but before the meat of the start call (the container-process exec trigger). Folks who want a post-create hook can add one with that name. I'd like to have renamed poststop to post-delete to avoid confusion like [1]. But the motivation for keeping hooks was backwards compatibility [2] so I've left the name alone. * Put each "...command is invoked..." lifecycle entry in its own list entry, to match the 'create' list entry. * Move the rules about what happens on hook failure into the lifecycle. This matches pre-split entries like: If any prestart hook fails, then the container MUST be stopped and the lifecycle continues at step 7. and avoids respecifying that information in a second location (config.md). * I added the warning section to try and follow post-split's generic "generates an error" approach while respecting the pre-split desire to see what failed (we had "then an error including the exit code and the stderr is returned to the caller" and "then an error is logged"). * I left the state 'id' context out, since Michael didn't want it [3]. [1]: opencontainers#395 Subject: Run post-stop hooks before the container sandbox is deleted. [2]: opencontainers#483 (comment) Subject: *: Remove hooks [3]: opencontainers#532 (comment) Subject: Restore hook language removed by create/start split Signed-off-by: W. Trevor King <wking@tremily.us>
I expect the lifecycle information was removed accidentally in be59415 (Split create and start, 2016-04-01, opencontainers#384), because for a time it seemed like that PR would also be removing hooks. Putting the lifecycle information back in, I made some tweaks to adjust to the new environment, for example: * Put the pre-start hooks after the 'start' call, but before the meat of the start call (the container-process exec trigger). Folks who want a post-create hook can add one with that name. I'd like to have renamed poststop to post-delete to avoid confusion like [1]. But the motivation for keeping hooks was backwards compatibility [2] so I've left the name alone. * Put each "...command is invoked..." lifecycle entry in its own list entry, to match the 'create' list entry. * Move the rules about what happens on hook failure into the lifecycle. This matches pre-split entries like: If any prestart hook fails, then the container MUST be stopped and the lifecycle continues at step 7. and avoids respecifying that information in a second location (config.md). * I added the warning section to try and follow post-split's generic "generates an error" approach while respecting the pre-split desire to see what failed (we had "then an error including the exit code and the stderr is returned to the caller" and "then an error is logged"). * I left the state 'id' context out, since Michael didn't want it [3]. * Make runtime.md references to "generate an error" and "log a warning" links, so readers have an easier time finding more detail on that wording. Where I reference a section, I'm still using the auto-generated anchor for that header and not the anchors which were added in 41839d7 (Merge pull request opencontainers#707 from mrunalp/anchor_tags, 2017-03-03) and similar. Mrunal suggested that the manually-added anchors were mainly intended for the validation tooling [4]. [1]: opencontainers#395 Subject: Run post-stop hooks before the container sandbox is deleted. [2]: opencontainers#483 (comment) Subject: *: Remove hooks [3]: opencontainers#532 (comment) Subject: Restore hook language removed by create/start split [4]: http://ircbot.wl.linuxfoundation.org/eavesdrop/%23opencontainers/%23opencontainers.2017-03-03.log.html#t2017-03-03T18:02:12 Signed-off-by: W. Trevor King <wking@tremily.us>
This drops all instances (outside the ChangeLog) turned up by
case-insensitive searches for 'hook'.
The desire to drop hooks has been around for a while now, but it
took a while to land a create/start split. By the time that split did
land, dropping hooks was still part of its motivation. Hooks are
hard to support robustly, and after the create/start split,
callers can do whatever they like at those times without having to go
through the runtime. There is still a use-case for folks who
prefer the old all-in-one UX, but we can support those folks with a
higher-level wrapper built on hookless create/start primatives.
There was some last-minute discussion of pre-pivot mount propagation
needing pre-start hooks, but that use case can be addressed by
manipulating the mounts array. With those arguments in place, the
consensus at today's meeting seemed in favor of removing hooks from
the spec.
Fixes #473.
Fixes #472.
Fixes #427.
Fixes #395.
Fixes #20.