fix(scheduler): fail closed on invalid HAMi device requests instead of binding GPU-less pods

pkg/device-plugin/nvidiadevice/nvinternal/plugin/server.go

@@ -167,7 +167,10 @@ func readFromConfigFile(sConfig *nvidia.NvidiaConfig, path string) (string, erro
 func LoadNvidiaDevicePluginConfig() (*config.Config, string, error) {
 	sConfig, err := config.LoadConfig(*ConfigFile)
 	if err != nil {
-		klog.Fatalf(`failed to load device config file %s: %v`, *ConfigFile, err)
+		// Fail closed rather than aborting the process: a bad config file
+		// must surface as an error the caller can act on, not a Fatalf that
+		// takes the plugin (and any test binary) down with it.
+		return nil, "", fmt.Errorf("load device config file %s: %w", *ConfigFile, err)
 	}
 	mode, err := readFromConfigFile(&sConfig.NvidiaConfig, ConfigFilePath)
 	if err != nil {

pkg/device-plugin/nvidiadevice/nvinternal/plugin/util.go

@@ -138,7 +138,10 @@ func (plugin *NvidiaDevicePlugin) validateContainerAllocation(ctr *corev1.Contai
 	if !ok {
 		return nil
 	}
-	req := dev.GenerateResourceRequests(ctr)
+	req, err := dev.GenerateResourceRequests(ctr)
+	if err != nil {
+		return err
+	}
 	for _, each := range allocated {
 		limit, bounded := plugin.memoryLimitMB(req, each.UUID)
 		if bounded && each.Usedmem > limit {

pkg/device/amd/device.go

@@ -200,25 +200,30 @@ func (dev *AMDDevices) GetResourceNames() device.ResourceNames {
 	}
 }
 
-func (dev *AMDDevices) GenerateResourceRequests(ctr *corev1.Container) device.ContainerDeviceRequest {
+func (dev *AMDDevices) GenerateResourceRequests(ctr *corev1.Container) (device.ContainerDeviceRequest, error) {
 	klog.Info("Start to count AMD devices for container ", ctr.Name)
 	amdResourceCount := corev1.ResourceName(dev.resourceCountName)
 	amdResourceMemory := corev1.ResourceName(dev.resourceMemoryName)
 	amdResourceCore := corev1.ResourceName(dev.resourceCoreName)
 	count, ok := ctr.Resources.Limits[amdResourceCount]
 	if ok {
 		if n, ok := count.AsInt64(); ok {
-			if n <= 0 || n > math.MaxInt32 {
+			if n == 0 {
+				// An explicit zero count means no device is requested,
+				// not an invalid request. See the nvidia backend.
+				return device.ContainerDeviceRequest{}, nil
+			}
+			if n < 0 || n > math.MaxInt32 {
 				klog.ErrorS(nil, "amd device count request is out of range", "container", ctr.Name, "request", n)
-				return device.ContainerDeviceRequest{}
+				return device.ContainerDeviceRequest{}, &device.ErrInvalidDeviceRequest{Container: ctr.Name, Device: "amd", Reason: fmt.Sprintf("device count %d is out of range", n)}
 			}
 			memnum := int32(0)
 			mem, memOK := ctr.Resources.Limits[amdResourceMemory]
 			if memOK {
 				memnums, ok := mem.AsInt64()
 				if !ok || memnums < 0 || memnums > math.MaxInt32 {
 					klog.ErrorS(nil, "amd device memory request is out of range", "container", ctr.Name, "request", mem.String())
-					return device.ContainerDeviceRequest{}
+					return device.ContainerDeviceRequest{}, &device.ErrInvalidDeviceRequest{Container: ctr.Name, Device: "amd", Reason: fmt.Sprintf("memory request %s is out of range", mem.String())}
 				}
 				memnum = int32(memnums)
 			}
@@ -234,7 +239,7 @@ func (dev *AMDDevices) GenerateResourceRequests(ctr *corev1.Container) device.Co
 				corePercentageNums, ok := corePercentage.AsInt64()
 				if !ok || corePercentageNums < 1 || corePercentageNums > 100 {
 					klog.ErrorS(nil, "amd device core percentage request is out of range", "container", ctr.Name, "request", corePercentage.String())
-					return device.ContainerDeviceRequest{}
+					return device.ContainerDeviceRequest{}, &device.ErrInvalidDeviceRequest{Container: ctr.Name, Device: "amd", Reason: fmt.Sprintf("core percentage request %s is out of range (must be an integer between 1 and 100)", corePercentage.String())}
 				}
 				corePercentageNum = int32(corePercentageNums)
 			}
@@ -248,10 +253,16 @@ func (dev *AMDDevices) GenerateResourceRequests(ctr *corev1.Container) device.Co
 				Memreq:           memnum,
 				MemPercentagereq: 0,
 				Coresreq:         corePercentageNum,
-			}
+			}, nil
 		}
+		// A quantity the apiserver accepts as an integer can still be too
+		// large for int64 (1Ei, 1e19). Falling through would report the
+		// container as device-less, which is the fail-open this change
+		// exists to remove.
+		klog.ErrorS(nil, "amd device count request is not a plain integer", "container", ctr.Name, "request", count.String())
+		return device.ContainerDeviceRequest{}, &device.ErrInvalidDeviceRequest{Container: ctr.Name, Device: "amd", Reason: fmt.Sprintf("device count %s is not a plain integer", count.String())}
 	}
-	return device.ContainerDeviceRequest{}
+	return device.ContainerDeviceRequest{}, nil
 }
 
 func (dev *AMDDevices) ScoreNode(node *corev1.Node, podDevices device.PodSingleDevice, previous []*device.DeviceUsage, policy string) float32 {

pkg/device/amd/device_test.go

@@ -238,7 +238,7 @@ func Test_GenerateResourceRequests(t *testing.T) {
 				},
 			},
 		}
-		got := dev.GenerateResourceRequests(ctr)
+		got, _ := dev.GenerateResourceRequests(ctr)
 		assert.DeepEqual(t, device.ContainerDeviceRequest{
 			Nums:             2,
 			Type:             AMDDevice,
@@ -257,7 +257,7 @@ func Test_GenerateResourceRequests(t *testing.T) {
 				},
 			},
 		}
-		got := dev.GenerateResourceRequests(ctr)
+		got, _ := dev.GenerateResourceRequests(ctr)
 		assert.DeepEqual(t, device.ContainerDeviceRequest{}, got)
 	})
 
@@ -270,7 +270,7 @@ func Test_GenerateResourceRequests(t *testing.T) {
 				},
 			},
 		}
