test: Improve csv modifier tests

Signed-off-by: Evan Lezar <elezar@nvidia.com>

Author: elezar

internal/modifier/cdi/spec.go

@@ -20,29 +20,51 @@ import (
 	"fmt"
 
 	"github.com/opencontainers/runtime-spec/specs-go"
-	"tags.cncf.io/container-device-interface/pkg/cdi"
+	cdiapi "tags.cncf.io/container-device-interface/pkg/cdi"
+	cdi "tags.cncf.io/container-device-interface/specs-go"
 
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/devices"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/oci"
 )
 
 // fromCDISpec represents the modifications performed from a raw CDI spec.
 type fromCDISpec struct {
-	cdiSpec *cdi.Spec
+	cdiSpec *cdiapi.Spec
 }
 
 var _ oci.SpecModifier = (*fromCDISpec)(nil)
 
 // Modify applies the mofiications defined by the raw CDI spec to the incomming OCI spec.
 func (m fromCDISpec) Modify(spec *specs.Spec) error {
 	for _, device := range m.cdiSpec.Devices {
-		device := device
-		cdiDevice := cdi.Device{
+		device := m.enrichDevice(device)
+		cdiDevice := cdiapi.Device{
 			Device: &device,
 		}
 		if err := cdiDevice.ApplyEdits(spec); err != nil {
-			return fmt.Errorf("failed to apply edits for device %q: %v", cdiDevice.GetQualifiedName(), err)
+			return fmt.Errorf("failed to apply edits for device %q: %v", m.cdiSpec.Kind+"="+device.Name, err)
 		}
 	}
 
 	return m.cdiSpec.ApplyEdits(spec)
 }
+
+func (m fromCDISpec) enrichDevice(device cdi.Device) cdi.Device {
+	if !devices.IsOverrideApplied() {
+		return device
+	}
+	// For testing we need to override the device node information to ensure
+	// that we don't trigger the CDI modification that requires the device node
+	// to exist and be a character device.
+	// The following condition is used to determine whether a failure to get
+	// the info is fatal:
+	// hasMinimalSpecification := d.Type != "" && (d.Major != 0 || d.Type == fifoDevice)
+	for i, dn := range device.ContainerEdits.DeviceNodes {
+		dn.Type = "c"
+		if dn.Major == 0 {
+			dn.Major = 99
+		}
+		device.ContainerEdits.DeviceNodes[i] = dn
+	}
+	return device
+}

internal/modifier/csv_test.go

@@ -17,40 +17,89 @@
 package modifier
 
 import (
+	"path/filepath"
 	"testing"
 
+	"github.com/opencontainers/runtime-spec/specs-go"
 	testlog "github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/require"
 
 	"github.com/NVIDIA/nvidia-container-toolkit/api/config/v1"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/config/image"
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/devices"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup/root"
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/test"
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/test/to"
 )
 
 func TestNewCSVModifier(t *testing.T) {
 	logger, _ := testlog.NewNullLogger()
 
+	defer devices.SetAllForTest()()
+
+	moduleRoot, err := test.GetModuleRoot()
+	require.NoError(t, err)
+
+	lookupRoot := filepath.Join(moduleRoot, "testdata", "lookup")
+
 	testCases := []struct {
-		description   string
-		cfg           *config.Config
-		envmap        map[string]string
-		expectedError error
-		expectedNil   bool
+		description         string
+		cfg                 config.Config
+		envmap              map[string]string
+		driverRootfs        string
+		devRootfs           string
+		expectedErrorString string
+		expectedSpec        specs.Spec
 	}{
 		{
 			description: "visible devices not set returns nil",
 			envmap:      map[string]string{},
-			expectedNil: true,
 		},
 		{
 			description: "visible devices empty returns nil",
 			envmap:      map[string]string{"NVIDIA_VISIBLE_DEVICES": ""},
-			expectedNil: true,
 		},
 		{
 			description: "visible devices 'void' returns nil",
 			envmap:      map[string]string{"NVIDIA_VISIBLE_DEVICES": "void"},
-			expectedNil: true,
+		},
+		{
+			description:  "visible devices all",
+			envmap:       map[string]string{"NVIDIA_VISIBLE_DEVICES": "all"},
+			driverRootfs: "rootfs-orin",
+			expectedSpec: specs.Spec{
+				Process: &specs.Process{
+					Env: []string{"NVIDIA_VISIBLE_DEVICES=void"},
+				},
+				Mounts: []specs.Mount{
+					{Source: "/usr/lib/aarch64-linux-gnu/nvidia/libcuda.so.1.1", Destination: "/usr/lib/aarch64-linux-gnu/nvidia/libcuda.so.1.1", Options: []string{"ro", "nosuid", "nodev", "rbind", "rprivate"}},
+					{Source: "/usr/lib/aarch64-linux-gnu/nvidia/libnvidia-ml.so.1", Destination: "/usr/lib/aarch64-linux-gnu/nvidia/libnvidia-ml.so.1", Options: []string{"ro", "nosuid", "nodev", "rbind", "rprivate"}},
+				},
+				Hooks: &specs.Hooks{
+					CreateContainer: []specs.Hook{
+						{
+							Path: "/usr/bin/nvidia-cdi-hook",
+							Args: []string{"nvidia-cdi-hook", "create-symlinks", "--link", "libcuda.so.1::/usr/lib/aarch64-linux-gnu/nvidia/libcuda.so"},
+							Env:  []string{"NVIDIA_CTK_DEBUG=false"},
+						},
+						{
+							Path: "/usr/bin/nvidia-cdi-hook",
+							Args: []string{"nvidia-cdi-hook", "update-ldcache", "--folder", "/usr/lib/aarch64-linux-gnu/nvidia"},
+							Env:  []string{"NVIDIA_CTK_DEBUG=false"},
+						},
+					},
+				},
+				Linux: &specs.Linux{
+					Devices: []specs.LinuxDevice{
+						{Path: "/dev/nvidia0", Type: "c", Major: 99},
+					},
+					Resources: &specs.LinuxResources{
+						Devices: []specs.LinuxDeviceCgroup{
+							{Allow: true, Type: "c", Major: to.Ptr[int64](99), Minor: to.Ptr[int64](0), Access: "rwm"},
+						},
+					},
+				},
+			},
 		},
 	}
 
@@ -59,24 +108,41 @@ func TestNewCSVModifier(t *testing.T) {
 			image, _ := image.New(
 				image.WithEnvMap(tc.envmap),
 			)
+			driverRoot := tc.driverRootfs
+			if driverRoot != "" {
+				driverRoot = filepath.Join(lookupRoot, tc.driverRootfs)
+			}
+			devRoot := tc.devRootfs
+			if devRoot != "" {
+				devRoot = filepath.Join(lookupRoot, tc.devRootfs)
+			}
+			driver := root.New(root.WithDriverRoot(driverRoot), root.WithDevRoot(devRoot))
+			// TODO: We should point to the factory root here.
+			tc.cfg.NVIDIAContainerCLIConfig.Root = driverRoot
+			// Override the CSV file search path for this root.
+			tc.cfg.NVIDIAContainerRuntimeConfig.Modes.CSV.MountSpecPath = filepath.Join(driverRoot, "/etc/nvidia-container-runtime/host-files-for-container.d")
+			tc.cfg.NVIDIACTKConfig.Path = "/usr/bin/nvidia-cdi-hook"
+
 			f := createFactory(
 				WithLogger(logger),
-				WithDriver(root.New()),
-				WithConfig(tc.cfg),
+				WithDriver(driver),
+				WithLogger(logger),
+				WithConfig(&tc.cfg),
 				WithImage(&image),
 			)
 			m, err := f.newCSVModifier()
-			if tc.expectedError != nil {
-				require.Error(t, err)
-			} else {
+			if tc.expectedErrorString == "" {
 				require.NoError(t, err)
-			}
-
-			if tc.expectedNil || tc.expectedError != nil {
-				require.Nil(t, m)
 			} else {
-				require.NotNil(t, m)
+				require.EqualError(t, err, tc.expectedErrorString)
+				return
 			}
+
+			s := specs.Spec{}
+			err = list{m}.Modify(&s)
+			require.NoError(t, err)
+
+			require.EqualValues(t, tc.expectedSpec, test.StripRoot(s, driverRoot))
 		})
 	}
 }