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")}