search

appc / spec

App Container Specification and Tooling (archived, see https://github.com/rkt/rkt/issues/4024)
Apache License 2.0
1.26k stars 146 forks source link

spec: additional event handlers #188

Open jheiss opened 9 years ago

jheiss commented 9 years ago

The current spec defines pre-start and post-stop event handlers, and says that processes will receive SIGTERM when the container needs to stop.

That seems insufficient to me. At a minimum I would add a pre-stop to allow more complicated shutdown procedures. E.g. clean shutdown of a database. docker/docker#6982 has a more extensive list of hooks that has been requested for Docker.

jonboulle commented 9 years ago

Re-capturing a question raised in #62:

Why explicitly limit event names? Can't an implementation define additional events/handlers? Examples of such handlers would be: ad-hoc maintenance tasks triggered manually (like running chef-server-ctl commands for Chef server); cron-like scheduled tasks; handling situations in multi-application containers when one of the processes exits. Spec may require that if an implementation handles events outside of the spec, the event names should be namespaced (e.g. rocket:cron or jetpack:task) to avoid possible collisions.

We certainly want a small set of very well defined eventHandlers that all executor implementations must support. As for extensions like ad-hoc tasks, is that worth codifying in image manifests at all, rather than just leaving it up to implementations? I think it's a bit of a grey area. Namespacing could be a good compromise and we could define this as a common approach for dealing with other fields like isolators (optional; small set of well-known fields; other fields accepted, but please namespace; commonly-used fields may be promoted to well-known in future versions of the spec)

cdaylward commented 9 years ago

The need for a "graceful shutdown" hook seems pretty clear (expecting all the existing software in the world, that people want in a container, to follow the spec's SIGTERM rule might be impractical). I'm not however sure if "pre-stop" captures this. If "pre-stop" is used for graceful shutdown, there is no way for an executor to know if a process should have terminated or not due to the hook. Should there be special-case event handlers for life cycle events (e.g. "stop") that take the place of SIGTERM? "pre-kill" also seems reasonable due to past experience (e.g. a script that sends a SIGUSR or triggers a core dump, etc, on a process that is has not terminated as expected or within the allotted time).

jonboulle commented 9 years ago

Beyond UNIX signals, it was raised in #12 that we should consider different kinds of termination signals such as HTTP GETs and/or Execs.

philips commented 9 years ago

@jonboulle I feel like it should always be an exec inside of the pod. If someone wants to implement a GET in that exec they can go for it.

philips commented 9 years ago

@cdaylward I see your ordering problem and what you are trying to capture with pre-kill. Essentially we need to define whether this thing will cause the process to exit or not.

In my mind pre-stop is called before the app's main pid is sent a SIGTERM. If the process exits after pre-stop is exec'd we will assume that this was intended. If we add restarts in the spec in the future this would not cause a restart of the app.

jonboulle commented 9 years ago

As part of this we should move away from the single unexplained SIGTERM reference in the intro text.

globalcitizen commented 9 years ago

If the additional event handlers are not added, it might be worth just removing them all together in favor of having people write this stuff in to their init scripts or (if calling directly) app itself. Reason being, if start and stop are the only hooks, it's basically needless duplication with poorly specified testing/failure modes. For imports from other formats, a wrapper-script could be generated.