mirror of
				https://github.com/NVIDIA/nvidia-container-toolkit
				synced 2025-06-26 18:18:24 +00:00 
			
		
		
		
	Refactor CSV discovery to make char device discovery clearer
Signed-off-by: Evan Lezar <elezar@nvidia.com>
This commit is contained in:
		
							parent
							
								
									7ec3cd0b5b
								
							
						
					
					
						commit
						2c1e356370
					
				| @ -24,13 +24,10 @@ import ( | ||||
| 	"github.com/sirupsen/logrus" | ||||
| ) | ||||
| 
 | ||||
| type csvDiscoverer struct { | ||||
| 	mounts | ||||
| 	filename  string | ||||
| 	mountType csv.MountSpecType | ||||
| } | ||||
| // charDevices is a discover for a list of character devices
 | ||||
| type charDevices mounts | ||||
| 
 | ||||
| var _ Discover = (*csvDiscoverer)(nil) | ||||
| var _ Discover = (*charDevices)(nil) | ||||
| 
 | ||||
| // NewFromCSVFiles 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
 | ||||
| @ -76,23 +73,17 @@ func NewFromCSVFile(logger *logrus.Logger, locators map[csv.MountSpecType]lookup | ||||
| 		return nil, fmt.Errorf("CSV file is empty") | ||||
| 	} | ||||
| 
 | ||||
| 	csvDiscoverers, err := newFromMountSpecs(logger, locators, targets) | ||||
| 	if err != nil { | ||||
| 		return nil, err | ||||
| 	} | ||||
| 	var discoverers []Discover | ||||
| 	for _, d := range csvDiscoverers { | ||||
| 		d.filename = filename | ||||
| 		discoverers = append(discoverers, d) | ||||
| 	} | ||||
| 
 | ||||
| 	return &list{discoverers: discoverers}, nil | ||||
| 	return newFromMountSpecs(logger, locators, targets) | ||||
| } | ||||
| 
 | ||||
| // newFromMountSpecs creates a discoverer for the CSV file. A logger is also supplied.
 | ||||
| // A list of csvDiscoverers is returned, with each being associated with a single MountSpecType.
 | ||||
