Fix incorrect parsing of create/run --volumes-from

Add a bunch of tests to ensure that --volumes-from
works as expected.

Also align the podman create and run man page.

Signed-off-by: Paul Holzinger <paul.holzinger@web.de>

5 files changed, 84 insertions, 19 deletions

diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go
index cfbcf6140..7e3dc7fb4 100644
--- a/cmd/podman/common/create.go
+++ b/cmd/podman/common/create.go
@@ -509,7 +509,7 @@ func GetCreateFlags(cf *ContainerCLIOpts) *pflag.FlagSet {
 		"volume", "v", containerConfig.Volumes(),
 		"Bind mount a volume into the container",
 	)
-	createFlags.StringSliceVar(
+	createFlags.StringArrayVar(
 		&cf.VolumesFrom,
 		"volumes-from", []string{},
 		"Mount volumes from the specified container(s)",
diff --git a/docs/source/markdown/podman-create.1.md b/docs/source/markdown/podman-create.1.md
index 9049ffb9f..4a8b311f0 100644
--- a/docs/source/markdown/podman-create.1.md
+++ b/docs/source/markdown/podman-create.1.md
@@ -1070,11 +1070,11 @@ change propagation properties of source mount. Say `/` is source mount for
 
 **--volumes-from**[=*CONTAINER*[:*OPTIONS*]]
 
-Mount volumes from the specified container(s).
-*OPTIONS* is a comma delimited list with the following available elements:
+Mount volumes from the specified container(s). Used to share volumes between
+containers. The *options* is a comma delimited list with the following available elements:
 
-* [rw|ro]
-* z
+* **rw**|**ro**
+* **z**
 
 Mounts already mounted volumes from a source container onto another
 container. You must supply the source's container-id or container-name.
@@ -1083,9 +1083,8 @@ the target container. You can share volumes even if the source container
 is not running.
 
 By default, Podman mounts the volumes in the same mode (read-write or
-read-only) as it is mounted in the source container. Optionally, you
-can change this by suffixing the container-id with either the `ro` or
-`rw` keyword.
+read-only) as it is mounted in the source container.
+You can change this by adding a `ro` or `rw` _option_.
 
 Labeling systems like SELinux require that proper labels are placed on volume
 content mounted into a container. Without a label, the security system might
diff --git a/docs/source/markdown/podman-run.1.md b/docs/source/markdown/podman-run.1.md
index 1a7b36a5e..47aa8827f 100644
--- a/docs/source/markdown/podman-run.1.md
+++ b/docs/source/markdown/podman-run.1.md
@@ -1100,7 +1100,7 @@ will convert /foo into a shared mount point. Alternatively, one can directly
 change propagation properties of source mount. Say, if _/_ is source mount for
 _/foo_, then use **mount --make-shared /** to convert _/_ into a shared mount.
 
-**--volumes-from**[=*container-id*[:*options*]]
+**--volumes-from**[=*CONTAINER*[:*OPTIONS*]]
 
 Mount volumes from the specified container(s). Used to share volumes between
 containers. The *options* is a comma delimited list with the following available elements:
@@ -1108,19 +1108,23 @@ containers. The *options* is a comma delimited list with the following available
 * **rw**|**ro**
 * **z**
 
-You can share volumes even if the source container is not running.
+Mounts already mounted volumes from a source container onto another
+container. You must supply the source's container-id or container-name.
+To share a volume, use the --volumes-from option when running
+the target container. You can share volumes even if the source container
+is not running.
 
 By default, Podman mounts the volumes in the same mode (read-write or
 read-only) as it is mounted in the source container.
-You can change this by adding a **ro** or **rw** _option_.
+You can change this by adding a `ro` or `rw` _option_.
 
 Labeling systems like SELinux require that proper labels are placed on volume
 content mounted into a container. Without a label, the security system might
 prevent the processes running inside the container from using the content. By
 default, Podman does not change the labels set by the OS.
 
-To change a label in the container context, you can add **z** to the volume mount.
-This suffix tells Podman to relabel file objects on the shared volumes. The **z**
+To change a label in the container context, you can add `z` to the volume mount.
+This suffix tells Podman to relabel file objects on the shared volumes. The `z`
 option tells Podman that two containers share the volume content. As a result,
 podman labels the content with a shared content label. Shared volume labels allow
 all containers to read/write content.
diff --git a/pkg/specgen/generate/storage.go b/pkg/specgen/generate/storage.go
index 7f55317ff..b225f79ee 100644
--- a/pkg/specgen/generate/storage.go
+++ b/pkg/specgen/generate/storage.go
@@ -195,9 +195,9 @@ func getVolumesFrom(volumesFrom []string, runtime *libpod.Runtime) (map[string]s
 		splitVol := strings.SplitN(volume, ":", 2)
 		if len(splitVol) == 2 {
 			splitOpts := strings.Split(splitVol[1], ",")
+			setRORW := false
+			setZ := false
 			for _, opt := range splitOpts {
-				setRORW := false
-				setZ := false
 				switch opt {
 				case "z":
 					if setZ {
diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go
index cbfb6bf59..4376bf309 100644
--- a/test/e2e/run_test.go
+++ b/test/e2e/run_test.go
@@ -733,23 +733,85 @@ USER mail`
 		err := os.MkdirAll(vol, 0755)
 		Expect(err).To(BeNil())
 
-		volFile := filepath.Join(vol, "test.txt")
+		filename := "test.txt"
+		volFile := filepath.Join(vol, filename)
 		data := "Testing --volumes-from!!!"
 		err = ioutil.WriteFile(volFile, []byte(data), 0755)
 		Expect(err).To(BeNil())
+		mountpoint := "/myvol/"
 
-		session := podmanTest.Podman([]string{"create", "--volume", vol + ":/myvol", redis, "sh"})
+		session := podmanTest.Podman([]string{"create", "--volume", vol + ":" + mountpoint, ALPINE, "cat", mountpoint + filename})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		ctrID := session.OutputToString()
 
-		session = podmanTest.Podman([]string{"run", "--volumes-from", ctrID, ALPINE, "echo", "'testing read-write!' >> myvol/test.txt"})
+		session = podmanTest.Podman([]string{"run", "--volumes-from", ctrID, ALPINE, "cat", mountpoint + filename})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal(data))
 
-		session = podmanTest.Podman([]string{"run", "--volumes-from", ctrID + ":z", ALPINE, "ls"})
+		session = podmanTest.Podman([]string{"run", "--volumes-from", ctrID, ALPINE, "sh", "-c", "echo test >> " + mountpoint + filename})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
+
+		session = podmanTest.Podman([]string{"start", "--attach", ctrID})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal(data + "test"))
+	})
+
+	It("podman run --volumes-from flag options", func() {
+		vol := filepath.Join(podmanTest.TempDir, "vol-test")
+		err := os.MkdirAll(vol, 0755)
+		Expect(err).To(BeNil())
+
+		filename := "test.txt"
+		volFile := filepath.Join(vol, filename)
+		data := "Testing --volumes-from!!!"
+		err = ioutil.WriteFile(volFile, []byte(data), 0755)
+		Expect(err).To(BeNil())
+		mountpoint := "/myvol/"
+
+		session := podmanTest.Podman([]string{"create", "--volume", vol + ":" + mountpoint, ALPINE, "cat", mountpoint + filename})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		ctrID := session.OutputToString()
+
+		// check that the read only option works
+		session = podmanTest.Podman([]string{"run", "--volumes-from", ctrID + ":ro", ALPINE, "touch", mountpoint + "abc.txt"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(1))
+		Expect(session.ErrorToString()).To(ContainSubstring("Read-only file system"))
+
+		// check that both z and ro options work
+		session = podmanTest.Podman([]string{"run", "--volumes-from", ctrID + ":ro,z", ALPINE, "cat", mountpoint + filename})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		Expect(session.OutputToString()).To(Equal(data))
+
+		// check that multiple ro/rw are not working
+		session = podmanTest.Podman([]string{"run", "--volumes-from", ctrID + ":ro,rw", ALPINE, "cat", mountpoint + filename})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(125))
+		Expect(session.ErrorToString()).To(ContainSubstring("cannot set ro or rw options more than once"))
+
+		// check that multiple z options are not working
+		session = podmanTest.Podman([]string{"run", "--volumes-from", ctrID + ":z,z,ro", ALPINE, "cat", mountpoint + filename})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(125))
+		Expect(session.ErrorToString()).To(ContainSubstring("cannot set :z more than once in mount options"))
+
+		// create new read only volume
+		session = podmanTest.Podman([]string{"create", "--volume", vol + ":" + mountpoint + ":ro", ALPINE, "cat", mountpoint + filename})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+		ctrID = session.OutputToString()
+
+		// check if the original volume was mounted as read only that --volumes-from also mount it as read only
+		session = podmanTest.Podman([]string{"run", "--volumes-from", ctrID, ALPINE, "touch", mountpoint + "abc.txt"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(1))
+		Expect(session.ErrorToString()).To(ContainSubstring("Read-only file system"))
 	})
 
 	It("podman run --volumes-from flag with built-in volumes", func() {