Why poststart and poststop hook are not consistent with the standard?

[duguhaotian]

The poststart hooks MUST be invoked by the runtime. If any poststart hook fails, the runtime MUST log a warning, but the remaining hooks and lifecycle continue as if the hook had succeeded.

The poststop hooks MUST be invoked by the runtime. If any poststop hook fails, the runtime MUST log a warning, but the remaining hooks and lifecycle continue as if the hook had succeeded.

link at

our runc implements like this

                        for i, hook := range c.config.Hooks.Poststart {
				if err := hook.Run(s); err != nil {
					if err := ignoreTerminateErrors(parent.terminate()); err != nil {
						logrus.Warn(err)
					}
					**return newSystemErrorWithCausef(err, "running poststart hook %d", i)**
				}
			}

                    for i, hook := range c.config.Hooks.Poststop {
			logrus.Infof("run poststop hook %d:%s", i, hook.Info())
			if err := hook.Run(s); err != nil {
				logrus.Errorf("running poststop hook %d: %s failed: %s", i, hook.Info(), err)
				**return newSystemErrorWithCausef(err, "running poststop hook %d:%s", i, hook.Info())**
			}
			logrus.Infof("poststop hook %d:%s done", i, hook.Info())
		}

If hook run is failed, we will return a err. But spec say: "but the remaining hooks and lifecycle continue as if the hook had succeeded."

Why we implement like this????

Thanks!!!!

[cyphar]

There are some other problems with how hooks are implemented, see #1710 (which I think this is ultimately a dup of).

Why we implement like this

runc had hook support before the specification, and the specification changed their semantics from the ones runc had but we haven't changed runc yet to follow the specification.

[wking]

… the specification changed their semantics from the ones runc had…

The spec almost dropped hooks when the create / start split landed and did not update the timing requirements with sufficient clarification for a split create/start. The current timing requirements largely landed in opencontainers/runtime-spec#532. So it's less “the specification changed their semantics” and more “it took almost a year after hooks landed before the spec regained a detailed opinion on when the hooks should be called”. In the interim, things were left up to runtimes, and runc went down one path with hook timing in its create/start split while the spec ended up going down a different path.

But +1 to this being a dup of #1710.

[duguhaotian]

Thanks for you reply