Merge branch 'main' into xrfxlp/1596-labeler-patch

Author: XRFXLP

labeler/pkg/devicecounts/device_counts.go

@@ -222,6 +222,53 @@ func (m *Manager) NodeLabelsAffectDeviceCounts(oldLabels, newLabels map[string]s
 	return !maps.Equal(oldInputLabels, newInputLabels)
 }
 
+// NodeResourcesAffectDeviceCounts reports whether an allocatable or capacity
+// change on a node could affect a device-count class whose CEL expression reads
+// from node.status.allocatable or node.status.capacity.
+func (m *Manager) NodeResourcesAffectDeviceCounts(oldNode, newNode *corev1.Node) bool {
+	if !m.Enabled() || oldNode == nil || newNode == nil {
+		return false
+	}
+
+	resourcesReferenced := false
+
+	for _, class := range m.classes {
+		if class.referencesNodeResources() {
+			resourcesReferenced = true
+			break
+		}
+	}
+
+	if !resourcesReferenced {
+		return false
+	}
+
+	oldAllocatable := resourceListToStringMap(oldNode.Status.Allocatable)
+	newAllocatable := resourceListToStringMap(newNode.Status.Allocatable)
+
+	if !maps.Equal(oldAllocatable, newAllocatable) {
+		return true
+	}
+
+	oldCapacity := resourceListToStringMap(oldNode.Status.Capacity)
+	newCapacity := resourceListToStringMap(newNode.Status.Capacity)
+
+	return !maps.Equal(oldCapacity, newCapacity)
+}
+
+func resourceListToStringMap(rl corev1.ResourceList) map[string]string {
+	if len(rl) == 0 {
+		return nil
+	}
+
+	result := make(map[string]string, len(rl))
+	for k, v := range rl {
+		result[string(k)] = v.String()
+	}
+
+	return result
+}
+
 func newDeviceCountCELEnv() (*cel.Env, error) {
 	// Keep the CEL surface intentionally small: expressions can only inspect
 	// the reconciled node, that node's associated ResourceSlices, and sum lists.
@@ -577,6 +624,16 @@ func (class compiledClass) referencesResourceSlices() bool {
 	return strings.Contains(class.CurrentExpression, "resourceSlices")
 }
 
+// referencesNodeResources is a cheap heuristic mirroring referencesResourceSlices.
+// It checks specifically for node.status.allocatable or node.status.capacity
+// rather than the broader node.status, so expressions that only reference
+// node.status.conditions (noisy with heartbeats) do not trigger unnecessary
+// allocatable/capacity comparisons on every node update.
+func (class compiledClass) referencesNodeResources() bool {
+	return strings.Contains(class.CurrentExpression, "node.status.allocatable") ||
+		strings.Contains(class.CurrentExpression, "node.status.capacity")
+}
+
 func matchLabels(actual, expected map[string]string) bool {
 	for key, expectedValue := range expected {
 		if actual[key] != expectedValue {

labeler/pkg/devicecounts/device_counts_test.go

@@ -223,6 +223,244 @@ func TestManagerRequiresResourceSlices(t *testing.T) {
 	})
 }
 
+func TestNodeResourcesAffectDeviceCounts(t *testing.T) {
+	nicConfig := Config{
+		Enabled: true,
+		Classes: []ClassConfig{
+			{
+				Name:    "nic",
+				Enabled: true,
+				Labels: Labels{
+					Current:  testNICCountCurrentLabel,
+					Expected: testNICCountExpectedLabel,
+				},
+				CurrentExpression: "int(node.status.allocatable['nvidia.com/mlnxnics'])",
+			},
+		},
+	}
+
+	gpuOnlyConfig := Config{
+		Enabled: true,
+		Classes: []ClassConfig{
+			{
+				Name:    "gpu",
+				Enabled: true,
+				Labels: Labels{
+					Current:  testGPUCountCurrentLabel,
+					Expected: testGPUCountExpectedLabel,
+				},
+				CurrentExpression: "int(node.metadata.labels['nvidia.com/gpu.count'])",
+			},
+		},
+	}
+
+	t.Run("triggers on allocatable NIC count change", func(t *testing.T) {
+		manager := newTestManager(t, nicConfig)
+		oldNode := testNode("node-a", map[string]string{})
+		oldNode.Status.Allocatable = corev1.ResourceList{
+			corev1.ResourceName("nvidia.com/mlnxnics"): resource.MustParse("0"),
+		}
+
+		newNode := oldNode.DeepCopy()
+		newNode.Status.Allocatable = corev1.ResourceList{
+			corev1.ResourceName("nvidia.com/mlnxnics"): resource.MustParse("4"),
+		}
+
+		require.True(t, manager.NodeResourcesAffectDeviceCounts(oldNode, newNode))
+	})
+
+	t.Run("triggers on allocatable key added", func(t *testing.T) {
+		manager := newTestManager(t, nicConfig)
+		oldNode := testNode("node-a", map[string]string{})
+
+		newNode := oldNode.DeepCopy()
+		newNode.Status.Allocatable = corev1.ResourceList{
+			corev1.ResourceName("nvidia.com/mlnxnics"): resource.MustParse("4"),
+		}
+
+		require.True(t, manager.NodeResourcesAffectDeviceCounts(oldNode, newNode))
+	})
+
+	t.Run("triggers on allocatable key removed", func(t *testing.T) {
+		manager := newTestManager(t, nicConfig)
+		oldNode := testNode("node-a", map[string]string{})
+		oldNode.Status.Allocatable = corev1.ResourceList{
+			corev1.ResourceName("nvidia.com/mlnxnics"): resource.MustParse("4"),
+		}
+
+		newNode := oldNode.DeepCopy()
+		newNode.Status.Allocatable = nil
+
+		require.True(t, manager.NodeResourcesAffectDeviceCounts(oldNode, newNode))
+	})
+
+	t.Run("triggers on capacity change", func(t *testing.T) {
+		capacityConfig := Config{
+			Enabled: true,
+			Classes: []ClassConfig{
+				{
+					Name:    "nic",
+					Enabled: true,
+					Labels: Labels{
+						Current:  testNICCountCurrentLabel,
+						Expected: testNICCountExpectedLabel,
+					},
+					CurrentExpression: "int(node.status.capacity['nvidia.com/mlnxnics'])",
+				},
+			},
+		}
+
+		manager := newTestManager(t, capacityConfig)
+		oldNode := testNode("node-a", map[string]string{})
+		oldNode.Status.Capacity = corev1.ResourceList{
+			corev1.ResourceName("nvidia.com/mlnxnics"): resource.MustParse("4"),
+		}
+
+		newNode := oldNode.DeepCopy()
+		newNode.Status.Capacity = corev1.ResourceList{
+			corev1.ResourceName("nvidia.com/mlnxnics"): resource.MustParse("8"),
+		}
+
+		require.True(t, manager.NodeResourcesAffectDeviceCounts(oldNode, newNode))
+	})
+
+	t.Run("does not trigger when allocatable is unchanged", func(t *testing.T) {
+		manager := newTestManager(t, nicConfig)
+		oldNode := testNode("node-a", map[string]string{})
+		oldNode.Status.Allocatable = corev1.ResourceList{
+			corev1.ResourceName("nvidia.com/mlnxnics"): resource.MustParse("4"),
+		}
+
+		newNode := oldNode.DeepCopy()
+
+		require.False(t, manager.NodeResourcesAffectDeviceCounts(oldNode, newNode))
+	})
+
+	t.Run("does not trigger for nil vs empty allocatable", func(t *testing.T) {
+		manager := newTestManager(t, nicConfig)
+		oldNode := testNode("node-a", map[string]string{})
+		oldNode.Status.Allocatable = nil
+
+		newNode := oldNode.DeepCopy()
+		newNode.Status.Allocatable = corev1.ResourceList{}
+
+		require.False(t, manager.NodeResourcesAffectDeviceCounts(oldNode, newNode))
+	})
+
+	t.Run("does not trigger when no class references node status", func(t *testing.T) {
+		manager := newTestManager(t, gpuOnlyConfig)
+		oldNode := testNode("node-a", map[string]string{
+			"nvidia.com/gpu.count": "8",
+		})
+		oldNode.Status.Allocatable = corev1.ResourceList{
+			corev1.ResourceName("nvidia.com/mlnxnics"): resource.MustParse("0"),
+		}
+
+		newNode := oldNode.DeepCopy()
+		newNode.Status.Allocatable = corev1.ResourceList{
+			corev1.ResourceName("nvidia.com/mlnxnics"): resource.MustParse("4"),
+		}
+
+		require.False(t, manager.NodeResourcesAffectDeviceCounts(oldNode, newNode))
+	})
+
+	t.Run("detects change when one of multiple classes references status", func(t *testing.T) {
+		mixedConfig := Config{
+			Enabled: true,
+			Classes: []ClassConfig{
+				{
+					Name:    "gpu",
+					Enabled: true,
+					Labels: Labels{
+						Current:  testGPUCountCurrentLabel,
+						Expected: testGPUCountExpectedLabel,
+					},
+					CurrentExpression: "int(node.metadata.labels['nvidia.com/gpu.count'])",
+				},
+				{
+					Name:    "nic",
+					Enabled: true,
+					Labels: Labels{
+						Current:  testNICCountCurrentLabel,
+						Expected: testNICCountExpectedLabel,
+					},
+					CurrentExpression: "int(node.status.allocatable['nvidia.com/mlnxnics'])",
+				},
+			},
+		}
+
+		manager := newTestManager(t, mixedConfig)
+		oldNode := testNode("node-a", map[string]string{
+			"nvidia.com/gpu.count": "8",
+		})
+		oldNode.Status.Allocatable = corev1.ResourceList{
+			corev1.ResourceName("nvidia.com/mlnxnics"): resource.MustParse("0"),
+		}
+
+		newNode := oldNode.DeepCopy()
+		newNode.Status.Allocatable = corev1.ResourceList{
+			corev1.ResourceName("nvidia.com/mlnxnics"): resource.MustParse("4"),
+		}
+
+		require.True(t, manager.NodeResourcesAffectDeviceCounts(oldNode, newNode))
+	})
+
+	t.Run("returns false for disabled manager", func(t *testing.T) {
+		var manager *Manager
+		oldNode := testNode("node-a", map[string]string{})
+		newNode := oldNode.DeepCopy()
+
+		require.False(t, manager.NodeResourcesAffectDeviceCounts(oldNode, newNode))
+	})
+}
+
+func TestReferencesNodeResources(t *testing.T) {
+	t.Run("returns true for allocatable expression", func(t *testing.T) {
+		class := compiledClass{
+			ClassConfig: ClassConfig{
+				CurrentExpression: "int(node.status.allocatable['nvidia.com/mlnxnics'])",
+			},
+		}
+		require.True(t, class.referencesNodeResources())
+	})
+
+	t.Run("returns true for capacity expression", func(t *testing.T) {
+		class := compiledClass{
+			ClassConfig: ClassConfig{
+				CurrentExpression: "int(node.status.capacity['nvidia.com/mlnxnics'])",
+			},
+		}
+		require.True(t, class.referencesNodeResources())
+	})
+
+	t.Run("returns false for label expression", func(t *testing.T) {
+		class := compiledClass{
+			ClassConfig: ClassConfig{
+				CurrentExpression: "int(node.metadata.labels['nvidia.com/gpu.count'])",
+			},
+		}
+		require.False(t, class.referencesNodeResources())
+	})
+
+	t.Run("returns false for resourceSlices expression", func(t *testing.T) {
+		class := compiledClass{
+			ClassConfig: ClassConfig{
+				CurrentExpression: "resourceSlices.size()",
+			},
+		}
+		require.False(t, class.referencesNodeResources())
+	})
+
+	t.Run("returns false for conditions expression", func(t *testing.T) {
+		class := compiledClass{
+			ClassConfig: ClassConfig{
+				CurrentExpression: "node.status.conditions.exists(c, c.type == 'Ready')",
+			},
+		}
+		require.False(t, class.referencesNodeResources())
+	})
+}
+
 func newTestManager(t *testing.T, config Config) *Manager {
 	t.Helper()
 

labeler/pkg/labeler/labeler.go

@@ -631,10 +631,12 @@ func hasReadyDriverPod(objs []any, excludePod *v1.Pod) bool {
 	return false
 }
 
-// nodeRequiresReconciliation returns true only when a node update changed an
-// input label the labeler reads from nodes. DCGM and driver labels are driven
-// by pod events, so the node UpdateFunc only needs to react to changes in kata
-// detection labels and the gpu-present label (for assumeDriverInstalled mode).
+// nodeRequiresReconciliation returns true when a node update changed an input
+// that the labeler reads from nodes: labels, allocatable, or capacity. DCGM
+// and driver labels are driven by pod events, so the node UpdateFunc only needs
+// to react to changes in kata detection labels, the gpu-present label (for
+// assumeDriverInstalled mode), and resource changes for status-backed device
+// count classes.
 func (l *Labeler) nodeRequiresReconciliation(oldObj, newObj any) bool {
 	oldNode, ok1 := oldObj.(*v1.Node)
 	newNode, ok2 := newObj.(*v1.Node)
@@ -657,7 +659,8 @@ func (l *Labeler) nodeRequiresReconciliation(oldObj, newObj any) bool {
 		}
 	}
 
-	return l.deviceCounts.NodeLabelsAffectDeviceCounts(oldNode.Labels, newNode.Labels)
+	return l.deviceCounts.NodeResourcesAffectDeviceCounts(oldNode, newNode) ||
+		l.deviceCounts.NodeLabelsAffectDeviceCounts(oldNode.Labels, newNode.Labels)
 }
 
 const gpuPresentLabel = "nvidia.com/gpu.present"