feat(labeler): replace GET + PUT to PATCH calls in labeler

commons/pkg/kubeclient/nodepatch.go

@@ -0,0 +1,84 @@
+// Copyright (c) 2026, NVIDIA CORPORATION.  All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package kubeclient
+
+import (
+	"encoding/json"
+	"fmt"
+
+	v1 "k8s.io/api/core/v1"
+)
+
+// NodeMergePatch builds an RFC 7386 JSON merge patch carrying the label and
+// annotation differences between original and modified. It returns a nil patch when
+// the two already agree, so callers can skip the write instead of spending an API
+// call on a no-op.
+//
+// The patch is assembled key by key rather than by marshalling modified, because
+// marshalling a Node emits every populated field. A caller that read original from an
+// informer cache holding a projected Node would then patch the projection's gaps back
+// over the real object. Emitting only keys that differ means fields missing from both
+// sides are left untouched.
+//
+// Spec fields such as taints and unschedulable are deliberately out of scope: a merge
+// patch replaces a list wholesale, so patching taints from a projected Node whose Spec
+// had been cleared would silently drop every taint on the real object.
+func NodeMergePatch(original, modified *v1.Node) ([]byte, error) {
+	metadata := map[string]any{}
+
+	if labels := stringMapMergePatch(original.Labels, modified.Labels); labels != nil {
+		metadata["labels"] = labels
+	}
+
+	if annotations := stringMapMergePatch(original.Annotations, modified.Annotations); annotations != nil {
+		metadata["annotations"] = annotations
+	}
+
+	if len(metadata) == 0 {
+		return nil, nil
+	}
+
+	patch, err := json.Marshal(map[string]any{"metadata": metadata})
+	if err != nil {
+		return nil, fmt.Errorf("marshal merge patch for node %s: %w", original.Name, err)
+	}
+
+	return patch, nil
+}
+
+// stringMapMergePatch returns the merge patch entries that turn original into
+// modified: added and changed keys map to their new value, removed keys map to nil so
+// the API server deletes them. It returns nil when the two maps already agree.
+func stringMapMergePatch(original, modified map[string]string) map[string]any {
+	patch := map[string]any{}
+
+	for key, value := range modified {
+		if current, exists := original[key]; !exists || current != value {
+			patch[key] = value
+		}
+	}
+
+	for key := range original {
+		if _, exists := modified[key]; !exists {
+			patch[key] = nil
+		}
+	}
+
+	if len(patch) == 0 {
+		return nil
+	}
+
+	return patch
+}

commons/pkg/kubeclient/nodepatch_test.go

