[Config | Hooks] Add "if supported by the platform" #472
Conversation
The spec currently suggests "Hooks MUST be called in the listed order." If hooks are not supported by the platform, this would be a requirement that could not be met. This PR adds "if supported by the platform" so that we're not imposing an impossible requirement on platforms that might not (currently) have support for hooks. Signed-off-by: Rob Dolin <RobDolin@microsoft.com>
|
I think we should just drop hooks (#473).
|
| @@ -245,7 +245,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 if supported by the platform. | |||
There was a problem hiding this comment.
This is fine only if the bundle were to support different platforms else one could just not add hooks to the config at all.
There was a problem hiding this comment.
On Tue, May 31, 2016 at 09:33:38AM -0700, Mrunal Patel wrote:
-Hooks MUST be called in the listed order.
+Hooks MUST be called in the listed order if supported by the platform.This is fine only if the bundle were to support different platforms
else one could just not add hooks to the config at all.
The new text here isn't clear, but based on 1, I think @RobDolinMS
means “Windows-based runtimes will error on configuration files with
meaningful content in ‘hooks’ (e.g. anything more than empty arrays
for the per-event properties)”. So for Windows this PR is aiming for
“you can't have hooks” not “you have a choice to not have hooks”.
The latter is already true, although it is made more explicit by the
in-flight #427.
There was a problem hiding this comment.
@wking If that is the intent, then we should clarify further whether the platform will error out or just not call the hooks.
There was a problem hiding this comment.
On Tue, May 31, 2016 at 10:34:39AM -0700, Mrunal Patel wrote:
-Hooks MUST be called in the listed order.
+Hooks MUST be called in the listed order if supported by the platform.@wking If that is the intent, then we should clarify further whether
the platform will error out or just not call the hooks.
I'm pretty sure “just not call the hooks” should not be valid runtime
behavior. This is related to, but maybe not quite covered by, the “…
unable to create the environment specified…” language @cyphar has
in-flight with #397.
There was a problem hiding this comment.
My preference here is definitely along the same lines (same as #476) -- if the runtime cannot support the functionality, then attempting to use it ought to result in an error (which stops users from scratching their heads wondering why their hooks aren't firing, for example).
|
Did this one ever reach consensus? |
|
On Fri, Nov 04, 2016 at 03:26:58PM -0700, Tianon Gravi wrote:
#559 means we'd need something like this PR if we really did want to |
|
Rejected Indeed, this seems like the wrong way to express that Windows may not be able to support hooks in the same was as other platforms. @RobDolinMS does Windows simply not currently support hooks, or are there fundamental issues which make them impossible to support? |
|
Would a member of @opencontainers/runtime-spec-maintainers please assign this to me? |
|
@mrunalp asks: If hooks are not supported, can they just be not added to the config? |
|
I believe this is covered by our wording that if a system cannot fulfill a field in the spec that the runtime should error out. Closing this one. |
Otherwise a runtime could silently ignore a property it didn't want to implement, which would be confusing for runtime callers [1]. This closes a potential loophole in the restriction from 766abd6 (runtime.md: Require 'create' to fail if config.json asks for the impossible, 2016-09-08, opencontainers#559). [1]: opencontainers#472 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
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>
Otherwise a runtime could silently ignore a property it didn't want to implement, which would be confusing for runtime callers [1]. This closes a potential loophole in the restriction from 766abd6 (runtime.md: Require 'create' to fail if config.json asks for the impossible, 2016-09-08, opencontainers#559). [1]: opencontainers#472 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
The spec currently suggests "Hooks MUST be called in the listed order."
If hooks are not supported by the platform, this would be a requirement that could not be met.
This PR adds "if supported by the platform" so that we're not imposing an impossible requirement on platforms that might not (currently) have support for hooks.
Signed-off-by: Rob Dolin RobDolin@microsoft.com