fix(device): re-check ResourceQuota in Fit() for non-NVIDIA backends

pkg/device/ascend/device.go

@@ -512,6 +512,12 @@ func (npu *Devices) Fit(devices []*device.DeviceUsage, request device.ContainerD
 			//This incurs an issue
 			memreq = dev.Totalmem * k.MemPercentagereq / 100
 		}
+		if !needTopology && !device.FitQuotaWithPodDevices(tmpDevs, allocated, pod.Namespace, k.Type,
+			int64(memreq), int64(k.Coresreq), npu.GetResourceNames().MemoryFactor) {
+			reason[common.ResourceQuotaNotFit]++
+			klog.V(3).InfoS(common.ResourceQuotaNotFit, "pod", klog.KObj(pod), "memreq", memreq, "coresreq", k.Coresreq)
+			continue
+		}
 		if dev.Totalmem-dev.Usedmem < memreq {
 			reason[common.CardInsufficientMemory]++
 			klog.V(5).InfoS(common.CardInsufficientMemory, "pod", klog.KObj(pod), "device", dev.ID, "device index", i, "device total memory", dev.Totalmem, "device used memory", dev.Usedmem, "request memory", memreq)
@@ -575,6 +581,9 @@ func (npu *Devices) Fit(devices []*device.DeviceUsage, request device.ContainerD
 
 	if needTopology {
 		if len(tmpDevs[k.Type]) == int(originReq) {
+			if !npu.fitQuotaForSelection(tmpDevs, allocated, pod, k.Type, reason) {
+				return false, tmpDevs, common.GenReason(reason, len(devices))
+			}
 			klog.V(5).InfoS("device allocate success", "pod", klog.KObj(pod), "allocate device", tmpDevs)
 			return true, tmpDevs, ""
 		} else if len(tmpDevs[k.Type]) > int(originReq) {
@@ -585,6 +594,9 @@ func (npu *Devices) Fit(devices []*device.DeviceUsage, request device.ContainerD
 				combination := npu.computeBestCombination(nodeInfo, int(originReq), tmpDevs[k.Type])
 				tmpDevs[k.Type] = combination
 			}
+			if !npu.fitQuotaForSelection(tmpDevs, allocated, pod, k.Type, reason) {
+				return false, tmpDevs, common.GenReason(reason, len(devices))
+			}
 			klog.V(5).InfoS("device allocate success", "pod", klog.KObj(pod), "best device combination", tmpDevs)
 			return true, tmpDevs, ""
 		}
@@ -597,6 +609,18 @@ func (npu *Devices) Fit(devices []*device.DeviceUsage, request device.ContainerD
 	return false, tmpDevs, common.GenReason(reason, len(devices))
 }
 
+// fitQuotaForSelection charges the namespace for exactly the cards that were
+// chosen. Passing no extra request means only the contents of tmpDevs and
+// anything already allocated to the pod are weighed.
+func (npu *Devices) fitQuotaForSelection(tmpDevs map[string]device.ContainerDevices, allocated *device.PodDevices, pod *corev1.Pod, devType string, reason map[string]int) bool {
+	if device.FitQuotaWithPodDevices(tmpDevs, allocated, pod.Namespace, devType, 0, 0, npu.GetResourceNames().MemoryFactor) {
+		return true
+	}
+	reason[common.ResourceQuotaNotFit]++
+	klog.V(3).InfoS(common.ResourceQuotaNotFit, "pod", klog.KObj(pod), "selected", len(tmpDevs[devType]))
+	return false
+}
+
 func hasNetworkID(devices []*device.DeviceUsage) bool {
 	for _, dev := range devices {
 		if dev.CustomInfo == nil {

pkg/device/ascend/device_test.go

@@ -32,6 +32,7 @@ import (
 	"k8s.io/klog/v2"
 
 	"github.com/Project-HAMi/HAMi/pkg/device"
+	"github.com/Project-HAMi/HAMi/pkg/device/common"
 	"github.com/Project-HAMi/HAMi/pkg/util"
 )
 
@@ -2457,3 +2458,149 @@ func TestDevices_AddResourceUsage(t *testing.T) {
 		})
 	}
 }
+
+// Fit() is the second gate on namespace quota, after admission. Usage is only
+// recorded at Filter time, so pods created together all clear admission against
+// the same figure and this is what stops them.
+func TestDevices_FitResourceQuota(t *testing.T) {
+	dev := &Devices{
+		config: VNPUConfig{
+			CommonWord:         "Ascend910A",
+			ChipName:           "910A",
+			ResourceName:       "huawei.com/Ascend910A",
+			ResourceMemoryName: "huawei.com/Ascend910A-memory",
+			MemoryAllocatable:  32768,
+			MemoryCapacity:     32768,
+			AICore:             30,
+		},
+	}
+
+	prevDevices := device.DevicesMap
+	device.DevicesMap = map[string]device.Devices{"Ascend910A": dev}
+	t.Cleanup(func() { device.DevicesMap = prevDevices })
+
+	usable := func() []*device.DeviceUsage {
+		return []*device.DeviceUsage{{
+			ID:        "dev-0",
+			Index:     0,
+			Count:     100,
+			Totalmem:  32768,
+			Totalcore: 30,
+			Type:      "Ascend910A",
+			Health:    true,
+		}}
+	}
+	request := device.ContainerDeviceRequest{
+		Nums:             1,
+		Memreq:           2184,
+		MemPercentagereq: 0,
+		Coresreq:         0,
+		Type:             "Ascend910A",
+	}
+	pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "ascend-quota"}}
+
+	qm := device.NewQuotaManager()
+	t.Cleanup(func() { delete(qm.Quotas, "ascend-quota") })
+
+	qm.Quotas["ascend-quota"] = &device.DeviceQuota{
+		"huawei.com/Ascend910A-memory": &device.Quota{Used: 0, Limit: 32768, LimitSet: true},
+	}
+	if fit, _, _ := dev.Fit(usable(), request, pod, &device.NodeInfo{}, &device.PodDevices{}); !fit {
+		t.Fatal("Fit() = false, want true: the request is inside the namespace quota")
+	}
+
+	qm.Quotas["ascend-quota"] = &device.DeviceQuota{
+		"huawei.com/Ascend910A-memory": &device.Quota{Used: 31000, Limit: 32768, LimitSet: true},
+	}
+	fit, _, reason := dev.Fit(usable(), request, pod, &device.NodeInfo{}, &device.PodDevices{})
+	if fit {
+		t.Error("Fit() = true, want false: the namespace quota is exhausted")
+	}
+	if !strings.Contains(reason, common.ResourceQuotaNotFit) {
+		t.Errorf("reason = %q, want it to mention %s", reason, common.ResourceQuotaNotFit)
+	}
+}
+
+// In topology mode the loop gathers every candidate card and picks the best
+// originReq subset afterwards, so quota has to be charged for the selection
+// rather than for each candidate. Charging per candidate would refuse cards the
+// final combination never uses and starve computeBestCombination.
+func TestDevices_FitResourceQuotaTopology(t *testing.T) {
+	dev := &Devices{
+		config: VNPUConfig{
+			CommonWord:         "Ascend910B2",
+			ChipName:           "910B2",
+			ResourceName:       "huawei.com/Ascend910B2",
+			ResourceMemoryName: "huawei.com/Ascend910B2-memory",
+			MemoryAllocatable:  65536,
+			MemoryCapacity:     65536,
+			AICore:             24,
+		},
+	}
+
+	prevDevices := device.DevicesMap
+	device.DevicesMap = map[string]device.Devices{"Ascend910B2": dev}
+	t.Cleanup(func() { device.DevicesMap = prevDevices })
+
+	usable := func() []*device.DeviceUsage {
+		devs := make([]*device.DeviceUsage, 0, 4)
+		for i := range 4 {
+			devs = append(devs, &device.DeviceUsage{
+				ID:        fmt.Sprintf("dev-%d", i),
+				Index:     uint(i),
+				Count:     100,
+				Totalmem:  65536,
+				Totalcore: 24,
+				Type:      "Ascend910B2",
+				Health:    true,
+				CustomInfo: map[string]any{
+					"NetworkID": float64(1),
+				},
+			})
+		}
+		return devs
+	}
+	request := device.ContainerDeviceRequest{
+		Nums:             2,
+		Memreq:           16384,
+		MemPercentagereq: 0,
+		Coresreq:         0,
+		Type:             "Ascend910B2",
+	}
+	pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "ascend-topo"}}
+
+	nodeInfo := &device.NodeInfo{ID: "node1", Devices: map[string][]device.DeviceInfo{}}
+	for i := range 4 {
+		nodeInfo.Devices["Ascend910B2"] = append(nodeInfo.Devices["Ascend910B2"], device.DeviceInfo{
+			ID:         fmt.Sprintf("dev-%d", i),
+			Index:      uint(i),
+			Health:     true,
+			CustomInfo: map[string]any{"NetworkID": float64(1)},
+		})
+	}
+
+	qm := device.NewQuotaManager()
+	t.Cleanup(func() { delete(qm.Quotas, "ascend-topo") })
+
+	qm.Quotas["ascend-topo"] = &device.DeviceQuota{
+		"huawei.com/Ascend910B2-memory": &device.Quota{Used: 0, Limit: 40000, LimitSet: true},
+	}
+	fit, res, reason := dev.Fit(usable(), request, pod, nodeInfo, &device.PodDevices{})
+	if !fit {
+		t.Fatalf("Fit() = false (reason %q), want true: the chosen pair is inside quota even though the candidate pool is not", reason)
+	}
+	if got := len(res["Ascend910B2"]); got != 2 {
+		t.Errorf("selected %d cards, want 2", got)
+	}
+
+	qm.Quotas["ascend-topo"] = &device.DeviceQuota{
+		"huawei.com/Ascend910B2-memory": &device.Quota{Used: 0, Limit: 20000, LimitSet: true},
+	}
+	fit, _, reason = dev.Fit(usable(), request, pod, nodeInfo, &device.PodDevices{})
+	if fit {
+		t.Error("Fit() = true, want false: the selected cards exceed the namespace quota")
+	}
+	if !strings.Contains(reason, common.ResourceQuotaNotFit) {
+		t.Errorf("reason = %q, want it to mention %s", reason, common.ResourceQuotaNotFit)
+	}
+}

