Merge pull request #4493 from mheon/add_removing_state

Add ContainerStateRemoving

8 files changed, 110 insertions, 96 deletions

diff --git a/libpod/boltdb_state_linux.go b/libpod/boltdb_state_linux.go
index 09a9be606..6ccda71bd 100644
--- a/libpod/boltdb_state_linux.go
+++ b/libpod/boltdb_state_linux.go
@@ -3,6 +3,8 @@
 package libpod
 
 import (
+	"github.com/containers/libpod/libpod/define"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
@@ -25,8 +27,12 @@ func replaceNetNS(netNSPath string, ctr *Container, newState *ContainerState) er
 			if err == nil {
 				newState.NetNS = ns
 			} else {
+				if ctr.ensureState(define.ContainerStateRunning, define.ContainerStatePaused) {
+					return errors.Wrapf(err, "error joning network namespace of container %s", ctr.ID())
+				}
+
 				logrus.Errorf("error joining network namespace for container %s: %v", ctr.ID(), err)
-				ctr.valid = false
+				ctr.state.NetNS = nil
 			}
 		}
 	} else {
diff --git a/libpod/container_api.go b/libpod/container_api.go
index b8cfe02f6..153a1d628 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -404,6 +404,11 @@ func (c *Container) Mount() (string, error) {
 			return "", err
 		}
 	}
+
+	if c.state.State == define.ContainerStateRemoving {
+		return "", errors.Wrapf(define.ErrCtrStateInvalid, "cannot mount container %s as it is being removed", c.ID())
+	}
+
 	defer c.newContainerEvent(events.Mount)
 	return c.mount()
 }
@@ -488,7 +493,12 @@ func (c *Container) Export(path string) error {
 			return err
 		}
 	}
-	defer c.newContainerEvent(events.Export)
+
+	if c.state.State == define.ContainerStateRemoving {
+		return errors.Wrapf(define.ErrCtrStateInvalid, "cannot mount container %s as it is being removed", c.ID())
+	}
+
+	defer c.newContainerEvent(events.Mount)
 	return c.export(path)
 }
 
@@ -674,6 +684,10 @@ func (c *Container) Refresh(ctx context.Context) error {
 		}
 	}
 
+	if c.state.State == define.ContainerStateRemoving {
+		return errors.Wrapf(define.ErrCtrStateInvalid, "cannot refresh containers that are being removed")
+	}
+
 	wasCreated := false
 	if c.state.State == define.ContainerStateCreated {
 		wasCreated = true
@@ -819,7 +833,6 @@ func (c *Container) Checkpoint(ctx context.Context, options ContainerCheckpointO
 			return err
 		}
 	}
-	defer c.newContainerEvent(events.Checkpoint)
 	return c.checkpoint(ctx, options)
 }
 
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 4ff1913b5..1e8a8a580 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -719,7 +719,8 @@ func (c *Container) isStopped() (bool, error) {
 	if err != nil {
 		return true, err
 	}
-	return c.state.State != define.ContainerStateRunning && c.state.State != define.ContainerStatePaused, nil
+
+	return !c.ensureState(define.ContainerStateRunning, define.ContainerStatePaused), nil
 }
 
 // save container state to the database
