From 2e0d3e9ea45f9ebb77ffe1f9022b46f0e429fb5e Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 22 Dec 2021 15:07:37 -0500 Subject: Support all volume mounts for rootless containers Fix handling of "bind" and "tmpfs" olumes to actually work. Allow bind, tmpfs local volumes to work in rootless mode. Also removed the string "error" from all error messages that begine with it. All Podman commands are printed with Error:, so this causes an ugly stutter. Fixes: https://github.com/containers/podman/issues/12013 Signed-off-by: Daniel J Walsh --- libpod/container_internal.go | 2 +- libpod/runtime_volume_linux.go | 42 +++++++++++++++++++++++------------------ libpod/volume_internal.go | 2 +- libpod/volume_internal_linux.go | 35 +++++++++------------------------- test/system/160-volumes.bats | 25 ++++++++++++++++++++++++ test/system/helpers.bash | 2 +- 6 files changed, 61 insertions(+), 47 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 7ae9daefa..2d12a90d1 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -762,7 +762,7 @@ func (c *Container) export(path string) error { if !c.state.Mounted { containerMount, err := c.runtime.store.Mount(c.ID(), c.config.MountLabel) if err != nil { - return errors.Wrapf(err, "error mounting container %q", c.ID()) + return errors.Wrapf(err, "mounting container %q", c.ID()) } mountPoint = containerMount defer func() { diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go index ed3cc971c..d4d9a4438 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_linux.go @@ -35,7 +35,7 @@ func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption) volume := newVolume(r) for _, option := range options { if err := option(volume); err != nil { - return nil, errors.Wrapf(err, "error running volume create option") + return nil, errors.Wrapf(err, "running volume create option") } } @@ -50,7 +50,7 @@ func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption) // Check if volume with given name exists. exists, err := r.state.HasVolume(volume.config.Name) if err != nil { - return nil, errors.Wrapf(err, "error checking if volume with name %s exists", volume.config.Name) + return nil, errors.Wrapf(err, "checking if volume with name %s exists", volume.config.Name) } if exists { return nil, errors.Wrapf(define.ErrVolumeExists, "volume with name %s already exists", volume.config.Name) @@ -67,9 +67,15 @@ func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption) if volume.config.Driver == define.VolumeDriverLocal { logrus.Debugf("Validating options for local driver") // Validate options - for key := range volume.config.Options { - switch key { - case "device", "o", "type", "UID", "GID", "SIZE", "INODES": + for key, val := range volume.config.Options { + switch strings.ToLower(key) { + case "device": + if strings.ToLower(volume.config.Options["type"]) == "bind" { + if _, err := os.Stat(val); err != nil { + return nil, errors.Wrapf(err, "invalid volume option %s for driver 'local'", key) + } + } + case "o", "type", "uid", "gid", "size", "inodes": // Do nothing, valid keys default: return nil, errors.Wrapf(define.ErrInvalidArg, "invalid mount option %s for driver 'local'", key) @@ -92,17 +98,17 @@ func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption) // Create the mountpoint of this volume volPathRoot := filepath.Join(r.config.Engine.VolumePath, volume.config.Name) if err := os.MkdirAll(volPathRoot, 0700); err != nil { - return nil, errors.Wrapf(err, "error creating volume directory %q", volPathRoot) + return nil, errors.Wrapf(err, "creating volume directory %q", volPathRoot) } if err := os.Chown(volPathRoot, volume.config.UID, volume.config.GID); err != nil { - return nil, errors.Wrapf(err, "error chowning volume directory %q to %d:%d", volPathRoot, volume.config.UID, volume.config.GID) + return nil, errors.Wrapf(err, "chowning volume directory %q to %d:%d", volPathRoot, volume.config.UID, volume.config.GID) } fullVolPath := filepath.Join(volPathRoot, "_data") if err := os.MkdirAll(fullVolPath, 0755); err != nil { - return nil, errors.Wrapf(err, "error creating volume directory %q", fullVolPath) + return nil, errors.Wrapf(err, "creating volume directory %q", fullVolPath) } if err := os.Chown(fullVolPath, volume.config.UID, volume.config.GID); err != nil { - return nil, errors.Wrapf(err, "error chowning volume directory %q to %d:%d", fullVolPath, volume.config.UID, volume.config.GID) + return nil, errors.Wrapf(err, "chowning volume directory %q to %d:%d", fullVolPath, volume.config.UID, volume.config.GID) } if err := LabelVolumePath(fullVolPath); err != nil { return nil, err @@ -132,7 +138,7 @@ func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption) lock, err := r.lockManager.AllocateLock() if err != nil { - return nil, errors.Wrapf(err, "error allocating lock for new volume") + return nil, errors.Wrapf(err, "allocating lock for new volume") } volume.lock = lock volume.config.LockID = volume.lock.ID() @@ -149,7 +155,7 @@ func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption) // Add the volume to state if err := r.state.AddVolume(volume); err != nil { - return nil, errors.Wrapf(err, "error adding volume to state") + return nil, errors.Wrapf(err, "adding volume to state") } defer volume.newVolumeEvent(events.Create) return volume, nil @@ -181,7 +187,7 @@ func makeVolumeInPluginIfNotExist(name string, options map[string]string, plugin createReq.Name = name createReq.Options = options if err := plugin.CreateVolume(createReq); err != nil { - return errors.Wrapf(err, "error creating volume %q in plugin %s", name, plugin.Name) + return errors.Wrapf(err, "creating volume %q in plugin %s", name, plugin.Name) } } @@ -225,13 +231,13 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool, timeo continue } - return errors.Wrapf(err, "error removing container %s that depends on volume %s", dep, v.Name()) + return errors.Wrapf(err, "removing container %s that depends on volume %s", dep, v.Name()) } logrus.Debugf("Removing container %s (depends on volume %q)", ctr.ID(), v.Name()) if err := r.removeContainer(ctx, ctr, force, false, false, timeout); err != nil { - return errors.Wrapf(err, "error removing container %s that depends on volume %s", ctr.ID(), v.Name()) + return errors.Wrapf(err, "removing container %s that depends on volume %s", ctr.ID(), v.Name()) } } } @@ -244,7 +250,7 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool, timeo // them. logrus.Errorf("Unmounting volume %s: %v", v.Name(), err) } else { - return errors.Wrapf(err, "error unmounting volume %s", v.Name()) + return errors.Wrapf(err, "unmounting volume %s", v.Name()) } } @@ -288,13 +294,13 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool, timeo if removalErr != nil { logrus.Errorf("Removing volume %s from plugin %s: %v", v.Name(), v.Driver(), removalErr) } - return errors.Wrapf(err, "error removing volume %s", v.Name()) + return errors.Wrapf(err, "removing volume %s", v.Name()) } // Free the volume's lock if err := v.lock.Free(); err != nil { if removalErr == nil { - removalErr = errors.Wrapf(err, "error freeing lock for volume %s", v.Name()) + removalErr = errors.Wrapf(err, "freeing lock for volume %s", v.Name()) } else { logrus.Errorf("Freeing lock for volume %q: %v", v.Name(), err) } @@ -304,7 +310,7 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool, timeo // from /var/lib/containers/storage/volumes if err := v.teardownStorage(); err != nil { if removalErr == nil { - removalErr = errors.Wrapf(err, "error cleaning up volume storage for %q", v.Name()) + removalErr = errors.Wrapf(err, "cleaning up volume storage for %q", v.Name()) } else { logrus.Errorf("Cleaning up volume storage for volume %q: %v", v.Name(), err) } diff --git a/libpod/volume_internal.go b/libpod/volume_internal.go index f69f1c044..f9e1ea87d 100644 --- a/libpod/volume_internal.go +++ b/libpod/volume_internal.go @@ -81,7 +81,7 @@ func (v *Volume) save() error { func (v *Volume) refresh() error { lock, err := v.runtime.lockManager.AllocateAndRetrieveLock(v.config.LockID) if err != nil { - return errors.Wrapf(err, "error acquiring lock %d for volume %s", v.config.LockID, v.Name()) + return errors.Wrapf(err, "acquiring lock %d for volume %s", v.config.LockID, v.Name()) } v.lock = lock diff --git a/libpod/volume_internal_linux.go b/libpod/volume_internal_linux.go index 45cd22385..abd31df0f 100644 --- a/libpod/volume_internal_linux.go +++ b/libpod/volume_internal_linux.go @@ -7,7 +7,6 @@ import ( "strings" "github.com/containers/podman/v3/libpod/define" - "github.com/containers/podman/v3/pkg/rootless" pluginapi "github.com/docker/go-plugins-helpers/volume" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -32,13 +31,6 @@ func (v *Volume) mount() error { return nil } - // We cannot mount 'local' volumes as rootless. - if !v.UsesVolumeDriver() && rootless.IsRootless() { - // This check should only be applied to 'local' driver - // so Volume Drivers must be excluded - 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 @@ -90,22 +82,27 @@ func (v *Volume) mount() error { // TODO: might want to cache this path in the runtime? mountPath, err := exec.LookPath("mount") if err != nil { - return errors.Wrapf(err, "error locating 'mount' binary") + return errors.Wrapf(err, "locating 'mount' binary") } mountArgs := []string{} if volOptions != "" { mountArgs = append(mountArgs, "-o", volOptions) } - if volType != "" { + switch volType { + case "": + case "bind": + mountArgs = append(mountArgs, "-o", volType) + default: mountArgs = append(mountArgs, "-t", volType) } + mountArgs = append(mountArgs, volDevice, v.config.MountPoint) mountCmd := exec.Command(mountPath, mountArgs...) logrus.Debugf("Running mount command: %s %s", mountPath, strings.Join(mountArgs, " ")) if output, err := mountCmd.CombinedOutput(); err != nil { logrus.Debugf("Mount %v failed with %v", mountCmd, err) - return errors.Wrapf(errors.Errorf(string(output)), "error mounting volume %s", v.Name()) + return errors.Errorf(string(output)) } logrus.Debugf("Mounted volume %s", v.Name()) @@ -139,20 +136,6 @@ func (v *Volume) unmount(force bool) error { return nil } - // We cannot unmount 'local' volumes as rootless. - if !v.UsesVolumeDriver() && 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-- } else { @@ -184,7 +167,7 @@ func (v *Volume) unmount(force bool) error { // Ignore EINVAL - the mount no longer exists. return nil } - return errors.Wrapf(err, "error unmounting volume %s", v.Name()) + return errors.Wrapf(err, "unmounting volume %s", v.Name()) } logrus.Debugf("Unmounted volume %s", v.Name()) } diff --git a/test/system/160-volumes.bats b/test/system/160-volumes.bats index 43462de36..1271b7c0b 100644 --- a/test/system/160-volumes.bats +++ b/test/system/160-volumes.bats @@ -319,5 +319,30 @@ EOF is "$output" "" "no more volumes to prune" } +@test "podman volume type=bind" { + myvoldir=${PODMAN_TMPDIR}/volume_$(random_string) + mkdir $myvoldir + touch $myvoldir/myfile + + myvolume=myvol$(random_string) + run_podman 125 volume create -o type=bind -o device=/bogus $myvolume + is "$output" "Error: invalid volume option device for driver 'local': stat /bogus: no such file or directory" "should fail with bogus directory not existing" + + run_podman volume create -o type=bind -o device=/$myvoldir $myvolume + is "$output" "$myvolume" "should successfully create myvolume" + + run_podman run --rm -v $myvolume:/vol:z $IMAGE \ + stat -c "%u:%s" /vol/myfile + is "$output" "0:0" "w/o keep-id: stat(file in container) == root" +} + +@test "podman volume type=tmpfs" { + myvolume=myvol$(random_string) + run_podman volume create -o type=tmpfs -o device=tmpfs $myvolume + is "$output" "$myvolume" "should successfully create myvolume" + + run_podman run --rm -v $myvolume:/vol $IMAGE stat -f -c "%T" /vol + is "$output" "tmpfs" "volume should be tmpfs" +} # vim: filetype=sh diff --git a/test/system/helpers.bash b/test/system/helpers.bash index 415c9010e..7be694dbd 100644 --- a/test/system/helpers.bash +++ b/test/system/helpers.bash @@ -63,7 +63,7 @@ function basic_setup() { for line in "${lines[@]}"; do set $line echo "# setup(): removing stray external container $1 ($2)" >&3 - run_podman rm $1 + run_podman rm -f $1 done # Clean up all images except those desired -- cgit v1.2.3-54-g00ecf