diff --git a/pkg/controller/service/controller.go b/pkg/controller/service/controller.go index 1348596c178..7322e5ac1a1 100644 --- a/pkg/controller/service/controller.go +++ b/pkg/controller/service/controller.go @@ -176,6 +176,20 @@ func New( s.nodeSyncLoop() }, UpdateFunc: func(old, cur interface{}) { + oldNode, ok := old.(*v1.Node) + if !ok { + return + } + + curNode, ok := cur.(*v1.Node) + if !ok { + return + } + + if !shouldSyncNode(oldNode, curNode) { + return + } + s.nodeSyncLoop() }, DeleteFunc: func(old interface{}) { @@ -649,6 +663,30 @@ func getNodeConditionPredicate() NodeConditionPredicate { } } +func shouldSyncNode(oldNode, newNode *v1.Node) bool { + if oldNode.Spec.Unschedulable != newNode.Spec.Unschedulable { + return true + } + + if !reflect.DeepEqual(oldNode.Labels, newNode.Labels) { + return true + } + + return nodeReadyConditionStatus(oldNode) != nodeReadyConditionStatus(newNode) +} + +func nodeReadyConditionStatus(node *v1.Node) v1.ConditionStatus { + for _, condition := range node.Status.Conditions { + if condition.Type != v1.NodeReady { + continue + } + + return condition.Status + } + + return "" +} + // nodeSyncLoop handles updating the hosts pointed to by all load // balancers whenever the set of nodes in the cluster changes. func (s *Controller) nodeSyncLoop() { diff --git a/pkg/controller/service/controller_test.go b/pkg/controller/service/controller_test.go index 41e822d1eef..0625b97c6ec 100644 --- a/pkg/controller/service/controller_test.go +++ b/pkg/controller/service/controller_test.go @@ -1486,3 +1486,122 @@ func TestListWithPredicate(t *testing.T) { }) } } + +func Test_shouldSyncNode(t *testing.T) { + testcases := []struct { + name string + oldNode *v1.Node + newNode *v1.Node + shouldSync bool + }{ + { + name: "spec.unschedable field changed", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: false, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + }, + shouldSync: true, + }, + { + name: "labels changed", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{}, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + labelNodeRoleExcludeBalancer: "", + }, + }, + }, + shouldSync: true, + }, + { + name: "ready condition changed", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: true, + }, + { + name: "not relevant condition changed and no ready condition", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeNetworkUnavailable, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeNetworkUnavailable, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + shouldSync := shouldSyncNode(testcase.oldNode, testcase.newNode) + if shouldSync != testcase.shouldSync { + t.Logf("actual shouldSyncNode: %v", shouldSync) + t.Logf("expected shouldSyncNode: %v", testcase.shouldSync) + t.Errorf("unexpected result from shouldSyncNode") + } + }) + } +}