| func newFromMountSpecs(logger *logrus.Logger, locators map[csv.MountSpecType]lookup.Locator, targets []*csv.MountSpec) ([]*csvDiscoverer, error) { | ||||
| 	var discoverers []*csvDiscoverer | ||||
| func newFromMountSpecs(logger *logrus.Logger, locators map[csv.MountSpecType]lookup.Locator, targets []*csv.MountSpec) (Discover, error) { | ||||
| 	if len(targets) == 0 { | ||||
| 		return &None{}, nil | ||||
| 	} | ||||
| 
 | ||||
| 	var discoverers []Discover | ||||
| 	candidatesByType := make(map[csv.MountSpecType][]string) | ||||
| 	for _, t := range targets { | ||||
| 		candidatesByType[t.Type] = append(candidatesByType[t.Type], t.Path) | ||||
| @ -103,45 +94,41 @@ func newFromMountSpecs(logger *logrus.Logger, locators map[csv.MountSpecType]loo | ||||
| 		if !exists { | ||||
| 			return nil, fmt.Errorf("no locator defined for '%v'", t) | ||||
| 		} | ||||
| 		d := csvDiscoverer{ | ||||
| 			mounts: mounts{ | ||||
| 				logger:   logger, | ||||
| 				lookup:   locator, | ||||
| 				required: candidates, | ||||
| 			}, | ||||
| 			mountType: t, | ||||
| 
 | ||||
| 		m := &mounts{ | ||||
| 			logger:   logger, | ||||
| 			lookup:   locator, | ||||
| 			required: candidates, | ||||
| 		} | ||||
| 
 | ||||
| 		switch t { | ||||
| 		case csv.MountSpecDev: | ||||
| 			// For device mount specs, we insert a charDevices into the list of discoverers.
 | ||||
| 			discoverers = append(discoverers, (*charDevices)(m)) | ||||
| 		default: | ||||
| 			discoverers = append(discoverers, m) | ||||
| 		} | ||||
| 		discoverers = append(discoverers, &d) | ||||
| 	} | ||||
| 
 | ||||
| 	return discoverers, nil | ||||
| 	return &list{discoverers: discoverers}, nil | ||||
| } | ||||
| 
 | ||||
| // Mounts returns the discovered mounts for the csvDiscoverer.
 | ||||
| // Note that if the discoverer is for the device MountSpecType, the list of mounts is empty.
 | ||||
| func (d *csvDiscoverer) Mounts() ([]Mount, error) { | ||||
| 	if d.mountType == csv.MountSpecDev { | ||||
| 		return d.None.Mounts() | ||||
| 	} | ||||
| 
 | ||||
| 	return d.mounts.Mounts() | ||||
| // Mounts returns the discovered mounts for the charDevices. Since this explicitly specifies a
 | ||||
| // device list, the mounts are nil.
 | ||||
| func (d *charDevices) Mounts() ([]Mount, error) { | ||||
| 	return nil, nil | ||||
| } | ||||
| 
 | ||||
| // Devices returns the discovered devices for the csvDiscoverer.
 | ||||
| // Note that if the discoverer is not for the device MountSpecType, the list of devices is empty.
 | ||||
| func (d *csvDiscoverer) Devices() ([]Device, error) { | ||||
| 	if d.mountType != csv.MountSpecDev { | ||||
| 		return d.None.Devices() | ||||
| 	} | ||||
| 
 | ||||
| 	mounts, err := d.mounts.Mounts() | ||||
| // Devices returns the discovered devices for the charDevices. Here the device nodes are first
 | ||||
| // discovered as mounts and these are converted to devices.
 | ||||
| func (d *charDevices) Devices() ([]Device, error) { | ||||
| 	devicesAsMounts, err := (*mounts)(d).Mounts() | ||||
| 	if err != nil { | ||||
| 		return nil, err | ||||
| 	} | ||||
| 	var devices []Device | ||||
| 	for _, mount := range mounts { | ||||
| 		device := Device(mount) | ||||
| 		devices = append(devices, device) | ||||
| 	for _, mount := range devicesAsMounts { | ||||
| 		devices = append(devices, Device(mount)) | ||||
| 	} | ||||
| 
 | ||||
| 	return devices, nil | ||||
|  | ||||
| @ -26,12 +26,12 @@ import ( | ||||
| 	"github.com/stretchr/testify/require" | ||||
| ) | ||||
| 
 | ||||
| func TestCSVDiscoverer(t *testing.T) { | ||||
| func TestCharDevices(t *testing.T) { | ||||
| 	logger, logHook := testlog.NewNullLogger() | ||||
| 
 | ||||
| 	testCases := []struct { | ||||
| 		description          string | ||||
| 		input                *csvDiscoverer | ||||
| 		input                *charDevices | ||||
| 		expectedMounts       []Mount | ||||
| 		expectedMountsError  error | ||||
| 		expectedDevicesError error | ||||
| @ -39,8 +39,8 @@ func TestCSVDiscoverer(t *testing.T) { | ||||
| 	}{ | ||||
| 		{ | ||||
| 			description: "dev mounts are empty", | ||||
| 			input: &csvDiscoverer{ | ||||
| 				mounts: mounts{ | ||||
| 			input: (*charDevices)( | ||||
| 				&mounts{ | ||||
| 					lookup: &lookup.LocatorMock{ | ||||
| 						LocateFunc: func(string) ([]string, error) { | ||||
| 							return []string{"located"}, nil | ||||
| @ -48,39 +48,14 @@ func TestCSVDiscoverer(t *testing.T) { | ||||
| 					}, | ||||
| 					required: []string{"required"}, | ||||
| 				}, | ||||
| 				mountType: "dev", | ||||
| 			}, | ||||
| 			), | ||||
| 			expectedDevices: []Device{{Path: "located"}}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			description: "dev devices returns error for nil lookup", | ||||
| 			input: &csvDiscoverer{ | ||||
| 				mountType: "dev", | ||||
| 			}, | ||||
| 			description:          "dev devices returns error for nil lookup", | ||||
| 			input:                &charDevices{}, | ||||
| 			expectedDevicesError: fmt.Errorf("no lookup defined"), | ||||
| 		}, | ||||
| 		{ | ||||
| 			description: "lib devices are empty", | ||||
| 			input: &csvDiscoverer{ | ||||
| 				mounts: mounts{ | ||||
| 					lookup: &lookup.LocatorMock{ | ||||
| 						LocateFunc: func(string) ([]string, error) { | ||||
| 							return []string{"located"}, nil | ||||
| 						}, | ||||
| 					}, | ||||
| 					required: []string{"required"}, | ||||
| 				}, | ||||
| 				mountType: "lib", | ||||
| 			}, | ||||
| 			expectedMounts: []Mount{{Path: "located"}}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			description: "lib mounts returns error for nil lookup", | ||||
| 			input: &csvDiscoverer{ | ||||
| 				mountType: "lib", | ||||
| 			}, | ||||
| 			expectedMountsError: fmt.Errorf("no lookup defined"), | ||||
| 		}, | ||||
| 	} | ||||
| 
 | ||||
| 	for _, tc := range testCases { | ||||
| @ -117,13 +92,14 @@ func TestNewFromMountSpec(t *testing.T) { | ||||
| 	} | ||||
| 
 | ||||
| 	testCases := []struct { | ||||
| 		description            string | ||||
| 		targets                []*csv.MountSpec | ||||
| 		expectedError          error | ||||
| 		expectedCSVDiscoverers []*csvDiscoverer | ||||
| 		description        string | ||||
| 		targets            []*csv.MountSpec | ||||
| 		expectedError      error | ||||
| 		expectedDiscoverer Discover | ||||
| 	}{ | ||||
| 		{ | ||||
| 			description: "empty targets returns empyt list", | ||||
| 			description:        "empty targets returns None discoverer list", | ||||
| 			expectedDiscoverer: &None{}, | ||||
| 		}, | ||||
| 		{ | ||||
| 			description: "unexpected locator returns error", | ||||
| @ -151,18 +127,16 @@ func TestNewFromMountSpec(t *testing.T) { | ||||
| 					Path: "dev1", | ||||
| 				}, | ||||
| 			}, | ||||
| 			expectedCSVDiscoverers: []*csvDiscoverer{ | ||||
| 				{ | ||||
| 					mountType: "dev", | ||||
| 					mounts: mounts{ | ||||
| 						logger:   logger, | ||||
| 						lookup:   locators["dev"], | ||||
| 						required: []string{"dev0", "dev1"}, | ||||
| 					}, | ||||
| 				}, | ||||
| 				{ | ||||
| 					mountType: "lib", | ||||
| 					mounts: mounts{ | ||||
| 			expectedDiscoverer: &list{ | ||||
| 				discoverers: []Discover{ | ||||
| 					(*charDevices)( | ||||
| 						&mounts{ | ||||
| 							logger:   logger, | ||||
| 							lookup:   locators["dev"], | ||||
| 							required: []string{"dev0", "dev1"}, | ||||
| 						}, | ||||
| 					), | ||||
| 					&mounts{ | ||||
| 						logger:   logger, | ||||
| 						lookup:   locators["lib"], | ||||
| 						required: []string{"lib0"}, | ||||
| @ -174,13 +148,13 @@ func TestNewFromMountSpec(t *testing.T) { | ||||
| 
 | ||||
| 	for _, tc := range testCases { | ||||
| 		t.Run(tc.description, func(t *testing.T) { | ||||
| 			discoverers, err := newFromMountSpecs(logger, locators, tc.targets) | ||||
| 			discoverer, err := newFromMountSpecs(logger, locators, tc.targets) | ||||
| 			if tc.expectedError != nil { | ||||
| 				require.Error(t, err) | ||||
| 				return | ||||
| 			} | ||||
| 			require.NoError(t, err) | ||||
| 			require.ElementsMatch(t, tc.expectedCSVDiscoverers, discoverers) | ||||
| 			require.EqualValues(t, tc.expectedDiscoverer, discoverer) | ||||
| 		}) | ||||
| 	} | ||||
| } | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user