Make gated modes (GDS, MOFED, GDRCOPY) optional in CDI

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

Author: elezar

internal/cdi/api.go

@@ -18,8 +18,9 @@ package cdi
 
 // Interface provides the API to the 'cdi' package
 //
-//go:generate moq -stub -out api_mock.go . Interface
+//go:generate moq -rm -fmt=goimports -stub -out api_mock.go . Interface
 type Interface interface {
 	CreateSpecFile() error
 	QualifiedName(string, string) string
+	AdditionalDevices() []string
 }

internal/cdi/api_mock.go

@@ -63,7 +63,8 @@ type cdiHandler struct {
 
 	imexChannels imex.Channels
 
-	cdilibs map[string]nvcdi.SpecGenerator
+	cdilibs         map[string]nvcdi.SpecGenerator
+	additionalModes []string
 }
 
 var _ Interface = &cdiHandler{}
@@ -134,18 +135,17 @@ func New(infolib info.Interface, nvmllib nvml.Interface, devicelib device.Interf
 		c.cdilibs["imex-channel"] = c.newImexChannelSpecGenerator()
 	}
 
-	var additionalModes []string
 	if c.gdrcopyEnabled {
-		additionalModes = append(additionalModes, "gdrcopy")
+		c.additionalModes = append(c.additionalModes, "gdrcopy")
 	}
 	if c.gdsEnabled {
-		additionalModes = append(additionalModes, "gds")
+		c.additionalModes = append(c.additionalModes, "gds")
 	}
 	if c.mofedEnabled {
-		additionalModes = append(additionalModes, "mofed")
+		c.additionalModes = append(c.additionalModes, "mofed")
 	}
 
