Add ability for volumes with options to mount/umount

When volume options and the local volume driver are specified,
the volume is intended to be mounted using the 'mount' command.
Supported options will be used to volume the volume before the
first container using it starts, and unmount the volume after the
last container using it dies.

This should work for any local filesystem, though at present I've
only tested with tmpfs and btrfs.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

docs/podman-volume-create.1.md

@@ -39,6 +39,8 @@ $ podman volume create myvol
 $ podman volume create
 
 $ podman volume create --label foo=bar myvol
+
+$ podman volume create --opt device=tmpfs --opt type=tmpfs --opt o=nodev,noexec myvol
 ```
 
 ## SEE ALSO

libpod/boltdb_state.go

@@ -159,6 +159,16 @@ func (s *BoltState) Refresh() error {
 			return err
 		}
 
+		allVolsBucket, err := getAllVolsBucket(tx)
+		if err != nil {
+			return err
+		}
+
+		volBucket, err := getVolBucket(tx)
+		if err != nil {
+			return err
+		}
+
 		// Iterate through all IDs. Check if they are containers.
 		// If they are, unmarshal their state, and then clear
 		// PID, mountpoint, and state for all of them
@@ -235,6 +245,44 @@ func (s *BoltState) Refresh() error {
 
 			return nil
 		})
+		if err != nil {
+			return err
+		}
+
+		// Now refresh volumes
+		err = allVolsBucket.ForEach(func(id, name []byte) error {
+			dbVol := volBucket.Bucket(id)
+			if dbVol == nil {
+				return errors.Wrapf(define.ErrInternal, "inconsistency in state - volume %s is in all volumes bucket but volume not found", string(id))
+			}
+
+			// Get the state
+			volStateBytes := dbVol.Get(stateKey)
+			if volStateBytes == nil {
+				// If the volume doesn't have a state, nothing to do
+				return nil
+			}
+
+			oldState := new(VolumeState)
+
+			if err := json.Unmarshal(volStateBytes, oldState); err != nil {
+				return errors.Wrapf(err, "error unmarshalling state for volume %s", string(id))
+			}
+
+			// Reset mount count to 0
+			oldState.MountCount = 0
+
+			newState, err := json.Marshal(oldState)
+			if err != nil {
+				return errors.Wrapf(err, "error marshalling state for volume %s", string(id))
+			}
+
+			if err := dbVol.Put(stateKey, newState); err != nil {
+				return errors.Wrapf(err, "error storing new state for volume %s", string(id))
+			}
+
+			return nil
+		})
 		return err
 	})
 	return err
@@ -1630,6 +1678,7 @@ func (s *BoltState) AllVolumes() ([]*Volume, error) {
 
 			volume := new(Volume)
 			volume.config = new(VolumeConfig)
+			volume.state = new(VolumeState)
 
 			if err := s.getVolumeFromDB(id, volume, volBucket); err != nil {
 				if errors.Cause(err) != define.ErrNSMismatch {

libpod/boltdb_state_internal.go

@@ -449,7 +449,7 @@ func (s *BoltState) getVolumeFromDB(name []byte, volume *Volume, volBkt *bolt.Bu
 		return errors.Wrapf(err, "error unmarshalling volume %s config from DB", string(name))
 	}
 
-	// Volume state is allowed to be nil for legacy compatability
+	// Volume state is allowed to be nil for legacy compatibility
 	volStateBytes := volDB.Get(stateKey)
 	if volStateBytes != nil {
 		if err := json.Unmarshal(volStateBytes, volume.state); err != nil {

libpod/container_internal.go

@@ -1205,7 +1205,7 @@ func (c *Container) restartWithTimeout(ctx context.Context, timeout uint) (err e
 // TODO: Add ability to override mount label so we can use this for Mount() too
 // TODO: Can we use this for export? Copying SHM into the export might not be
 // good
-func (c *Container) mountStorage() (string, error) {
+func (c *Container) mountStorage() (_ string, Err error) {
 	var err error
 	// Container already mounted, nothing to do
 	if c.state.Mounted {
@@ -1225,6 +1225,40 @@ func (c *Container) mountStorage() (string, error) {
 		if err := os.Chown(c.config.ShmDir, c.RootUID(), c.RootGID()); err != nil {
 			return "", errors.Wrapf(err, "failed to chown %s", c.config.ShmDir)
 		}
+		defer func() {
+			if Err != nil {
+				if err := c.unmountSHM(c.config.ShmDir); err != nil {
+					logrus.Errorf("Error unmounting SHM for container %s after mount error: %v", c.ID(), err)
+				}
+			}
+		}()
+	}
+
+	// Request a mount of all named volumes
+	for _, v := range c.config.NamedVolumes {
+		vol, err := c.runtime.state.Volume(v.Name)
+		if err != nil {
+			return "", errors.Wrapf(err, "error retrieving named volume %s for container %s", v.Name, c.ID())
+		}
+
+		if vol.needsMount() {
+			vol.lock.Lock()
+			if err := vol.mount(); err != nil {
+				vol.lock.Unlock()
+				return "", errors.Wrapf(err, "error mounting volume %s for container %s", vol.Name(), c.ID())
+			}
+			vol.lock.Unlock()
+			defer func() {
+				if Err == nil {
+					return
+				}
+				vol.lock.Lock()
+				defer vol.lock.Unlock()
+				if err := vol.unmount(false); err != nil {
+					logrus.Errorf("Error unmounting volume %s after error mounting container %s: %v", vol.Name(), c.ID(), err)
+				}
+			}()
+		}
 	}
 
 	// TODO: generalize this mount code so it will mount every mount in ctr.config.Mounts
@@ -1270,13 +1304,46 @@ func (c *Container) cleanupStorage() error {
 		return err
 	}
 
+	var cleanupErr error
+
+	// Request an unmount of all named volumes
+	for _, v := range c.config.NamedVolumes {
+		vol, err := c.runtime.state.Volume(v.Name)
+		if err != nil {
+			if cleanupErr != nil {
+				logrus.Errorf("Error unmounting container %s: %v", c.ID(), cleanupErr)
+			}
+			cleanupErr = errors.Wrapf(err, "error retrieving named volume %s for container %s", v.Name, c.ID())
+
+			// We need to try and unmount every volume, so continue
+			// if they fail.
+			continue
+		}
+
+		if vol.needsMount() {
+			vol.lock.Lock()
+			if err := vol.unmount(false); err != nil {
+				if cleanupErr != nil {
+					logrus.Errorf("Error unmounting container %s: %v", c.ID(), cleanupErr)
+				}
+				cleanupErr = errors.Wrapf(err, "error unmounting volume %s for container %s", vol.Name(), c.ID())
+			}
+			vol.lock.Unlock()
+		}
+	}
+
 	c.state.Mountpoint = ""
 	c.state.Mounted = false
 
 	if c.valid {
-		return c.save()
+		if err := c.save(); err != nil {
+			if cleanupErr != nil {
+				logrus.Errorf("Error unmounting container %s: %v", c.ID(), cleanupErr)
+			}
+			cleanupErr = err
+		}
 	}
-	return nil
+	return cleanupErr
 }
 
 // Unmount the a container and free its resources

libpod/container_internal_linux.go

@@ -61,7 +61,7 @@ func (c *Container) unmountSHM(mount string) error {
 
 // prepare mounts the container and sets up other required resources like net
 // namespaces
-func (c *Container) prepare() (err error) {
+func (c *Container) prepare() (Err error) {
 	var (
 		wg                              sync.WaitGroup
 		netNS                           ns.NetNS
@@ -108,31 +108,42 @@ func (c *Container) prepare() (err error) {
 		logrus.Debugf("Created root filesystem for container %s at %s", c.ID(), c.state.Mountpoint)
 	}()
 
-	defer func() {
-		if err != nil {
-			if err2 := c.cleanupNetwork(); err2 != nil {
-				logrus.Errorf("Error cleaning up container %s network: %v", c.ID(), err2)
-			}
-			if err2 := c.cleanupStorage(); err2 != nil {
-				logrus.Errorf("Error cleaning up container %s storage: %v", c.ID(), err2)
-			}
-		}
-	}()
-
 	wg.Wait()
 
+	var createErr error
 	if createNetNSErr != nil {
-		if mountStorageErr != nil {
+		createErr = createNetNSErr
-			logrus.Error(createNetNSErr)
-			return mountStorageErr
-		}
-		return createNetNSErr
 	}
 	if mountStorageErr != nil {
-		return mountStorageErr
+		logrus.Errorf("Error preparing container %s: %v", c.ID(), createErr)
+		createErr = mountStorageErr
 	}
 
-	// Save the container
+	// Only trigger storage cleanup if mountStorage was successful.
+	// Otherwise, we may mess up mount counters.
+	if createNetNSErr != nil && mountStorageErr == nil {
+		if err := c.cleanupStorage(); err != nil {
+			// createErr is guaranteed non-nil, so print
+			// unconditionally
+			logrus.Errorf("Error preparing container %s: %v", c.ID(), createErr)
+			createErr = errors.Wrapf(err, "error unmounting storage for container %s after network create failure", c.ID())
+		}
+	}
+
+	// It's OK to unconditionally trigger network cleanup. If the network
+	// isn't ready it will do nothing.
+	if createErr != nil {
+		if err := c.cleanupNetwork(); err != nil {
+			logrus.Errorf("Error preparing container %s: %v", c.ID(), createErr)
+			createErr = errors.Wrapf(err, "error cleaning up container %s network after setup failure", c.ID())
+		}
+	}
+
+	if createErr != nil {
+		return createErr
+	}
+
+	// Save changes to container state
 	return c.save()
 }
 

libpod/runtime_volume_linux.go

@@ -48,6 +48,19 @@ func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption)
 	}
 	volume.config.CreatedTime = time.Now()
 
+	if volume.config.Driver == "local" {
+		logrus.Debugf("Validating options for local driver")
+		// Validate options
+		for key := range volume.config.Options {
+			switch key {
+			case "device", "o", "type":
+				// Do nothing, valid keys
+			default:
+				return nil, errors.Wrapf(define.ErrInvalidArg, "invalid mount option %s for driver 'local'", key)
+			}
+		}
+	}
+
 	// Create the mountpoint of this volume
 	volPathRoot := filepath.Join(r.config.VolumePath, volume.config.Name)
 	if err := os.MkdirAll(volPathRoot, 0700); err != nil {
@@ -102,6 +115,11 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool) error
 		return define.ErrVolumeRemoved
 	}
 
+	// Update volume status to pick up a potential removal from state
+	if err := v.update(); err != nil {
+		return err
+	}
+
 	deps, err := r.state.VolumeInUse(v)
 	if err != nil {
 		return err
@@ -137,6 +155,11 @@ 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())
+	}
+
 	// Set volume as invalid so it can no longer be used
 	v.valid = false
 

libpod/volume.go

@@ -38,14 +38,6 @@ type VolumeConfig struct {
 	// a list of mount options. For other drivers, they are passed to the
 	// volume driver handling the volume.
 	Options map[string]string `json:"volumeOptions,omitempty"`
-	// Type is the type of the volume. This is only used with the local
-	// driver. It the the filesystem that we will attempt to mount - nfs,
-	// tmpfs, etc.
-	Type string `json:"type,omitempty"`
-	// Device is the device of the volume. This is only used with the local
-	// driver, and only with some filesystem types (e.g., not required by
-	// tmpfs). It is the device to mount.
-	Device string `json:"device,omitempty"`
 	// Whether this volume was created for a specific container and will be
 	// removed with it.
 	IsCtrSpecific bool `json:"ctrSpecific"`
@@ -101,10 +93,9 @@ func (v *Volume) MountPoint() string {
 // Options return the volume's options
 func (v *Volume) Options() map[string]string {
 	options := make(map[string]string)
-	for key, value := range v.config.Options {
+	for k, v := range v.config.Options {
-		options[key] = value
+		options[k] = v
 	}
-
 	return options
 }
 

libpod/volume_internal.go

@@ -3,6 +3,8 @@ package libpod
 import (
 	"os"
 	"path/filepath"
+
+	"github.com/containers/libpod/libpod/define"
 )
 
 // Creates a new volume
@@ -20,3 +22,25 @@ func newVolume(runtime *Runtime) (*Volume, error) {
 func (v *Volume) teardownStorage() error {
 	return os.RemoveAll(filepath.Join(v.runtime.config.VolumePath, v.Name()))
 }
+
+// Volumes with options set, or a filesystem type, or a device to mount need to
+// be mounted and unmounted.
+func (v *Volume) needsMount() bool {
+	return len(v.config.Options) > 0 && v.config.Driver == "local"
+}
+
+// update() updates the volume state from the DB.
+func (v *Volume) update() error {
+	if err := v.runtime.state.UpdateVolume(v); err != nil {
+		return err
+	}
+	if !v.valid {
+		return define.ErrVolumeRemoved
+	}
+	return nil
+}
+
+// save() saves the volume state to the DB
+func (v *Volume) save() error {
+	return v.runtime.state.SaveVolume(v)
+}

libpod/volume_internal_linux.go

@@ -0,0 +1,128 @@
+// +build linux
+
+package libpod
+
+import (
+	"io/ioutil"
+	"os/exec"
+
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+	"golang.org/x/sys/unix"
+)
+
+// mount mounts the volume if necessary.
+// A mount is necessary if a volume has any options set.
+// If a mount is necessary, v.state.MountCount will be incremented.
+// If it was 0 when the increment occurred, the volume will be mounted on the
+// host. Otherwise, we assume it is already mounted.
+// Must be done while the volume is locked.
+// Is a no-op on volumes that do not require a mount (as defined by
+// volumeNeedsMount())
+func (v *Volume) mount() error {
+	if !v.needsMount() {
+		return nil
+	}
+
+	// Update the volume from the DB to get an accurate mount counter.
+	if err := v.update(); err != nil {
+		return err
+	}
+
+	// If the count is non-zero, the volume is already mounted.
+	// Nothing to do.
+	if v.state.MountCount > 0 {
+		v.state.MountCount = v.state.MountCount + 1
+		logrus.Debugf("Volume %s mount count now at %d", v.Name(), v.state.MountCount)
+		return v.save()
+	}
+
+	volDevice := v.config.Options["device"]
+	volType := v.config.Options["type"]
+	volOptions := v.config.Options["o"]
+
+	// Some filesystems (tmpfs) don't have a device, but we still need to
+	// give the kernel something.
+	if volDevice == "" && volType != "" {
+		volDevice = volType
+	}
+
+	// We need to use the actual mount command.
+	// Convincing unix.Mount to use the same semantics as the mount command
+	// itself seems prohibitively difficult.
+	// 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")
+	}
+	mountArgs := []string{}
+	if volOptions != "" {
+		mountArgs = append(mountArgs, "-o", volOptions)
+	}
+	if volType != "" {
+		mountArgs = append(mountArgs, "-t", volType)
+	}
+	mountArgs = append(mountArgs, volDevice, v.config.MountPoint)
+	mountCmd := exec.Command(mountPath, mountArgs...)
+
+	errPipe, err := mountCmd.StderrPipe()
+	if err != nil {
+		return errors.Wrapf(err, "getting stderr pipe")
+	}
+	if err := mountCmd.Start(); err != nil {
+		out, err2 := ioutil.ReadAll(errPipe)
+		if err2 != nil {
+			return errors.Wrapf(err2, "error reading mount STDERR")
+		}
+		return errors.Wrapf(errors.New(string(out)), "error mounting volume %s", v.Name())
+	}
+
+	logrus.Debugf("Mounted volume %s", v.Name())
+
+	// Increment the mount counter
+	v.state.MountCount = v.state.MountCount + 1
+	logrus.Debugf("Volume %s mount count now at %d", v.Name(), v.state.MountCount)
+	return v.save()
+}
+
+// unmount unmounts the volume if necessary.
+// Unmounting a volume that is not mounted is a no-op.
+// Unmounting a volume that does not require a mount is a no-op.
+// The volume must be locked for this to occur.
+// The mount counter will be decremented if non-zero. If the counter reaches 0,
+// the volume will really be unmounted, as no further containers are using the
+// volume.
+// If force is set, the volume will be unmounted regardless of mount counter.
+func (v *Volume) unmount(force bool) error {
+	if !v.needsMount() {
+		return nil
+	}
+
+	// Update the volume from the DB to get an accurate mount counter.
+	if err := v.update(); err != nil {
+		return err
+	}
+
+	if v.state.MountCount == 0 {
+		logrus.Debugf("Volume %s already unmounted", v.Name())
+		return nil
+	}
+
+	if !force {
+		v.state.MountCount = v.state.MountCount - 1
+	} else {
+		v.state.MountCount = 0
+	}
+
+	logrus.Debugf("Volume %s mount count now at %d", v.Name(), v.state.MountCount)
+
+	if v.state.MountCount == 0 {
+		// Unmount the volume
+		if err := unix.Unmount(v.config.MountPoint, unix.MNT_DETACH); err != nil {
+			return errors.Wrapf(err, "error unmounting volume %s", v.Name())
+		}
+		logrus.Debugf("Unmounted volume %s", v.Name())
+	}
+
+	return v.save()
+}

libpod/volume_internal_unsupported.go

@@ -0,0 +1,15 @@
+// +build !linux
+
+package libpod
+
+import (
+	"github.com/containers/libpod/libpod/define"
+)
+
+func (v *Volume) mount() error {
+	return define.ErrNotImplemented
+}
+
+func (v *Volume) unmount(force bool) error {
+	return define.ErrNotImplemented
+}

pkg/adapter/runtime.go

@@ -185,7 +185,7 @@ func (r *LocalRuntime) CreateVolume(ctx context.Context, c *cliconfig.VolumeCrea
 		options = append(options, libpod.WithVolumeLabels(labels))
 	}
 
-	if len(options) != 0 {
+	if len(opts) != 0 {
 		options = append(options, libpod.WithVolumeOptions(opts))
 	}
 	newVolume, err := r.NewVolume(ctx, options...)

pkg/varlinkapi/volumes.go

@@ -67,7 +67,8 @@ func (i *LibpodAPI) GetVolumes(call iopodman.VarlinkCall, args []string, all boo
 			Labels:     v.Labels(),
 			MountPoint: v.MountPoint(),
 			Name:       v.Name(),
-			Options:    v.Options(),
+			// TODO change types here to be correct
+			//Options:    v.Options(),
 		}
 		volumes = append(volumes, newVol)
 	}

test/e2e/run_volume_test.go

@@ -5,11 +5,14 @@ package integration
 import (
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
+	"strings"
 
 	. "github.com/containers/libpod/test/utils"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gexec"
 )
 
 var _ = Describe("Podman run with volumes", func() {
@@ -190,4 +193,60 @@ var _ = Describe("Podman run with volumes", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Not(Equal(0)))
 	})
+
+	It("podman run with tmpfs named volume mounts and unmounts", func() {
+		SkipIfRootless()
+		volName := "testvol"
+		mkVolume := podmanTest.Podman([]string{"volume", "create", "--opt", "type=tmpfs", "--opt", "device=tmpfs", "--opt", "o=nodev", "testvol"})
+		mkVolume.WaitWithDefaultTimeout()
+		Expect(mkVolume.ExitCode()).To(Equal(0))
+
+		// Volume not mounted on create
+		mountCmd1, err := gexec.Start(exec.Command("mount"), GinkgoWriter, GinkgoWriter)
+		Expect(err).To(BeNil())
+		mountCmd1.Wait(90)
+		Expect(mountCmd1.ExitCode()).To(Equal(0))
+		os.Stdout.Sync()
+		os.Stderr.Sync()
+		mountOut1 := strings.Join(strings.Fields(fmt.Sprintf("%s", mountCmd1.Out.Contents())), " ")
+		fmt.Printf("Output: %s", mountOut1)
+		Expect(strings.Contains(mountOut1, volName)).To(BeFalse())
+
+		ctrName := "testctr"
+		podmanSession := podmanTest.Podman([]string{"run", "-d", "--name", ctrName, "-v", fmt.Sprintf("%s:/testvol", volName), ALPINE, "top"})
+		podmanSession.WaitWithDefaultTimeout()
+		Expect(podmanSession.ExitCode()).To(Equal(0))
+
+		// Volume now mounted as container is running
+		mountCmd2, err := gexec.Start(exec.Command("mount"), GinkgoWriter, GinkgoWriter)
+		Expect(err).To(BeNil())
+		mountCmd2.Wait(90)
+		Expect(mountCmd2.ExitCode()).To(Equal(0))
+		os.Stdout.Sync()
+		os.Stderr.Sync()
+		mountOut2 := strings.Join(strings.Fields(fmt.Sprintf("%s", mountCmd2.Out.Contents())), " ")
+		fmt.Printf("Output: %s", mountOut2)
+		Expect(strings.Contains(mountOut2, volName)).To(BeTrue())
+
+		// Stop the container to unmount
+		podmanStopSession := podmanTest.Podman([]string{"stop", "--timeout", "0", ctrName})
+		podmanStopSession.WaitWithDefaultTimeout()
+		Expect(podmanStopSession.ExitCode()).To(Equal(0))
+
+		// We have to force cleanup so the unmount happens
+		podmanCleanupSession := podmanTest.Podman([]string{"container", "cleanup", ctrName})
+		podmanCleanupSession.WaitWithDefaultTimeout()
+		Expect(podmanCleanupSession.ExitCode()).To(Equal(0))
+
+		// Ensure volume is unmounted
+		mountCmd3, err := gexec.Start(exec.Command("mount"), GinkgoWriter, GinkgoWriter)
+		Expect(err).To(BeNil())
+		mountCmd3.Wait(90)
+		Expect(mountCmd3.ExitCode()).To(Equal(0))
+		os.Stdout.Sync()
+		os.Stderr.Sync()
+		mountOut3 := strings.Join(strings.Fields(fmt.Sprintf("%s", mountCmd3.Out.Contents())), " ")
+		fmt.Printf("Output: %s", mountOut3)
+		Expect(strings.Contains(mountOut3, volName)).To(BeFalse())
+	})
 })

test/e2e/volume_create_test.go

@@ -57,4 +57,10 @@ var _ = Describe("Podman volume create", func() {
 		Expect(match).To(BeTrue())
 		Expect(len(check.OutputToStringArray())).To(Equal(1))
 	})
+
+	It("podman create volume with bad volume option", func() {
+		session := podmanTest.Podman([]string{"volume", "create", "--opt", "badOpt=bad"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Not(Equal(0)))
+	})
 })