@@ -0,0 +1,145 @@
+// Copyright (c) 2026, NVIDIA CORPORATION.  All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package kubeclient
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func node(labels, annotations map[string]string) *v1.Node {
+	return &v1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "node-1",
+			Labels:      labels,
+			Annotations: annotations,
+		},
+	}
+}
+
+func TestNodeMergePatch_MetadataChanges_ReturnsExpectedPatch(t *testing.T) {
+	tests := []struct {
+		name     string
+		original *v1.Node
+		modified *v1.Node
+		expected string
+	}{
+		{
+			name:     "no change produces no patch",
+			original: node(map[string]string{"a": "1"}, map[string]string{"b": "2"}),
+			modified: node(map[string]string{"a": "1"}, map[string]string{"b": "2"}),
+			expected: "",
+		},
+		{
+			name:     "adds a label",
+			original: node(map[string]string{"a": "1"}, nil),
+			modified: node(map[string]string{"a": "1", "b": "2"}, nil),
+			expected: `{"metadata":{"labels":{"b":"2"}}}`,
+		},
+		{
+			name:     "changes a label without mentioning the others",
+			original: node(map[string]string{"a": "1", "b": "2"}, nil),
+			modified: node(map[string]string{"a": "9", "b": "2"}, nil),
+			expected: `{"metadata":{"labels":{"a":"9"}}}`,
+		},
+		{
+			name:     "removes a label with an explicit null",
+			original: node(map[string]string{"a": "1", "b": "2"}, nil),
+			modified: node(map[string]string{"a": "1"}, nil),
+			expected: `{"metadata":{"labels":{"b":null}}}`,
+		},
+		{
+			name:     "adds an annotation",
+			original: node(nil, nil),
+			modified: node(nil, map[string]string{"bootstrap": "true"}),
+			expected: `{"metadata":{"annotations":{"bootstrap":"true"}}}`,
+		},
+		{
+			name:     "carries labels and annotations in a single patch",
+			original: node(map[string]string{"a": "1"}, nil),
+			modified: node(map[string]string{"a": "2"}, map[string]string{"bootstrap": "true"}),
+			expected: `{"metadata":{"annotations":{"bootstrap":"true"},"labels":{"a":"2"}}}`,
+		},
+		{
+			name:     "a nil map and an empty map are the same thing",
+			original: node(nil, nil),
+			modified: node(map[string]string{}, map[string]string{}),
+			expected: "",
+		},
+		{
+			name:     "sets a label onto a node that had none",
+			original: node(nil, nil),
+			modified: node(map[string]string{"a": "1"}, nil),
+			expected: `{"metadata":{"labels":{"a":"1"}}}`,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			patch, err := NodeMergePatch(tt.original, tt.modified)
+			require.NoError(t, err)
+
+			if tt.expected == "" {
+				assert.Nil(t, patch, "equivalent nodes must not cost an API call")
+				return
+			}
+
+			assert.JSONEq(t, tt.expected, string(patch))
+		})
+	}
+}
+
+// TestNodeMergePatchLeavesProjectedFieldsAlone pins the reason the patch is built key
+// by key. Informer caches often hold a projected Node — the labeler's transform keeps
+// only one annotation and clears Spec entirely — and a patch derived from that
+// projection must not describe the fields the projection dropped, or it would erase
+// them on the real object.
+func TestNodeMergePatch_ProjectedFields_LeavesThemAlone(t *testing.T) {
+	projected := &v1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "node-1",
+			Labels:      map[string]string{"gpu": "true"},
+			Annotations: map[string]string{"kept": "yes"},
+		},
+	}
+
+	modified := projected.DeepCopy()
+	modified.Labels["driver.installed"] = "true"
+
+	patch, err := NodeMergePatch(projected, modified)
+	require.NoError(t, err)
+
+	assert.JSONEq(t, `{"metadata":{"labels":{"driver.installed":"true"}}}`, string(patch))
+	assert.NotContains(t, string(patch), "annotations",
+		"an untouched annotation must not appear in the patch")
+	assert.NotContains(t, string(patch), "spec",
+		"a cleared Spec must never reach the patch, or real taints would be dropped")
+}
+
+func TestNodeMergePatch_SpecChanges_ReturnsNoPatch(t *testing.T) {
+	original := node(nil, nil)
+	modified := original.DeepCopy()
+	modified.Spec.Unschedulable = true
+	modified.Spec.Taints = []v1.Taint{{Key: "held", Effect: v1.TaintEffectNoSchedule}}
+
+	patch, err := NodeMergePatch(original, modified)
+	require.NoError(t, err)
+
+	assert.Nil(t, patch, "spec is out of scope until a caller needs it")
+}

labeler/pkg/labeler/labeler.go

