Skip to content

Commit 2b6838d

Browse files
committed
Move poststart hook from runc create to runc start
The runtime-spec [1] currently says: > 6. Runtime's start command is invoked with the unique identifier of > the container. > 7. The startContainer hooks MUST be invoked by the runtime. If any > startContainer hook fails, the runtime MUST generate an error, stop > the container, and continue the lifecycle at step 12. > 8. The runtime MUST run the user-specified program, as specified by > process. > 9. The poststart hooks MUST be invoked by the runtime. If any > poststart hook fails, the runtime MUST generate an error, stop the > container, and continue the lifecycle at step 12. > ... > 11. Runtime's delete command is invoked with the unique identifier of > the container. > 12. The container MUST be destroyed by undoing the steps performed > during create phase (step 2). > 13. 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. Currently, we do 9 before 8 (heck, even before 6), which is clearly against the spec and results in issues like the one described in [2]. Let's move running poststart hook to after the user-specified process has started. NOTE this patch only fixes the order and does not implement removing the container when the poststart hook failed (as this part of the spec is controversial -- destroy et al and should probably be, and currently are, part of "runc delete"). [1]: https://github.com/opencontainers/runtime-spec/blob/main/runtime.md#lifecycle [2]: #5182 Reported-by: ningmingxiao <ning.mingxiao@zte.com.cn> Reported-by: Erik Sjölund <erik.sjolund@gmail.com> Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1 parent 2253475 commit 2b6838d

File tree

3 files changed

+37
-15
lines changed

3 files changed

+37
-15
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1616
- `EXTRA_BUILDTAGS` make variable is deprecated in favor of `RUNC_BUILDTAGS`
1717
and will be removed in runc 1.6. (#5171)
1818

19+
### Fixed ###
20+
- The poststart hooks are now executed after starting the user-specified
21+
process, fixing a runtime-spec conformance issue. (#4347, #5186)
22+
1923
## [1.5.0-rc.1] - 2026-03-12
2024

2125
> 憎しみを束ねてもそれは脆い!

libcontainer/container_linux.go

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,11 @@ func (c *Container) Exec() error {
232232

233233
func (c *Container) exec() error {
234234
path := filepath.Join(c.stateDir, execFifoFilename)
235-
return handleFifo(path, c.initProcess.pid())
235+
if err := handleFifo(path, c.initProcess.pid()); err != nil {
236+
return err
237+
}
238+
239+
return c.postStart()
236240
}
237241

238242
func handleFifo(path string, pid int) error {
@@ -256,6 +260,29 @@ func handleFifo(path string, pid int) error {
256260
}
257261
}
258262

263+
func (c *Container) postStart() (retErr error) {
264+
if !c.config.HasHook(configs.Poststart) {
265+
return nil
266+
}
267+
268+
defer func() {
269+
if retErr != nil {
270+
// A poststart hook failed; kill the container.
271+
if err := c.signal(unix.SIGKILL); err != nil && !errors.Is(err, ErrNotRunning) {
272+
logrus.WithError(err).Warn("kill after failed poststart")
273+
}
274+
// We're still init's parent so wait is required.
275+
c.initProcess.wait()
276+
}
277+
}()
278+
279+
s, err := c.currentOCIState()
280+
if err != nil {
281+
return err
282+
}
283+
return c.config.Hooks.Run(configs.Poststart, s)
284+
}
285+
259286
func readFromExecFifo(execFifo io.Reader) error {
260287
data, err := io.ReadAll(execFifo)
261288
if err != nil {
@@ -374,19 +401,6 @@ func (c *Container) start(process *Process) (retErr error) {
374401

375402
if process.Init {
376403
c.fifo.Close()
377-
if c.config.HasHook(configs.Poststart) {
378-
s, err := c.currentOCIState()
379-
if err != nil {
380-
return err
381-
}
382-
383-
if err := c.config.Hooks.Run(configs.Poststart, s); err != nil {
384-
if err := ignoreTerminateErrors(parent.terminate()); err != nil {
385-
logrus.Warn(fmt.Errorf("error running poststart hook: %w", err))
386-
}
387-
return err
388-
}
389-
}
390404
}
391405
return nil
392406
}

tests/integration/hooks.bats

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,11 @@ function teardown() {
3535
echo "testing hook $hook"
3636
update_config '.hooks |= {"'$hook'": [{"path": "/bin/true"}, {"path": "/bin/false"}]}'
3737
runc run "test_hook-$hook"
38-
[[ "$output" != "Hello World" ]]
38+
# Failed poststart hooks results in container being killed,
39+
# but only after it has started, so output may or may not appear.
40+
if [ "$hook" != "poststart" ]; then
41+
[[ "$output" != "Hello World" ]]
42+
fi
3943
[ "$status" -ne 0 ]
4044
[[ "$output" == *"error running $hook hook #1:"* ]]
4145
done

0 commit comments

Comments
 (0)