Merge pull request #311 from crosbymichael/destory-state

Implement Container States

checkpoint.go

@@ -30,14 +30,8 @@ var checkpointCommand = cli.Command{
 		if err != nil {
 			fatal(err)
 		}
+		defer destroy(container)
 		options := criuOptions(context)
-		status, err := container.Status()
-		if err != nil {
-			fatal(err)
-		}
-		if status == libcontainer.Checkpointed {
-			fatal(fmt.Errorf("Container with id %s already checkpointed", context.GlobalString("id")))
-		}
 		// these are the mandatory criu options for a container
 		setPageServer(context, options)
 		setManageCgroupsMode(context, options)

libcontainer/container.go

@@ -30,6 +30,23 @@ const (
 	Destroyed
 )
 
+func (s Status) String() string {
+	switch s {
+	case Running:
+		return "running"
+	case Pausing:
+		return "pausing"
+	case Paused:
+		return "paused"
+	case Checkpointed:
+		return "checkpointed"
+	case Destroyed:
+		return "destroyed"
+	default:
+		return "undefined"
+	}
+}
+
 // BaseState represents the platform agnostic pieces relating to a
 // running container's state
 type BaseState struct {

libcontainer/container_linux.go

@@ -37,6 +37,7 @@ type linuxContainer struct {
 	criuPath      string
 	m             sync.Mutex
 	criuVersion   int
+	state         containerState
 }
 
 // State represents a running container's state
@@ -183,7 +184,14 @@ func (c *linuxContainer) Start(process *Process) error {
 		return newSystemError(err)
 	}
 	if doInit {
-		c.updateState(parent)
+		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{
@@ -320,48 +328,29 @@ func newPipe() (parent *os.File, child *os.File, err error) {
 func (c *linuxContainer) Destroy() error {
 	c.m.Lock()
 	defer c.m.Unlock()
-	status, err := c.currentStatus()
+	return c.state.destroy()
-	if err != nil {
-		return err
-	}
-	if status != Destroyed {
-		return newGenericError(fmt.Errorf("container is not destroyed"), ContainerNotStopped)
-	}
-	if !c.config.Namespaces.Contains(configs.NEWPID) {
-		if err := killCgroupProcesses(c.cgroupManager); err != nil {
-			logrus.Warn(err)
-		}
-	}
-	err = c.cgroupManager.Destroy()
-	if rerr := os.RemoveAll(c.root); err == nil {
-		err = rerr
-	}
-	c.initProcess = nil
-	if c.config.Hooks != nil {
-		s := configs.HookState{
-			Version: c.config.Version,
-			ID:      c.id,
-			Root:    c.config.Rootfs,
-		}
-		for _, hook := range c.config.Hooks.Poststop {
-			if err := hook.Run(s); err != nil {
-				return err
-			}
-		}
-	}
-	return err
 }
 
 func (c *linuxContainer) Pause() error {
 	c.m.Lock()
 	defer c.m.Unlock()
-	return c.cgroupManager.Freeze(configs.Frozen)
+	if err := c.cgroupManager.Freeze(configs.Frozen); err != nil {
+		return err
+	}
+	return c.state.transition(&pausedState{
+		c: c,
+	})
 }
 
 func (c *linuxContainer) Resume() error {
 	c.m.Lock()
 	defer c.m.Unlock()
-	return c.cgroupManager.Freeze(configs.Thawed)
+	if err := c.cgroupManager.Freeze(configs.Thawed); err != nil {
+		return err
+	}
+	return c.state.transition(&runningState{
+		c: c,
+	})
 }
 
 func (c *linuxContainer) NotifyOOM() (<-chan struct{}, error) {
@@ -459,7 +448,7 @@ func (c *linuxContainer) Checkpoint(criuOpts *CriuOpts) error {
 	}
 
 	if criuOpts.ImagesDirectory == "" {
-		criuOpts.ImagesDirectory = filepath.Join(c.root, "criu.image")
+		return fmt.Errorf("invalid directory to save checkpoint")
 	}
 
 	// Since a container can be C/R'ed multiple times,
@@ -578,11 +567,9 @@ func (c *linuxContainer) addCriuRestoreMount(req *criurpc.CriuReq, m *configs.Mo
 func (c *linuxContainer) Restore(process *Process, criuOpts *CriuOpts) error {
 	c.m.Lock()
 	defer c.m.Unlock()
-
 	if err := c.checkCriuVersion("1.5.2"); err != nil {
 		return err
 	}
-
 	if criuOpts.WorkDirectory == "" {
 		criuOpts.WorkDirectory = filepath.Join(c.root, "criu.work")
 	}
@@ -591,22 +578,19 @@ func (c *linuxContainer) Restore(process *Process, criuOpts *CriuOpts) error {
 	if err := os.Mkdir(criuOpts.WorkDirectory, 0655); err != nil && !os.IsExist(err) {
 		return err
 	}
-
 	workDir, err := os.Open(criuOpts.WorkDirectory)
 	if err != nil {
 		return err
 	}
 	defer workDir.Close()
-
 	if criuOpts.ImagesDirectory == "" {
-		criuOpts.ImagesDirectory = filepath.Join(c.root, "criu.image")
+		return fmt.Errorf("invalid directory to restore checkpoint")
 	}
 	imageDir, err := os.Open(criuOpts.ImagesDirectory)
 	if err != nil {
 		return err
 	}
 	defer imageDir.Close()
-
 	// CRIU has a few requirements for a root directory:
 	// * it must be a mount point
 	// * its parent must not be overmounted
@@ -617,18 +601,15 @@ func (c *linuxContainer) Restore(process *Process, criuOpts *CriuOpts) error {
 		return err
 	}
 	defer os.Remove(root)
-
 	root, err = filepath.EvalSymlinks(root)
 	if err != nil {
 		return err
 	}
-
 	err = syscall.Mount(c.config.Rootfs, root, "", syscall.MS_BIND|syscall.MS_REC, "")
 	if err != nil {
 		return err
 	}
 	defer syscall.Unmount(root, syscall.MNT_DETACH)
-
 	t := criurpc.CriuReqType_RESTORE
 	req := &criurpc.CriuReq{
 		Type: &t,
@@ -696,15 +677,13 @@ func (c *linuxContainer) Restore(process *Process, criuOpts *CriuOpts) error {
 		fds    []string
 		fdJSON []byte
 	)
-
 	if fdJSON, err = ioutil.ReadFile(filepath.Join(criuOpts.ImagesDirectory, descriptorsFilename)); err != nil {
 		return err
 	}
 
-	if err = json.Unmarshal(fdJSON, &fds); err != nil {
+	if err := json.Unmarshal(fdJSON, &fds); err != nil {
 		return err
 	}
-
 	for i := range fds {
 		if s := fds[i]; strings.Contains(s, "pipe:") {
 			inheritFd := new(criurpc.InheritFd)
@@ -713,12 +692,7 @@ func (c *linuxContainer) Restore(process *Process, criuOpts *CriuOpts) error {
 			req.Opts.InheritFd = append(req.Opts.InheritFd, inheritFd)
 		}
 	}
-
+	return c.criuSwrk(process, req, criuOpts, true)
-	err = c.criuSwrk(process, req, criuOpts, true)
-	if err != nil {
-		return err
-	}
-	return nil
 }
 
 func (c *linuxContainer) criuApplyCgroups(pid int, req *criurpc.CriuReq) error {
@@ -913,82 +887,123 @@ func (c *linuxContainer) criuNotifications(resp *criurpc.CriuResp, process *Proc
 	if notify == nil {
 		return fmt.Errorf("invalid response: %s", resp.String())
 	}
-
 	switch {
 	case notify.GetScript() == "post-dump":
-		if !opts.LeaveRunning {
+		f, err := os.Create(filepath.Join(c.root, "checkpoint"))
-			f, err := os.Create(filepath.Join(c.root, "checkpoint"))
+		if err != nil {
-			if err != nil {
+			return err
-				return err
-			}
-			f.Close()
 		}
-		break
+		f.Close()
-
 	case notify.GetScript() == "network-unlock":
 		if err := unlockNetwork(c.config); err != nil {
 			return err
 		}
-		break
-
 	case notify.GetScript() == "network-lock":
 		if err := lockNetwork(c.config); err != nil {
 			return err
 		}
-		break
-
 	case notify.GetScript() == "post-restore":
 		pid := notify.GetPid()
 		r, err := newRestoredProcess(int(pid), fds)
 		if err != nil {
 			return err
 		}
-
+		process.ops = r
-		// TODO: crosbymichael restore previous process information by saving the init process information in
+		if err := c.state.transition(&restoredState{
-		// the container's state file or separate process state files.
+			imageDir: opts.ImagesDirectory,
+			c:        c,
+		}); err != nil {
+			return err
+		}
 		if err := c.updateState(r); err != nil {
 			return err
 		}
-		process.ops = r
+		if err := os.Remove(filepath.Join(c.root, "checkpoint")); err != nil {
-		break
+			if !os.IsNotExist(err) {
+				logrus.Error(err)
+			}
+		}
 	}
-
 	return nil
 }
 
 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
 	}
+	return c.saveState(state)
+}
+
+func (c *linuxContainer) saveState(s *State) error {
 	f, err := os.Create(filepath.Join(c.root, stateFilename))
 	if err != nil {
 		return err
 	}
 	defer f.Close()
-	os.Remove(filepath.Join(c.root, "checkpoint"))
+	return json.NewEncoder(f).Encode(s)
-	return json.NewEncoder(f).Encode(state)
+}
+
+func (c *linuxContainer) deleteState() error {
+	return os.Remove(filepath.Join(c.root, stateFilename))
 }
 
 func (c *linuxContainer) currentStatus() (Status, error) {
-	if _, err := os.Stat(filepath.Join(c.root, "checkpoint")); err == nil {
+	if err := c.refreshState(); err != nil {
-		return Checkpointed, nil
+		return -1, err
 	}
+	return c.state.status(), nil
+}
+
+// refreshState needs to be called to verify that the current state on the
+// container is what is true.  Because consumers of libcontainer can use it
+// out of process we need to verify the container's status based on runtime
+// information and not rely on our in process info.
+func (c *linuxContainer) refreshState() error {
+	paused, err := c.isPaused()
+	if err != nil {
+		return err
+	}
+	if paused {
+		return c.state.transition(&pausedState{c: c})
+	}
+	running, err := c.isRunning()
+	if err != nil {
+		return err
+	}
+	if running {
+		return c.state.transition(&runningState{c: c})
+	}
+	return c.state.transition(&stoppedState{c: c})
+}
+
+func (c *linuxContainer) isRunning() (bool, error) {
 	if c.initProcess == nil {
-		return Destroyed, nil
+		return false, nil
 	}
 	// return Running if the init process is alive
 	if err := syscall.Kill(c.initProcess.pid(), 0); err != nil {
 		if err == syscall.ESRCH {
-			return Destroyed, nil
+			return false, nil
 		}
-		return 0, newSystemError(err)
+		return false, newSystemError(err)
 	}
-	if c.config.Cgroups != nil && c.config.Cgroups.Resources != nil && c.config.Cgroups.Resources.Freezer == configs.Frozen {
+	return true, nil
-		return Paused, nil
+}
+
+func (c *linuxContainer) isPaused() (bool, error) {
+	data, err := ioutil.ReadFile(filepath.Join(c.cgroupManager.GetPaths()["freezer"], "freezer.state"))
+	if err != nil {
+		if os.IsNotExist(err) {
+			return false, nil
+		}
+		return false, newSystemError(err)
 	}
-	return Running, nil
+	return bytes.Equal(bytes.TrimSpace(data), []byte("FROZEN")), nil
 }
 
 func (c *linuxContainer) currentState() (*State, error) {

libcontainer/container_linux_test.go

@@ -161,6 +161,7 @@ func TestGetContainerState(t *testing.T) {
 			},
 		},
 	}
+	container.state = &nullState{c: container}
 	state, err := container.State()
 	if err != nil {
 		t.Fatal(err)

libcontainer/factory_linux.go

@@ -166,7 +166,7 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err
 	if err := os.MkdirAll(containerRoot, 0700); err != nil {
 		return nil, newGenericError(err, SystemError)
 	}
-	return &linuxContainer{
+	c := &linuxContainer{
 		id:            id,
 		root:          containerRoot,
 		config:        config,
@@ -174,7 +174,9 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err
 		initArgs:      l.InitArgs,
 		criuPath:      l.CriuPath,
 		cgroupManager: l.NewCgroupsManager(config.Cgroups, nil),
-	}, nil
+	}
+	c.state = &stoppedState{c: c}
+	return c, nil
 }
 
 func (l *LinuxFactory) Load(id string) (Container, error) {
@@ -191,7 +193,7 @@ func (l *LinuxFactory) Load(id string) (Container, error) {
 		processStartTime: state.InitProcessStartTime,
 		fds:              state.ExternalDescriptors,
 	}
-	return &linuxContainer{
+	c := &linuxContainer{
 		initProcess:   r,
 		id:            id,
 		config:        &state.Config,
@@ -200,7 +202,9 @@ func (l *LinuxFactory) Load(id string) (Container, error) {
 		criuPath:      l.CriuPath,
 		cgroupManager: l.NewCgroupsManager(state.Config.Cgroups, state.CgroupPaths),
 		root:          containerRoot,
-	}, nil
+	}
+	c.state = &nullState{c: c}
+	return c, nil
 }
 
 func (l *LinuxFactory) Type() string {

libcontainer/integration/checkpoint_test.go

@@ -128,8 +128,8 @@ func TestCheckpoint(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if state != libcontainer.Checkpointed {
+	if state != libcontainer.Running {
-		t.Fatal("Unexpected state: ", state)
+		t.Fatal("Unexpected state checkpoint: ", state)
 	}
 
 	stdinW.Close()
@@ -167,7 +167,7 @@ func TestCheckpoint(t *testing.T) {
 		t.Fatal(err)
 	}
 	if state != libcontainer.Running {
-		t.Fatal("Unexpected state: ", state)
+		t.Fatal("Unexpected restore state: ", state)
 	}
 
 	pid, err = restoreProcessConfig.Pid()

libcontainer/state_linux.go

@@ -0,0 +1,217 @@
+// +build linux
+
+package libcontainer
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+
+	"github.com/Sirupsen/logrus"
+	"github.com/opencontainers/runc/libcontainer/configs"
+)
+
+func newStateTransitionError(from, to containerState) error {
+	return &stateTransitionError{
+		From: from.status().String(),
+		To:   to.status().String(),
+	}
+}
+
+// stateTransitionError is returned when an invalid state transition happens from one
+// state to another.
+type stateTransitionError struct {
+	From string
+	To   string
+}
+
+func (s *stateTransitionError) Error() string {
+	return fmt.Sprintf("invalid state transition from %s to %s", s.From, s.To)
+}
+
+type containerState interface {
+	transition(containerState) error
+	destroy() error
+	status() Status
+}
+
+func destroy(c *linuxContainer) error {
+	if !c.config.Namespaces.Contains(configs.NEWPID) {
+		if err := killCgroupProcesses(c.cgroupManager); err != nil {
+			logrus.Warn(err)
+		}
+	}
+	err := c.cgroupManager.Destroy()
+	if rerr := os.RemoveAll(c.root); err == nil {
+		err = rerr
+	}
+	c.initProcess = nil
+	if herr := runPoststopHooks(c); err == nil {
+		err = herr
+	}
+	return err
+}
+
+func runPoststopHooks(c *linuxContainer) error {
+	if c.config.Hooks != nil {
+		s := configs.HookState{
+			Version: c.config.Version,
+			ID:      c.id,
+			Root:    c.config.Rootfs,
+		}
+		for _, hook := range c.config.Hooks.Poststop {
+			if err := hook.Run(s); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+// stoppedState represents a container is a stopped/destroyed state.
+type stoppedState struct {
+	c *linuxContainer
+}
+
+func (b *stoppedState) status() Status {
+	return Destroyed
+}
+
+func (b *stoppedState) transition(s containerState) error {
+	switch s.(type) {
+	case *runningState:
+		b.c.state = s
+		return nil
+	case *restoredState:
+		b.c.state = s
+		return nil
+	case *stoppedState:
+		return nil
+	}
+	return newStateTransitionError(b, s)
+}
+
+func (b *stoppedState) destroy() error {
+	return destroy(b.c)
+}
+
+// runningState represents a container that is currently running.
+type runningState struct {
+	c *linuxContainer
+}
+
+func (r *runningState) status() Status {
+	return Running
+}
+
+func (r *runningState) transition(s containerState) error {
+	switch s.(type) {
+	case *stoppedState:
+		running, err := r.c.isRunning()
+		if err != nil {
+			return err
+		}
+		if running {
+			return newGenericError(fmt.Errorf("container still running"), ContainerNotStopped)
+		}
+		r.c.state = s
+		return nil
+	case *pausedState:
+		r.c.state = s
+		return nil
+	case *runningState, *nullState:
+		return nil
+	}
+	return newStateTransitionError(r, s)
+}
+
+func (r *runningState) destroy() error {
+	running, err := r.c.isRunning()
+	if err != nil {
+		return err
+	}
+	if running {
+		return newGenericError(fmt.Errorf("container is not destroyed"), ContainerNotStopped)
+	}
+	return destroy(r.c)
+}
+
+// pausedState represents a container that is currently pause.  It cannot be destroyed in a
+// paused state and must transition back to running first.
+type pausedState struct {
+	c *linuxContainer
+}
+
+func (p *pausedState) status() Status {
+	return Paused
+}
+
+func (p *pausedState) transition(s containerState) error {
+	switch s.(type) {
+	case *runningState:
+		p.c.state = s
+		return nil
+	case *pausedState:
+		return nil
+	}
+	return newStateTransitionError(p, s)
+}
+
+func (p *pausedState) destroy() error {
+	return newGenericError(fmt.Errorf("container is paused"), ContainerPaused)
+}
+
+// restoredState is the same as the running state but also has accociated checkpoint
+// information that maybe need destroyed when the container is stopped and destory is called.
+type restoredState struct {
+	imageDir string
+	c        *linuxContainer
+}
+
+func (r *restoredState) status() Status {
+	return Running
+}
+
+func (r *restoredState) transition(s containerState) error {
+	switch s.(type) {
+	case *stoppedState:
+		return nil
+	case *runningState:
+		return nil
+	}
+	return newStateTransitionError(r, s)
+}
+
+func (r *restoredState) destroy() error {
+	if _, err := os.Stat(filepath.Join(r.c.root, "checkpoint")); err != nil {
+		if !os.IsNotExist(err) {
+			return err
+		}
+	}
+	return destroy(r.c)
+}
+
+// nullState is used whenever a container is restored, loaded, or setting additional
+// processes inside and it should not be destroyed when it is exiting.
+type nullState struct {
+	c *linuxContainer
+	s Status
+}
+
+func (n *nullState) status() Status {
+	return n.s
+}
+
+func (n *nullState) transition(s containerState) error {
+	switch s.(type) {
+	case *restoredState:
+		n.c.state = s
+	default:
+		// do nothing for null states
+	}
+	return nil
+}
+
+func (n *nullState) destroy() error {
+	return nil
+}

libcontainer/state_linux_test.go

@@ -0,0 +1,85 @@
+// +build linux
+
+package libcontainer
+
+import "testing"
+
+func TestStateStatus(t *testing.T) {
+	states := map[containerState]Status{
+		&stoppedState{}:  Destroyed,
+		&runningState{}:  Running,
+		&restoredState{}: Running,
+		&pausedState{}:   Paused,
+	}
+	for s, status := range states {
+		if s.status() != status {
+			t.Fatalf("state returned %s but expected %s", s.status(), status)
+		}
+	}
+}
+
+func isStateTransitionError(err error) bool {
+	_, ok := err.(*stateTransitionError)
+	return ok
+}
+
+func TestStoppedStateTransition(t *testing.T) {
+	s := &stoppedState{c: &linuxContainer{}}
+	valid := []containerState{
+		&stoppedState{},
+		&runningState{},
+		&restoredState{},
+	}
+	for _, v := range valid {
+		if err := s.transition(v); err != nil {
+			t.Fatal(err)
+		}
+	}
+	err := s.transition(&pausedState{})
+	if err == nil {
+		t.Fatal("transition to paused state should fail")
+	}
+	if !isStateTransitionError(err) {
+		t.Fatal("expected stateTransitionError")
+	}
+}
+
+func TestPausedStateTransition(t *testing.T) {
+	s := &pausedState{c: &linuxContainer{}}
+	valid := []containerState{
+		&pausedState{},
+		&runningState{},
+	}
+	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) {
+	s := &restoredState{c: &linuxContainer{}}
+	valid := []containerState{
+		&stoppedState{},
+		&runningState{},
+	}
+	for _, v := range valid {
+		if err := s.transition(v); err != nil {
+			t.Fatal(err)
+		}
+	}
+	err := s.transition(&nullState{})
+	if err == nil {
+		t.Fatal("transition to null state should fail")
+	}
+	if !isStateTransitionError(err) {
+		t.Fatal("expected stateTransitionError")
+	}
+}

restore.go

@@ -5,7 +5,6 @@ package main
 import (
 	"fmt"
 	"os"
-	"path/filepath"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/codegangsta/cli"
@@ -109,24 +108,12 @@ func restoreContainer(context *cli.Context, spec *specs.LinuxSpec, config *confi
 
 	// ensure that the container is always removed if we were the process
 	// that created it.
-	defer func() {
+	defer destroy(container)
-		if err != nil {
+	process := &libcontainer.Process{
-			return
+		Stdin:  os.Stdin,
-		}
+		Stdout: os.Stdout,
-		status, err := container.Status()
+		Stderr: os.Stderr,
-		if err != nil {
+	}
-			logrus.Error(err)
-		}
-		if status != libcontainer.Checkpointed {
-			if err := container.Destroy(); err != nil {
-				logrus.Error(err)
-			}
-			if err := os.RemoveAll(options.ImagesDirectory); err != nil {
-				logrus.Error(err)
-			}
-		}
-	}()
-	process := &libcontainer.Process{}
 	tty, err := newTty(spec.Process.Terminal, process, rootuid)
 	if err != nil {
 		return -1, err
@@ -134,16 +121,6 @@ func restoreContainer(context *cli.Context, spec *specs.LinuxSpec, config *confi
 	handler := newSignalHandler(tty)
 	defer handler.Close()
 	if err := container.Restore(process, options); err != nil {
-		cstatus, cerr := container.Status()
-		if cerr != nil {
-			logrus.Error(cerr)
-		}
-		if cstatus == libcontainer.Destroyed {
-			dest := filepath.Join(context.GlobalString("root"), context.GlobalString("id"))
-			if errVal := os.RemoveAll(dest); errVal != nil {
-				logrus.Error(errVal)
-			}
-		}
 		return -1, err
 	}
 	return handler.forward(process)

start.go

@@ -142,13 +142,7 @@ func setupSocketActivation(spec *specs.LinuxSpec, listenFds string) {
 }
 
 func destroy(container libcontainer.Container) {
-	status, err := container.Status()
+	if err := container.Destroy(); err != nil {
-	if err != nil {
 		logrus.Error(err)
 	}
-	if status != libcontainer.Checkpointed {
-		if err := container.Destroy(); err != nil {
-			logrus.Error(err)
-		}
-	}
 }