From 4415446c321ed9e31facfe25c9e230f5257bb872 Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Fri, 2 Oct 2015 11:16:50 -0700 Subject: [PATCH] Add state pattern for container state transition Signed-off-by: Michael Crosby Add state status() method Signed-off-by: Michael Crosby Allow multiple checkpoint on restore Signed-off-by: Michael Crosby Handle leave-running state Signed-off-by: Michael Crosby 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 Remove isDestroyed method Signed-off-by: Michael Crosby Handling Pausing from freezer state Signed-off-by: Rajasekaran freezer status Signed-off-by: Rajasekaran Fixing review comments Signed-off-by: Rajasekaran Added comment when freezer not available Signed-off-by: Rajasekaran Signed-off-by: Michael Crosby Conflicts: libcontainer/container_linux.go Change checkFreezer logic to isPaused() Signed-off-by: Michael Crosby Remove state base and factor out destroy func Signed-off-by: Michael Crosby Add unit test for state transitions Signed-off-by: Michael Crosby --- checkpoint.go | 8 +- libcontainer/container.go | 17 ++ libcontainer/container_linux.go | 177 ++++++++-------- libcontainer/container_linux_test.go | 1 + libcontainer/factory_linux.go | 12 +- libcontainer/integration/checkpoint_test.go | 6 +- libcontainer/state_linux.go | 217 ++++++++++++++++++++ libcontainer/state_linux_test.go | 85 ++++++++ restore.go | 35 +--- start.go | 8 +- 10 files changed, 435 insertions(+), 131 deletions(-) create mode 100644 libcontainer/state_linux.go create mode 100644 libcontainer/state_linux_test.go diff --git a/checkpoint.go b/checkpoint.go index 9df558c8d..c1ca44f63 100644 --- 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 index 6292fd185..051c8cf68 100644 --- 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 index de98e97ca..62b228a12 100644 --- 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 index 73577f5c7..2e9d1322d 100644 --- 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) diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index 70513f7b6..c2d359edb 100644 --- a/libcontainer/factory_linux.go +++ b/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 { diff --git a/libcontainer/integration/checkpoint_test.go b/libcontainer/integration/checkpoint_test.go index fc027fa89..62cab2a37 100644 --- a/libcontainer/integration/checkpoint_test.go +++ b/libcontainer/integration/checkpoint_test.go @@ -128,8 +128,8 @@ func TestCheckpoint(t *testing.T) { t.Fatal(err) } - if state != libcontainer.Checkpointed { - t.Fatal("Unexpected state: ", state) + if state != libcontainer.Running { + 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() diff --git a/libcontainer/state_linux.go b/libcontainer/state_linux.go new file mode 100644 index 000000000..5e1bb731f --- /dev/null +++ b/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 +} diff --git a/libcontainer/state_linux_test.go b/libcontainer/state_linux_test.go new file mode 100644 index 000000000..e2740ca63 --- /dev/null +++ b/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") + } +} diff --git a/restore.go b/restore.go index b5b20fddd..db1a0fd1f 100644 --- a/restore.go +++ b/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() { - if err != nil { - return - } - status, err := container.Status() - 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{} + defer destroy(container) + process := &libcontainer.Process{ + Stdin: os.Stdin, + Stdout: os.Stdout, + Stderr: os.Stderr, + } 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) diff --git a/start.go b/start.go index c39fa017f..802b3bf32 100644 --- a/start.go +++ b/start.go @@ -142,13 +142,7 @@ func setupSocketActivation(spec *specs.LinuxSpec, listenFds string) { } func destroy(container libcontainer.Container) { - status, err := container.Status() - if err != nil { + if err := container.Destroy(); err != nil { logrus.Error(err) } - if status != libcontainer.Checkpointed { - if err := container.Destroy(); err != nil { - logrus.Error(err) - } - } }