From f89749bd8a4b4e8ebe640b77fd21037e195c37c1 Mon Sep 17 00:00:00 2001 From: mesutoezdil Date: Sun, 11 Oct 2026 14:44:18 +0200 Subject: [PATCH] fix(plugin): remove the node capacity entries when reporting is off With -report_node_capacity the plugin patches amd.com/gpumem and amd.com/gpucores into the node status. Kubelet keeps an extended resource it did not advertise itself, so after turning the option off the old values stayed and kubelet rejected Pods asking for more than the stale amount. Clear the two resources when they are still listed and reporting is off, and grant the nodes/status patch permission unconditionally so the plugin may do so. Signed-off-by: mesutoezdil --- helm/amd-gpu/templates/rbac.yaml | 5 +++-- internal/pkg/plugin/plugin.go | 4 ++++ internal/pkg/utils/utils.go | 25 +++++++++++++++++++++++ internal/pkg/utils/utils_test.go | 35 ++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 2 deletions(-) diff --git a/helm/amd-gpu/templates/rbac.yaml b/helm/amd-gpu/templates/rbac.yaml index 0b448e9b..5cc0cb2e 100644 --- a/helm/amd-gpu/templates/rbac.yaml +++ b/helm/amd-gpu/templates/rbac.yaml @@ -10,11 +10,12 @@ rules: - apiGroups: [""] resources: ["nodes"] verbs: ["get", "patch"] -{{- if .Values.dp.reportNodeCapacity }} + # Publishes amd.com/gpumem and amd.com/gpucores with dp.reportNodeCapacity, + # and removes them again once it is turned off: kubelet keeps a value it + # did not advertise itself. - apiGroups: [""] resources: ["nodes/status"] verbs: ["patch"] -{{- end }} - apiGroups: [""] resources: ["events"] verbs: ["create"] diff --git a/internal/pkg/plugin/plugin.go b/internal/pkg/plugin/plugin.go index cc1f1794..cee3b280 100644 --- a/internal/pkg/plugin/plugin.go +++ b/internal/pkg/plugin/plugin.go @@ -385,6 +385,10 @@ func (p *AMDGPUPlugin) RegisterInAnnotation() error { if err := utils.PatchNodeStatusCapacity(node, healthyCapacity(devices)); err != nil { glog.Errorf("patch node capacity error: %v", err) } + } else if err := utils.ClearNodeStatusCapacity(node, gpuMemResource, gpuCoresResource); err != nil { + // A run with reporting on left these behind; kubelet would keep + // rejecting Pods that ask for more than the stale value. + glog.Errorf("clear stale node capacity error: %v", err) } warnModeChangeInUse(node, devices) diff --git a/internal/pkg/utils/utils.go b/internal/pkg/utils/utils.go index 704972a6..7c7fd0af 100644 --- a/internal/pkg/utils/utils.go +++ b/internal/pkg/utils/utils.go @@ -140,6 +140,31 @@ func PatchNodeStatusCapacity(node *corev1.Node, resources corev1.ResourceList) e return err } +// ClearNodeStatusCapacity removes the named resources from the node's status +// capacity and allocatable when it still lists them. Kubelet never drops an +// extended resource it does not advertise itself, so a value patched earlier +// stays until something removes it. +func ClearNodeStatusCapacity(node *corev1.Node, names ...corev1.ResourceName) error { + stale := map[string]any{} + for _, n := range names { + _, inCapacity := node.Status.Capacity[n] + _, inAllocatable := node.Status.Allocatable[n] + if inCapacity || inAllocatable { + stale[string(n)] = nil + } + } + if len(stale) == 0 { + return nil + } + bytes, err := json.Marshal(map[string]any{"status": map[string]any{"capacity": stale, "allocatable": stale}}) + if err != nil { + return err + } + _, err = GetClient().CoreV1().Nodes(). + Patch(context.Background(), node.Name, k8stypes.StrategicMergePatchType, bytes, metav1.PatchOptions{}, "status") + return err +} + func PatchNodeAnnotations(node *corev1.Node, annotations map[string]string) error { type patchMetadata struct { Annotations map[string]string `json:"annotations,omitempty"` diff --git a/internal/pkg/utils/utils_test.go b/internal/pkg/utils/utils_test.go index 5955a7e9..28f18afa 100644 --- a/internal/pkg/utils/utils_test.go +++ b/internal/pkg/utils/utils_test.go @@ -174,6 +174,41 @@ func TestDecodePodDevicesMalformed(t *testing.T) { } } +func TestClearNodeStatusCapacity(t *testing.T) { + stale := corev1.ResourceList{"amd.com/gpumem": resource.MustParse("16304"), "amd.com/gpucores": resource.MustParse("32"), "cpu": resource.MustParse("4")} + node := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "n"}, Status: corev1.NodeStatus{Capacity: stale, Allocatable: stale.DeepCopy()}} + cs := setup(t, node) + if err := ClearNodeStatusCapacity(node, "amd.com/gpumem", "amd.com/gpucores"); err != nil { + t.Fatal(err) + } + got, err := cs.CoreV1().Nodes().Get(context.Background(), "n", metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + for _, list := range []corev1.ResourceList{got.Status.Capacity, got.Status.Allocatable} { + for _, name := range []corev1.ResourceName{"amd.com/gpumem", "amd.com/gpucores"} { + if _, still := list[name]; still { + t.Errorf("%s is still listed after the clear", name) + } + } + if _, ok := list["cpu"]; !ok { + t.Error("an unrelated resource was removed") + } + } + + // Nothing stale: no write. + clean := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "m"}} + cs = setup(t, clean) + if err := ClearNodeStatusCapacity(clean, "amd.com/gpumem"); err != nil { + t.Fatal(err) + } + for _, a := range cs.Actions() { + if a.GetVerb() == "patch" { + t.Fatal("a node without the resources must not be patched") + } + } +} + func TestPatchNodeStatusCapacity(t *testing.T) { cs := setup(t, &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "n"}}) res := corev1.ResourceList{"amd.com/gpumem": resource.MustParse("16304")}