Skip to content

Commit

Permalink
libcontainer: Set 'status' in hook stdin
Browse files Browse the repository at this point in the history 
Finish off the work started in a344b2d (sync up `HookState` with OCI
spec `State`, 2016-12-19, opencontainers#1201).

And drop HookState, since there's no need for a local alias for
specs.State.  And add len guards to avoid computing the state when
.Hooks is non-nil but the particular phase we're looking at is empty.

Also set c.initProcess in newInitProcess to support OCIState calls
from within initProcess.start().  I think the cyclic references
between linuxContainer and initProcess are unfortunate, but didn't
want to address that here.

I've also left the timing of the Prestart hooks alone, although the
spec calls for them to happen before start (not as part of creation)
[1,2].  Once the timing gets fixed we can drop the
initProcessStartTime hacks which initProcess.start currently needs.

I'm not sure why we trigger the prestart hooks in response to both
procReady and procHooks.  But we've had two prestart rounds in
initProcess.start since 2f27649 (Move pre-start hooks after container
mounts, 2016-02-17, opencontainers#568).  I've left that alone too.

[1]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#prestart
[2]: opencontainers#1710

Signed-off-by: W. Trevor King <wking@tremily.us>
  • Loading branch information
@wking
wking authored and Dongsu Park committed May 29, 2018
1 parent dd67ab1 commit 2063acd
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 59 deletions.
13 changes: 5 additions & 8 deletions libcontainer/configs/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,26 +265,23 @@ func (hooks Hooks) MarshalJSON() ([]byte, error) {
})
}

// HookState is the payload provided to a hook on execution.
type HookState specs.State

type Hook interface {
// Run executes the hook with the provided state.
Run(HookState) error
Run(*specs.State) error
}

