Merge pull request #4287 from mheon/anonymous_volumes

Add support for anonymous volumes to `podman run -v`

docs/podman-create.1.md

@@ -800,7 +800,7 @@ Set the UTS mode for the container
     **ns**: specify the user namespace to use.
     Note: the host mode gives the container access to changing the host's hostname and is therefore considered insecure.
 
-**--volume**, **-v**[=*[HOST-DIR:CONTAINER-DIR[:OPTIONS]]*]
+**--volume**, **-v**[=*[[SOURCE-VOLUME|HOST-DIR:]CONTAINER-DIR[:OPTIONS]]*]
 
 Create a bind mount. If you specify, ` -v /HOST-DIR:/CONTAINER-DIR`, podman
 bind mounts `/HOST-DIR` in the host to `/CONTAINER-DIR` in the podman
@@ -810,11 +810,23 @@ container. The `OPTIONS` are a comma delimited list and can be:
 * [z|Z]
 * [`[r]shared`|`[r]slave`|`[r]private`]
 
-The `CONTAINER-DIR` must be an absolute path such as `/src/docs`. The `HOST-DIR`
-must be an absolute path as well. Podman bind-mounts the `HOST-DIR` to the
-path you specify. For example, if you supply the `/foo` value, Podman creates a bind-mount.
+The `CONTAINER-DIR` must be an absolute path such as `/src/docs`. The volume
+will be mounted into the container at this directory.
 
-You can specify multiple  **-v** options to mount one or more mounts to a
+Volumes may specify a source as well, as either a directory on the host or the
+name of a named volume. If no source is given, the volume will be created as an
+anonymous named volume with a randomly generated name, and will be removed when
+the container is removed via the `--rm` flag or `podman rm --volumes`.
+
+If a volume source is specified, it must be a path on the host or the name of a
+named volume. Host paths are allowed to be absolute or relative; relative paths
+are resolved relative to the directory Podman is run in. Any source that does
+not begin with a `.` or `/` it will be treated as the name of a named volume.
+If a volume with that name does not exist, it will be created. Volumes created
+with names are not anonymous and are not removed by `--rm` and
+`podman rm --volumes`.
+
+You can specify multiple  **-v** options to mount one or more volumes into a
 container.
 
 You can add `:ro` or `:rw` suffix to a volume to mount it  read-only or

docs/podman-run.1.md

@@ -839,7 +839,7 @@ Set the UTS mode for the container
 
 **NOTE**: the host mode gives the container access to changing the host's hostname and is therefore considered insecure.
 
-**--volume**, **-v**[=*[HOST-DIR-OR-VOUME-NAME:CONTAINER-DIR[:OPTIONS]]*]
+**--volume**, **-v**[=*[[SOURCE-VOLUME|HOST-DIR:]CONTAINER-DIR[:OPTIONS]]*]
 
 Create a bind mount. If you specify, ` -v /HOST-DIR:/CONTAINER-DIR`, Podman
 bind mounts `/HOST-DIR` in the host to `/CONTAINER-DIR` in the Podman
@@ -853,11 +853,23 @@ create one.
 * [`z`|`Z`]
 * [`[r]shared`|`[r]slave`|`[r]private`]
 
-The `/CONTAINER-DIR` must be an absolute path such as `/src/docs`. The `/HOST-DIR`
-must be an absolute path as well. Podman bind-mounts the `HOST-DIR` to the
-path you specify. For example, if you supply the `/foo` value, Podman creates a bind-mount.
+The `CONTAINER-DIR` must be an absolute path such as `/src/docs`. The volume
+will be mounted into the container at this directory.
 
-You can specify multiple  **-v** options to mount one or more mounts to a
+Volumes may specify a source as well, as either a directory on the host or the
+name of a named volume. If no source is given, the volume will be created as an
+anonymous named volume with a randomly generated name, and will be removed when
+the container is removed via the `--rm` flag or `podman rm --volumes`.
+
+If a volume source is specified, it must be a path on the host or the name of a
+named volume. Host paths are allowed to be absolute or relative; relative paths
+are resolved relative to the directory Podman is run in. Any source that does
+not begin with a `.` or `/` it will be treated as the name of a named volume.
+If a volume with that name does not exist, it will be created. Volumes created
+with names are not anonymous and are not removed by `--rm` and
+`podman rm --volumes`.
+
+You can specify multiple  **-v** options to mount one or more volumes into a
 container.
 
 You can add `:ro` or `:rw` suffix to a volume to mount it  read-only or

libpod/runtime_ctr.go