@@ -26,6 +26,7 @@ import (
 	resourcev1 "k8s.io/api/resource/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/informers"
 	resourceinformers "k8s.io/client-go/informers/resource/v1"
 	"k8s.io/client-go/kubernetes"
@@ -35,6 +36,7 @@ import (
 
 	listersv1 "k8s.io/client-go/listers/core/v1"
 
+	"github.com/nvidia/nvsentinel/commons/pkg/kubeclient"
 	"github.com/nvidia/nvsentinel/commons/pkg/managed"
 	"github.com/nvidia/nvsentinel/commons/pkg/stringutil"
 	"github.com/nvidia/nvsentinel/labeler/pkg/devicecounts"
@@ -782,25 +784,24 @@ func (l *Labeler) updateNodeLabelsForPod(nodeName, expectedDCGMVersion, expected
 	}
 
 	err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {
-		node, err := l.clientset.CoreV1().Nodes().Get(l.ctx, nodeName, metav1.GetOptions{})
+		current, err := l.nodeForPatch(nodeName)
 		if err != nil {
 			return err
 		}
 
-		if node.Labels == nil {
-			node.Labels = make(map[string]string)
+		desired := current.DeepCopy()
+		if desired.Labels == nil {
+			desired.Labels = make(map[string]string)
 		}
 
-		needsUpdate := l.updateDriverAndDCGMLabels(node, expectedDriverLabel, expectedDCGMVersion)
+		needsUpdate := l.updateDriverAndDCGMLabels(desired, expectedDriverLabel, expectedDCGMVersion)
 
 		if !needsUpdate {
 			slog.Debug("Node already has correct pod-related labels", "node", nodeName)
 			return nil
 		}
 
-		_, err = l.clientset.CoreV1().Nodes().Update(l.ctx, node, metav1.UpdateOptions{})
-
-		return err
+		return l.patchNode(current, desired)
 	})
 	if err != nil {
 		metrics.NodeUpdateFailures.Inc()
@@ -838,24 +839,68 @@ func (l *Labeler) updateNodeLabelsAttempt(nodeName string) error {
 		return fmt.Errorf("failed to calculate desired node labels for %s: %w", nodeName, err)
 	}
 
-	node, err := l.clientset.CoreV1().Nodes().Get(l.ctx, nodeName, metav1.GetOptions{})
+	current, err := l.nodeForPatch(nodeName)
 	if err != nil {
-		return fmt.Errorf("get node %s: %w", nodeName, err)
+		return err
 	}
 
-	if node.Labels == nil {
-		node.Labels = make(map[string]string)
+	desired := current.DeepCopy()
+	if desired.Labels == nil {
+		desired.Labels = make(map[string]string)
 	}
 
-	needsUpdate := l.reconcileNodeLabelsInPlace(node, driverLabel, dcgmVersion)
+	needsUpdate := l.reconcileNodeLabelsInPlace(desired, driverLabel, dcgmVersion)
 	if !needsUpdate {
 		slog.Debug("Node labels are correct", "node", nodeName)
 		return nil
 	}
 
-	_, err = l.clientset.CoreV1().Nodes().Update(l.ctx, node, metav1.UpdateOptions{})
+	return l.patchNode(current, desired)
+}
+
+// nodeForPatch returns the Node to diff a write against, preferring the informer
+// cache so that a label write costs one API call rather than a GET followed by a
+// write. The cached Node is a projection — the transform keeps the labels and the
+// DCGM bootstrap annotation — which is enough, because the patch only ever
+// describes keys the caller changed itself.
+//
+// The returned Node is the cache's own object on the hit path, so callers must
+// treat it as read-only and reconcile against a copy.
+func (l *Labeler) nodeForPatch(nodeName string) (*v1.Node, error) {
+	// Before the caches are warm a miss says nothing about the node, so read through
+	// to the API server rather than skipping a label the node genuinely needs.
+	if l.allInformersSynced() {
+		if node, err := l.getNodeFromCache(nodeName); err == nil {
+			return node, nil
+		}
+	}
+
+	node, err := l.clientset.CoreV1().Nodes().Get(l.ctx, nodeName, metav1.GetOptions{})
 	if err != nil {
-		return fmt.Errorf("update node %s: %w", nodeName, err)
+		return nil, fmt.Errorf("get node %s: %w", nodeName, err)
+	}
+
+	return node, nil
+}
+
+// patchNode writes the difference between current and desired as a JSON merge patch.
+// A patch that would say nothing is skipped, so a reconcile that finds the node
+// already correct costs no API call at all.
+func (l *Labeler) patchNode(current, desired *v1.Node) error {
+	patch, err := kubeclient.NodeMergePatch(current, desired)
+	if err != nil {
+		return err
+	}
+
+	if patch == nil {
+		slog.Debug("Node labels already match the cached state", "node", current.Name)
+		return nil
+	}
+
+	if _, err := l.clientset.CoreV1().Nodes().Patch(
+		l.ctx, current.Name, types.MergePatchType, patch, metav1.PatchOptions{},
+	); err != nil {
+		return fmt.Errorf("patch node %s: %w", current.Name, err)
 	}
 
 	return nil