oci: implemented CDI device mapping (sylabs#1459)

* oci: implemented CDI device mapping

* first batch of post-comments revisions

* revisions following elezar's comments

* added `--device` to CHANGELOG

* fixed out-of-order calls to addCDIDevices & Update

* added sync.Once getting & refreshing CDI registry

* cdi unit-test stub

* added some more cdi unit-tests

* more unit-tests

* better testing for empty mounts lists

* finishing touches on cdi unit-test

* first stab at cdi e2e-test

* DT's temp fix for userns mapping limitation

* ignore VSCode debugging build targets

* renamed CDI json template file

* added initial deviceNode testing

* changed flag to --cdi-dirs (from --cdidirs)

.gitignore

@@ -115,3 +115,12 @@ debian/singularity-ce*/
 debian/tmp/
 
 .LICENSE_DEPENDENCIES.csv
+
+# VSCode debugging build targets
+__debug_bin
+*/__debug_bin
+*/*/__debug_bin
+*/*/*/__debug_bin
+*/*/*/*/__debug_bin
+*/*/*/*/*/__debug_bin
+*/*/*/*/*/*/__debug_bin

CHANGELOG.md

@@ -16,6 +16,10 @@
 - Added `remote get-login-password` subcommand that allows the user to
   retrieve a CLI token to interact with the OCI registry of a
   Singularity Enterprise instance.
+- Added `--device` flag to "action" commands (`run`/`exec`/`shell`) when run in
+  OCI mode (`--oci`). Currently supports passing one or more (comma-separated)
+  fully-qualified CDI device names, and those devices will then be made
+  available inside the container.
 
 ### Bug Fixes
 

cmd/internal/cli/action_flags.go

@@ -37,6 +37,8 @@ var (
 	singularityEnvFile string
 	noMount            []string
 	proot              string
+	device             []string
+	cdiDirs            []string
 
 	isBoot          bool
 	isFakeroot      bool
@@ -105,7 +107,7 @@ var actionBindFlag = cmdline.Flag{
 	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.",
+	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.",
 	EnvKeys:      []string{"BIND", "BINDPATH"},
 	Tag:          "<spec>",
 	EnvHandler:   cmdline.EnvAppendValue,
@@ -131,7 +133,7 @@ var actionHomeFlag = cmdline.Flag{
 	DefaultValue: CurrentUser.HomeDir,
 	Name:         "home",
 	ShortHand:    "H",
-	Usage:        "a home directory specification.  spec can either be a src path or src:dest pair.  src is the source path of the home directory outside the container and dest overrides the home directory within the container.",
+	Usage:        "a home directory specification. spec can either be a src path or src:dest pair. src is the source path of the home directory outside the container and dest overrides the home directory within the container.",
 	EnvKeys:      []string{"HOME"},
 	Tag:          "<spec>",
 }
@@ -812,6 +814,24 @@ var actionOCIFlag = cmdline.Flag{
 	EnvKeys:      []string{"OCI"},
 }
 
+// --device
+var actionDevice = cmdline.Flag{
+	ID:           "actionDevice",
+	Value:        &device,
+	DefaultValue: []string{},
+	Name:         "device",
+	Usage:        "fully-qualified CDI device name(s). A fully-qualified CDI device name consists of a VENDOR, CLASS, and NAME, which are combined as follows: <VENDOR>/<CLASS>=<NAME> (e.g. vendor.com/device=mydevice). Multiple fully-qualified CDI device names can be given as a comma separated list.",
+}
+
+// --cdi-dirs
+var actionCdiDirs = cmdline.Flag{
+	ID:           "actionCdiDirs",
+	Value:        &cdiDirs,
+	DefaultValue: []string{},
+	Name:         "cdi-dirs",
+	Usage:        "comma-separated list of directories in which CDI should look for device definition JSON files. If omitted, default will be: /etc/cdi,/var/run/cdi",
+}
+
 func init() {
 	addCmdInit(func(cmdManager *cmdline.CommandManager) {
 		cmdManager.RegisterCmd(ExecCmd)
@@ -906,5 +926,7 @@ func init() {
 		cmdManager.RegisterFlagForCmd(&actionSIFFUSEFlag, actionsCmd...)
 		cmdManager.RegisterFlagForCmd(&actionProotFlag, actionsCmd...)
 		cmdManager.RegisterFlagForCmd(&actionOCIFlag, actionsCmd...)
+		cmdManager.RegisterFlagForCmd(&actionDevice, actionsCmd...)
+		cmdManager.RegisterFlagForCmd(&actionCdiDirs, actionsCmd...)
 	})
 }

cmd/internal/cli/actions.go

@@ -388,6 +388,8 @@ func launchContainer(cmd *cobra.Command, image string, containerCmd string, cont
 		launcher.OptKeyInfo(ki),
 		launcher.OptSIFFuse(sifFUSE),
 		launcher.OptCacheDisabled(disableCache),
+		launcher.OptDevice(device),
+		launcher.OptCdiDirs(cdiDirs),
 	}
 
 	// Explicitly use the interface type here, as we will add alternative launchers later...

e2e/actions/actions.go

@@ -2466,5 +2466,6 @@ func E2ETests(env e2e.TestEnv) testhelper.Tests {
 		"ociShell":   c.actionOciShell,   // singularity shell --oci
 		"ociNetwork": c.actionOciNetwork, // singularity exec --oci --net
 		"ociBinds":   c.actionOciBinds,   // singularity exec --oci --bind / --mount
+		"ociCdi":     c.actionOciCdi,     // singularity exec --oci --cdi
 	}
 }

e2e/actions/oci.go

@@ -6,11 +6,16 @@
 package actions
 
 import (
+	"encoding/json"
+	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"
+	"text/template"
 
+	cdispecs "github.com/container-orchestrated-devices/container-device-interface/specs-go"
 	"github.com/sylabs/singularity/e2e/internal/e2e"
 	"github.com/sylabs/singularity/internal/pkg/util/fs"
 )
@@ -476,3 +481,218 @@ func (c actionTests) actionOciBinds(t *testing.T) {
 		})
 	}
 }
+
+func (c actionTests) actionOciCdi(t *testing.T) {
+	// Grab the reference OCI archive we're going to use
+	e2e.EnsureOCIArchive(t, c.env)
+	imageRef := "oci-archive:" + c.env.OCIArchivePath
+
+	// Set up a custom subtestWorkspace object that will holds the collection of temporary directories (nested under the main temporary directory, mainDir) that each test will use.
+	type subtestWorkspace struct {
+		mainDir   string
+		jsonsDir  string
+		mountDirs []string
+	}
+
+	// Create a function to create a fresh subtestWorkspace, with distinct temporary directories, that each individual subtest will use
+	setupIndivSubtestWorkspace := func(t *testing.T, numMountDirs int) *subtestWorkspace {
+		stws := subtestWorkspace{}
+		mainDir, cleanup := e2e.MakeTempDir(t, c.env.TestDir, "", "")
+		t.Cleanup(func() {
+			if !t.Failed() {
+				e2e.Privileged(cleanup)
+			}
+		})
+		stws.mainDir = mainDir
+
+		// No need to do anything with the cleanup functions returned here, because the directories created are all going to be children of (tw.)mainDir, whose cleanup was already registered above.
+		stws.jsonsDir, _ = e2e.MakeTempDir(t, stws.mainDir, "cdi-jsons-", "")
+		stws.mountDirs = make([]string, 0, numMountDirs)
+		for len(stws.mountDirs) < numMountDirs {
+			dir, _ := e2e.MakeTempDir(t, stws.mainDir, fmt.Sprintf("mount-dir-%d-", len(stws.mountDirs)+1), "")
+			// Make writable to all, due to current nested userns mapping restrictions.
+			// Will work without this once crun-specific single mapping is present.
+			os.Chmod(dir, 0o777)
+			stws.mountDirs = append(stws.mountDirs, dir)
+		}
+
+		return &stws
+	}
+
+	// Set up the JSON template that we're going to populate on a per-subtest basis with particular CDI spec values
+	e2eMountTemplateFilename := "cditemplate.json.tpl"
+	cdiJSONTemplateFilePath := filepath.Join("..", "test", "cdi", e2eMountTemplateFilename)
+	funcMap := template.FuncMap{
+		// The name "title" is what the function will be called in the template text.
+		"tojson": func(o any) string {
+			s, _ := json.Marshal(o)
+			return string(s)
+		},
+	}
+	cdiJSONTemplate, err := template.New(e2eMountTemplateFilename).Funcs(funcMap).ParseFiles(cdiJSONTemplateFilePath)
+	if err != nil {
+		t.Errorf("Could not read JSON template for CDI e2e tests from file %#v", cdiJSONTemplateFilePath)
+		return
+	}
+
+	// The set of actual subtests
+	var wantUID uint32 = 1000
+	var wantGID uint32 = 1000
+	tests := []struct {
+		name        string
+		devices     []string
+		wantExit    int
+		postRun     func(t *testing.T)
+		DeviceNodes []cdispecs.DeviceNode
+		Mounts      []cdispecs.Mount
+		Env         []string
+	}{
+		{
+			name: "ValidMounts",
+			devices: []string{
+				"singularityCEtesting.sylabs.io/device=TesterDevice",
+			},
+			wantExit:    0,
+			DeviceNodes: []cdispecs.DeviceNode{},
+			Mounts: []cdispecs.Mount{
+				{
+					ContainerPath: "/tmp/mount1",
+					Options:       []string{"rw", "bind", "users"},
+				},
+				{
+					ContainerPath: "/tmp/mount3",
+					Options:       []string{"rw", "bind", "users"},
+				},
+				{
+					ContainerPath: "/tmp/mount13",
+					Options:       []string{"rw", "bind", "users"},
+				},
+				{
+					ContainerPath: "/tmp/mount17",
+					Options:       []string{"rw", "bind", "users"},
+				},
+			},
+			Env: []string{
+				"ABCD=QWERTY",
+				"EFGH=ASDFGH",
+				"IJKL=ZXCVBN",
+			},
+		},
+		{
+			name: "InvalidDevice",
+			devices: []string{
+				"singularityCEtesting.sylabs.io/device=DoesNotExist",
+			},
+			wantExit:    255,
+			DeviceNodes: []cdispecs.DeviceNode{},
+			Mounts:      []cdispecs.Mount{},
+			Env:         []string{},
+		},
+		{
+			name: "KmsgDevice",
+			devices: []string{
+				"singularityCEtesting.sylabs.io/device=TesterDevice",
+			},
+			wantExit: 0,
+			DeviceNodes: []cdispecs.DeviceNode{
+				{
+					HostPath:    "/dev/kmsg",
+					Path:        "/dev/kmsg",
+					Permissions: "rw",
+					Type:        "c",
+					UID:         &wantUID,
+					GID:         &wantGID,
+				},
+			},
+		},
+	}
+
+	for _, profile := range e2e.OCIProfiles {
+		t.Run(profile.String(), func(t *testing.T) {
+			for _, tt := range tests {
+				t.Run(tt.name, func(t *testing.T) {
+					stws := setupIndivSubtestWorkspace(t, len(tt.Mounts))
+
+					// Populate the HostPath values we're going to feed into the CDI JSON template, based on the subtestWorkspace we just created
+					for i, d := range stws.mountDirs {
+						tt.Mounts[i].HostPath = d
+					}
+
+					// Inject this subtest's values into the template to create the CDI JSON file
+					cdiJSONFilePath := filepath.Join(stws.jsonsDir, fmt.Sprintf("%s-cdi.json", tt.name))
+					cdiJSONFile, err := os.OpenFile(cdiJSONFilePath, os.O_CREATE|os.O_WRONLY, 0o644)
+					if err != nil {
+						t.Errorf("could not create file %#v for writing CDI JSON: %v", cdiJSONFilePath, err)
+					}
+					if err = cdiJSONTemplate.Execute(cdiJSONFile, tt); err != nil {
+						t.Errorf("error executing template %#v to create CDI JSON: %v", cdiJSONTemplateFilePath, err)
+						return
+					}
+					cdiJSONFile.Close()
+
+					// Create a list of test strings, each of which will be echoed into a separate file in a separate mount in the container.
+					testfileStrings := make([]string, 0, len(tt.Mounts))
+					for i := range tt.Mounts {
+						testfileStrings = append(testfileStrings, fmt.Sprintf("test_string_for_mount_%d_in_test_%s", i, tt.name))
+					}
+
+					// Generate the command to be executed in the container
+					// Start by printing all environment variables, to test using e2e.ContainMatch conditions later
+					execCmd := "/bin/env"
+					for _, d := range tt.DeviceNodes {
+						testFlag := "-f"
+						switch d.Type {
+						case "c":
+							testFlag = "-c"
+						}
+						execCmd += fmt.Sprintf(" && test %s %s", testFlag, d.Path)
+					}
+					for i, m := range tt.Mounts {
+						// Add a separate teststring echo statement for each mount
+						execCmd += fmt.Sprintf(" && echo %s > %s/testfile_%d", testfileStrings[i], m.ContainerPath, i)
+					}
+
+					// Create a postRun function to check that the testfiles written to the container mounts made their way to the right host temporary directories
+					testMountsAndEnv := func(t *testing.T) {
+						for i, m := range tt.Mounts {
+							testfileFilename := filepath.Join(m.HostPath, fmt.Sprintf("testfile_%d", i))
+							b, err := os.ReadFile(testfileFilename)
+							if err != nil {
+								t.Errorf("could not read testfile %s", testfileFilename)
+								return
+							}
+
+							s := string(b)
+							if s != testfileStrings[i]+"\n" {
+								t.Errorf("mismatched testfileString; expected %#v, got %#v (mount: %#v)", s, testfileStrings[i], m)
+							}
+						}
+					}
+
+					// Create a set of e2e.SingularityCmdResultOp objects to test that environment variables have been correctly injected into the container
+					envExpects := make([]e2e.SingularityCmdResultOp, 0, len(tt.Env))
+					for _, e := range tt.Env {
+						envExpects = append(envExpects, e2e.ExpectOutput(e2e.ContainMatch, e))
+					}
+
+					c.env.RunSingularity(
+						t,
+						e2e.AsSubtest(tt.name),
+						e2e.WithCommand("exec"),
+						e2e.WithArgs(
+							"--device",
+							strings.Join(tt.devices, ","),
+							"--cdi-dirs",
+							stws.jsonsDir,
+							imageRef,
+							"/bin/sh", "-c", execCmd),
+						e2e.WithProfile(profile),
+						e2e.ExpectExit(tt.wantExit, envExpects...),
+						e2e.PostRun(tt.postRun),
+						e2e.PostRun(testMountsAndEnv),
+					)
+				})
+			}
+		})
+	}
+}

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/blang/semver/v4 v4.0.0
 	github.com/buger/goterm v1.0.4
 	github.com/buger/jsonparser v1.1.1
+	github.com/container-orchestrated-devices/container-device-interface v0.5.4
 	github.com/containerd/containerd v1.7.0
 	github.com/containernetworking/cni v1.1.2
 	github.com/containernetworking/plugins v1.2.0
@@ -33,6 +34,7 @@ require (
 	github.com/opencontainers/umoci v0.4.7
 	github.com/pelletier/go-toml/v2 v2.0.7
 	github.com/pkg/errors v0.9.1
+	github.com/samber/lo v1.38.1
 	github.com/seccomp/libseccomp-golang v0.10.0
 	github.com/shopspring/decimal v1.3.1
 	github.com/sigstore/sigstore v1.6.0
@@ -71,7 +73,6 @@ require (
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/cilium/ebpf v0.9.1 // indirect
 	github.com/cloudflare/circl v1.1.0 // indirect
-	github.com/container-orchestrated-devices/container-device-interface v0.5.4 // indirect
 	github.com/containers/libtrust v0.0.0-20230121012942-c1716e8a8d01 // indirect
 	github.com/containers/ocicrypt v1.1.7 // indirect
 	github.com/containers/storage v1.45.3 // indirect
@@ -174,6 +175,7 @@ require (
 	go.mozilla.org/pkcs7 v0.0.0-20210826202110-33d05740a352 // indirect
 	go.opentelemetry.io/otel v1.14.0 // indirect
 	go.opentelemetry.io/otel/trace v1.14.0 // indirect
+	golang.org/x/exp v0.0.0-20220823124025-807a23277127 // indirect
 	golang.org/x/mod v0.8.0 // indirect
 	golang.org/x/net v0.8.0 // indirect
 	golang.org/x/sync v0.1.0 // indirect

go.sum

@@ -592,6 +592,8 @@ github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
 github.com/safchain/ethtool v0.2.0 h1:dILxMBqDnQfX192cCAPjZr9v2IgVXeElHPy435Z/IdE=
 github.com/safchain/ethtool v0.2.0/go.mod h1:WkKB1DnNtvsMlDmQ50sgwowDJV/hGbJSOvJoEXs1AJQ=
+github.com/samber/lo v1.38.1 h1:j2XEAqXKb09Am4ebOg31SpvzUTTs6EN3VfgeLUhPdXM=
+github.com/samber/lo v1.38.1/go.mod h1:+m/ZKRl6ClXCE2Lgf3MsQlWfh4bn1bz6CXEOxnEXnEA=
 github.com/sebdah/goldie/v2 v2.5.3 h1:9ES/mNN+HNUbNWpVAlrzuZ7jE+Nrczbj8uFRjM7624Y=
 github.com/seccomp/libseccomp-golang v0.9.2-0.20220502022130-f33da4d89646/go.mod h1:JA8cRccbGaA1s33RQf7Y1+q9gHmZX1yB/z9WDN1C6fg=
 github.com/seccomp/libseccomp-golang v0.10.0 h1:aA4bp+/Zzi0BnWZ2F1wgNBs5gTpm+na2rWM6M9YjLpY=
@@ -764,6 +766,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
 golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
+golang.org/x/exp v0.0.0-20220823124025-807a23277127 h1:S4NrSKDfihhl3+4jSTgwoIevKxX9p7Iv9x++OEIptDo=
+golang.org/x/exp v0.0.0-20220823124025-807a23277127/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE=
 golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
 golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=

internal/pkg/runtime/launcher/native/launcher_linux.go

@@ -70,6 +70,13 @@ func NewLauncher(opts ...launcher.Option) (*Launcher, error) {
 			return nil, fmt.Errorf("%w", err)
 		}
 	}
+	if len(lo.Devices) > 0 {
+		return nil, fmt.Errorf("CDI device mappings unsupported in native launcher")
+	}
+
+	if len(lo.CdiDirs) > 0 {
+		return nil, fmt.Errorf("CDI device mappings unsupported in native launcher")
+	}
 
 	// Initialize empty default Singularity Engine and OCI configuration
 	engineConfig := singularityConfig.NewConfig()