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>
This commit is contained in:
Matthew Heon
2019-12-17 21:41:31 -05:00
parent b2f05e0e84
commit 88917e4a93
2 changed files with 29 additions and 3 deletions

View File

@ -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)
}
}

View File

@ -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)