Skip to content

Commit 94059a5

Browse files
Dongsu Parkcyphar
authored andcommitted
libcontainer: call Prestart, Poststart hooks from correct places
So far Prestart, Poststart hooks have been called from the context of create, which does not satisfy the runtime spec. That's why the runtime-tools validation tests like `hooks_stdin.t` have failed. Let's move call sites of Prestart, Poststart to correct places. Unfortunately as for the Poststart hook, it's not possible to tell whether a specific call site is from create context or run context. That's why we needed to allow Create and Run methods to accept another parameter `action` (of type `CtAct`). Doing that, it's possible to set a variable `initProcess.IsCreate` that allows us distinguish Create from Run. It depends on a pending PR #1741. See also #1710. Signed-off-by: Dongsu Park <[email protected]> Signed-off-by: Aleksa Sarai <[email protected]>
1 parent 98f5be6 commit 94059a5

File tree

2 files changed

+35
-36
lines changed

2 files changed

+35
-36
lines changed

libcontainer/container_linux.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -262,9 +262,25 @@ func (c *linuxContainer) Exec() error {
262262
return c.exec()
263263
}
264264

265-
func (c *linuxContainer) exec() error {
265+
func (c *linuxContainer) exec() (errExec error) {
266266
path := filepath.Join(c.root, execFifoFilename)
267267

268+
if c.config.Hooks != nil && len(c.config.Hooks.Poststart) > 0 {
269+
defer func() {
270+
s, err := c.currentOCIState()
271+
if err != nil {
272+
errExec = err
273+
return
274+
}
275+
for i, hook := range c.config.Hooks.Poststart {
276+
if err := hook.Run(s); err != nil {
277+
errExec = newSystemErrorWithCausef(err, "running poststart hook %d", i)
278+
return
279+
}
280+
}
281+
}()
282+
}
283+
268284
fifoOpen := make(chan struct{})
269285
select {
270286
case <-awaitProcessExit(c.initProcess.pid(), fifoOpen):
@@ -355,17 +371,14 @@ func (c *linuxContainer) start(process *Process, isCreate bool) error {
355371
}
356372
c.initProcessStartTime = state.InitProcessStartTime
357373

358-
if c.config.Hooks != nil && len(c.config.Hooks.Poststart) > 0 {
374+
if c.config.Hooks != nil && len(c.config.Hooks.Prestart) > 0 {
359375
s, err := c.currentOCIState()
360376
if err != nil {
361377
return err
362378
}
363-
for i, hook := range c.config.Hooks.Poststart {
379+
for i, hook := range c.config.Hooks.Prestart {
364380
if err := hook.Run(s); err != nil {
365-
if err := ignoreTerminateErrors(parent.terminate()); err != nil {
366-
logrus.Warn(err)
367-
}
368-
return newSystemErrorWithCausef(err, "running poststart hook %d", i)
381+
return newSystemErrorWithCausef(err, "running prestart hook %d", i)
369382
}
370383
}
371384
}

libcontainer/process_linux.go

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,21 @@ func (p *initProcess) start() error {
345345
sentResume bool
346346
)
347347

348+
// runc start
349+
if !p.IsCreate {
350+
if p.config.Config.Hooks != nil && len(p.config.Config.Hooks.Poststart) > 0 {
351+
s, err := p.container.currentOCIState()
352+
if err != nil {
353+
return err
354+
}
355+
for i, hook := range p.config.Config.Hooks.Poststart {
356+
if err := hook.Run(s); err != nil {
357+
return newSystemErrorWithCausef(err, "running poststart hook %d", i)
358+
}
359+
}
360+
}
361+
}
362+
348363
ierr := parseSync(p.parentPipe, func(sync *syncT) error {
349364
switch sync.Type {
350365
case procReady:
@@ -364,21 +379,6 @@ func (p *initProcess) start() error {
364379
return newSystemErrorWithCause(err, "setting Intel RDT config for ready process")
365380
}
366381
}
367-
368-
if p.config.Config.Hooks != nil && len(p.config.Config.Hooks.Prestart) > 0 {
369-
s, err := p.container.currentOCIState()
370-
if err != nil {
371-
return err
372-
}
373-
// initProcessStartTime hasn't been set yet.
374-
s.Pid = p.cmd.Process.Pid
375-
s.Status = "creating"
376-
for i, hook := range p.config.Config.Hooks.Prestart {
377-
if err := hook.Run(s); err != nil {
378-
return newSystemErrorWithCausef(err, "running prestart hook %d", i)
379-
}
380-
}
381-
}
382382
}
383383
// Sync with child.
384384
if err := writeSync(p.parentPipe, procRun); err != nil {
@@ -395,20 +395,6 @@ func (p *initProcess) start() error {
395395
return newSystemErrorWithCause(err, "setting Intel RDT config for procHooks process")
396396
}
397397
}
398-
if p.config.Config.Hooks != nil && len(p.config.Config.Hooks.Prestart) > 0 {
399-
s, err := p.container.currentOCIState()
400-
if err != nil {
401-
return err
402-
}
403-
// initProcessStartTime hasn't been set yet.
404-
s.Pid = p.cmd.Process.Pid
405-
s.Status = "creating"
406-
for i, hook := range p.config.Config.Hooks.Prestart {
407-
if err := hook.Run(s); err != nil {
408-
return newSystemErrorWithCausef(err, "running prestart hook %d", i)
409-
}
410-
}
411-
}
412398
// Sync with child.
413399
if err := writeSync(p.parentPipe, procResume); err != nil {
414400
return newSystemErrorWithCause(err, "writing syncT 'resume'")

0 commit comments

Comments
 (0)