Skip to content

Commit

Permalink
libcontainer: call Prestart, Poststart hooks from correct places
Browse files Browse the repository at this point in the history
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 opencontainers#1741.
See also opencontainers#1710.

Signed-off-by: Dongsu Park <dongsu@kinvolk.io>
  • Loading branch information
Dongsu Park committed May 30, 2018
1 parent aedc95e commit 0c199c4
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 35 deletions.
25 changes: 19 additions & 6 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,21 @@ func (c *linuxContainer) Exec() error {
func (c *linuxContainer) exec() error {
path := filepath.Join(c.root, execFifoFilename)

defer func() {
if c.config.Hooks != nil && len(c.config.Hooks.Poststart) > 0 {
s, err := c.currentOCIState()
if err != nil {
return
}
for i, hook := range c.config.Hooks.Poststart {
if err := hook.Run(s); err != nil {
fmt.Printf("%v", newSystemErrorWithCausef(err, "running poststart hook %d", i))
return
}
}
}
}()

fifoOpen := make(chan struct{})
select {
case <-awaitProcessExit(c.initProcess.pid(), fifoOpen):
Expand Down Expand Up @@ -366,17 +381,15 @@ func (c *linuxContainer) start(process *Process, isInit, isCreate bool) error {
}
c.initProcessStartTime = state.InitProcessStartTime

if c.config.Hooks != nil && len(c.config.Hooks.Poststart) > 0 {
if c.config.Hooks != nil && len(c.config.Hooks.Prestart) > 0 {
s, err := c.currentOCIState()
if err != nil {
return err
}
for i, hook := range c.config.Hooks.Poststart {
s.Status = "created"
for i, hook := range c.config.Hooks.Prestart {
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)
return newSystemErrorWithCausef(err, "running prestart hook %d", i)
}
}
}
Expand Down
44 changes: 15 additions & 29 deletions libcontainer/process_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,21 @@ func (p *initProcess) start() error {
sentResume bool
)

// runc start
if !p.IsCreate {
if p.config.Config.Hooks != nil && len(p.config.Config.Hooks.Poststart) > 0 {
s, err := p.container.currentOCIState()
if err != nil {
return err
}
for i, hook := range p.config.Config.Hooks.Poststart {
if err := hook.Run(s); err != nil {
return newSystemErrorWithCausef(err, "running poststart hook %d", i)
}
}
}
}

ierr := parseSync(p.parentPipe, func(sync *syncT) error {
switch sync.Type {
case procReady:
Expand All @@ -340,21 +355,6 @@ func (p *initProcess) start() error {
return newSystemErrorWithCause(err, "setting Intel RDT config for ready process")
}
}

if p.config.Config.Hooks != nil && len(p.config.Config.Hooks.Prestart) > 0 {
s, err := p.container.currentOCIState()
if err != nil {
return err
}
// initProcessStartTime hasn't been set yet.
s.Pid = p.cmd.Process.Pid
s.Status = "creating"
for i, hook := range p.config.Config.Hooks.Prestart {
if err := hook.Run(s); err != nil {
return newSystemErrorWithCausef(err, "running prestart hook %d", i)
}
}
}
}
// Sync with child.
if err := writeSync(p.parentPipe, procRun); err != nil {
Expand All @@ -371,20 +371,6 @@ func (p *initProcess) start() error {
return newSystemErrorWithCause(err, "setting Intel RDT config for procHooks process")
}
}
if p.config.Config.Hooks != nil && len(p.config.Config.Hooks.Prestart) > 0 {
s, err := p.container.currentOCIState()
if err != nil {
return err
}
// initProcessStartTime hasn't been set yet.
s.Pid = p.cmd.Process.Pid
s.Status = "creating"
for i, hook := range p.config.Config.Hooks.Prestart {
if err := hook.Run(s); err != nil {
return newSystemErrorWithCausef(err, "running prestart hook %d", i)
}
}
}
// Sync with child.
if err := writeSync(p.parentPipe, procResume); err != nil {
return newSystemErrorWithCause(err, "writing syncT 'resume'")
Expand Down

0 comments on commit 0c199c4

Please sign in to comment.