-		got := dev.GenerateResourceRequests(ctr)
+		got, _ := dev.GenerateResourceRequests(ctr)
 		assert.Equal(t, int32(100), got.Coresreq)
 	})
 
@@ -285,7 +285,7 @@ func Test_GenerateResourceRequests(t *testing.T) {
 				},
 			},
 		}
-		got := dev.GenerateResourceRequests(ctr)
+		got, _ := dev.GenerateResourceRequests(ctr)
 		assert.Equal(t, int32(42), got.Coresreq)
 	})
 
@@ -299,7 +299,7 @@ func Test_GenerateResourceRequests(t *testing.T) {
 					},
 				},
 			}
-			got := dev.GenerateResourceRequests(ctr)
+			got, _ := dev.GenerateResourceRequests(ctr)
 			assert.Equal(t, int32(cores), got.Coresreq)
 		}
 		for _, cores := range []int64{0, 101, 150, 200, -1} {
@@ -311,7 +311,7 @@ func Test_GenerateResourceRequests(t *testing.T) {
 					},
 				},
 			}
-			got := dev.GenerateResourceRequests(ctr)
+			got, _ := dev.GenerateResourceRequests(ctr)
 			assert.DeepEqual(t, device.ContainerDeviceRequest{}, got)
 		}
 		for _, rawCore := range []string{"50m", "99.1"} {
@@ -323,17 +323,25 @@ func Test_GenerateResourceRequests(t *testing.T) {
 					},
 				},
 			}
-			got := dev.GenerateResourceRequests(ctr)
+			got, _ := dev.GenerateResourceRequests(ctr)
 			assert.DeepEqual(t, device.ContainerDeviceRequest{}, got)
 		}
 	})
 