@@ -1057,6 +1058,8 @@ func (c *Container) initAndStart(ctx context.Context) (err error) {
 	// If we are ContainerStateUnknown, throw an error
 	if c.state.State == define.ContainerStateUnknown {
 		return errors.Wrapf(define.ErrCtrStateInvalid, "container %s is in an unknown state", c.ID())
+	} else if c.state.State == define.ContainerStateRemoving {
+		return errors.Wrapf(define.ErrCtrStateInvalid, "cannot start container %s as it is being removed", c.ID())
 	}
 
 	// If we are running, do nothing
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index f051f40e9..586de0776 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -21,6 +21,7 @@ import (
 	"github.com/containernetworking/plugins/pkg/ns"
 	"github.com/containers/buildah/pkg/secrets"
 	"github.com/containers/libpod/libpod/define"
+	"github.com/containers/libpod/libpod/events"
 	"github.com/containers/libpod/pkg/annotations"
 	"github.com/containers/libpod/pkg/apparmor"
 	"github.com/containers/libpod/pkg/cgroups"
@@ -699,6 +700,8 @@ func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointO
 		return err
 	}
 
+	defer c.newContainerEvent(events.Checkpoint)
+
 	if options.TargetFile != "" {
 		if err = c.exportCheckpoint(options.TargetFile, options.IgnoreRootfs); err != nil {
 			return err
@@ -770,7 +773,7 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti
 		return err
 	}
 
-	if (c.state.State != define.ContainerStateConfigured) && (c.state.State != define.ContainerStateExited) {
+	if !c.ensureState(define.ContainerStateConfigured, define.ContainerStateExited) {
 		return errors.Wrapf(define.ErrCtrStateInvalid, "container %s is running or paused, cannot restore", c.ID())
 	}
 
diff --git a/libpod/define/containerstate.go b/libpod/define/containerstate.go
index ab2527b3e..e7d258e21 100644
--- a/libpod/define/containerstate.go
+++ b/libpod/define/containerstate.go
@@ -25,6 +25,9 @@ const (
 	// ContainerStateExited indicates the the container has stopped and been
 	// cleaned up
 	ContainerStateExited ContainerStatus = iota
+	// ContainerStateRemoving indicates the container is in the process of
+	// being removed.
+	ContainerStateRemoving ContainerStatus = iota
 )
 
 // ContainerStatus returns a string representation for users
@@ -45,6 +48,8 @@ func (t ContainerStatus) String() string {
 		return "paused"
 	case ContainerStateExited:
 		return "exited"
+	case ContainerStateRemoving:
+		return "removing"
 	}
 	return "bad state"
 }
@@ -67,6 +72,8 @@ func StringToContainerStatus(status string) (ContainerStatus, error) {
 		return ContainerStatePaused, nil
 	case ContainerStateExited.String():
 		return ContainerStateExited, nil
+	case ContainerStateRemoving.String():
+		return ContainerStateRemoving, nil
 	default:
 		return ContainerStateUnknown, errors.Wrapf(ErrInvalidArg, "unknown container state: %s", status)
 	}
diff --git a/libpod/options.go b/libpod/options.go
index f7f14eb26..a9b775dc3 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -768,16 +768,8 @@ func WithIPCNSFrom(nsCtr *Container) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if !nsCtr.valid {
-			return define.ErrCtrRemoved
-		}
-
-		if nsCtr.ID() == ctr.ID() {
-			return errors.Wrapf(define.ErrInvalidArg, "must specify another container")
-		}
-
-		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
-			return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		if err := checkDependencyContainer(nsCtr, ctr); err != nil {
+			return err
 		}
 
 		ctr.config.IPCNsCtr = nsCtr.ID()
@@ -796,16 +788,8 @@ func WithMountNSFrom(nsCtr *Container) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if !nsCtr.valid {
-			return define.ErrCtrRemoved
-		}
-
-		if nsCtr.ID() == ctr.ID() {
-			return errors.Wrapf(define.ErrInvalidArg, "must specify another container")
-		}
-
-		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
-			return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		if err := checkDependencyContainer(nsCtr, ctr); err != nil {
+			return err
 		}
 
 		ctr.config.MountNsCtr = nsCtr.ID()
@@ -824,22 +808,14 @@ func WithNetNSFrom(nsCtr *Container) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if !nsCtr.valid {
-			return define.ErrCtrRemoved
-		}
-
-		if nsCtr.ID() == ctr.ID() {
-			return errors.Wrapf(define.ErrInvalidArg, "must specify another container")
+		if err := checkDependencyContainer(nsCtr, ctr); err != nil {
+			return err
 		}
 
 		if ctr.config.CreateNetNS {
 			return errors.Wrapf(define.ErrInvalidArg, "cannot join another container's net ns as we are making a new net ns")
 		}
 
-		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
-			return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
-		}
-
 		ctr.config.NetNsCtr = nsCtr.ID()
 
 		return nil
@@ -856,16 +832,8 @@ func WithPIDNSFrom(nsCtr *Container) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if !nsCtr.valid {
-			return define.ErrCtrRemoved
-		}
-
-		if nsCtr.ID() == ctr.ID() {
-			return errors.Wrapf(define.ErrInvalidArg, "must specify another container")
-		}
-
-		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
-			return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		if err := checkDependencyContainer(nsCtr, ctr); err != nil {
+			return err
 		}
 
 		if ctr.config.NoCgroups {
@@ -888,16 +856,8 @@ func WithUserNSFrom(nsCtr *Container) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if !nsCtr.valid {
-			return define.ErrCtrRemoved
-		}
-
-		if nsCtr.ID() == ctr.ID() {
-			return errors.Wrapf(define.ErrInvalidArg, "must specify another container")
-		}
-
-		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
-			return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		if err := checkDependencyContainer(nsCtr, ctr); err != nil {
+			return err
 		}
 
 		ctr.config.UserNsCtr = nsCtr.ID()
@@ -917,16 +877,8 @@ func WithUTSNSFrom(nsCtr *Container) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if !nsCtr.valid {
-			return define.ErrCtrRemoved
-		}
-
-		if nsCtr.ID() == ctr.ID() {
-			return errors.Wrapf(define.ErrInvalidArg, "must specify another container")
-		}
-
-		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
-			return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		if err := checkDependencyContainer(nsCtr, ctr); err != nil {
+			return err
 		}
 
 		ctr.config.UTSNsCtr = nsCtr.ID()
@@ -945,16 +897,8 @@ func WithCgroupNSFrom(nsCtr *Container) CtrCreateOption {
 			return define.ErrCtrFinalized
 		}
 
-		if !nsCtr.valid {
-			return define.ErrCtrRemoved
-		}
-
-		if nsCtr.ID() == ctr.ID() {
-			return errors.Wrapf(define.ErrInvalidArg, "must specify another container")
-		}
-
-		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
-			return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		if err := checkDependencyContainer(nsCtr, ctr); err != nil {
+			return err
 		}
 
 		ctr.config.CgroupNsCtr = nsCtr.ID()
diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index 7069d3494..ae401013c 100644
--- a/libpod/runtime_ctr.go
+++ b/libpod/runtime_ctr.go
@@ -489,32 +489,19 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool,
 		}
 	}
 
-	var cleanupErr error
-	// Remove the container from the state
-	if c.config.Pod != "" {
-		// If we're removing the pod, the container will be evicted
-		// from the state elsewhere
-		if !removePod {
-			if err := r.state.RemoveContainerFromPod(pod, c); err != nil {
-				cleanupErr = err
-			}
-		}
-	} else {
-		if err := r.state.RemoveContainer(c); err != nil {
-			cleanupErr = err
-		}
+	// Set ContainerStateRemoving and remove exec sessions
+	c.state.State = define.ContainerStateRemoving
+	c.state.ExecSessions = nil
+
+	if err := c.save(); err != nil {
+		return errors.Wrapf(err, "unable to set container %s removing state in database", c.ID())
 	}
 
-	// Set container as invalid so it can no longer be used
-	c.valid = false
+	var cleanupErr error
 
 	// Clean up network namespace, cgroups, mounts
 	if err := c.cleanup(ctx); err != nil {
-		if cleanupErr == nil {
-			cleanupErr = errors.Wrapf(err, "error cleaning up container %s", c.ID())
-		} else {
-			logrus.Errorf("cleanup network, cgroups, mounts: %v", err)
-		}
+		cleanupErr = errors.Wrapf(err, "error cleaning up container %s", c.ID())
 	}
 
 	// Stop the container's storage
@@ -540,6 +527,29 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool,
 		}
 	}
 