pkg/device/cambricon/device.go

@@ -378,6 +378,12 @@ func (cam *CambriconDevices) Fit(devices []*device.DeviceUsage, request device.C
 			//This incurs an issue
 			memreq = dev.Totalmem * k.MemPercentagereq / 100
 		}
+		if !device.FitQuotaWithPodDevices(tmpDevs, allocated, pod.Namespace, k.Type,
+			int64(memreq), int64(k.Coresreq), cam.GetResourceNames().MemoryFactor) {
+			reason[common.ResourceQuotaNotFit]++
+			klog.V(3).InfoS(common.ResourceQuotaNotFit, "pod", klog.KObj(pod), "memreq", memreq, "coresreq", k.Coresreq)
+			continue
+		}
 		if dev.Totalmem-dev.Usedmem < memreq {
 			reason[common.CardInsufficientMemory]++
 			klog.V(5).InfoS(common.CardInsufficientMemory, "pod", klog.KObj(pod), "device", dev.ID, "device index", i, "device total memory", dev.Totalmem, "device used memory", dev.Usedmem, "request memory", memreq)

pkg/device/cambricon/device_test.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/client-go/kubernetes/fake"
 
 	"github.com/Project-HAMi/HAMi/pkg/device"
+	"github.com/Project-HAMi/HAMi/pkg/device/common"
 	"github.com/Project-HAMi/HAMi/pkg/util/client"
 )
 
