Split ParseBindPath from config Get/Setters & add basic tests, from sylabs 319

cmd/internal/cli/action_flags.go

@@ -15,6 +15,7 @@ import (
 var (
 	AppName            string
 	BindPaths          []string
+	Mounts             []string
 	HomePath           string
 	OverlayPath        []string
 	ScratchPath        []string
@@ -83,7 +84,7 @@ var actionAppFlag = cmdline.Flag{
 var actionBindFlag = cmdline.Flag{
 	ID:           "actionBindFlag",
 	Value:        &BindPaths,
-	DefaultValue: cmdline.StringArray{}, // to allow commas in bind path
+	DefaultValue: []string{},
 	Name:         "bind",
 	ShortHand:    "B",
 	Usage:        "a user-bind path specification.  spec has the format src[:dest[:opts]], where src and dest are outside and inside paths.  If dest is not given, it is set equal to src.  Mount options ('opts') may be specified as 'ro' (read-only) or 'rw' (read/write, which is the default). Multiple bind paths can be given by a comma separated list.",
@@ -92,6 +93,19 @@ var actionBindFlag = cmdline.Flag{
 	EnvHandler:   cmdline.EnvAppendValue,
 }
 
+// --mount
+var actionMountFlag = cmdline.Flag{
+	ID:           "actionMountFlag",
+	Value:        &Mounts,
+	DefaultValue: []string{},
+	Name:         "mount",
+	Usage:        "a mount specification e.g. 'type=bind,source=/opt,destination=/hostopt'.",
+	EnvKeys:      []string{"MOUNT"},
+	Tag:          "<spec>",
+	EnvHandler:   cmdline.EnvAppendValue,
+	StringArray:  true,
+}
+
 // -H|--home
 var actionHomeFlag = cmdline.Flag{
 	ID:           "actionHomeFlag",
@@ -656,6 +670,7 @@ func init() {
 		cmdManager.RegisterFlagForCmd(&actionHostnameFlag, actionsInstanceCmd...)
 		cmdManager.RegisterFlagForCmd(&actionIpcNamespaceFlag, actionsInstanceCmd...)
 		cmdManager.RegisterFlagForCmd(&actionKeepPrivsFlag, actionsInstanceCmd...)
+		cmdManager.RegisterFlagForCmd(&actionMountFlag, actionsInstanceCmd...)
 		cmdManager.RegisterFlagForCmd(&actionNetNamespaceFlag, actionsInstanceCmd...)
 		cmdManager.RegisterFlagForCmd(&actionNetworkArgsFlag, actionsInstanceCmd...)
 		cmdManager.RegisterFlagForCmd(&actionNetworkFlag, actionsInstanceCmd...)

cmd/internal/cli/actions_linux.go

@@ -411,10 +411,21 @@ func execStarter(cobraCmd *cobra.Command, image string, args []string, name stri
 		img.File.Close()
 	}
 
+	// First get binds from -B/--bind and env var
 	binds, err := singularityConfig.ParseBindPath(BindPaths)
 	if err != nil {
 		sylog.Fatalf("while parsing bind path: %s", err)
 	}
+
+	// Now add binds from one or more --mount and env var.
+	for _, m := range Mounts {
+		bps, err := singularityConfig.ParseMountString(m)
+		if err != nil {
+			sylog.Fatalf("while parsing mount %q: %s", m, err)
+		}
+		binds = append(binds, bps...)
+	}
+
 	engineConfig.SetBindPath(binds)
 	generator.AddProcessEnv("SINGULARITY_BIND", strings.Join(BindPaths, ","))
 

cmd/internal/cli/build.go

@@ -31,6 +31,7 @@ import (
 var buildArgs struct {
 	sections      []string
 	bindPaths     []string
+	mounts        []string
 	arch          string
 	builderURL    string
 	libraryURL    string
@@ -240,6 +241,19 @@ var buildBindFlag = cmdline.Flag{
 		"Multiple bind paths can be given by a comma separated list. (not supported with remote build)",
 }
 
+// --mount
+var buildMountFlag = cmdline.Flag{
+	ID:           "buildMountFlag",
+	Value:        &buildArgs.mounts,
+	DefaultValue: []string{},
+	Name:         "mount",
+	Usage:        "a mount specification e.g. 'type=bind,source=/opt,destination=/hostopt'.",
+	EnvKeys:      []string{"MOUNT"},
+	Tag:          "<spec>",
+	EnvHandler:   cmdline.EnvAppendValue,
+	StringArray:  true,
+}
+
 // --writable-tmpfs
 var buildWritableTmpfsFlag = cmdline.Flag{
 	ID:           "buildWritableTmpfsFlag",
@@ -283,6 +297,7 @@ func init() {
 		cmdManager.RegisterFlagForCmd(&buildNvFlag, buildCmd)
 		cmdManager.RegisterFlagForCmd(&buildRocmFlag, buildCmd)
 		cmdManager.RegisterFlagForCmd(&buildBindFlag, buildCmd)
+		cmdManager.RegisterFlagForCmd(&buildMountFlag, buildCmd)
 		cmdManager.RegisterFlagForCmd(&buildWritableTmpfsFlag, buildCmd)
 	})
 }

cmd/internal/cli/build_linux.go

@@ -115,6 +115,12 @@ func runBuild(cmd *cobra.Command, args []string) {
 		}
 		os.Setenv("SINGULARITY_BINDPATH", strings.Join(buildArgs.bindPaths, ","))
 	}
+	if len(buildArgs.mounts) > 0 {
+		if buildArgs.remote {
+			sylog.Fatalf("--mount option is not supported for remote build")
+		}
+		os.Setenv("SINGULARITY_MOUNT", strings.Join(buildArgs.mounts, "\n"))
+	}
 	if buildArgs.writableTmpfs {
 		if buildArgs.remote {
 			sylog.Fatalf("--writable-tmpfs option is not supported for remote build")

e2e/actions/actions.go

@@ -1070,7 +1070,9 @@ func (c actionTests) actionBinds(t *testing.T) {
 	hostCanaryFileWithColon := filepath.Join(hostCanaryDir, "file:colon")
 
 	canaryFileBind := hostCanaryFile + ":" + contCanaryFile
+	canaryFileMount := "type=bind,source=" + hostCanaryFile + ",destination=" + contCanaryFile
 	canaryDirBind := hostCanaryDir + ":" + contCanaryDir
+	canaryDirMount := "type=bind,source=" + hostCanaryDir + ",destination=" + contCanaryDir
 
 	hostHomeDir := filepath.Join(workspace, "home")
 	hostWorkDir := filepath.Join(workspace, "workdir")
@@ -1512,6 +1514,31 @@ func (c actionTests) actionBinds(t *testing.T) {
 			},
 			exit: 255,
 		},
+		// For the --mount variants we are really just verifying the CLI
+		// acceptance of one or more --mount flags. Translation from --mount
+		// strings to BindPath structs is checked in unit tests. The
+		// functionality of bind mounts of various kinds is already checked
+		// above, with --bind flags. No need to duplicate all of these.
+		{
+			name: "MountSingle",
+			args: []string{
+				"--mount", canaryFileMount,
+				sandbox,
+				"test", "-f", contCanaryFile,
+			},
+			exit: 0,
+		},
+		{
+			name: "MountNested",
+			args: []string{
+				"--mount", canaryDirMount,
+				"--mount", "source=" + hostCanaryFile + ",destination=" + filepath.Join(contCanaryDir, "file3"),
+				sandbox,
+				"test", "-f", "/canary/file3",
+			},
+			postRun: checkHostFile(filepath.Join(hostCanaryDir, "file3")),
+			exit:    0,
+		},
 	}
 
 	for _, profile := range e2e.Profiles {
@@ -2071,6 +2098,33 @@ func (c actionTests) bindImage(t *testing.T) {
 			},
 			exit: 0,
 		},
+		// For the --mount variants we are really just verifying the CLI
+		// acceptance of one or more image bind mount strings. Translation from
+		// --mount strings to BindPath structs is checked in unit tests. The
+		// functionality of image mounts of various kinds is already checked
+		// above, with --bind flags. No need to duplicate all of these.
+		{
+			name:    "MountSifWithID",
+			profile: e2e.UserProfile,
+			args: []string{
+				// rootfs ID is now '4'
+				"--mount", "type=bind,source=" + c.env.ImagePath + ",destination=/rootfs,id=4",
+				c.env.ImagePath,
+				"test", "-d", "/rootfs/etc",
+			},
+			exit: 0,
+		},
+		{
+			name:    "MountSifDataExt3AndSquash",
+			profile: e2e.UserProfile,
+			args: []string{
+				"--mount", "type=bind,source=" + sifExt3Image + ",destination=/ext3,image-src=/",
+				"--mount", "type=bind,source=" + sifSquashImage + ",destination=/squash,image-src=/",
+				c.env.ImagePath,
+				"test", "-f", filepath.Join("/squash", squashMarkerFile), "-a", "-f", "/ext3/ext3_marker",
+			},
+			exit: 0,
+		},
 	}
 
 	for _, tt := range tests {
@@ -2327,10 +2381,10 @@ func E2ETests(env e2e.TestEnv) testhelper.Tests {
 		"issue 5690":            c.issue5690,           // https://github.com/hpcng/singularity/issues/5690
 		"issue 6165":            c.issue6165,           // https://github.com/hpcng/singularity/issues/6165
 		"network":               c.actionNetwork,       // test basic networking
-		"binds":                 c.actionBinds,         // test various binds
+		"binds":                 c.actionBinds,         // test various binds with --bind and --mount
 		"exit and signals":      c.exitSignals,         // test exit and signals propagation
 		"fuse mount":            c.fuseMount,           // test fusemount option
-		"bind image":            c.bindImage,           // test bind image
+		"bind image":            c.bindImage,           // test bind image with --bind and --mount
 		"umask":                 c.actionUmask,         // test umask propagation
 		"no-mount":              c.actionNoMount,       // test --no-mount
 		"compat":                c.actionCompat,        // test --compat

e2e/imgbuild/imgbuild.go

@@ -1333,6 +1333,63 @@ func (c imgBuildTests) buildBindMount(t *testing.T) {
 			},
 			exit: 255,
 		},
+		{
+			name: "Mount test dir to /mnt",
+			buildOption: []string{
+				"--mount", "type=bind,source=" + dir + ",destination=/mnt",
+			},
+			buildPost: []string{
+				"cat /mnt/canary",
+			},
+			buildTest: []string{
+				"cat /mnt/canary",
+			},
+			exit: 0,
+		},
+		{
+			name: "Mount test dir to multiple directory",
+			buildOption: []string{
+				"--mount", "type=bind,source=" + dir + ",destination=/mnt",
+				"--mount", "type=bind,source=" + dir + ",destination=/opt",
+			},
+			buildPost: []string{
+				"cat /mnt/canary",
+				"cat /opt/canary",
+			},
+			buildTest: []string{
+				"cat /mnt/canary",
+				"cat /opt/canary",
+			},
+			exit: 0,
+		},
+		{
+			name: "Mount test dir to /mnt read-only",
+			buildOption: []string{
+				"--mount", "type=bind,source=" + dir + ",destination=/mnt,ro",
+			},
+			buildPost: []string{
+				"mkdir /mnt/should_fail",
+			},
+			exit: 255,
+		},
+		{
+			name: "Mount test dir to non-existent image directory",
+			buildOption: []string{
+				"--mount", "type=bind,source=" + dir + ",destination=/fake/dir",
+			},
+			buildPost: []string{
+				"cat /mnt/canary",
+			},
+			exit: 255,
+		},
+		{
+			name: "Mount test dir with remote",
+			buildOption: []string{
+				"--mount", "type=bind,source=" + dir + ",destination=/mnt",
+				"--remote",
+			},
+			exit: 255,
+		},
 	}
 
 	sandboxImage := filepath.Join(tmpdir, "build-sandbox")

internal/pkg/build/stage.go

@@ -109,7 +109,7 @@ func (s *stage) runPostScript(configFile, sessionResolv, sessionHosts string) er
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stderr
 		cmd.Dir = "/"
-		cmd.Env = currentEnvNoSingularity([]string{"NV", "ROCM", "BINDPATH"})
+		cmd.Env = currentEnvNoSingularity([]string{"NV", "ROCM", "BINDPATH", "MOUNT"})
 
 		sylog.Infof("Running post scriptlet")
 		return cmd.Run()
@@ -135,7 +135,7 @@ func (s *stage) runTestScript(configFile, sessionResolv, sessionHosts string) er
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stderr
 		cmd.Dir = "/"
-		cmd.Env = currentEnvNoSingularity([]string{"NV", "ROCM", "BINDPATH", "WRITABLE_TMPFS"})
+		cmd.Env = currentEnvNoSingularity([]string{"NV", "ROCM", "BINDPATH", "MOUNT", "WRITABLE_TMPFS"})
 
 		sylog.Infof("Running testscript")
 		return cmd.Run()

pkg/cmdline/flag.go

@@ -13,8 +13,6 @@ import (
 	"github.com/spf13/pflag"
 )
 
-type StringArray []string
-
 // Flag holds information about a command flag
 type Flag struct {
 	ID           string
@@ -29,6 +27,10 @@ type Flag struct {
 	Required     bool
 	EnvKeys      []string
 	EnvHandler   EnvHandler
+	// When Value is a []String:
+	// If true, will use pFlag StringArrayVar(P) type, where values are not split on comma.
+	// If false, will use pFlag StringSliceVar(P) type, where a single value is split on commas.
+	StringArray bool
 }
 
 // flagManager manages cobra command flags and store them
@@ -78,9 +80,11 @@ func (m *flagManager) registerFlagForCmd(flag *Flag, cmds ...*cobra.Command) err
 	case string:
 		m.registerStringVar(flag, cmds)
 	case []string:
-		m.registerStringSliceVar(flag, cmds)
-	case StringArray:
-		m.registerStringArrayVar(flag, cmds)
+		if flag.StringArray {
+			m.registerStringArrayVar(flag, cmds)
+		} else {
+			m.registerStringSliceVar(flag, cmds)
+		}
 	case bool:
 		m.registerBoolVar(flag, cmds)
 	case int:
@@ -121,9 +125,9 @@ func (m *flagManager) registerStringSliceVar(flag *Flag, cmds []*cobra.Command)
 func (m *flagManager) registerStringArrayVar(flag *Flag, cmds []*cobra.Command) error {
 	for _, c := range cmds {
 		if flag.ShortHand != "" {
-			c.Flags().StringArrayVarP(flag.Value.(*[]string), flag.Name, flag.ShortHand, ([]string)(flag.DefaultValue.(StringArray)), flag.Usage)
+			c.Flags().StringArrayVarP(flag.Value.(*[]string), flag.Name, flag.ShortHand, flag.DefaultValue.([]string), flag.Usage)
 		} else {
-			c.Flags().StringArrayVar(flag.Value.(*[]string), flag.Name, ([]string)(flag.DefaultValue.(StringArray)), flag.Usage)
+			c.Flags().StringArrayVar(flag.Value.(*[]string), flag.Name, flag.DefaultValue.([]string), flag.Usage)
 		}
 		m.setFlagOptions(flag, c)
 	}

pkg/cmdline/flag_test.go

@@ -17,6 +17,7 @@ var (
 	testString      string
 	testBool        bool
 	testStringSlice []string
+	testStringArray []string
 	testInt         int
 	testUint32      uint32
 )
@@ -151,6 +152,34 @@ var ttData = []struct {
 		},
 		cmd: parentCmd,
 	},
+	{
+		desc: "string array flag",
+		flag: &Flag{
+			ID:           "testStringArrayFlag",
+			Value:        &testStringArray,
+			DefaultValue: testStringArray,
+			Name:         "string-array",
+			Usage:        "a string array flag",
+			EnvKeys:      []string{"STRING_ARRAY"},
+			StringArray:  true,
+		},
+		cmd:      parentCmd,
+		envValue: "arg1,arg2",
+		// Should not split on commas
+		matchValue: "[\"arg1,arg2\"]",
+	},
+	{
+		desc: "string array flag (short)",
+		flag: &Flag{
+			ID:           "testStringArrayShortFlag",
+			Value:        &testStringArray,
+			DefaultValue: testStringArray,
+			Name:         "string-array-short",
+			ShortHand:    "a",
+			Usage:        "a string array flag (short)",
+		},
+		cmd: parentCmd,
+	},
 	{
 		desc: "int flag",
 		flag: &Flag{