Fix various state bugs for pause and destroy

There were issues where a process could die before pausing completed
leaving the container in an inconsistent state and unable to be
destoryed.  This makes sure that if the container is paused and the
process is dead it will unfreeze the cgroup before removing them.

Signed-off-by: Michael Crosby <[email protected]>

Author: crosbymichael

libcontainer/container_linux.go

@@ -189,29 +189,27 @@ func (c *linuxContainer) Start(process *Process) error {
 		}
 		return newSystemError(err)
 	}
+	c.state = &runningState{
+		c: c,
+	}
 	if doInit {
 		if err := c.updateState(parent); err != nil {
 			return err
 		}
-	} else {
-		c.state.transition(&nullState{
-			c: c,
-			s: Running,
-		})
-	}
-	if c.config.Hooks != nil {
-		s := configs.HookState{
-			Version: c.config.Version,
-			ID:      c.id,
-			Pid:     parent.pid(),
-			Root:    c.config.Rootfs,
-		}
-		for _, hook := range c.config.Hooks.Poststart {
-			if err := hook.Run(s); err != nil {
-				if err := parent.terminate(); err != nil {
-					logrus.Warn(err)
+		if c.config.Hooks != nil {
+			s := configs.HookState{
+				Version: c.config.Version,
+				ID:      c.id,
+				Pid:     parent.pid(),
+				Root:    c.config.Rootfs,
+			}
+			for _, hook := range c.config.Hooks.Poststart {
+				if err := hook.Run(s); err != nil {
+					if err := parent.terminate(); err != nil {
+						logrus.Warn(err)
+					}
+					return newSystemError(err)
 				}
-				return newSystemError(err)
 			}
 		}
 	}
@@ -340,6 +338,13 @@ func (c *linuxContainer) Destroy() error {
 func (c *linuxContainer) Pause() error {
 	c.m.Lock()
 	defer c.m.Unlock()
+	status, err := c.currentStatus()
+	if err != nil {
+		return err
+	}
+	if status != Running {
+		return newGenericError(fmt.Errorf("container not running"), ContainerNotRunning)
+	}
 	if err := c.cgroupManager.Freeze(configs.Frozen); err != nil {
 		return err
 	}
@@ -351,6 +356,13 @@ func (c *linuxContainer) Pause() error {
 func (c *linuxContainer) Resume() error {
 	c.m.Lock()
 	defer c.m.Unlock()
+	status, err := c.currentStatus()
+	if err != nil {
+		return err
+	}
+	if status != Paused {
+		return newGenericError(fmt.Errorf("container not paused"), ContainerNotPaused)
+	}
 	if err := c.cgroupManager.Freeze(configs.Thawed); err != nil {
 		return err
 	}
@@ -939,9 +951,6 @@ func (c *linuxContainer) criuNotifications(resp *criurpc.CriuResp, process *Proc
 
 func (c *linuxContainer) updateState(process parentProcess) error {
 	c.initProcess = process
-	if err := c.refreshState(); err != nil {
-		return err
-	}
 	state, err := c.currentState()
 	if err != nil {
 		return err
@@ -1017,35 +1026,36 @@ func (c *linuxContainer) isPaused() (bool, error) {
 }
 
 func (c *linuxContainer) currentState() (*State, error) {
-	status, err := c.currentStatus()
-	if err != nil {
-		return nil, err
-	}
-	if status == Destroyed {
-		return nil, newGenericError(fmt.Errorf("container destroyed"), ContainerNotExists)
-	}
-	startTime, err := c.initProcess.startTime()
-	if err != nil {
-		return nil, newSystemError(err)
+	var (
+		startTime           string
+		externalDescriptors []string
+		pid                 = -1
+	)
+	if c.initProcess != nil {
+		pid = c.initProcess.pid()
+		startTime, _ = c.initProcess.startTime()
+		externalDescriptors = c.initProcess.externalDescriptors()
 	}
 	state := &State{
 		BaseState: BaseState{
 			ID:                   c.ID(),
 			Config:               *c.config,
-			InitProcessPid:       c.initProcess.pid(),
+			InitProcessPid:       pid,
 			InitProcessStartTime: startTime,
 		},
 		CgroupPaths:         c.cgroupManager.GetPaths(),
 		NamespacePaths:      make(map[configs.NamespaceType]string),
-		ExternalDescriptors: c.initProcess.externalDescriptors(),
-	}
-	for _, ns := range c.config.Namespaces {
-		state.NamespacePaths[ns.Type] = ns.GetPath(c.initProcess.pid())
+		ExternalDescriptors: externalDescriptors,
 	}
-	for _, nsType := range configs.NamespaceTypes() {
-		if _, ok := state.NamespacePaths[nsType]; !ok {
-			ns := configs.Namespace{Type: nsType}
-			state.NamespacePaths[ns.Type] = ns.GetPath(c.initProcess.pid())
+	if pid > 0 {
+		for _, ns := range c.config.Namespaces {
+			state.NamespacePaths[ns.Type] = ns.GetPath(pid)
+		}
+		for _, nsType := range configs.NamespaceTypes() {
+			if _, ok := state.NamespacePaths[nsType]; !ok {
+				ns := configs.Namespace{Type: nsType}
+				state.NamespacePaths[ns.Type] = ns.GetPath(pid)
+			}
 		}
 	}
 	return state, nil

libcontainer/error.go

@@ -16,6 +16,7 @@ const (
 	ContainerPaused
 	ContainerNotStopped
 	ContainerNotRunning
+	ContainerNotPaused
 
 	// Process errors
 	ProcessNotExecuted
@@ -46,6 +47,8 @@ func (c ErrorCode) String() string {
 		return "Container is not running"
 	case ConsoleExists:
 		return "Console exists for process"
+	case ContainerNotPaused:
+		return "Container is not paused"
 	default:
 		return "Unknown error"
 	}

libcontainer/state_linux.go

@@ -49,6 +49,7 @@ func destroy(c *linuxContainer) error {
 	if herr := runPoststopHooks(c); err == nil {
 		err = herr
 	}
+	c.state = &stoppedState{c: c}
 	return err
 }
 
@@ -116,10 +117,10 @@ func (r *runningState) transition(s containerState) error {
 		}
 		r.c.state = s
 		return nil
-	case *pausedState:
+	case *pausedState, *nullState:
 		r.c.state = s
 		return nil
-	case *runningState, *nullState:
+	case *runningState:
 		return nil
 	}
 	return newStateTransitionError(r, s)
@@ -148,7 +149,7 @@ func (p *pausedState) status() Status {
 
 func (p *pausedState) transition(s containerState) error {
 	switch s.(type) {
-	case *runningState:
+	case *runningState, *stoppedState:
 		p.c.state = s
 		return nil
 	case *pausedState:
@@ -158,6 +159,16 @@ func (p *pausedState) transition(s containerState) error {
 }
 
 func (p *pausedState) destroy() error {
+	isRunning, err := p.c.isRunning()
+	if err != nil {
+		return err
+	}
+	if !isRunning {
+		if err := p.c.cgroupManager.Freeze(configs.Thawed); err != nil {
+			return err
+		}
+		return destroy(p.c)
+	}
 	return newGenericError(fmt.Errorf("container is paused"), ContainerPaused)
 }
 

libcontainer/state_linux_test.go

@@ -49,19 +49,13 @@ func TestPausedStateTransition(t *testing.T) {
 	valid := []containerState{
 		&pausedState{},
 		&runningState{},
+		&stoppedState{},
 	}
 	for _, v := range valid {
 		if err := s.transition(v); err != nil {
 			t.Fatal(err)
 		}
 	}
-	err := s.transition(&stoppedState{})
-	if err == nil {
-		t.Fatal("transition to stopped state should fail")
-	}
-	if !isStateTransitionError(err) {
-		t.Fatal("expected stateTransitionError")
-	}
 }
 
 func TestRestoredStateTransition(t *testing.T) {