Add state pattern for container state transition

Signed-off-by: Michael Crosby <[email protected]> Add state status() method Signed-off-by: Michael Crosby <[email protected]> Allow multiple checkpoint on restore Signed-off-by: Michael Crosby <[email protected]> Handle leave-running state Signed-off-by: Michael Crosby <[email protected]> Fix state transitions for inprocess Because the tests use libcontainer in process between the various states we need to ensure that that usecase works as well as the out of process one. Signed-off-by: Michael Crosby <[email protected]> Remove isDestroyed method Signed-off-by: Michael Crosby <[email protected]> Handling Pausing from freezer state Signed-off-by: Rajasekaran <[email protected]> freezer status Signed-off-by: Rajasekaran <[email protected]> Fixing review comments Signed-off-by: Rajasekaran <[email protected]> Added comment when freezer not available Signed-off-by: Rajasekaran <[email protected]> Signed-off-by: Michael Crosby <[email protected]> Conflicts: libcontainer/container_linux.go Change checkFreezer logic to isPaused() Signed-off-by: Michael Crosby <[email protected]> Remove state base and factor out destroy func Signed-off-by: Michael Crosby <[email protected]> Add unit test for state transitions Signed-off-by: Michael Crosby <[email protected]>
kolyshkin · Dec 17, 2015 · 4415446 · 4415446
1 parent 9d6ce71
commit 4415446
Show file tree

Hide file tree

Showing 10 changed files with 435 additions and 131 deletions.
diff --git a/checkpoint.go b/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)

diff --git a/libcontainer/container.go b/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 {

diff --git a/libcontainer/container_linux.go b/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()
-	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
+	return c.state.destroy()
 }
 
 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)
 		}
 	}
-
-	err = c.criuSwrk(process, req, criuOpts, true)
-	if err != nil {
-		return err
-	}
-	return nil
+	return c.criuSwrk(process, req, criuOpts, true)
 }
 
 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"))
-			if err != nil {
-				return err
-			}
-			f.Close()
+		f, err := os.Create(filepath.Join(c.root, "checkpoint"))
+		if err != nil {
+			return err
 		}
-		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
 		}
-
-		// TODO: crosbymichael restore previous process information by saving the init process information in
-		// the container's state file or separate process state files.
+		process.ops = r
+		if err := c.state.transition(&restoredState{
+			imageDir: opts.ImagesDirectory,
+			c:        c,
+		}); err != nil {
+			return err
+		}
 		if err := c.updateState(r); err != nil {
 			return err
 		}
-		process.ops = r
-		break
+		if err := os.Remove(filepath.Join(c.root, "checkpoint")); err != nil {
+			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(state)
+	return json.NewEncoder(f).Encode(s)
+}
+
+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 {
-		return Checkpointed, nil
+	if err := c.refreshState(); err != 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 Paused, nil
+	return true, 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) {

diff --git a/libcontainer/container_linux_test.go b/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)