[no-relnote] Refactor creation of discoverer for tegra systems

This change updates the way we construct a discoverer for tegra systems
to be more flexible in terms of how the SOURCES of the mount specs can
be specified. This allows for subsequent changes like adding (or removing)
mount specs at the point of construction.

Signed-off-by: Evan Lezar <[email protected]>

Author: elezar

internal/platform-support/tegra/csv.go

@@ -17,36 +17,30 @@
 package tegra
 
 import (
-	"fmt"
-
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/platform-support/tegra/csv"
 )
 
-// newDiscovererFromCSVFiles creates a discoverer for the specified CSV files. A logger is also supplied.
-// The constructed discoverer is comprised of a list, with each element in the list being associated with a
-// single CSV files.
-func (o tegraOptions) newDiscovererFromCSVFiles() (discover.Discover, error) {
-	if len(o.csvFiles) == 0 {
-		o.logger.Warningf("No CSV files specified")
+func (o options) newDiscovererFromMountSpecs() (discover.Discover, error) {
+	pathsByType := o.MountSpecPathsByType()
+	if len(pathsByType) == 0 {
+		o.logger.Warningf("No mount specs specified")
 		return discover.None{}, nil
 	}
 
-	targetsByType := getTargetsFromCSVFiles(o.logger, o.csvFiles)
-
 	devices := discover.NewCharDeviceDiscoverer(
 		o.logger,
 		o.devRoot,
-		targetsByType[csv.MountSpecDev],
+		pathsByType[csv.MountSpecDev],
 	)
 
 	directories := discover.NewMounts(
 		o.logger,
 		lookup.NewDirectoryLocator(lookup.WithLogger(o.logger), lookup.WithRoot(o.driverRoot)),
 		o.driverRoot,
-		targetsByType[csv.MountSpecDir],
+		pathsByType[csv.MountSpecDir],
 	)
 
 	// We create a discoverer for mounted libraries and add additional .so
@@ -57,14 +51,14 @@ func (o tegraOptions) newDiscovererFromCSVFiles() (discover.Discover, error) {
 			o.logger,
 			o.symlinkLocator,
 			o.driverRoot,
-			targetsByType[csv.MountSpecLib],
+			pathsByType[csv.MountSpecLib],
 		),
 		"",
 		o.hookCreator,
 	)
 
 	// We process the explicitly requested symlinks.
-	symlinkTargets := o.ignorePatterns.Apply(targetsByType[csv.MountSpecSym]...)
+	symlinkTargets := pathsByType[csv.MountSpecSym]
 	o.logger.Debugf("Filtered symlink targets: %v", symlinkTargets)
 	symlinks := discover.NewMounts(
 		o.logger,
@@ -85,35 +79,34 @@ func (o tegraOptions) newDiscovererFromCSVFiles() (discover.Discover, error) {
 	return d, nil
 }
 
-// getTargetsFromCSVFiles returns the list of mount specs from the specified CSV files.
-// These are aggregated by mount spec type.
-// TODO: We use a function variable here to allow this to be overridden for testing.
-// This should be properly mocked.
-var getTargetsFromCSVFiles = func(logger logger.Interface, files []string) map[csv.MountSpecType][]string {
-	targetsByType := make(map[csv.MountSpecType][]string)
-	for _, filename := range files {
-		targets, err := loadCSVFile(logger, filename)
-		if err != nil {
-			logger.Warningf("Skipping CSV file %v: %v", filename, err)
-			continue
-		}
-		for _, t := range targets {
-			targetsByType[t.Type] = append(targetsByType[t.Type], t.Path)
-		}
+// MountSpecsFromCSVFiles returns a MountSpecPathsByTyper for the specified list
+// of CSV files.
+func MountSpecsFromCSVFiles(logger logger.Interface, csvFiles ...string) MountSpecPathsByTyper {
+	var tts []MountSpecPathsByTyper
+
+	for _, filename := range csvFiles {
+		tts = append(tts, &fromCSVFile{logger, filename})
 	}
-	return targetsByType
+	return Merge(tts...)
 }
 
-// loadCSVFile loads the specified CSV file and returns the list of mount specs
-func loadCSVFile(logger logger.Interface, filename string) ([]*csv.MountSpec, error) {
+type fromCSVFile struct {
+	logger   logger.Interface
+	filename string
+}
+
+// MountSpecPathsByType returns mountspecs defined in the specified CSV file.
+func (t *fromCSVFile) MountSpecPathsByType() MountSpecPathsByType {
 	// Create a discoverer for each file-kind combination
-	targets, err := csv.NewCSVFileParser(logger, filename).Parse()
+	targets, err := csv.NewCSVFileParser(t.logger, t.filename).Parse()
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse CSV file: %v", err)
-	}
-	if len(targets) == 0 {
-		return nil, fmt.Errorf("CSV file is empty")
+		t.logger.Warningf("failed to parse CSV file %v: %v", t.filename, err)
+		return nil
 	}
 
-	return targets, nil
+	targetsByType := make(MountSpecPathsByType)
+	for _, t := range targets {
+		targetsByType[t.Type] = append(targetsByType[t.Type], t.Path)
+	}
+	return targetsByType
 }

internal/platform-support/tegra/csv_test.go

@@ -24,7 +24,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
-	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup"
 
 	"github.com/NVIDIA/nvidia-container-toolkit/internal/platform-support/tegra/csv"
@@ -34,7 +33,7 @@ func TestDiscovererFromCSVFiles(t *testing.T) {
 	logger, _ := testlog.NewNullLogger()
 	testCases := []struct {
 		description         string
-		moutSpecs           map[csv.MountSpecType][]string
+		moutSpecs           MountSpecPathsByType
 		ignorePatterns      []string
 		symlinkLocator      lookup.Locator
 		symlinkChainLocator lookup.Locator
@@ -186,19 +185,19 @@ func TestDiscovererFromCSVFiles(t *testing.T) {
 	hookCreator := discover.NewHookCreator()
 	for _, tc := range testCases {
 		t.Run(tc.description, func(t *testing.T) {
-			defer setGetTargetsFromCSVFiles(tc.moutSpecs)()
-
-			o := tegraOptions{
-				logger:              logger,
-				hookCreator:         hookCreator,
-				csvFiles:            []string{"dummy"},
-				ignorePatterns:      tc.ignorePatterns,
+			o := options{
+				logger:      logger,
+				hookCreator: hookCreator,
+				MountSpecPathsByTyper: Filter(
+					tc.moutSpecs,
+					Symlinks(tc.ignorePatterns...),
+				),
 				symlinkLocator:      tc.symlinkLocator,
 				symlinkChainLocator: tc.symlinkChainLocator,
 				resolveSymlink:      tc.symlinkResolver,
 			}
 
-			d, err := o.newDiscovererFromCSVFiles()
+			d, err := o.newDiscovererFromMountSpecs()
 			require.ErrorIs(t, err, tc.expectedError)
 
 			hooks, err := d.Hooks()
@@ -212,14 +211,3 @@ func TestDiscovererFromCSVFiles(t *testing.T) {
 		})
 	}
 }
-
-func setGetTargetsFromCSVFiles(override map[csv.MountSpecType][]string) func() {
-	original := getTargetsFromCSVFiles
-	getTargetsFromCSVFiles = func(logger logger.Interface, files []string) map[csv.MountSpecType][]string {
-		return override
-	}
-
-	return func() {
-		getTargetsFromCSVFiles = original
-	}
-}

internal/platform-support/tegra/mount_specs.go

@@ -0,0 +1,113 @@
+/**
+# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+**/
+
+package tegra
+
+import "github.com/NVIDIA/nvidia-container-toolkit/internal/platform-support/tegra/csv"
+
+// A MountSpecPathsByTyper provides a function to return mount specs paths by
+// mount type.
+// The MountSpecTypes are one of: dev, dir, lib, sym and define how these should
+// be included in a container (or represented in the associated CDI spec).
+type MountSpecPathsByTyper interface {
+	MountSpecPathsByType() MountSpecPathsByType
+}
+
+type MountSpecPathsByType map[csv.MountSpecType][]string
+
+var _ MountSpecPathsByTyper = (MountSpecPathsByType)(nil)
+
+// MountSpecPathsByType for a variable of type MountSpecPathsByType returns the
+// underlying data structure.
+// This allows for using this type in functions such as Merge and Filter.
+func (m MountSpecPathsByType) MountSpecPathsByType() MountSpecPathsByType {
+	return m
+}
+
+type merge []MountSpecPathsByTyper
+
+// Merge combines the MountSpecPathsByType for the specified sources.
+func Merge(sources ...MountSpecPathsByTyper) MountSpecPathsByTyper {
+	return merge(sources)
+}
+
+// MountSpecPathsByType for a set of merged mount specs combines the list of
+// paths per type.
+func (ts merge) MountSpecPathsByType() MountSpecPathsByType {
+	targetsByType := make(MountSpecPathsByType)
+	for _, t := range ts {
+		if t == nil {
+			continue
+		}
+		for tType, targets := range t.MountSpecPathsByType() {
+			targetsByType[tType] = append(targetsByType[tType], targets...)
+		}
+	}
+	return targetsByType
+}
+
+type filterMountSpecs struct {
+	from   MountSpecPathsByTyper
+	remove MountSpecPathsByTyper
+}
+
+// Filter removes the specified MountSpecPaths (by type) from the specified
+// set of MountSpecPaths.
+// Here the paths in the remove set are treated as patterns, and elements in
+// from that match any specified pattern are filtered out.
+func Filter(from MountSpecPathsByTyper, remove MountSpecPathsByTyper) MountSpecPathsByTyper {
+	return filterMountSpecs{
+		from:   from,
+		remove: remove,
+	}
+}
+
+// MountSpecPathsByType for a filter get the mountspecs defined in the source
+// and apply the specified per-type filters.
+func (m filterMountSpecs) MountSpecPathsByType() MountSpecPathsByType {
+	ms := m.from.MountSpecPathsByType()
+	if len(ms) == 0 {
+		return ms
+	}
+
+	for t, patterns := range m.remove.MountSpecPathsByType() {
+		paths := ms[t]
+		if len(paths) == 0 {
+			continue
+		}
+		filtered := ignoreMountSpecPatterns(patterns).Apply(paths...)
+		ms[t] = filtered
+	}
+
+	return ms
+}
+
+// DeviceNodes creates a set of MountSpecPaths for the specified device nodes.
+// These have the MoutSpecDev type.
+func DeviceNodes(dn ...string) MountSpecPathsByTyper {
+	return MountSpecPathsByType{
+		csv.MountSpecDev: dn,
+	}
+}
+
+// DeviceNodes creates a set of MountSpecPaths for the specified symlinks.
+// These have the MountSpecSym type.
+func Symlinks(s ...string) MountSpecPathsByTyper {
+	return MountSpecPathsByType{
+		csv.MountSpecSym: s,
+	}
+}

internal/platform-support/tegra/options.go

@@ -0,0 +1,105 @@
+/**
+# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+**/
+
+package tegra
+
+import (
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/discover"
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/logger"
+	"github.com/NVIDIA/nvidia-container-toolkit/internal/lookup"
+)
+
+type options struct {
+	logger             logger.Interface
+	driverRoot         string
+	devRoot            string
+	hookCreator        discover.HookCreator
+	ldconfigPath       string
+	librarySearchPaths []string
+
+	// The following can be overridden for testing
+	symlinkLocator      lookup.Locator
+	symlinkChainLocator lookup.Locator
+	// TODO: This should be replaced by a regular mock
+	resolveSymlink func(string) (string, error)
+
+	MountSpecPathsByTyper
+}
+
+// Option defines a functional option for configuring a Tegra discoverer.
+type Option func(*options)
+
+// WithLogger sets the logger for the discoverer.
+func WithLogger(logger logger.Interface) Option {
+	return func(o *options) {
+		o.logger = logger
+	}
+}
+
+// WithDriverRoot sets the driver root for the discoverer.
+func WithDriverRoot(driverRoot string) Option {
+	return func(o *options) {
+		o.driverRoot = driverRoot
+	}
+}
+
+// WithDevRoot sets the /dev root.
+// If this is unset, the driver root is assumed.
+func WithDevRoot(devRoot string) Option {
+	return func(o *options) {
+		o.devRoot = devRoot
+	}
+}
+
+// WithHookCreator sets the hook creator for the discoverer.
+func WithHookCreator(hookCreator discover.HookCreator) Option {
+	return func(o *options) {
+		o.hookCreator = hookCreator
+	}
+}
+
+// WithLdconfigPath sets the path to the ldconfig program
+func WithLdconfigPath(ldconfigPath string) Option {
+	return func(o *options) {
+		o.ldconfigPath = ldconfigPath
+	}
+}
+
+// WithLibrarySearchPaths sets the library search paths for the discoverer.
+func WithLibrarySearchPaths(librarySearchPaths ...string) Option {
+	return func(o *options) {
+		o.librarySearchPaths = librarySearchPaths
+	}
+}
+
+// WithMountSpecsByPath sets the source of MountSpec paths per type.
+// If multiple values are supplied, these are merged.
+func WithMountSpecsByPath(msfp ...MountSpecPathsByTyper) Option {
+	return func(o *options) {
+		o.MountSpecPathsByTyper = Merge(msfp...)
+	}
+}
+
+// MountSpecPathsByType returns the mounts specs by path configured for these
+// options.
+// For an unconfigured MountSpecPathsByTyper no mountspecs are returned.
+func (o options) MountSpecPathsByType() MountSpecPathsByType {
+	if o.MountSpecPathsByTyper == nil {
+		return nil
+	}
+	return o.MountSpecPathsByTyper.MountSpecPathsByType()
+}