+	t.Run("zero count is device-less, not invalid", func(t *testing.T) {
+		ctr := &corev1.Container{Resources: corev1.ResourceRequirements{Limits: corev1.ResourceList{
+			"amd.com/gpu": *resource.NewQuantity(0, resource.DecimalSI),
+		}}}
+		got, err := dev.GenerateResourceRequests(ctr)
+		assert.NilError(t, err)
+		assert.DeepEqual(t, device.ContainerDeviceRequest{}, got)
+	})
+
 	for _, tc := range []struct {
 		name     string
 		resource corev1.ResourceName
 		value    int64
 	}{
-		{name: "rejects zero count", resource: "amd.com/gpu", value: 0},
 		{name: "rejects overflowing count", resource: "amd.com/gpu", value: math.MaxInt32 + 1},
 		{name: "rejects negative memory", resource: "amd.com/gpu-mem", value: -1},
 		{name: "rejects overflowing memory", resource: "amd.com/gpu-mem", value: math.MaxInt32 + 1},
@@ -346,7 +354,9 @@ func Test_GenerateResourceRequests(t *testing.T) {
 			}
 			limits[tc.resource] = *resource.NewQuantity(tc.value, resource.DecimalSI)
 			ctr := &corev1.Container{Resources: corev1.ResourceRequirements{Limits: limits}}
-			assert.DeepEqual(t, device.ContainerDeviceRequest{}, dev.GenerateResourceRequests(ctr))
+			got, err := dev.GenerateResourceRequests(ctr)
+			assert.DeepEqual(t, device.ContainerDeviceRequest{}, got)
+			assert.ErrorContains(t, err, "out of range")
 		})
 	}
 }

pkg/device/ascend/device.go

@@ -364,7 +364,7 @@ func (dev *Devices) CheckHealth(devType string, n *corev1.Node) (bool, bool) {
 	return device.CheckHealth(devType, dev.GetResourceNames().ResourceCountName, n)
 }
 
