Skip to content

[release-1.15] fix ascend vnpu addResource #5377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
volcano-sh-bot merged 2 commits into from
Jun 9, 2026
+329 −16
Merged

[release-1.15] fix ascend vnpu addResource #5377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa111fd
fix addResource
DSFans2014 Jun 2, 2026
f3b53d6
add test case
DSFans2014 Jun 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 48 additions & 16 deletions pkg/scheduler/api/devices/ascend/hami/device_info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ type AscendDevice struct {
DeviceInfo *devices.DeviceInfo
DeviceUsage *devices.DeviceUsage
Score float64
PodMap map[string]*devices.DeviceUsage
}

type AscendDevices struct {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
}
Comment on lines +208 to +216

@gemini-code-assist gemini-code-assist Bot Jun 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If dev.PodMap is nil, assigning a value to it will cause a runtime panic (panic: assignment to entry in nil map). Although PodMap is initialized in NewAscendDevices and DeepCopy, it is safer to lazily initialize it if it is nil to prevent potential panics if AscendDevice is constructed or modified elsewhere without proper initialization.

		if dev.PodMap == nil {
			dev.PodMap = make(map[string]*devices.DeviceUsage)
		}
		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)
		}

@JesseStutler JesseStutler Jun 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DSFans2014 Could you check it?

@DSFans2014 DSFans2014 Jun 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New instances of AscendDevice are created through NewAscendDevices, DeepCopy, and getDeviceSnapshot. All of these interfaces initialize the PodMap, so there will be no null pointer issues currently.
From the perspective of defensive programming, I can add null pointer checks.

@DSFans2014 DSFans2014 Jun 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I submitted a PR on the bot for this change: volcano-sh-bot#1. I'm not sure if this is the correct way to modify the bot's branch. If not, please let me know how to do it. @JesseStutler
I also have added this change to #5378 which will be merged to master

@JesseStutler JesseStutler Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, If there are other places already init the struct, I think we can safely merge it, because AI may also have illusions and can't get the context of other places of codes

}
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
Expand Down
Loading
Loading