From fa111fd654b231a288579faa3ff0e2b7b047d9a7 Mon Sep 17 00:00:00 2001 From: james Date: Tue, 2 Jun 2026 10:16:02 +0800 Subject: [PATCH 1/2] fix addResource Signed-off-by: james --- .../api/devices/ascend/hami/device_info.go | 64 ++++++++++++++----- 1 file changed, 48 insertions(+), 16 deletions(-) diff --git a/pkg/scheduler/api/devices/ascend/hami/device_info.go b/pkg/scheduler/api/devices/ascend/hami/device_info.go index 37b03dd3ad4..5f3898457ab 100644 --- a/pkg/scheduler/api/devices/ascend/hami/device_info.go +++ b/pkg/scheduler/api/devices/ascend/hami/device_info.go @@ -60,6 +60,7 @@ type AscendDevice struct { DeviceInfo *devices.DeviceInfo DeviceUsage *devices.DeviceUsage Score float64 + PodMap map[string]*devices.DeviceUsage } type AscendDevices struct { @@ -115,6 +116,7 @@ func NewAscendDevices(name string, node *v1.Node) map[string]*AscendDevices { Usedmem: 0, Usedcores: 0, }, + PodMap: make(map[string]*devices.DeviceUsage), } asDevices.Devices[nd.ID] = cur_dev klog.V(5).Infof("add device. ID %s dev_info %+v", cur_dev.DeviceInfo.ID, cur_dev.DeviceInfo) @@ -137,22 +139,14 @@ func GetAscendDeviceNames() []string { return deviceNames } -func (ads *AscendDevices) AddResourceUsage(id string, cores int32, mem int32) error { - dev, ok := ads.Devices[id] - if !ok { - return fmt.Errorf("ascend device %s not found", id) - } +func (ads *AscendDevices) AddResourceUsage(dev *AscendDevice, cores int32, mem int32) error { dev.DeviceUsage.Used++ dev.DeviceUsage.Usedcores += cores dev.DeviceUsage.Usedmem += mem return nil } -func (ads *AscendDevices) SubResourceUsage(id string, cores int32, mem int32) error { - dev, ok := ads.Devices[id] - if !ok { - return fmt.Errorf("ascend device %s not found", id) - } +func (ads *AscendDevices) SubResourceUsage(dev *AscendDevice, cores int32, mem int32) error { dev.DeviceUsage.Used-- dev.DeviceUsage.Usedcores -= cores dev.DeviceUsage.Usedmem -= mem @@ -170,7 +164,7 @@ func (ads *AscendDevices) SubResource(pod *v1.Pod) { if ads == nil { return } - ano_key := devices.InRequestDevices[ads.Type] + ano_key := devices.SupportDevices[ads.Type] ano, ok := pod.Annotations[ano_key] if !ok { return @@ -181,12 +175,21 @@ func (ads *AscendDevices) SubResource(pod *v1.Pod) { return } for _, cono_dev := range con_devs { - ads.SubResourceUsage(cono_dev.UUID, cono_dev.Usedcores, cono_dev.Usedmem) + dev, ok := ads.Devices[cono_dev.UUID] + if !ok { + klog.Warningf("ascend device %s not found", cono_dev.UUID) + continue + } + if _, ok := dev.PodMap[string(pod.UID)]; ok { + delete(dev.PodMap, string(pod.UID)) + ads.SubResourceUsage(dev, cono_dev.Usedcores, cono_dev.Usedmem) + klog.V(5).Infof("sub resource usage for pod %s. device %s usedmem %d", pod.Name, dev.DeviceInfo.ID, cono_dev.Usedmem) + } } } func (ads *AscendDevices) addResource(annotations map[string]string, pod *v1.Pod) { - ano_key := devices.InRequestDevices[ads.Type] + ano_key := devices.SupportDevices[ads.Type] ano, ok := annotations[ano_key] if !ok { return @@ -197,7 +200,20 @@ func (ads *AscendDevices) addResource(annotations map[string]string, pod *v1.Pod return } for _, cono_dev := range con_devs { - ads.AddResourceUsage(cono_dev.UUID, cono_dev.Usedcores, cono_dev.Usedmem) + dev, ok := ads.Devices[cono_dev.UUID] + if !ok { + klog.Warningf("ascend device %s not found", cono_dev.UUID) + continue + } + if _, ok := dev.PodMap[string(pod.UID)]; !ok { + dev.PodMap[string(pod.UID)] = &devices.DeviceUsage{ + Used: 1, + Usedcores: cono_dev.Usedcores, + Usedmem: cono_dev.Usedmem, + } + ads.AddResourceUsage(dev, cono_dev.Usedcores, cono_dev.Usedmem) + klog.V(5).Infof("add resource usage for pod %s. device %s usedmem %d", pod.Name, dev.DeviceInfo.ID, cono_dev.Usedmem) + } } } @@ -361,6 +377,14 @@ func (ads *AscendDevices) DeepCopy() interface{} { DeviceInfo: dev.DeviceInfo, DeviceUsage: newUsage, Score: dev.Score, + PodMap: make(map[string]*devices.DeviceUsage), + } + for k, v := range dev.PodMap { + newDev.PodMap[k] = &devices.DeviceUsage{ + Used: v.Used, + Usedmem: v.Usedmem, + Usedcores: v.Usedcores, + } } cp.Devices[id] = newDev } @@ -481,7 +505,7 @@ func fit(req *devices.ContainerDeviceRequest, dev *AscendDevice) bool { func getDeviceSnapshot(ads *AscendDevices) []*AscendDevice { dupDevs := make([]*AscendDevice, 0, len(ads.Devices)) for _, dev := range ads.Devices { - dup_dev := &AscendDevice{ + dupDev := &AscendDevice{ config: dev.config, nodeRegisterAnno: dev.nodeRegisterAnno, useUUIDAnno: dev.useUUIDAnno, @@ -493,8 +517,16 @@ func getDeviceSnapshot(ads *AscendDevices) []*AscendDevice { Usedmem: dev.DeviceUsage.Usedmem, Usedcores: dev.DeviceUsage.Usedcores, }, + PodMap: make(map[string]*devices.DeviceUsage), + } + for k, v := range dev.PodMap { + dupDev.PodMap[k] = &devices.DeviceUsage{ + Used: v.Used, + Usedmem: v.Usedmem, + Usedcores: v.Usedcores, + } } - dupDevs = append(dupDevs, dup_dev) + dupDevs = append(dupDevs, dupDev) } return dupDevs } From f3b53d604f3fa5e9d6cd6ce5bd680c9c4dc4ac6a Mon Sep 17 00:00:00 2001 From: james Date: Tue, 2 Jun 2026 12:21:35 +0800 Subject: [PATCH 2/2] add test case Signed-off-by: james --- .../devices/ascend/hami/device_info_test.go | 281 ++++++++++++++++++ 1 file changed, 281 insertions(+) diff --git a/pkg/scheduler/api/devices/ascend/hami/device_info_test.go b/pkg/scheduler/api/devices/ascend/hami/device_info_test.go index 157c37be902..6c60be4e9f4 100644 --- a/pkg/scheduler/api/devices/ascend/hami/device_info_test.go +++ b/pkg/scheduler/api/devices/ascend/hami/device_info_test.go @@ -25,6 +25,9 @@ import ( "github.com/stretchr/testify/assert" "gopkg.in/yaml.v2" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" ) var config_yaml = ` @@ -389,3 +392,281 @@ func Test_verifyReq(t *testing.T) { }) } } + +func TestAscendDevices_AddResource(t *testing.T) { + tests := []struct { + name string + ascendDevices *AscendDevices + pod *v1.Pod + expectedUsage map[string]*devices.DeviceUsage // device ID -> expected DeviceUsage + }{ + { + name: "Successfully add single device resource", + ascendDevices: createTestAscendDevices("node1", "Ascend910", map[string]*AscendDevice{ + "device-1": createTestAscendDevice("device-1", 100, 32000, 0, 0, 0, nil), + }), + pod: createTestPod("test-pod", "ns1", map[string]string{ + devices.SupportDevices["Ascend910"]: "device-1,Ascend910,16000,50", + }), + expectedUsage: map[string]*devices.DeviceUsage{ + "device-1": { + Used: 1, + Usedmem: 16000, + Usedcores: 50, + }, + }, + }, + { + name: "Successfully add multiple device resources", + ascendDevices: createTestAscendDevices("node1", "Ascend910", map[string]*AscendDevice{ + "device-1": createTestAscendDevice("device-1", 100, 32000, 0, 0, 0, nil), + "device-2": createTestAscendDevice("device-2", 100, 32000, 0, 0, 0, nil), + }), + pod: createTestPod("test-pod", "ns1", map[string]string{ + devices.SupportDevices["Ascend910"]: "device-1,Ascend910,8000,25:device-2,Ascend910,8000,25", + }), + expectedUsage: map[string]*devices.DeviceUsage{ + "device-1": { + Used: 1, + Usedmem: 8000, + Usedcores: 25, + }, + "device-2": { + Used: 1, + Usedmem: 8000, + Usedcores: 25, + }, + }, + }, + { + name: "Add resource to existing device usage - accumulate", + ascendDevices: func() *AscendDevices { + ads := createTestAscendDevices("node1", "Ascend910", map[string]*AscendDevice{ + "device-1": createTestAscendDevice("device-1", 100, 32000, 1, 8000, 25, map[string]*devices.DeviceUsage{ + "existing-pod": {Used: 1, Usedmem: 8000, Usedcores: 25}, + }), + }) + return ads + }(), + pod: createTestPod("new-pod", "ns1", map[string]string{ + devices.SupportDevices["Ascend910"]: "device-1,Ascend910,8000,25", + }), + expectedUsage: map[string]*devices.DeviceUsage{ + "device-1": { + Used: 2, // Should increase from 1 to 2 + Usedmem: 16000, // Should increase from 8000 to 16000 + Usedcores: 50, // Should increase from 25 to 50 + }, + }, + }, + { + name: "Add same pod multiple times - should not duplicate", + ascendDevices: createTestAscendDevices("node1", "Ascend910", map[string]*AscendDevice{ + "device-1": createTestAscendDevice("device-1", 100, 32000, 0, 0, 0, nil), + }), + pod: createTestPod("test-pod", "ns1", map[string]string{ + devices.SupportDevices["Ascend910"]: "device-1,Ascend910,16000,50", + }), + expectedUsage: map[string]*devices.DeviceUsage{ + "device-1": { + Used: 1, + Usedmem: 16000, + Usedcores: 50, + }, + }, + }, + { + name: "Pod has invalid annotation format", + ascendDevices: createTestAscendDevices("node1", "Ascend910", map[string]*AscendDevice{ + "device-1": createTestAscendDevice("device-1", 100, 32000, 0, 0, 0, nil), + }), + pod: createTestPod("test-pod", "ns1", map[string]string{ + devices.SupportDevices["Ascend910"]: "invalid format", + }), + expectedUsage: map[string]*devices.DeviceUsage{ + "device-1": { + Used: 0, + Usedmem: 0, + Usedcores: 0, + }, + }, + }, + { + name: "Pod missing required annotation", + ascendDevices: createTestAscendDevices("node1", "Ascend910", map[string]*AscendDevice{ + "device-1": createTestAscendDevice("device-1", 100, 32000, 0, 0, 0, nil), + }), + pod: createTestPod("test-pod", "ns1", map[string]string{}), + expectedUsage: map[string]*devices.DeviceUsage{ + "device-1": { + Used: 0, + Usedmem: 0, + Usedcores: 0, + }, + }, + }, + { + name: "Device not found in devices map", + ascendDevices: createTestAscendDevices("node1", "Ascend910", map[string]*AscendDevice{ + "device-1": createTestAscendDevice("device-1", 100, 32000, 0, 0, 0, nil), + }), + pod: createTestPod("test-pod", "ns1", map[string]string{ + devices.SupportDevices["Ascend910"]: "non-existent-device,Ascend910,16000,50", + }), + expectedUsage: map[string]*devices.DeviceUsage{ + "device-1": { + Used: 0, + Usedmem: 0, + Usedcores: 0, + }, + }, + }, + { + name: "Multiple containers with different devices", + ascendDevices: createTestAscendDevices("node1", "Ascend910", map[string]*AscendDevice{ + "device-1": createTestAscendDevice("device-1", 100, 32000, 0, 0, 0, nil), + "device-2": createTestAscendDevice("device-2", 100, 32000, 0, 0, 0, nil), + "device-3": createTestAscendDevice("device-3", 100, 32000, 0, 0, 0, nil), + }), + pod: createTestPod("test-pod", "ns1", map[string]string{ + devices.SupportDevices["Ascend910"]: "device-1,Ascend910,4000,10:device-2,Ascend910,4000,10", + }), + expectedUsage: map[string]*devices.DeviceUsage{ + "device-1": { + Used: 1, + Usedmem: 4000, + Usedcores: 10, + }, + "device-2": { + Used: 1, + Usedmem: 4000, + Usedcores: 10, + }, + "device-3": { + Used: 0, + Usedmem: 0, + Usedcores: 0, + }, + }, + }, + { + name: "Nil AscendDevices receiver", + ascendDevices: nil, + pod: createTestPod("test-pod", "ns1", map[string]string{ + devices.SupportDevices["Ascend910"]: `[{"UUID":"device-1","Type":"Ascend910","Usedmem":16000,"Usedcores":50}]`, + }), + expectedUsage: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Handle nil receiver case separately + if tt.ascendDevices == nil { + defer func() { + if r := recover(); r != nil { + t.Errorf("AddResource panicked with nil receiver: %v", r) + } + }() + tt.ascendDevices.AddResource(tt.pod) + return + } + + // For test case where adding same pod twice + if tt.name == "Add same pod multiple times - should not duplicate" { + // First addition + tt.ascendDevices.AddResource(tt.pod) + // Second addition (should not duplicate) + tt.ascendDevices.AddResource(tt.pod) + } else { + tt.ascendDevices.AddResource(tt.pod) + } + + // Verify resource usage for each device + for deviceID, expected := range tt.expectedUsage { + dev, exists := tt.ascendDevices.Devices[deviceID] + if !exists && expected.Used > 0 { + t.Errorf("Device %s not found in AscendDevices", deviceID) + continue + } + if dev != nil { + if dev.DeviceUsage.Used != expected.Used { + t.Errorf("Device %s: Expected Used=%d, got %d", deviceID, expected.Used, dev.DeviceUsage.Used) + } + if dev.DeviceUsage.Usedmem != expected.Usedmem { + t.Errorf("Device %s: Expected Usedmem=%d, got %d", deviceID, expected.Usedmem, dev.DeviceUsage.Usedmem) + } + if dev.DeviceUsage.Usedcores != expected.Usedcores { + t.Errorf("Device %s: Expected Usedcores=%d, got %d", deviceID, expected.Usedcores, dev.DeviceUsage.Usedcores) + } + } + } + }) + } +} + +func createTestAscendDevices(nodeName, deviceType string, devices map[string]*AscendDevice) *AscendDevices { + return &AscendDevices{ + NodeName: nodeName, + Type: deviceType, + Devices: devices, + Policy: binpackPolicy, + } +} + +func createTestAscendDevice(id string, totalCores, totalMem int32, used, usedmem, usedcores int32, existingPods map[string]*devices.DeviceUsage) *AscendDevice { + device := &AscendDevice{ + DeviceInfo: &devices.DeviceInfo{ + ID: id, + Devcore: totalCores, + Devmem: totalMem, + Count: 1, + }, + DeviceUsage: &devices.DeviceUsage{ + Used: used, + Usedmem: usedmem, + Usedcores: usedcores, + }, + PodMap: make(map[string]*devices.DeviceUsage), + config: config.VNPUConfig{ + CommonWord: "Ascend910", + ResourceName: "huawei.com/Ascend910", + ResourceMemoryName: "huawei.com/Ascend910Memory", + MemoryCapacity: 32000, + MemoryAllocatable: 32000, + }, + } + + // Add existing pods to PodMap + for podUID, usage := range existingPods { + device.PodMap[podUID] = &devices.DeviceUsage{ + Used: usage.Used, + Usedmem: usage.Usedmem, + Usedcores: usage.Usedcores, + } + } + + return device +} + +func createTestPod(name, namespace string, annotations map[string]string) *v1.Pod { + return &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + UID: getTestUID(name), + Annotations: annotations, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "test-container", + }, + }, + }, + } +} + +func getTestUID(podName string) types.UID { + return types.UID(podName + "-uid-1234") +}