-func (dev *Devices) GenerateResourceRequests(ctr *corev1.Container) device.ContainerDeviceRequest {
+func (dev *Devices) GenerateResourceRequests(ctr *corev1.Container) (device.ContainerDeviceRequest, error) {
 	ascendResourceCount := corev1.ResourceName(dev.config.ResourceName)
 	ascendResourceMem := corev1.ResourceName(dev.config.ResourceMemoryName)
 	ascendResourceCore := corev1.ResourceName(dev.config.ResourceCoreName)
@@ -377,9 +377,14 @@ func (dev *Devices) GenerateResourceRequests(ctr *corev1.Container) device.Conta
 		klog.V(3).Infof("Counting %s devices", dev.config.CommonWord)
 		if n, ok := v.AsInt64(); ok {
 			klog.Info("Found AscendDevices devices")
-			if n <= 0 || n > math.MaxInt32 {
+			if n == 0 {
+				// An explicit zero count means no device is requested,
+				// not an invalid request. See the nvidia backend.
+				return device.ContainerDeviceRequest{}, nil
+			}
+			if n < 0 || n > math.MaxInt32 {
 				klog.ErrorS(nil, "ascend device count request is out of range", "container", ctr.Name, "request", n)
-				return device.ContainerDeviceRequest{}
+				return device.ContainerDeviceRequest{}, &device.ErrInvalidDeviceRequest{Container: ctr.Name, Device: dev.config.CommonWord, Reason: fmt.Sprintf("device count %d is out of range", n)}
 			}
 			memnum := 0
 			mem, ok := ctr.Resources.Limits[ascendResourceMem]
@@ -390,15 +395,15 @@ func (dev *Devices) GenerateResourceRequests(ctr *corev1.Container) device.Conta
 				// Negative quantities such as -1m return ok=false from AsInt64, so reject by sign first.
 				if mem.Sign() < 0 {
 					klog.ErrorS(nil, "ascend device memory request is negative", "container", ctr.Name, "request", mem.String(), "device", dev.config.CommonWord)
-					return device.ContainerDeviceRequest{}
+					return device.ContainerDeviceRequest{}, &device.ErrInvalidDeviceRequest{Container: ctr.Name, Device: dev.config.CommonWord, Reason: fmt.Sprintf("memory request %s is negative", mem.String())}
 				}
 				memnums, ok := mem.AsInt64()
 				if ok {
 					// Ascend memory is in MB, so an over-int32 value such as a byte quantity 16Gi is a wrong-unit mistake.
 					if memnums > math.MaxInt32 {
 						klog.ErrorS(nil, "ascend device memory request is out of range; memory unit is treated as MB not Byte, so a quantity such as 16Gi is invalid, request 16384 for 16GB instead",
 							"container", ctr.Name, "request", mem.String(), "device", dev.config.CommonWord)
-						return device.ContainerDeviceRequest{}
+						return device.ContainerDeviceRequest{}, &device.ErrInvalidDeviceRequest{Container: ctr.Name, Device: dev.config.CommonWord, Reason: fmt.Sprintf("memory request %s is out of range; memory unit is treated as MB not Byte", mem.String())}
 					}
 					if dev.config.MemoryFactor > 1 {
 						rawMemnums := memnums
@@ -407,7 +412,7 @@ func (dev *Devices) GenerateResourceRequests(ctr *corev1.Container) device.Conta
 						if memnums > math.MaxInt32 {
 							klog.ErrorS(nil, "ascend device memory request overflows int32 after applying memory factor; memory unit is treated as MB not Byte",
 								"container", ctr.Name, "raw", rawMemnums, "scaled", memnums, "factor", dev.config.MemoryFactor)
-							return device.ContainerDeviceRequest{}
+							return device.ContainerDeviceRequest{}, &device.ErrInvalidDeviceRequest{Container: ctr.Name, Device: dev.config.CommonWord, Reason: fmt.Sprintf("memory request %d overflows int32 after applying memory factor %d", rawMemnums, dev.config.MemoryFactor)}
 						}
 						klog.V(4).Infof("Update Ascend memory request. before %d, after %d, factor %d", rawMemnums, memnums, dev.config.MemoryFactor)
 					}
@@ -426,7 +431,7 @@ func (dev *Devices) GenerateResourceRequests(ctr *corev1.Container) device.Conta
 					corenums, valid := cv.AsInt64()
 					if !valid || corenums < 0 || corenums > 100 {
 						klog.ErrorS(nil, "ascend device core request is out of range", "container", ctr.Name, "request", cv.String())
-						return device.ContainerDeviceRequest{}
+						return device.ContainerDeviceRequest{}, &device.ErrInvalidDeviceRequest{Container: ctr.Name, Device: dev.config.CommonWord, Reason: fmt.Sprintf("core request %s is out of range (must be an integer between 0 and 100)", cv.String())}
 					}
 					corenum = int32(corenums)
 				}
@@ -443,10 +448,16 @@ func (dev *Devices) GenerateResourceRequests(ctr *corev1.Container) device.Conta
 				Memreq:           int32(memnum),
 				MemPercentagereq: int32(mempnum),
 				Coresreq:         corenum,
-			}
+			}, nil
 		}
+		// A quantity the apiserver accepts as an integer can still be too
+		// large for int64 (1Ei, 1e19). Falling through would report the
+		// container as device-less, which is the fail-open this change
+		// exists to remove.
+		klog.ErrorS(nil, "ascend device count request is not a plain integer", "container", ctr.Name, "request", v.String())
+		return device.ContainerDeviceRequest{}, &device.ErrInvalidDeviceRequest{Container: ctr.Name, Device: "ascend", Reason: fmt.Sprintf("device count %s is not a plain integer", v.String())}
 	}
-	return device.ContainerDeviceRequest{}
+	return device.ContainerDeviceRequest{}, nil
 }
 
 func (dev *Devices) ScoreNode(node *corev1.Node, podDevices device.PodSingleDevice, previous []*device.DeviceUsage, policy string) float32 {

pkg/device/ascend/device_test.go

@@ -1766,7 +1766,7 @@ func Test_GenerateResourceRequests(t *testing.T) {
 					},
 				},
 			}
-			result := dev.GenerateResourceRequests(&test.args)
+			result, _ := dev.GenerateResourceRequests(&test.args)
 
 			assert.Equal(t, result, test.want)
 		})
@@ -1816,7 +1816,7 @@ func Test_GenerateResourceRequests_VNPUCoreMode(t *testing.T) {
 					},
 				},
 			}
-			result := dev.GenerateResourceRequests(&test.args)
+			result, _ := dev.GenerateResourceRequests(&test.args)
 
 			assert.Equal(t, result, test.want)
 		})
@@ -1961,7 +1961,7 @@ func Test_GenerateResourceRequestsFactor(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			result := test.dev.GenerateResourceRequests(&req)
+			result, _ := test.dev.GenerateResourceRequests(&req)
 			assert.Equal(t, result, test.want)
 		})
 	}
