run: remove --inject-volume

The syntax of `--inject-volume` has been somewhat contentious [1][1] and
it seems pretty clear we do not want the flag to remain as-is.
Furthermore, there's actually no immediate need for it since the same
functionality can be achieved through a combination of the `--volume`
and `--mount` flags (which landed in the same PR as `--inject-volume`).
Hence, remove it for now, guide people towards the alternative, and we
can put further thought into re-adding something similar in future
(which would essentially just be a convenience).

[1]: rkt#1656
[2]: rkt#1582

Documentation/subcommands/run.md

@@ -130,9 +130,9 @@ The volume is then mounted into each app running to the pod based on information
 
 ### Mounting Volumes without Mount Points
 
-If the ACI doesn't have any mount points defined in its manifest, you can still mount volumes using the `--mount` or `--inject-volume` flags.
+If the ACI doesn't have any mount points defined in its manifest, you can still mount volumes using the `--mount` flag.
 
-With `--mount` you define a mapping between volumes and a path in the app. This will override any mount points in the image manifest.
+With `--mount` you define a mapping between volumes and a path in the app. This will supplement and override any mount points in the image manifest.
 In the following example, the `--mount` option is positioned after the app name; it defines the mount only in that app:
 
 ```
@@ -150,12 +150,6 @@ It defines mounts on all apps: both app1 and app2 will have `/srv/logs` accessib
         example.com/app1 example.com/app2
 ```
 
-`--inject-volume` is convenient when you don't want to share volumes between the apps in the pod. It simply maps a path in the host to a path in the app:
-
-```
-# rkt run example.com/app1 --inject-volume /srv/data:/var/data
-```
-
 ### MapReduce Example
 
 Let's say we want to read data from the host directory `/opt/tenant1/work` to power a MapReduce-style worker. We'll call this app `example.com/reduce-worker`.

rkt/cli_apps.go

@@ -155,42 +155,6 @@ func (ae *appExec) Set(s string) error {
 	return nil
 }
 
-// appInjectVolume is for --inject-volume flags in the form of: --inject-volume source=PATH,target=PATH
-type appInjectVolume apps.Apps
-
-func (aam *appInjectVolume) Set(s string) error {
-	app := (*apps.Apps)(aam).Last()
-	if app == nil {
-		return fmt.Errorf("--inject-volume must follow an image")
-	}
-
-	p := strings.SplitN(s, ":", 2)
-	if len(p) != 2 {
-		return fmt.Errorf("must be SOURCE_PATH:TARGET_PATH")
-	}
-
-	volName, err := types.SanitizeACName(p[1])
-	if err != nil {
-		return fmt.Errorf("empty target in volume")
-	}
-
-	rVol := types.Volume{Name: *types.MustACName(volName), Source: p[0], Kind: "host"}
-	mount := schema.Mount{Volume: rVol.Name, Path: p[1]}
-
-	(*apps.Apps)(aam).Volumes = append((*apps.Apps)(aam).Volumes, rVol)
-	app.Mounts = append(app.Mounts, mount)
-
-	return nil
-}
-
-func (aam *appInjectVolume) Type() string {
-	return "appInjectVolume"
-}
-
-func (aam *appInjectVolume) String() string {
-	return ""
-}
-
 // appMount is for --mount flags in the form of: --mount volume=VOLNAME,target=PATH
 type appMount apps.Apps
 

rkt/run.go