@@ -1164,3 +1165,79 @@ func TestDevices_AddResourceUsage(t *testing.T) {
 		})
 	}
 }
+
+// Fit() is the second gate on quota. Admission alone cannot hold a namespace
+// to its limit, because usage is only recorded at Filter time, so pods created
+// together all pass admission against the same figure. Until now only nvidia
+// re-checked here.
+func TestDevices_FitResourceQuota(t *testing.T) {
+	dev := InitMLUDevice(CambriconConfig{
+		ResourceCountName:  "cambricon.com/mlu",
+		ResourceMemoryName: "cambricon.com/mlu.smlu.vmemory",
+		ResourceCoreName:   "cambricon.com/mlu.smlu.vcore",
+	})
+
+	prevDevices := device.DevicesMap
+	device.DevicesMap = map[string]device.Devices{CambriconMLUDevice: dev}
+	t.Cleanup(func() { device.DevicesMap = prevDevices })
+
+	usable := func() []*device.DeviceUsage {
+		return []*device.DeviceUsage{{
+			ID:        "dev-0",
+			Index:     0,
+			Count:     100,
+			Usedmem:   0,
+			Totalmem:  100000,
+			Totalcore: 100,
+			Usedcores: 0,
+			Type:      CambriconMLUDevice,
+			Health:    true,
+		}}
+	}
+	request := device.ContainerDeviceRequest{
+		Nums:             1,
+		Memreq:           2560,
+		MemPercentagereq: 0,
+		Coresreq:         50,
+		Type:             CambriconMLUDevice,
+	}
+	pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "mlu-quota"}}
+
+	qm := device.NewQuotaManager()
+	t.Cleanup(func() { delete(qm.Quotas, "mlu-quota") })
+
+	qm.Quotas["mlu-quota"] = &device.DeviceQuota{
+		"cambricon.com/mlu.smlu.vmemory": &device.Quota{Used: 0, Limit: 20, LimitSet: true},
+	}
+	fit, _, _ := dev.Fit(usable(), request, pod, &device.NodeInfo{}, &device.PodDevices{})
+	if !fit {
+		t.Fatal("Fit() = false, want true: the request is inside the namespace quota")
+	}
+
+	qm.Quotas["mlu-quota"] = &device.DeviceQuota{
+		"cambricon.com/mlu.smlu.vmemory": &device.Quota{Used: 4864, Limit: 20, LimitSet: true},
+	}
+	fit, _, reason := dev.Fit(usable(), request, pod, &device.NodeInfo{}, &device.PodDevices{})
+	if fit {
+		t.Error("Fit() = true, want false: the namespace quota is exhausted")
+	}
+	if !strings.Contains(reason, common.ResourceQuotaNotFit) {
+		t.Errorf("reason = %q, want it to mention %s", reason, common.ResourceQuotaNotFit)
+	}
+
+	qm.Quotas["mlu-quota"] = &device.DeviceQuota{
+		"cambricon.com/mlu.smlu.vcore": &device.Quota{Used: 80, Limit: 100, LimitSet: true},
+	}
+	fit, _, reason = dev.Fit(usable(), request, pod, &device.NodeInfo{}, &device.PodDevices{})
+	if fit {
+		t.Error("Fit() = true, want false: the namespace core quota is exhausted")
+	}
+	if !strings.Contains(reason, common.ResourceQuotaNotFit) {
+		t.Errorf("reason = %q, want it to mention %s", reason, common.ResourceQuotaNotFit)
+	}
+
+	delete(qm.Quotas, "mlu-quota")
+	if fit, _, _ = dev.Fit(usable(), request, pod, &device.NodeInfo{}, &device.PodDevices{}); !fit {
+		t.Error("Fit() = false, want true: no quota is set for this namespace")
+	}
+}

