Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure volumes can be removed when they fail to unmount #4256

Merged
merged 1 commit into from
Oct 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions libpod/define/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ var (
// CGroup.
ErrNoCgroups = errors.New("this container does not have a cgroup")

// ErrRootless indicates that the given command cannot but run without
// root.
ErrRootless = errors.New("operation requires root privileges")

// ErrRuntimeStopped indicates that the runtime has already been shut
// down and no further operations can be performed on it
ErrRuntimeStopped = errors.New("runtime has already been stopped")
Expand Down
9 changes: 8 additions & 1 deletion libpod/runtime_volume_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,14 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool) error

// If the volume is still mounted - force unmount it
if err := v.unmount(true); err != nil {
return errors.Wrapf(err, "error unmounting volume %s", v.Name())
if force {
// If force is set, evict the volume, even if errors
// occur. Otherwise we'll never be able to get rid of
// them.
logrus.Errorf("Error unmounting volume %s: %v", v.Name(), err)
} else {
return errors.Wrapf(err, "error unmounting volume %s", v.Name())
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at line 174 we do RemoveVolume. Should we return with an error there only if the --force option isn't in play too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're safe to error there - if removing the volume from the DB fails it's always an error

}

// Set volume as invalid so it can no longer be used
Expand Down
25 changes: 25 additions & 0 deletions libpod/volume_internal_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"io/ioutil"
"os/exec"

"github.com/containers/libpod/libpod/define"
"github.com/containers/libpod/pkg/rootless"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"golang.org/x/sys/unix"
Expand All @@ -24,6 +26,11 @@ func (v *Volume) mount() error {
return nil
}

// We cannot mount volumes as rootless.
if rootless.IsRootless() {
return errors.Wrapf(define.ErrRootless, "cannot mount volumes without root privileges")
}

// Update the volume from the DB to get an accurate mount counter.
if err := v.update(); err != nil {
return err
Expand Down Expand Up @@ -108,6 +115,20 @@ func (v *Volume) unmount(force bool) error {
return nil
}

// We cannot unmount volumes as rootless.
if rootless.IsRootless() {
// If force is set, just clear the counter and bail without
// error, so we can remove volumes from the state if they are in
// an awkward configuration.
if force {
logrus.Errorf("Volume %s is mounted despite being rootless - state is not sane", v.Name())
v.state.MountCount = 0
return v.save()
}

return errors.Wrapf(define.ErrRootless, "cannot mount or unmount volumes without root privileges")
}

if !force {
v.state.MountCount = v.state.MountCount - 1
} else {
Expand All @@ -119,6 +140,10 @@ func (v *Volume) unmount(force bool) error {
if v.state.MountCount == 0 {
// Unmount the volume
if err := unix.Unmount(v.config.MountPoint, unix.MNT_DETACH); err != nil {
if err == unix.EINVAL {
// Ignore EINVAL - the mount no longer exists.
return nil
}
return errors.Wrapf(err, "error unmounting volume %s", v.Name())
}
logrus.Debugf("Unmounted volume %s", v.Name())
Expand Down