// NewFunctionHook will call the provided function when the hook is run.
func NewFunctionHook(f func(HookState) error) FuncHook {
func NewFunctionHook(f func(*specs.State) error) FuncHook {
return FuncHook{
run: f,
}
}

type FuncHook struct {
run func(HookState) error
run func(*specs.State) error
}

func (f FuncHook) Run(s HookState) error {
func (f FuncHook) Run(s *specs.State) error {
return f.run(s)
}

Expand All @@ -307,7 +304,7 @@ type CommandHook struct {
Command
}

func (c Command) Run(s HookState) error {
func (c Command) Run(s *specs.State) error {
b, err := json.Marshal(s)
if err != nil {
return err
Expand Down
14 changes: 9 additions & 5 deletions libcontainer/configs/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runtime-spec/specs-go"
)

func TestUnmarshalHooks(t *testing.T) {
Expand Down Expand Up @@ -101,7 +102,7 @@ func TestMarshalUnmarshalHooks(t *testing.T) {
}

func TestMarshalHooksWithUnexpectedType(t *testing.T) {
fHook := configs.NewFunctionHook(func(configs.HookState) error {
fHook := configs.NewFunctionHook(func(*specs.State) error {
return nil
})
hook := configs.Hooks{
Expand All @@ -119,14 +120,15 @@ func TestMarshalHooksWithUnexpectedType(t *testing.T) {
}

func TestFuncHookRun(t *testing.T) {
state := configs.HookState{
state := &specs.State{
Version: "1",
ID: "1",
Status: "created",
Pid: 1,
Bundle: "/bundle",
}

fHook := configs.NewFunctionHook(func(s configs.HookState) error {
fHook := configs.NewFunctionHook(func(s *specs.State) error {
if !reflect.DeepEqual(state, s) {
t.Errorf("Expected state %+v to equal %+v", state, s)
}
Expand All @@ -137,9 +139,10 @@ func TestFuncHookRun(t *testing.T) {
}

func TestCommandHookRun(t *testing.T) {
state := configs.HookState{
state := &specs.State{
Version: "1",
ID: "1",
Status: "created",
Pid: 1,
Bundle: "/bundle",
}
Expand All @@ -160,9 +163,10 @@ func TestCommandHookRun(t *testing.T) {
}

func TestCommandHookRunTimeout(t *testing.T) {
state := configs.HookState{
state := &specs.State{
Version: "1",
ID: "1",
Status: "created",
Pid: 1,
Bundle: "/bundle",
}
Expand Down
7 changes: 7 additions & 0 deletions libcontainer/container.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"time"

"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runtime-spec/specs-go"
)

// Status is the status of a container.
Expand Down Expand Up @@ -85,6 +86,12 @@ type BaseContainer interface {
// SystemError - System error.
State() (*State, error)

// OCIState returns the current container's state information.
//
// errors:
// SystemError - System error.
OCIState() (*specs.State, error)

// Returns the current config of the container.
Config() configs.Config

Expand Down
58 changes: 40 additions & 18 deletions libcontainer/container_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/opencontainers/runc/libcontainer/intelrdt"
"github.com/opencontainers/runc/libcontainer/system"
"github.com/opencontainers/runc/libcontainer/utils"
"github.com/opencontainers/runtime-spec/specs-go"

"github.com/golang/protobuf/proto"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -156,6 +157,12 @@ func (c *linuxContainer) State() (*State, error) {
return c.currentState()
}

func (c *linuxContainer) OCIState() (*specs.State, error) {
c.m.Lock()
defer c.m.Unlock()
return c.currentOCIState()
}

func (c *linuxContainer) Processes() ([]int, error) {
pids, err := c.cgroupManager.GetAllPids()
if err != nil {
Expand Down Expand Up @@ -359,14 +366,10 @@ func (c *linuxContainer) start(process *Process, isInit bool) error {
}
c.initProcessStartTime = state.InitProcessStartTime

if c.config.Hooks != nil {
bundle, annotations := utils.Annotations(c.config.Labels)
s := configs.HookState{
Version: c.config.Version,
ID: c.id,
Pid: parent.pid(),
Bundle: bundle,
Annotations: annotations,
if c.config.Hooks != nil && len(c.config.Hooks.Poststart) > 0 {
s, err := c.currentOCIState()
if err != nil {
return err
}
for i, hook := range c.config.Hooks.Poststart {
if err := hook.Run(s); err != nil {
Expand Down Expand Up @@ -506,7 +509,7 @@ func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, c
if err != nil {
return nil, err
}
return &initProcess{
init := &initProcess{
cmd: cmd,
childPipe: childPipe,
parentPipe: parentPipe,
Expand All @@ -517,7 +520,9 @@ func (c *linuxContainer) newInitProcess(p *Process, cmd *exec.Cmd, parentPipe, c
process: p,
bootstrapData: data,
sharePidns: sharePidns,
}, nil
}
c.initProcess = init
return init, nil
}

func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, parentPipe, childPipe *os.File) (*setnsProcess, error) {
Expand Down Expand Up @@ -1485,14 +1490,10 @@ func (c *linuxContainer) criuNotifications(resp *criurpc.CriuResp, process *Proc
return err
}
case notify.GetScript() == "setup-namespaces":
if c.config.Hooks != nil {
bundle, annotations := utils.Annotations(c.config.Labels)
s := configs.HookState{
Version: c.config.Version,
ID: c.id,
Pid: int(notify.GetPid()),
Bundle: bundle,
Annotations: annotations,
if c.config.Hooks != nil && len(c.config.Hooks.Prestart) > 0 {
s, err := c.currentOCIState()
if err != nil {
return nil
}
for i, hook := range c.config.Hooks.Prestart {
if err := hook.Run(s); err != nil {
Expand Down Expand Up @@ -1687,6 +1688,27 @@ func (c *linuxContainer) currentState() (*State, error) {
return state, nil
}

func (c *linuxContainer) currentOCIState() (*specs.State, error) {
bundle, annotations := utils.Annotations(c.config.Labels)
state := &specs.State{
Version: specs.Version,
ID: c.ID(),
Bundle: bundle,
Annotations: annotations,
}
status, err := c.currentStatus()
if err != nil {
return nil, err
}
state.Status = status.String()
if status != Stopped {
if c.initProcess != nil {
state.Pid = c.initProcess.pid()
}
}
return state, nil
}

// orderNamespacePaths sorts namespace paths into a list of paths that we
// can setns in order.
func (c *linuxContainer) orderNamespacePaths(namespaces map[configs.NamespaceType]string) ([]string, error) {
Expand Down
3 changes: 2 additions & 1 deletion libcontainer/factory_linux_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/mount"
"github.com/opencontainers/runc/libcontainer/utils"
"github.com/opencontainers/runtime-spec/specs-go"

"golang.org/x/sys/unix"
)
Expand Down Expand Up @@ -229,6 +230,6 @@ func marshal(path string, v interface{}) error {

type unserializableHook struct{}

func (unserializableHook) Run(configs.HookState) error {
func (unserializableHook) Run(*specs.State) error {
return nil
}
7 changes: 4 additions & 3 deletions libcontainer/integration/exec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/opencontainers/runc/libcontainer"
"github.com/opencontainers/runc/libcontainer/cgroups/systemd"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runtime-spec/specs-go"

"golang.org/x/sys/unix"
)
Expand Down Expand Up @@ -1137,7 +1138,7 @@ func TestHook(t *testing.T) {

config.Hooks = &configs.Hooks{
Prestart: []configs.Hook{
configs.NewFunctionHook(func(s configs.HookState) error {
configs.NewFunctionHook(func(s *specs.State) error {
if s.Bundle != expectedBundle {
t.Fatalf("Expected prestart hook bundlePath '%s'; got '%s'", expectedBundle, s.Bundle)
}
Expand All @@ -1154,7 +1155,7 @@ func TestHook(t *testing.T) {
}),
},
Poststart: []configs.Hook{
configs.NewFunctionHook(func(s configs.HookState) error {
configs.NewFunctionHook(func(s *specs.State) error {
if s.Bundle != expectedBundle {
t.Fatalf("Expected poststart hook bundlePath '%s'; got '%s'", expectedBundle, s.Bundle)
}
Expand All @@ -1167,7 +1168,7 @@ func TestHook(t *testing.T) {
}),
},
Poststop: []configs.Hook{
configs.NewFunctionHook(func(s configs.HookState) error {
configs.NewFunctionHook(func(s *specs.State) error {
if s.Bundle != expectedBundle {
t.Fatalf("Expected poststop hook bundlePath '%s'; got '%s'", expectedBundle, s.Bundle)
}
Expand Down
30 changes: 14 additions & 16 deletions libcontainer/process_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,15 +340,14 @@ func (p *initProcess) start() error {
}
}

if p.config.Config.Hooks != nil {
bundle, annotations := utils.Annotations(p.container.config.Labels)
s := configs.HookState{
Version: p.container.config.Version,
ID: p.container.id,
Pid: p.pid(),
Bundle: bundle,
Annotations: annotations,
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)
Expand All @@ -371,15 +370,14 @@ func (p *initProcess) start() error {
return newSystemErrorWithCause(err, "setting Intel RDT config for procHooks process")
}
}
if p.config.Config.Hooks != nil {
bundle, annotations := utils.Annotations(p.container.config.Labels)
s := configs.HookState{
Version: p.container.config.Version,
ID: p.container.id,
Pid: p.pid(),
Bundle: bundle,
Annotations: annotations,
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)
Expand Down
12 changes: 4 additions & 8 deletions libcontainer/state_linux.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"path/filepath"

"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/utils"

"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
Expand Down Expand Up @@ -62,13 +61,10 @@ func destroy(c *linuxContainer) error {
}

func runPoststopHooks(c *linuxContainer) error {
if c.config.Hooks != nil {
bundle, annotations := utils.Annotations(c.config.Labels)
s := configs.HookState{
Version: c.config.Version,
ID: c.id,
Bundle: bundle,
Annotations: annotations,
if c.config.Hooks != nil && len(c.config.Hooks.Poststop) > 0 {
s, err := c.currentOCIState()
if err != nil {
return err
}
for _, hook := range c.config.Hooks.Poststop {
if err := hook.Run(s); err != nil {
Expand Down

0 comments on commit 2063acd

Please sign in to comment.