Skip to content

Commit

Permalink
Volumes: Only remove from DB if plugin removal succeeds
Browse files Browse the repository at this point in the history
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 <mheon@redhat.com>
  • Loading branch information
mheon committed Aug 20, 2021
1 parent 89818f7 commit f0d0c48
Showing 1 changed file with 9 additions and 6 deletions.
15 changes: 9 additions & 6 deletions libpod/runtime_volume_linux.go
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit f0d0c48

Please sign in to comment.