Merge branch 'main' into ladithyav-readme-simplification-2

Author: lalitadithya

.golangci.yml

@@ -14,7 +14,7 @@
 
 version: "2"
 run:
-  go: "1.23"
+  go: "1.27"
   modules-download-mode: readonly
   issues-exit-code: 1
   tests: false

.versions.yaml

@@ -31,7 +31,7 @@
 
 # Language Versions
 languages:
-  go: '1.26.3'
+  go: '1.27.0'
   python: '3.13'
 
 # Build & Package Management
@@ -41,7 +41,7 @@ build_tools:
 
 # Go Development Tools
 go_tools:
-  golangci_lint: 'v2.5.0'
+  golangci_lint: 'v2.13.1'
   gotestsum: 'v1.13.0'
   gocover_cobertura: 'v1.4.0'
   setup_envtest: 'v0.0.0-20260119123727-a2de7e94d2dd'

CONTRIBUTING.md

@@ -58,7 +58,7 @@ When reporting issues:
 ## Development Setup
 
 **Prerequisites**:
-- Go 1.25+ (see `.versions.yaml` for exact version)
+- Go 1.27+ (see `.versions.yaml` for exact version)
 - Kubernetes cluster (for testing)
 - Docker (for container builds)
 - Make (for build targets)

DEVELOPMENT.md

@@ -40,7 +40,7 @@ make show-versions
 ### Prerequisites
 
 **Core Tools** (required):