+	// Remove the container from the state
+	if c.config.Pod != "" {
+		// If we're removing the pod, the container will be evicted
+		// from the state elsewhere
+		if !removePod {
+			if err := r.state.RemoveContainerFromPod(pod, c); err != nil {
+				if cleanupErr == nil {
+					cleanupErr = err
+				} else {
+					logrus.Errorf("Error removing container %s from database: %v", c.ID(), err)
+				}
+			}
+		}
+	} else {
+		if err := r.state.RemoveContainer(c); err != nil {
+			if cleanupErr == nil {
+				cleanupErr = err
+			} else {
+				logrus.Errorf("Error removing container %s from database: %v", c.ID(), err)
+			}
+		}
+	}
+
 	// Deallocate the container's lock
 	if err := c.lock.Free(); err != nil {
 		if cleanupErr == nil {
@@ -549,6 +559,9 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool,
 		}
 	}
 
+	// Set container as invalid so it can no longer be used
+	c.valid = false
+
 	c.newContainerEvent(events.Remove)
 
 	if !removeVolume {
diff --git a/libpod/util.go b/libpod/util.go
index bae2f4eb8..30e5cd4c3 100644
--- a/libpod/util.go
+++ b/libpod/util.go
@@ -206,3 +206,28 @@ func DefaultSeccompPath() (string, error) {
 	}
 	return config.SeccompDefaultPath, nil
 }
+
+// CheckDependencyContainer verifies the given container can be used as a
+// dependency of another container.
+// Both the dependency to check and the container that will be using the
+// dependency must be passed in.
+// It is assumed that ctr is locked, and depCtr is unlocked.
+func checkDependencyContainer(depCtr, ctr *Container) error {
+	state, err := depCtr.State()
+	if err != nil {
+		return errors.Wrapf(err, "error accessing dependency container %s state", depCtr.ID())
+	}
+	if state == define.ContainerStateRemoving {
+		return errors.Wrapf(define.ErrCtrStateInvalid, "cannot use container %s as a dependency as it is being removed", depCtr.ID())
+	}
+
+	if depCtr.ID() == ctr.ID() {
+		return errors.Wrapf(define.ErrInvalidArg, "must specify another container")
+	}
+
+	if ctr.config.Pod != "" && depCtr.PodID() != ctr.config.Pod {
+		return errors.Wrapf(define.ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, depCtr.ID())
+	}
+
+	return nil
+}