Skip to content

Commit

Permalink
Support volume bind mounts for rootless containers
Browse files Browse the repository at this point in the history 
Fix handling of "bind" volumes to actually work.
Allow bind 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: containers#12013

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
  • Loading branch information
@rhatdan
rhatdan committed Dec 22, 2021
1 parent 2aea0a5 commit 71edc1e
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 24 deletions.
2 changes: 1 addition & 1 deletion libpod/container_internal.go
View file
Expand Up @@ -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() {
Expand Down
45 changes: 28 additions & 17 deletions libpod/runtime_volume_linux.go
View file
Expand Up @@ -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")
}
}

Expand All @@ -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)
Expand All @@ -67,9 +67,20 @@ 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 {
for key, val := range volume.config.Options {
switch key {
case "device", "o", "type", "UID", "GID", "SIZE", "INODES":
case "device":
switch volume.config.Options["type"] {
case "bind":
if _, err := os.Stat(val); err != nil {
return nil, errors.Wrapf(err, "invalid volume option %s for driver 'local'", key)
}
case "tmpfs":
if val != "tmpfs" {
return nil, errors.Wrapf(define.ErrInvalidArg, "invalid tmpfs device must be tmpfs: %q", val)
}
}
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)
Expand All @@ -92,17 +103,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
Expand Down Expand Up @@ -132,7 +143,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()
Expand All @@ -149,7 +160,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
Expand Down Expand Up @@ -181,7 +192,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)
}
}

Expand Down Expand Up @@ -225,13 +236,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())
}
}
}
Expand All @@ -244,7 +255,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())
}
}

Expand Down Expand Up @@ -288,13 +299,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)
}
Expand All @@ -304,7 +315,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)
}
Expand Down
12 changes: 12 additions & 0 deletions libpod/volume.go
View file
Expand Up @@ -249,6 +249,18 @@ func (v *Volume) IsDangling() (bool, error) {
return len(ctrs) == 0, nil
}

var rootlessVolume = map[string]bool{
"": true,
"local": true,
"bind": true,
"tmpfs": true,
}

// RequiresRoot determines whether the volume driver will work in rootless mode
func (v *Volume) RequiresRoot() bool {
return !rootlessVolume[v.config.Driver]
}

// UsesVolumeDriver determines whether the volume uses a volume driver. Volume
// drivers are pluggable backends for volumes that will manage the storage and
// mounting.
Expand Down
2 changes: 1 addition & 1 deletion libpod/volume_internal.go
View file
Expand Up @@ -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

Expand Down
15 changes: 10 additions & 5 deletions libpod/volume_internal_linux.go
View file
Expand Up @@ -33,7 +33,7 @@ func (v *Volume) mount() error {
}

// We cannot mount 'local' volumes as rootless.
if !v.UsesVolumeDriver() && rootless.IsRootless() {
if rootless.IsRootless() && v.RequiresRoot() {
// 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")
Expand Down Expand Up @@ -90,22 +90,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())
Expand Down Expand Up @@ -184,7 +189,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())
}
Expand Down
27 changes: 27 additions & 0 deletions test/system/160-volumes.bats
View file
Expand Up @@ -319,5 +319,32 @@ 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 125 volume create -o type=tmpfs -o device=/bogus $myvolume
is "$output" "Error: invalid tmpfs device must be tmpfs: \"/bogus\": invalid argument" "should fail with bogus directory not existing"
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

0 comments on commit 71edc1e

Please sign in to comment.