From f0d0c48d2e38a6a34e687ef8c75caef30790bcdd Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 18 Aug 2021 14:19:11 -0400 Subject: [PATCH] Volumes: Only remove from DB if plugin removal succeeds Originally, Podman would unconditionally remove volumes from the DB, even if they failed to be removed from the volume plugin; this was a safety measure to ensure that `volume rm` can always remove a volume from the database, even if the plugin is misbehaving. However, this is a significant deivation from Docker, which refuses to remove if the plugin errors. These errors can be legitimate configuration issues which the user should address before the volume is removed, so Podman should also use this behaviour. Fixes #11214 Signed-off-by: Matthew Heon --- libpod/runtime_volume_linux.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go index 3d5bc8bb29..f489fbbb50 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_linux.go @@ -234,11 +234,6 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool) error // Set volume as invalid so it can no longer be used v.valid = false - // Remove the volume from the state - if err := r.state.RemoveVolume(v); err != nil { - return errors.Wrapf(err, "error removing volume %s", v.Name()) - } - var removalErr error // If we use a volume plugin, we need to remove from the plugin. @@ -266,11 +261,19 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool) error req := new(pluginapi.RemoveRequest) req.Name = v.Name() if err := v.plugin.RemoveVolume(req); err != nil { - removalErr = errors.Wrapf(err, "volume %s could not be removed from plugin %s, but it has been removed from Podman", v.Name(), v.Driver()) + return errors.Wrapf(err, "volume %s could not be removed from plugin %s", v.Name(), v.Driver()) } } } + // Remove the volume from the state + if err := r.state.RemoveVolume(v); err != nil { + if removalErr != nil { + logrus.Errorf("Error removing volume %s from plugin %s: %v", v.Name(), v.Driver(), removalErr) + } + return errors.Wrapf(err, "error removing volume %s", v.Name()) + } + // Free the volume's lock if err := v.lock.Free(); err != nil { if removalErr == nil {