@@ -2081,7 +2081,7 @@ func Test_GenerateResourceRequests_OutOfRangeValues(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			result := test.dev.GenerateResourceRequests(&test.args)
+			result, _ := test.dev.GenerateResourceRequests(&test.args)
 			assert.Equal(t, result, test.want)
 		})
 	}
@@ -2141,7 +2141,7 @@ func Test_GenerateResourceRequests_MemoryFactorOverflow(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			result := test.dev.GenerateResourceRequests(&test.args)
+			result, _ := test.dev.GenerateResourceRequests(&test.args)
 			assert.Equal(t, result, device.ContainerDeviceRequest{})
 		})
 	}
@@ -3708,7 +3708,7 @@ func Test_GenerateResourceRequests_CoresValidation(t *testing.T) {
 					},
 				},
 			}
-			req := dev.GenerateResourceRequests(ctr)
+			req, _ := dev.GenerateResourceRequests(ctr)
 			if tt.wantReq {
 				assert.Equal(t, int32(1), req.Nums)
 				assert.Equal(t, int32(tt.cores), req.Coresreq)

pkg/device/awsneuron/device.go

@@ -272,16 +272,22 @@ func (dev *AWSNeuronDevices) GetResourceNames() device.ResourceNames {
 	}
 }
 
-func (dev *AWSNeuronDevices) GenerateResourceRequests(ctr *corev1.Container) device.ContainerDeviceRequest {
+func (dev *AWSNeuronDevices) GenerateResourceRequests(ctr *corev1.Container) (device.ContainerDeviceRequest, error) {
 	klog.Info("Start to count awsNeuron devices for container ", ctr.Name)
 	awsResourceCount := corev1.ResourceName(dev.resourceCountName)
 	awsResourceCores := corev1.ResourceName(dev.resourceCoreName)
 	v, ok := resourceQuantity(ctr, awsResourceCount)
 	if ok {
+		// An explicit zero count means no device is requested, not an
+		// invalid request. See the nvidia backend. MutateAdmission keeps
+		// using the shared validator, which still rejects zero there.
+		if zero, isInt := v.AsInt64(); isInt && zero == 0 {
+			return device.ContainerDeviceRequest{}, nil
+		}
 		n, err := validateResourceRequest(v, dev.resourceCountName, maxAWSNeuronDeviceCount)
 		if err != nil {
 			klog.ErrorS(err, "Invalid awsNeuron device request", "container", ctr.Name)
-			return device.ContainerDeviceRequest{}
+			return device.ContainerDeviceRequest{}, &device.ErrInvalidDeviceRequest{Container: ctr.Name, Device: "awsneuron", Reason: err.Error()}
 		}
 		klog.InfoS("Detected awsNeuron device request",
 			"container", ctr.Name,
@@ -294,14 +300,14 @@ func (dev *AWSNeuronDevices) GenerateResourceRequests(ctr *corev1.Container) dev
 			// A zero core request denotes a whole AWS Neuron device. Fit converts
 			// it to the selected device's actual addressable core count.
 			Coresreq: 0,
-		}
+		}, nil
 	} else {
 		core, ok := resourceQuantity(ctr, awsResourceCores)
 		if ok {
 			n, err := validateResourceRequest(core, dev.resourceCoreName, maxAWSNeuronCoreCount)
 			if err != nil {
 				klog.ErrorS(err, "Invalid awsNeuron core request", "container", ctr.Name)
-				return device.ContainerDeviceRequest{}
+				return device.ContainerDeviceRequest{}, &device.ErrInvalidDeviceRequest{Container: ctr.Name, Device: "awsneuron", Reason: err.Error()}
 			}
 			klog.InfoS("Detected awsNeuron device request",
 				"container", ctr.Name,
@@ -312,10 +318,10 @@ func (dev *AWSNeuronDevices) GenerateResourceRequests(ctr *corev1.Container) dev
 				Memreq:           0,
 				MemPercentagereq: 0,
 				TotalCoresreq:    n,
-			}
+			}, nil
 		}
 	}
-	return device.ContainerDeviceRequest{}
+	return device.ContainerDeviceRequest{}, nil
 }
 
 func (dev *AWSNeuronDevices) ScoreNode(node *corev1.Node, podDevices device.PodSingleDevice, previous []*device.DeviceUsage, policy string) float32 {