@@ -86,7 +86,6 @@ func init() {
 	cmdRun.Flags().Var((*appAsc)(&rktApps), "signature", "local signature file to use in validating the preceding image")
 	cmdRun.Flags().Var((*appExec)(&rktApps), "exec", "override the exec command for the preceding image")
 	cmdRun.Flags().Var((*appMount)(&rktApps), "mount", "mount point binding a volume to a path within an app")
-	cmdRun.Flags().Var((*appInjectVolume)(&rktApps), "inject-volume", "inject a volume into an app. Syntax: --inject-volume=SOURCE:TARGET")
 
 	flagPorts = portList{}
 

tests/rkt_volume_test.go

@@ -56,19 +56,19 @@ var volTests = []struct {
 		`/bin/sh -c "export FILE=/dir1/file CONTENT=3 ; ^RKT_BIN^ --debug --insecure-skip-verify run --mds-register=false --inherit-env=true --volume=dir1,kind=host,source=^TMPDIR^ --mount=volume=dir1,target=dir1 ^VOL_RO_WRITE_FILE^"`,
 		`Cannot write to file "/dir1/file": open /dir1/file: read-only file system`,
 	},
-	// Check that the volume still contain the file previously written
+	// Check that the volume still contains the file previously written
 	{
 		`/bin/sh -c "export FILE=/dir1/file ; ^RKT_BIN^ --debug --insecure-skip-verify run --mds-register=false --inherit-env=true --volume=dir1,kind=host,source=^TMPDIR^ --mount=volume=dir1,target=dir1 ^VOL_RO_READ_FILE^"`,
 		`<<<2>>>`,
 	},
-	// Check that inject-volume works without any mountpoint in the image
+	// Check that injecting a rw mount/volume works without any mountpoint in the image manifest
 	{
-		`/bin/sh -c "export FILE=/dir1/file CONTENT=1 ; ^RKT_BIN^ --debug --insecure-skip-verify run --mds-register=false --inherit-env=true ^INJECT_RW^" --inject-volume=^TMPDIR^:dir1`,
+		`/bin/sh -c "export FILE=/dir1/file CONTENT=1 ; ^RKT_BIN^ --debug --insecure-skip-verify run --mds-register=false --inherit-env=true --volume=dir1,kind=host,source=^TMPDIR^ ^VOL_ADD_MOUNT_RW^" --mount=volume=dir1,target=dir1`,
 		`<<<1>>>`,
 	},
-	// Check that inject-volume works with a ro mountpoint in the image
+	// Check that injecting a ro mount/volume works without any mountpoint in the image manifest
 	{
-		`/bin/sh -c "export FILE=/dir1/file CONTENT=1 ; ^RKT_BIN^ --debug --insecure-skip-verify run --mds-register=false --inherit-env=true ^INJECT_RO^ --inject-volume=^TMPDIR^:/dir1"`,
+		`/bin/sh -c "export FILE=/dir1/file CONTENT=1 ; ^RKT_BIN^ --debug --insecure-skip-verify run --mds-register=false --inherit-env=true --volume=dir1,kind=host,source=^TMPDIR^,readOnly=true ^VOL_ADD_MOUNT_RO^ --mount=volume=dir1,target=dir1`,
 		`Cannot write to file "/dir1/file": open /dir1/file: read-only file system`,
 	},
 }
@@ -86,10 +86,10 @@ func TestVolumes(t *testing.T) {
 	defer os.Remove(volRoReadFileImage)
 	volRoWriteFileImage := patchTestACI("rkt-inspect-vol-ro-write-file.aci", "--exec=/inspect --write-file --read-file", "--mounts=dir1,path=/dir1,readOnly=true")
 	defer os.Remove(volRoWriteFileImage)
-	injectImageRW := patchTestACI("rkt-inspect-inject-rw.aci", "--exec=/inspect --write-file --read-file")
-	defer os.Remove(injectImageRW)
-	injectImageRO := patchTestACI("rkt-inspect-inject-ro.aci", "--exec=/inspect --write-file --read-file", "--mounts=dir1,path=/dir1,readOnly=true")
-	defer os.Remove(injectImageRO)
+	volAddMountRwImage := patchTestACI("rkt-inspect-vol-add-mount-rw.aci", "--exec=/inspect --write-file --read-file")
+	defer os.Remove(volAddMountRwImage)
+	volAddMountRoImage := patchTestACI("rkt-inspect-vol-add-mount-ro.aci", "--exec=/inspect --write-file --read-file")
+	defer os.Remove(volAddMountRoImage)
 	ctx := newRktRunCtx()
 	defer ctx.cleanup()
 
@@ -110,8 +110,8 @@ func TestVolumes(t *testing.T) {
 		cmd = strings.Replace(cmd, "^VOL_RO_WRITE_FILE^", volRoWriteFileImage, -1)
 		cmd = strings.Replace(cmd, "^VOL_RW_READ_FILE^", volRwReadFileImage, -1)
 		cmd = strings.Replace(cmd, "^VOL_RW_WRITE_FILE^", volRwWriteFileImage, -1)
-		cmd = strings.Replace(cmd, "^INJECT_RW^", injectImageRW, -1)
-		cmd = strings.Replace(cmd, "^INJECT_RO^", injectImageRO, -1)
+		cmd = strings.Replace(cmd, "^VOL_ADD_MOUNT_RW^", volAddMountRwImage, -1)
+		cmd = strings.Replace(cmd, "^VOL_ADD_MOUNT_RO^", volAddMountRoImage, -1)
 
 		t.Logf("Running test #%v", i)
 		child := spawnOrFail(t, cmd)