@@ -295,21 +295,32 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (c *Contai
 	// Maintain an array of them - we need to lock them later.
 	ctrNamedVolumes := make([]*Volume, 0, len(ctr.config.NamedVolumes))
 	for _, vol := range ctr.config.NamedVolumes {
-		// Check if it exists already
-		dbVol, err := r.state.Volume(vol.Name)
-		if err == nil {
-			ctrNamedVolumes = append(ctrNamedVolumes, dbVol)
-			// The volume exists, we're good
-			continue
-		} else if errors.Cause(err) != define.ErrNoSuchVolume {
-			return nil, errors.Wrapf(err, "error retrieving named volume %s for new container", vol.Name)
+		isAnonymous := false
+		if vol.Name == "" {
+			// Anonymous volume. We'll need to create it.
+			// It needs a name first.
+			vol.Name = stringid.GenerateNonCryptoID()
+			isAnonymous = true
+		} else {
+			// Check if it exists already
+			dbVol, err := r.state.Volume(vol.Name)
+			if err == nil {
+				ctrNamedVolumes = append(ctrNamedVolumes, dbVol)
+				// The volume exists, we're good
+				continue
+			} else if errors.Cause(err) != define.ErrNoSuchVolume {
+				return nil, errors.Wrapf(err, "error retrieving named volume %s for new container", vol.Name)
+			}
 		}
 
 		logrus.Debugf("Creating new volume %s for container", vol.Name)
 
 		// The volume does not exist, so we need to create it.
-		newVol, err := r.newVolume(ctx, WithVolumeName(vol.Name), withSetCtrSpecific(),
-			WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID()))
+		volOptions := []VolumeCreateOption{WithVolumeName(vol.Name), WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID())}
+		if isAnonymous {
+			volOptions = append(volOptions, withSetCtrSpecific())
+		}
+		newVol, err := r.newVolume(ctx, volOptions...)
 		if err != nil {
 			return nil, errors.Wrapf(err, "error creating named volume %q", vol.Name)
 		}

pkg/adapter/containers.go

@@ -437,7 +437,7 @@ func (r *LocalRuntime) Run(ctx context.Context, c *cliconfig.RunValues, exitCode
 	}
 
 	if c.IsSet("rm") {
-		if err := r.Runtime.RemoveContainer(ctx, ctr, false, false); err != nil {
+		if err := r.Runtime.RemoveContainer(ctx, ctr, false, true); err != nil {
 			logrus.Errorf("Error removing container %s: %v", ctr.ID(), err)
 		}
 	}
@@ -1053,7 +1053,7 @@ func (r *LocalRuntime) CleanupContainers(ctx context.Context, cli *cliconfig.Cle
 
 // Only used when cleaning up containers
 func removeContainer(ctx context.Context, ctr *libpod.Container, runtime *LocalRuntime) error {
-	if err := runtime.RemoveContainer(ctx, ctr, false, false); err != nil {
+	if err := runtime.RemoveContainer(ctx, ctr, false, true); err != nil {
 		return errors.Wrapf(err, "failed to cleanup and remove container %v", ctr.ID())
 	}
 	return nil

pkg/spec/storage.go

@@ -11,7 +11,6 @@ import (
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/pkg/util"
 	pmount "github.com/containers/storage/pkg/mount"
-	"github.com/containers/storage/pkg/stringid"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -648,7 +647,7 @@ func (config *CreateConfig) getVolumeMounts() (map[string]spec.Mount, map[string
 	mounts := make(map[string]spec.Mount)
 	volumes := make(map[string]*libpod.ContainerNamedVolume)
 
-	volumeFormatErr := errors.Errorf("incorrect volume format, should be host-dir:ctr-dir[:option]")
+	volumeFormatErr := errors.Errorf("incorrect volume format, should be [host-dir:]ctr-dir[:option]")
 
 	for _, vol := range config.Volumes {
 		var (
@@ -665,7 +664,11 @@ func (config *CreateConfig) getVolumeMounts() (map[string]spec.Mount, map[string
 
 		src = splitVol[0]
 		if len(splitVol) == 1 {
-			dest = src
+			// This is an anonymous named volume. Only thing given
+			// is destination.
+			// Name/source will be blank, and populated by libpod.
+			src = ""
+			dest = splitVol[0]
 		} else if len(splitVol) > 1 {
 			dest = splitVol[1]
 		}
@@ -675,8 +678,11 @@ func (config *CreateConfig) getVolumeMounts() (map[string]spec.Mount, map[string
 			}
 		}
 
-		if err := parse.ValidateVolumeHostDir(src); err != nil {
-			return nil, nil, err
+		// Do not check source dir for anonymous volumes
+		if len(splitVol) > 1 {
+			if err := parse.ValidateVolumeHostDir(src); err != nil {
+				return nil, nil, err
+			}
 		}
 		if err := parse.ValidateVolumeCtrDir(dest); err != nil {
 			return nil, nil, err
@@ -736,8 +742,8 @@ func (config *CreateConfig) getImageVolumes() (map[string]spec.Mount, map[string
 			}
 			mounts[vol] = mount
 		} else {
+			// Anonymous volumes have no name.
 			namedVolume := new(libpod.ContainerNamedVolume)
-			namedVolume.Name = stringid.GenerateNonCryptoID()
 			namedVolume.Options = []string{"rprivate", "rw", "nodev"}
 			namedVolume.Dest = cleanDest
 			volumes[vol] = namedVolume

test/e2e/run_volume_test.go

@@ -280,4 +280,81 @@ var _ = Describe("Podman run with volumes", func() {
 		session2.WaitWithDefaultTimeout()
 		Expect(session2.ExitCode()).To(Equal(0))
 	})
+
+	It("podman run with anonymous volume", func() {
+		list1 := podmanTest.Podman([]string{"volume", "list", "--quiet"})
+		list1.WaitWithDefaultTimeout()
+		Expect(list1.ExitCode()).To(Equal(0))
+		Expect(list1.OutputToString()).To(Equal(""))
+
+		session := podmanTest.Podman([]string{"create", "-v", "/test", ALPINE, "top"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		list2 := podmanTest.Podman([]string{"volume", "list", "--quiet"})
+		list2.WaitWithDefaultTimeout()
+		Expect(list2.ExitCode()).To(Equal(0))
+		arr := list2.OutputToStringArray()
+		Expect(len(arr)).To(Equal(1))
+		Expect(arr[0]).To(Not(Equal("")))
+	})
+
+	It("podman rm -v removes anonymous volume", func() {
+		list1 := podmanTest.Podman([]string{"volume", "list", "--quiet"})
+		list1.WaitWithDefaultTimeout()
+		Expect(list1.ExitCode()).To(Equal(0))
+		Expect(list1.OutputToString()).To(Equal(""))
+
+		ctrName := "testctr"
+		session := podmanTest.Podman([]string{"create", "--name", ctrName, "-v", "/test", ALPINE, "top"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		list2 := podmanTest.Podman([]string{"volume", "list", "--quiet"})
+		list2.WaitWithDefaultTimeout()
+		Expect(list2.ExitCode()).To(Equal(0))
+		arr := list2.OutputToStringArray()
+		Expect(len(arr)).To(Equal(1))
+		Expect(arr[0]).To(Not(Equal("")))
+
+		remove := podmanTest.Podman([]string{"rm", "-v", ctrName})
+		remove.WaitWithDefaultTimeout()
+		Expect(remove.ExitCode()).To(Equal(0))
+
+		list3 := podmanTest.Podman([]string{"volume", "list", "--quiet"})
+		list3.WaitWithDefaultTimeout()
+		Expect(list3.ExitCode()).To(Equal(0))
+		Expect(list3.OutputToString()).To(Equal(""))
+	})
+
+	It("podman rm -v retains named volume", func() {
+		list1 := podmanTest.Podman([]string{"volume", "list", "--quiet"})
+		list1.WaitWithDefaultTimeout()
+		Expect(list1.ExitCode()).To(Equal(0))
+		Expect(list1.OutputToString()).To(Equal(""))
+
+		ctrName := "testctr"
+		volName := "testvol"
+		session := podmanTest.Podman([]string{"create", "--name", ctrName, "-v", fmt.Sprintf("%s:/test", volName), ALPINE, "top"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+
+		list2 := podmanTest.Podman([]string{"volume", "list", "--quiet"})
+		list2.WaitWithDefaultTimeout()
+		Expect(list2.ExitCode()).To(Equal(0))
+		arr := list2.OutputToStringArray()
+		Expect(len(arr)).To(Equal(1))
+		Expect(arr[0]).To(Equal(volName))
+
+		remove := podmanTest.Podman([]string{"rm", "-v", ctrName})
+		remove.WaitWithDefaultTimeout()
+		Expect(remove.ExitCode()).To(Equal(0))
+
+		list3 := podmanTest.Podman([]string{"volume", "list", "--quiet"})
+		list3.WaitWithDefaultTimeout()
+		Expect(list3.ExitCode()).To(Equal(0))
+		arr2 := list3.OutputToStringArray()
+		Expect(len(arr2)).To(Equal(1))
+		Expect(arr2[0]).To(Equal(volName))
+	})
 })