pkg/device/hygon/device.go

@@ -301,6 +301,12 @@ func (dcu *DCUDevices) Fit(devices []*device.DeviceUsage, request device.Contain
 			//This incurs an issue
 			memreq = dev.Totalmem * k.MemPercentagereq / 100
 		}
+		if !device.FitQuotaWithPodDevices(tmpDevs, allocated, pod.Namespace, k.Type,
+			int64(memreq), int64(k.Coresreq), dcu.GetResourceNames().MemoryFactor) {
+			reason[common.ResourceQuotaNotFit]++
+			klog.V(3).InfoS(common.ResourceQuotaNotFit, "pod", klog.KObj(pod), "memreq", memreq, "coresreq", k.Coresreq)
+			continue
+		}
 		if dev.Totalmem-dev.Usedmem < memreq {
 			reason[common.CardInsufficientMemory]++
 			klog.V(5).InfoS(common.CardInsufficientMemory, "pod", klog.KObj(pod), "device", dev.ID, "device index", i, "device total memory", dev.Totalmem, "device used memory", dev.Usedmem, "request memory", memreq)

pkg/device/hygon/device_test.go

@@ -21,6 +21,7 @@ import (
 	"errors"
 	"flag"
 	"fmt"
+	"strings"
 	"testing"
 
 	"gotest.tools/v3/assert"
@@ -31,6 +32,7 @@ import (
 	"k8s.io/klog/v2"
 
 	"github.com/Project-HAMi/HAMi/pkg/device"
+	"github.com/Project-HAMi/HAMi/pkg/device/common"
 	"github.com/Project-HAMi/HAMi/pkg/util"
 	"github.com/Project-HAMi/HAMi/pkg/util/client"
 )
@@ -1257,3 +1259,59 @@ func TestDevices_AddResourceUsage(t *testing.T) {
 		})
 	}
 }
