Merge pull request #6229 from mheon/small_kata_fix

Cleanup OCI runtime before storage

2 files changed, 23 insertions, 15 deletions

diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 3fcf687ec..5baa5fc1c 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -1562,21 +1562,24 @@ func (c *Container) cleanup(ctx context.Context) error {
 		lastError = errors.Wrapf(err, "error removing container %s network", c.ID())
 	}
 
-	// Unmount storage
-	if err := c.cleanupStorage(); err != nil {
+	// Remove the container from the runtime, if necessary.
+	// Do this *before* unmounting storage - some runtimes (e.g. Kata)
+	// apparently object to having storage removed while the container still
+	// exists.
+	if err := c.cleanupRuntime(ctx); err != nil {
 		if lastError != nil {
-			logrus.Errorf("Error unmounting container %s storage: %v", c.ID(), err)
+			logrus.Errorf("Error removing container %s from OCI runtime: %v", c.ID(), err)
 		} else {
-			lastError = errors.Wrapf(err, "error unmounting container %s storage", c.ID())
+			lastError = err
 		}
 	}
 
-	// Remove the container from the runtime, if necessary
-	if err := c.cleanupRuntime(ctx); err != nil {
+	// Unmount storage
+	if err := c.cleanupStorage(); err != nil {
 		if lastError != nil {
-			logrus.Errorf("Error removing container %s from OCI runtime: %v", c.ID(), err)
+			logrus.Errorf("Error unmounting container %s storage: %v", c.ID(), err)
 		} else {
-			lastError = err
+			lastError = errors.Wrapf(err, "error unmounting container %s storage", c.ID())
 		}
 	}
 
diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index 1d880531e..c670822a0 100644
--- a/libpod/runtime_ctr.go
+++ b/libpod/runtime_ctr.go
@@ -488,20 +488,25 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool,
 		}
 	}
 
+	var cleanupErr error
+
+	// Clean up network namespace, cgroups, mounts.
+	// Do this before we set ContainerStateRemoving, to ensure that we can
+	// actually remove from the OCI runtime.
+	if err := c.cleanup(ctx); err != nil {
+		cleanupErr = errors.Wrapf(err, "error cleaning up container %s", c.ID())
+	}
+
 	// Set ContainerStateRemoving
 	c.state.State = define.ContainerStateRemoving
 
 	if err := c.save(); err != nil {
+		if cleanupErr != nil {
+			logrus.Errorf(err.Error())
+		}
 		return errors.Wrapf(err, "unable to set container %s removing state in database", c.ID())
 	}
 
-	var cleanupErr error
-
-	// Clean up network namespace, cgroups, mounts
-	if err := c.cleanup(ctx); err != nil {
-		cleanupErr = errors.Wrapf(err, "error cleaning up container %s", c.ID())
-	}
-
 	// Stop the container's storage
 	if err := c.teardownStorage(); err != nil {
 		if cleanupErr == nil {