Remove volumes after containers in pod remove

When trying to reproduce #4704 I noticed that the named volumes
from the Postgres containers in the reproducer weren't being
removed by `podman pod rm -f` saying that the container they were
attached to was still in use. This was rather odd, considering
they were only in use by one container, and that container was in
the process of being removed with the pod.

After a bit of tracing, I realized that the cause is the ordering
of container removal when we remove a pod. Normally, it's done
in removeContainer() before volume removal (which is the last
thing in that function). However, when we are removing a pod, we
remove containers all at once, after removeContainer has already
finished - meaning the container still exists when we try to
remove its volumes, and thus the volume can't be removed.

Solution: collect a list of all named volumes in use by the pod,
and remove them all at once after every container in the pod is
gone. This ensures that there are no dependency issues.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

2 files changed, 29 insertions, 3 deletions

diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index ae401013c..d272e4549 100644
--- a/libpod/runtime_ctr.go
+++ b/libpod/runtime_ctr.go
@@ -573,7 +573,7 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool,
 			if !volume.IsCtrSpecific() {
 				continue
 			}
-			if err := runtime.removeVolume(ctx, volume, false); err != nil && err != define.ErrNoSuchVolume && err != define.ErrVolumeBeingUsed {
+			if err := runtime.removeVolume(ctx, volume, false); err != nil && errors.Cause(err) != define.ErrNoSuchVolume {
 				logrus.Errorf("cleanup volume (%s): %v", v, err)
 			}
 		}
diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go
index 704aaf9d0..563d9728a 100644
--- a/libpod/runtime_pod_linux.go
+++ b/libpod/runtime_pod_linux.go
@@ -225,11 +225,20 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool)
 		}
 	}
 
+	ctrNamedVolumes := make(map[string]*ContainerNamedVolume)
+
 	// Second loop - all containers are good, so we should be clear to
 	// remove.
 	for _, ctr := range ctrs {
-		// Remove the container
-		if err := r.removeContainer(ctx, ctr, force, true, true); err != nil {
+		// Remove the container.
+		// Do NOT remove named volumes. Instead, we're going to build a
+		// list of them to be removed at the end, once the containers
+		// have been removed by RemovePodContainers.
+		for _, vol := range ctr.config.NamedVolumes {
+			ctrNamedVolumes[vol.Name] = vol
+		}
+
+		if err := r.removeContainer(ctx, ctr, force, false, true); err != nil {
 			if removalErr != nil {
 				removalErr = err
 			} else {
@@ -246,6 +255,23 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool)
 		return err
 	}
 
+	for volName := range ctrNamedVolumes {
+		volume, err := r.state.Volume(volName)
+		if err != nil && errors.Cause(err) != define.ErrNoSuchVolume {
+			logrus.Errorf("Error retrieving volume %s: %v", volName, err)
+			continue
+		}
+		if !volume.IsCtrSpecific() {
+			continue
+		}
+		if err := r.removeVolume(ctx, volume, false); err != nil {
+			if errors.Cause(err) == define.ErrNoSuchVolume || errors.Cause(err) == define.ErrVolumeRemoved {
+				continue
+			}
+			logrus.Errorf("Error removing volume %s: %v", volName, err)
+		}
+	}
+
 	// Remove pod cgroup, if present
 	if p.state.CgroupPath != "" {
 		logrus.Debugf("Removing pod cgroup %s", p.state.CgroupPath)