-	for _, mode := range additionalModes {
+	for _, mode := range c.additionalModes {
 		lib, err := nvcdi.New(
 			nvcdi.WithInfoLib(c.infolib),
 			nvcdi.WithLogger(c.logger),
@@ -166,6 +166,7 @@ func New(infolib info.Interface, nvmllib nvml.Interface, devicelib device.Interf
 
 // CreateSpecFile creates a CDI spec file for the specified devices.
 func (cdi *cdiHandler) CreateSpecFile() error {
+	var emptySpecs []string
 	for class, cdilib := range cdi.cdilibs {
 		cdi.logger.Infof("Generating CDI spec for resource: %s/%s", cdi.vendor, class)
 
@@ -197,10 +198,22 @@ func (cdi *cdiHandler) CreateSpecFile() error {
 
 		err = spec.Save(filepath.Join(cdiRoot, specName+".json"))
 		if err != nil {
+			// TODO: This is a brittle check since it relies on exact string matches.
+			// We should pull this functionality into the CDI tooling instead.
+			if strings.Contains(err.Error(), "invalid device, empty device edits") {
+				klog.ErrorS(err, "Ignoring empty CDI specs", "vendor", cdi.vendor, "class", class)
+				emptySpecs = append(emptySpecs, class)
+				continue
+			}
 			return fmt.Errorf("failed to save CDI spec: %v", err)
 		}
 	}
 
+	// Remove the classes with empty specs from the supported types.
+	for _, emptySpec := range emptySpecs {
+		delete(cdi.cdilibs, emptySpec)
+	}
+
 	return nil
 }
 
@@ -233,3 +246,18 @@ func (cdi *cdiHandler) getRootTransformer() transform.Transformer {
 func (cdi *cdiHandler) QualifiedName(class string, id string) string {
 	return cdiparser.QualifiedName(cdi.vendor, class, id)
 }
+
+// AdditionalDevices returns the optional CDI devices based on the device plugin
+// configuration.
+// Here we check for requested modes as well as whether the modes have a valid
+// CDI spec associated with them.
+func (cdi *cdiHandler) AdditionalDevices() []string {
+	var devices []string
+	for _, mode := range cdi.additionalModes {
+		if cdi.cdilibs[mode] == nil {
+			continue
+		}
+		devices = append(devices, cdi.QualifiedName(mode, "all"))
+	}
+	return devices
+}

internal/cdi/cdi.go

@@ -30,6 +30,10 @@ func NewNullHandler() Interface {
 	return &null{}
 }
 
+func (n *null) AdditionalDevices() []string {
+	return nil
+}
+
 // CreateSpecFile is a no-op for the null handler.
 func (n *null) CreateSpecFile() error {
 	return nil

internal/cdi/null.go

@@ -380,15 +380,8 @@ func (plugin *nvidiaDevicePlugin) updateResponseForCDI(response *pluginapi.Conta
 	for _, channel := range plugin.imexChannels {
 		devices = append(devices, plugin.cdiHandler.QualifiedName("imex-channel", channel.ID))
 	}
-	if *plugin.config.Flags.GDSEnabled {
-		devices = append(devices, plugin.cdiHandler.QualifiedName("gds", "all"))
-	}
-	if *plugin.config.Flags.MOFEDEnabled {
-		devices = append(devices, plugin.cdiHandler.QualifiedName("mofed", "all"))
-	}
-	if *plugin.config.Flags.GDRCopyEnabled {
-		devices = append(devices, plugin.cdiHandler.QualifiedName("gdrcopy", "all"))
-	}
+
+	devices = append(devices, plugin.cdiHandler.AdditionalDevices()...)
 
 	if len(devices) == 0 {
 		return nil

internal/plugin/server.go

@@ -33,6 +33,7 @@ func TestCDIAllocateResponse(t *testing.T) {
 		deviceIds            []string
 		deviceListStrategies []string
 		CDIPrefix            string
+		AdditionalCDIDevices []string
 		GDSEnabled           bool
 		MOFEDEnabled         bool
 		imexChannels         []*imex.Channel
@@ -91,7 +92,7 @@ func TestCDIAllocateResponse(t *testing.T) {
 			description:          "mofed devices are selected if configured",
 			deviceListStrategies: []string{"cdi-annotations"},
 			CDIPrefix:            "cdi.k8s.io/",
-			MOFEDEnabled:         true,
+			AdditionalCDIDevices: []string{"nvidia.com/mofed=all"},
 			expectedResponse: pluginapi.ContainerAllocateResponse{
 				Annotations: map[string]string{
 					"cdi.k8s.io/nvidia-device-plugin_uuid": "nvidia.com/mofed=all",
@@ -102,7 +103,7 @@ func TestCDIAllocateResponse(t *testing.T) {
 			description:          "gds devices are selected if configured",
 			deviceListStrategies: []string{"cdi-annotations"},
 			CDIPrefix:            "cdi.k8s.io/",
-			GDSEnabled:           true,
+			AdditionalCDIDevices: []string{"nvidia.com/gds=all"},
 			expectedResponse: pluginapi.ContainerAllocateResponse{
 				Annotations: map[string]string{
 					"cdi.k8s.io/nvidia-device-plugin_uuid": "nvidia.com/gds=all",
@@ -114,8 +115,7 @@ func TestCDIAllocateResponse(t *testing.T) {
 			deviceIds:            []string{"gpu0"},
 			deviceListStrategies: []string{"cdi-annotations"},
 			CDIPrefix:            "cdi.k8s.io/",
-			GDSEnabled:           true,
-			MOFEDEnabled:         true,
+			AdditionalCDIDevices: []string{"nvidia.com/gds=all", "nvidia.com/mofed=all"},
 			expectedResponse: pluginapi.ContainerAllocateResponse{
 				Annotations: map[string]string{
 					"cdi.k8s.io/nvidia-device-plugin_uuid": "nvidia.com/gpu=gpu0,nvidia.com/gds=all,nvidia.com/mofed=all",
@@ -152,6 +152,9 @@ func TestCDIAllocateResponse(t *testing.T) {
 					QualifiedNameFunc: func(c string, s string) string {
 						return "nvidia.com/" + c + "=" + s
 					},
+					AdditionalDevicesFunc: func() []string {
+						return tc.AdditionalCDIDevices
+					},
 				},
 				deviceListStrategies: deviceListStrategies,
 				cdiAnnotationPrefix:  tc.CDIPrefix,