+
+// Fit() is the second gate on namespace quota, after admission. Usage is only
+// recorded at Filter time, so pods created together all clear admission against
+// the same figure and this is what stops them.
+func TestDevices_FitResourceQuota(t *testing.T) {
+	dev := InitDCUDevice(HygonConfig{
+		ResourceCountName:  "hygon.com/dcunum",
+		ResourceMemoryName: "hygon.com/dcumem",
+		ResourceCoreName:   "hygon.com/dcucores",
+	})
+
+	prevDevices := device.DevicesMap
+	device.DevicesMap = map[string]device.Devices{HygonDCUDevice: dev}
+	t.Cleanup(func() { device.DevicesMap = prevDevices })
+
+	usable := func() []*device.DeviceUsage {
+		return []*device.DeviceUsage{{
+			ID:        "dev-0",
+			Index:     0,
+			Count:     100,
+			Totalmem:  10000,
+			Totalcore: 100,
+			Type:      HygonDCUDevice,
+			Health:    true,
+		}}
+	}
+	request := device.ContainerDeviceRequest{
+		Nums:             1,
+		Memreq:           200,
+		MemPercentagereq: 0,
+		Coresreq:         50,
+		Type:             HygonDCUDevice,
+	}
+	pod := &corev1.Pod{ObjectMeta: metav1.ObjectMeta{Namespace: "dcu-quota"}}
+
+	qm := device.NewQuotaManager()
+	t.Cleanup(func() { delete(qm.Quotas, "dcu-quota") })
+
+	qm.Quotas["dcu-quota"] = &device.DeviceQuota{
+		"hygon.com/dcumem": &device.Quota{Used: 0, Limit: 1000, LimitSet: true},
+	}
+	if fit, _, _ := dev.Fit(usable(), request, pod, &device.NodeInfo{}, &device.PodDevices{}); !fit {
+		t.Fatal("Fit() = false, want true: the request is inside the namespace quota")
+	}
+
+	qm.Quotas["dcu-quota"] = &device.DeviceQuota{
+		"hygon.com/dcumem": &device.Quota{Used: 900, Limit: 1000, LimitSet: true},
+	}
+	fit, _, reason := dev.Fit(usable(), request, pod, &device.NodeInfo{}, &device.PodDevices{})
+	if fit {
+		t.Error("Fit() = true, want false: the namespace quota is exhausted")
+	}
+	if !strings.Contains(reason, common.ResourceQuotaNotFit) {
+		t.Errorf("reason = %q, want it to mention %s", reason, common.ResourceQuotaNotFit)
+	}
+}