-- [Go 1.25+](https://golang.org/dl/) - See `.versions.yaml` for exact version
+- [Go 1.27+](https://golang.org/dl/) - See `.versions.yaml` for exact version
 - [Docker](https://docs.docker.com/get-docker/)
 - [kubectl](https://kubernetes.io/docs/tasks/tools/install-kubectl/)
 - [Helm 3.0+](https://helm.sh/docs/intro/install/)

api/gen/go/device/v1alpha1/gpu.pb.go

@@ -1,8 +1,6 @@
 module github.com/nvidia/nvsentinel/api
 
-go 1.26.0
-
-toolchain go1.26.3
+go 1.27.0
 
 require (
 	google.golang.org/grpc v1.83.1

api/go.mod

@@ -1,8 +1,6 @@
 module github.com/nvidia/nvsentinel/commons
 
-go 1.26.0
-
-toolchain go1.26.3
+go 1.27.0
 
 require (
 	github.com/BurntSushi/toml v1.6.0

commons/go.mod

@@ -19,6 +19,9 @@ import (
 	"github.com/prometheus/client_golang/prometheus/promauto"
 )
 
+// labelMonitor is the agent-name label shared by every publisher counter.
+const labelMonitor = "monitor"
+
 // Counters are registered against the default registry via promauto.
 // The `monitor` label is the agent name; `code` on sendsError is a gRPC
 // status-code string (bounded cardinality, ~17 values).
@@ -28,22 +31,22 @@ var (
 			Name: "nvsentinel_health_events_publisher_skipped_pc_unavailable_total",
 			Help: "Total health-event sends skipped because the platform-connector Unix socket was missing.",
 		},
-		[]string{"monitor"},
+		[]string{labelMonitor},
 	)
 
 	sendsSuccess = promauto.NewCounterVec(
 		prometheus.CounterOpts{
 			Name: "nvsentinel_health_events_publisher_sends_success_total",
 			Help: "Total successful health-event sends.",
 		},
-		[]string{"monitor"},
+		[]string{labelMonitor},
 	)
 
 	sendsError = promauto.NewCounterVec(
 		prometheus.CounterOpts{
 			Name: "nvsentinel_health_events_publisher_sends_error_total",
 			Help: "Total failed health-event sends after retries exhausted (excluding socket-missing skips).",
 		},
-		[]string{"monitor", "code"},
+		[]string{labelMonitor, "code"},
 	)
 )

commons/pkg/healthpub/metrics.go

@@ -19,6 +19,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"sync"
 	"time"
 
@@ -50,7 +51,15 @@ func (p *NodePatcher) Patch(
 	cached *v1.Node,
 	mutate func(*v1.Node) error,
 ) (bool, error) {
-	current, err := p.currentNode(ctx, nodes, nodeName, cached)
+	var current *v1.Node
+
+	err := retry.OnError(nodePatchBackoff(), isRetryableNodePatchError, func() error {
+		var err error
+
+		current, err = p.currentNode(ctx, nodes, nodeName, cached)
+
+		return err
+	})
 	if err != nil {
 		return false, err
 	}
@@ -85,11 +94,13 @@ func (p *NodePatcher) Patch(
 		if errors.IsConflict(err) {
 			patchErr := err
 
-			current, err = nodes.Get(ctx, nodeName, metav1.GetOptions{})
+			refreshed, err := nodes.Get(ctx, nodeName, metav1.GetOptions{})
 			if err != nil {
 				return fmt.Errorf("refresh node %q after patch conflict: %w", nodeName, err)
 			}
 
+			current = refreshed
+
 			return fmt.Errorf("patch node %q: %w", nodeName, patchErr)
 		}
 
@@ -134,8 +145,6 @@ func (p *NodePatcher) currentNode(
 		return nil, fmt.Errorf("refresh node %q while pending write is not in cache: %w", nodeName, err)
 	}
 
-	p.pendingVersions.CompareAndDelete(nodeName, writtenVersionValue)
-
 	return current, nil
 }
 
@@ -156,40 +165,36 @@ func isRetryableNodePatchError(err error) bool {
 		errors.IsServiceUnavailable(err)
 }
 
-// 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.
+// NodeMergePatch builds an RFC 7386 JSON merge patch carrying differences in labels,
+// annotations, taints, and unschedulable state. It returns a nil patch when the two
+// nodes already agree, so callers can skip a no-op write.
 //
-// CreateTwoWayMergePatch compares metadata-only projections of the two Nodes.
-// Excluding every other field from both inputs ensures an informer projection cannot
-// patch its gaps back over the live object.
+// CreateTwoWayMergePatch compares projections containing only the fields this helper
+// supports. Excluding every other field from both inputs ensures an informer
+// projection cannot patch its gaps back over the live object.
 //
-// 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.
+// Taints are emitted only when the caller changed them. A projected Node whose Spec
+// is empty on both sides therefore cannot erase taints from the real object.
 func NodeMergePatch(original, modified *v1.Node) ([]byte, error) {
-	originalMetadata := &v1.Node{
-		ObjectMeta: metav1.ObjectMeta{
-			Labels:      original.Labels,
-			Annotations: original.Annotations,
-		},
-	}
-	modifiedMetadata := &v1.Node{
-		ObjectMeta: metav1.ObjectMeta{
-			Labels:      modified.Labels,
-			Annotations: modified.Annotations,
-		},
+	originalProjection := projectNodePatchableFields(original)
+	modifiedProjection := projectNodePatchableFields(modified)
+
+	specChanged := !reflect.DeepEqual(original.Spec.Taints, modified.Spec.Taints) ||
+		original.Spec.Unschedulable != modified.Spec.Unschedulable
+	if specChanged {
+		// Lists in spec, such as taints, are replaced wholesale. ResourceVersion
+		// prevents a stale list from overwriting a concurrent update.
+		modifiedProjection.ResourceVersion = original.ResourceVersion
 	}
 
-	originalJSON, err := json.Marshal(originalMetadata)
+	originalJSON, err := json.Marshal(originalProjection)
 	if err != nil {
-		return nil, fmt.Errorf("marshal original metadata for node %q: %w", original.Name, err)
+		return nil, fmt.Errorf("marshal original patch projection for node %q: %w", original.Name, err)
 	}
 
-	modifiedJSON, err := json.Marshal(modifiedMetadata)
+	modifiedJSON, err := json.Marshal(modifiedProjection)
 	if err != nil {
-		return nil, fmt.Errorf("marshal modified metadata for node %q: %w", original.Name, err)
+		return nil, fmt.Errorf("marshal modified patch projection for node %q: %w", original.Name, err)
 	}
 
 	patch, err := strategicpatch.CreateTwoWayMergePatch(originalJSON, modifiedJSON, v1.Node{})
@@ -203,3 +208,18 @@ func NodeMergePatch(original, modified *v1.Node) ([]byte, error) {
 
 	return patch, nil
 }
+
+// projectNodePatchableFields restricts patch generation to the Node fields this
+// helper intentionally supports, preventing callbacks from patching unrelated fields.
+func projectNodePatchableFields(node *v1.Node) *v1.Node {
+	return &v1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Labels:      node.Labels,
+			Annotations: node.Annotations,
+		},
+		Spec: v1.NodeSpec{
+			Taints:        node.Spec.Taints,
+			Unschedulable: node.Spec.Unschedulable,
+		},
+	}
+}