koord-manager: nodeslo-controller enqueue request when node labels up…

…dated (#2201) Signed-off-by: joey <zchengjoey@gmail.com>
koordinator-sh · Sep 25, 2024 · f959cdb · f959cdb
1 parent b24da87
commit f959cdb
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 9 deletions.
diff --git a/pkg/slo-controller/nodemetric/node_event_handler.go b/pkg/slo-controller/nodemetric/node_event_handler.go
@@ -50,7 +50,7 @@ func (n *EnqueueRequestForNode) Create(ctx context.Context, e event.CreateEvent,
 func (n *EnqueueRequestForNode) Update(ctx context.Context, e event.UpdateEvent, q workqueue.RateLimitingInterface) {
 	newNode, oldNode := e.ObjectNew.(*corev1.Node), e.ObjectOld.(*corev1.Node)
 	// TODO, only use for noderesource
-	if !isNodeAllocatableUpdated(newNode, oldNode) {
+	if !isNodeUpdated(newNode, oldNode) {
 		return
 	}
 	q.Add(reconcile.Request{
@@ -75,10 +75,10 @@ func (n *EnqueueRequestForNode) Delete(ctx context.Context, e event.DeleteEvent,
 func (n *EnqueueRequestForNode) Generic(ctx context.Context, e event.GenericEvent, q workqueue.RateLimitingInterface) {
 }
 
-// isNodeAllocatableUpdated returns whether the new node's allocatable is different from the old one's
-func isNodeAllocatableUpdated(newNode *corev1.Node, oldNode *corev1.Node) bool {
+// isNodeUpdated returns whether the new node's allocatable or labels is different from the old one's
+func isNodeUpdated(newNode *corev1.Node, oldNode *corev1.Node) bool {
 	if newNode == nil || oldNode == nil {
 		return false
 	}
-	return !reflect.DeepEqual(oldNode.Status.Allocatable, newNode.Status.Allocatable)
+	return !reflect.DeepEqual(oldNode.Status.Allocatable, newNode.Status.Allocatable) || !reflect.DeepEqual(oldNode.Labels, newNode.Labels)
 }
diff --git a/pkg/slo-controller/nodemetric/node_event_handler_test.go b/pkg/slo-controller/nodemetric/node_event_handler_test.go
@@ -33,13 +33,13 @@ func Test_isNodeAllocatableUpdated(t *testing.T) {
 	assert := assert.New(t)
 	newNode := &corev1.Node{}
 	oldNode := &corev1.Node{}
-	assert.Equal(false, isNodeAllocatableUpdated(nil, oldNode))
-	assert.Equal(false, isNodeAllocatableUpdated(newNode, nil))
-	assert.Equal(false, isNodeAllocatableUpdated(nil, nil))
-	assert.Equal(false, isNodeAllocatableUpdated(newNode, oldNode))
+	assert.Equal(false, isNodeUpdated(nil, oldNode))
+	assert.Equal(false, isNodeUpdated(newNode, nil))
+	assert.Equal(false, isNodeUpdated(nil, nil))
+	assert.Equal(false, isNodeUpdated(newNode, oldNode))
 	newNode.Status.Allocatable = corev1.ResourceList{}
 	newNode.Status.Allocatable["test"] = resource.Quantity{}
-	assert.Equal(true, isNodeAllocatableUpdated(newNode, oldNode))
+	assert.Equal(true, isNodeUpdated(newNode, oldNode))
 }
 
 func Test_EnqueueRequestForNode(t *testing.T) {
@@ -154,6 +154,65 @@ func Test_EnqueueRequestForNode(t *testing.T) {
 			},
 			hasEvent: false,
 		},
+		{
+			name: "update node labels",
+			fn: func(handler *EnqueueRequestForNode, q workqueue.RateLimitingInterface) {
+				handler.Update(context.TODO(), event.UpdateEvent{
+					ObjectOld: &corev1.Node{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+							Labels: map[string]string{
+								"test": "test",
+							},
+						},
+					},
+					ObjectNew: &corev1.Node{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+							Labels: map[string]string{
+								"test": "test1",
+							},
+						},
+					},
+				}, q)
+			},
+			hasEvent:  true,
+			eventName: "node1",
+		},
+		{
+			name: "allocatable and labels not updated",
+			fn: func(handler *EnqueueRequestForNode, q workqueue.RateLimitingInterface) {
+				handler.Update(context.TODO(), event.UpdateEvent{
+					ObjectOld: &corev1.Node{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+							Labels: map[string]string{
+								"test": "test",
+							},
+						},
+						Status: corev1.NodeStatus{
+							Allocatable: corev1.ResourceList{
+								corev1.ResourceCPU: *resource.NewQuantity(500, resource.DecimalSI),
+							},
+						},
+					},
+					ObjectNew: &corev1.Node{
+						ObjectMeta: metav1.ObjectMeta{
+							Name: "node1",
+							Labels: map[string]string{
+								"test": "test",
+							},
+						},
+						Status: corev1.NodeStatus{
+							Allocatable: corev1.ResourceList{
+								corev1.ResourceCPU: *resource.NewQuantity(500, resource.DecimalSI),
+							},
+						},
+					},
+				}, q)
+			},
+			hasEvent: false,
+		},
 	}
 
 	for _, tc := range testCases {