Wait for mount command to finish when mounting volume

command.Start() just starts the command. That catches some
errors, but the nasty ones - bad options and similar - happen
when the command runs. Use CombinedOutput() instead - it waits
for the command to exit, and thus catches non-0 exit of the
`mount` command (invalid options, for example).

STDERR from the `mount` command is directly used, which isn't
necessarily the best, but we can't really get much more info on
what went wrong.

Fixes containers#4303

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

libpod/volume_internal_linux.go

@@ -3,8 +3,8 @@
 package libpod
 
 import (
-	"io/ioutil"
 	"os/exec"
+	"strings"
 
 	"github.com/containers/libpod/libpod/define"
 	"github.com/containers/libpod/pkg/rootless"
@@ -72,16 +72,10 @@ func (v *Volume) mount() error {
 	mountArgs = append(mountArgs, volDevice, v.config.MountPoint)
 	mountCmd := exec.Command(mountPath, mountArgs...)
 
-	errPipe, err := mountCmd.StderrPipe()
-	if err != nil {
-		return errors.Wrapf(err, "error getting stderr pipe for mount")
-	}
-	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("Running mount command: %s %s", mountPath, strings.Join(mountArgs, " "))
+	if output, err := mountCmd.CombinedOutput(); err != nil {
+		logrus.Debugf("Mount failed with %v", err)
+		return errors.Wrapf(errors.Errorf(string(output)), "error mounting volume %s", v.Name())
 	}
 
 	logrus.Debugf("Mounted volume %s", v.Name())

test/e2e/run_volume_test.go

@@ -358,10 +358,22 @@ var _ = Describe("Podman run with volumes", func() {
 		Expect(arr2[0]).To(Equal(volName))
 	})
 
+
 	It("podman run image volume is not noexec", func() {
 		session := podmanTest.Podman([]string{"run", "--rm", redis, "grep", "/data", "/proc/self/mountinfo"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		Expect(session.OutputToString()).To(Not(ContainSubstring("noexec")))
 	})
+
+	It("podman mount with invalid option fails", func() {
+		volName := "testVol"
+		volCreate := podmanTest.Podman([]string{"volume", "create", "--opt", "type=tmpfs", "--opt", "device=tmpfs", "--opt", "o=invalid", volName})
+		volCreate.WaitWithDefaultTimeout()
+		Expect(volCreate.ExitCode()).To(Equal(0))
+
+		volMount := podmanTest.Podman([]string{"run", "--rm", "-v", fmt.Sprintf("%s:/tmp", volName), ALPINE, "ls"})
+		volMount.WaitWithDefaultTimeout()
+		Expect(volMount.ExitCode()).To(Not(Equal(0)))
+	})
 })