Remove container from state before cleaning up.

Attempt to cleanup as much of the container as possible, even if one
of the cleanup stages fails.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

Closes: #895
Approved by: mheon

2 files changed, 40 insertions, 21 deletions

diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 84724e013..f6d8fc32a 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -261,10 +261,6 @@ func (c *Container) setupStorage(ctx context.Context) error {
 
 // Tear down a container's storage prior to removal
 func (c *Container) teardownStorage() error {
-	if !c.valid {
-		return errors.Wrapf(ErrCtrRemoved, "container %s is not valid", c.ID())
-	}
-
 	if c.state.State == ContainerStateRunning || c.state.State == ContainerStatePaused {
 		return errors.Wrapf(ErrCtrStateInvalid, "cannot remove storage for container %s as it is running or paused", c.ID())
 	}
@@ -818,7 +814,10 @@ func (c *Container) cleanupStorage() error {
 	c.state.Mountpoint = ""
 	c.state.Mounted = false
 
-	return c.save()
+	if c.valid {
+		return c.save()
+	}
+	return nil
 }
 
 // Unmount the a container and free its resources
diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index bde9db764..aa49b2d51 100644
--- a/libpod/runtime_ctr.go
+++ b/libpod/runtime_ctr.go
@@ -238,24 +238,41 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool)
 		return errors.Wrapf(ErrCtrExists, "container %s has dependent containers which must be removed before it: %s", c.ID(), depsStr)
 	}
 
-	// Clean up network namespace, cgroups, mounts
-	if err := c.cleanup(); err != nil {
-		return err
-	}
-
-	// Stop the container's storage
-	if err := c.teardownStorage(); err != nil {
-		return err
-	}
-
+	var cleanupErr error
 	// Remove the container from the state
 	if c.config.Pod != "" {
 		if err := r.state.RemoveContainerFromPod(pod, c); err != nil {
-			return err
+			if cleanupErr == nil {
+				cleanupErr = err
+			} else {
+				logrus.Errorf("removing container from pod: %v", err)
+			}
 		}
 	} else {
 		if err := r.state.RemoveContainer(c); err != nil {
-			return err
+			if cleanupErr == nil {
+				cleanupErr = err
+			} else {
+				logrus.Errorf("removing container: %v", err)
+			}
+		}
+	}
+
+	// Clean up network namespace, cgroups, mounts
+	if err := c.cleanup(); err != nil {
+		if cleanupErr == nil {
+			cleanupErr = err
+		} else {
+			logrus.Errorf("cleanup network, cgroups, mounts: %v", err)
+		}
+	}
+
+	// Stop the container's storage
+	if err := c.teardownStorage(); err != nil {
+		if cleanupErr == nil {
+			cleanupErr = err
+		} else {
+			logrus.Errorf("cleanup storage: %v", err)
 		}
 	}
 
@@ -263,16 +280,19 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool)
 	// Only do this if we're not ContainerStateConfigured - if we are,
 	// we haven't been created in the runtime yet
 	if c.state.State != ContainerStateConfigured {
-		err = c.delete(ctx)
-		if err != nil {
-			return err
+		if err := c.delete(ctx); err != nil {
+			if cleanupErr == nil {
+				cleanupErr = err
+			} else {
+				logrus.Errorf("delete container: %v", err)
+			}
 		}
 	}
 
 	// Set container as invalid so it can no longer be used
 	c.valid = false
 
-	return nil
+	return cleanupErr
 }
 
 // GetContainer retrieves a container by its ID