From 5a73ab9310ea810d01b005e8b40dcfda5fb79d87 Mon Sep 17 00:00:00 2001 From: Alexander Constantinescu Date: Wed, 20 Jul 2022 23:36:45 +0200 Subject: [PATCH 1/2] De-duping node "update enqueuing"/sync predicates --- .../controllers/service/controller.go | 123 +- .../controllers/service/controller_test.go | 1918 ++++++++++++++++- 2 files changed, 1958 insertions(+), 83 deletions(-) diff --git a/staging/src/k8s.io/cloud-provider/controllers/service/controller.go b/staging/src/k8s.io/cloud-provider/controllers/service/controller.go index 5265c718a46..f1944d1fd41 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/service/controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/service/controller.go @@ -175,7 +175,7 @@ func New( return } - if !shouldSyncNode(oldNode, curNode) { + if !shouldSyncUpdatedNode(oldNode, curNode) { return } @@ -254,7 +254,7 @@ func (c *Controller) Run(ctx context.Context, workers int) { func (c *Controller) triggerNodeSync() { c.nodeSyncLock.Lock() defer c.nodeSyncLock.Unlock() - newHosts, err := listWithPredicate(c.nodeLister, c.getNodeConditionPredicate()) + newHosts, err := listWithPredicates(c.nodeLister, allNodePredicates...) if err != nil { runtime.HandleError(fmt.Errorf("Failed to retrieve current set of nodes from node lister: %v", err)) // if node list cannot be retrieve, trigger full node sync to be safe. @@ -450,7 +450,7 @@ func (c *Controller) syncLoadBalancerIfNeeded(ctx context.Context, service *v1.S } func (c *Controller) ensureLoadBalancer(ctx context.Context, service *v1.Service) (*v1.LoadBalancerStatus, error) { - nodes, err := listWithPredicate(c.nodeLister, c.getNodeConditionPredicate()) + nodes, err := listWithPredicates(c.nodeLister, allNodePredicates...) if err != nil { return nil, err } @@ -679,58 +679,8 @@ func nodeSlicesEqualForLB(x, y []*v1.Node) bool { return nodeNames(x).Equal(nodeNames(y)) } -func (c *Controller) getNodeConditionPredicate() NodeConditionPredicate { - return func(node *v1.Node) bool { - if _, hasExcludeBalancerLabel := node.Labels[v1.LabelNodeExcludeBalancers]; hasExcludeBalancerLabel { - return false - } - - // Remove nodes that are about to be deleted by the cluster autoscaler. - for _, taint := range node.Spec.Taints { - if taint.Key == ToBeDeletedTaint { - klog.V(4).Infof("Ignoring node %v with autoscaler taint %+v", node.Name, taint) - return false - } - } - - // If we have no info, don't accept - if len(node.Status.Conditions) == 0 { - return false - } - for _, cond := range node.Status.Conditions { - // We consider the node for load balancing only when its NodeReady condition status - // is ConditionTrue - if cond.Type == v1.NodeReady && cond.Status != v1.ConditionTrue { - klog.V(4).Infof("Ignoring node %v with %v condition status %v", node.Name, cond.Type, cond.Status) - return false - } - } - return true - } -} - -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 "" +func shouldSyncUpdatedNode(oldNode, newNode *v1.Node) bool { + return respectsPredicates(oldNode, allNodePredicates...) != respectsPredicates(newNode, allNodePredicates...) } // nodeSyncInternal handles updating the hosts pointed to by all load @@ -778,7 +728,7 @@ func (c *Controller) nodeSyncService(svc *v1.Service) bool { return false } klog.V(4).Infof("nodeSyncService started for service %s/%s", svc.Namespace, svc.Name) - hosts, err := listWithPredicate(c.nodeLister, c.getNodeConditionPredicate()) + hosts, err := listWithPredicates(c.nodeLister, allNodePredicates...) if err != nil { runtime.HandleError(fmt.Errorf("failed to retrieve node list: %v", err)) return true @@ -987,19 +937,70 @@ func (c *Controller) patchStatus(service *v1.Service, previousStatus, newStatus // some set of criteria defined by the function. type NodeConditionPredicate func(node *v1.Node) bool -// listWithPredicate gets nodes that matches predicate function. -func listWithPredicate(nodeLister corelisters.NodeLister, predicate NodeConditionPredicate) ([]*v1.Node, error) { +var ( + allNodePredicates []NodeConditionPredicate = []NodeConditionPredicate{ + nodeIncludedPredicate, + nodeSchedulablePredicate, + nodeUnTaintedPredicate, + nodeReadyPredicate, + } +) + +// We consider the node for load balancing only when the node is not labelled for exclusion. +func nodeIncludedPredicate(node *v1.Node) bool { + _, hasExcludeBalancerLabel := node.Labels[v1.LabelNodeExcludeBalancers] + return !hasExcludeBalancerLabel +} + +// We consider the node for load balancing only when the node is schedulable. +func nodeSchedulablePredicate(node *v1.Node) bool { + return !node.Spec.Unschedulable +} + +// We consider the node for load balancing only when its not tainted for deletion by the cluster autoscaler. +func nodeUnTaintedPredicate(node *v1.Node) bool { + for _, taint := range node.Spec.Taints { + if taint.Key == ToBeDeletedTaint { + return false + } + } + return true +} + +// We consider the node for load balancing only when its NodeReady condition status is ConditionTrue +func nodeReadyPredicate(node *v1.Node) bool { + for _, cond := range node.Status.Conditions { + if cond.Type == v1.NodeReady { + return cond.Status == v1.ConditionTrue + } + } + return false +} + +// listWithPredicate gets nodes that matches all predicate functions. +func listWithPredicates(nodeLister corelisters.NodeLister, predicates ...NodeConditionPredicate) ([]*v1.Node, error) { nodes, err := nodeLister.List(labels.Everything()) if err != nil { return nil, err } + return filterWithPredicates(nodes, predicates...), nil +} +func filterWithPredicates(nodes []*v1.Node, predicates ...NodeConditionPredicate) []*v1.Node { var filtered []*v1.Node for i := range nodes { - if predicate(nodes[i]) { + if respectsPredicates(nodes[i], predicates...) { filtered = append(filtered, nodes[i]) } } - - return filtered, nil + return filtered +} + +func respectsPredicates(node *v1.Node, predicates ...NodeConditionPredicate) bool { + for _, p := range predicates { + if !p(node) { + return false + } + } + return true } diff --git a/staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go b/staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go index d1a998b6be2..69452bb990e 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go +++ b/staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go @@ -1564,8 +1564,7 @@ func TestPatchStatus(t *testing.T) { } } -func Test_getNodeConditionPredicate(t *testing.T) { - validNodeStatus := v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: "Test"}}} +func Test_respectsPredicates(t *testing.T) { tests := []struct { name string @@ -1575,20 +1574,18 @@ func Test_getNodeConditionPredicate(t *testing.T) { {want: false, input: &v1.Node{}}, {want: true, input: &v1.Node{Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}}, {want: false, input: &v1.Node{Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionFalse}}}}}, - {want: true, input: &v1.Node{Spec: v1.NodeSpec{Unschedulable: true}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}}, + {want: false, input: &v1.Node{Spec: v1.NodeSpec{Unschedulable: true}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}}, - {want: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{}}}}, - {want: false, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{v1.LabelNodeExcludeBalancers: ""}}}}, + {want: true, input: &v1.Node{Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{}}}}, + {want: false, input: &v1.Node{Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{v1.LabelNodeExcludeBalancers: ""}}}}, {want: false, input: &v1.Node{Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}, Spec: v1.NodeSpec{Taints: []v1.Taint{{Key: ToBeDeletedTaint, Value: fmt.Sprint(time.Now().Unix()), Effect: v1.TaintEffectNoSchedule}}}}}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c := &Controller{} - - if result := c.getNodeConditionPredicate()(tt.input); result != tt.want { - t.Errorf("getNodeConditionPredicate() = %v, want %v", result, tt.want) + if result := respectsPredicates(tt.input, allNodePredicates...); result != tt.want { + t.Errorf("matchesPredicates() = %v, want %v", result, tt.want) } }) } @@ -1645,7 +1642,7 @@ func TestListWithPredicate(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - get, err := listWithPredicate(fakeInformerFactory.Core().V1().Nodes().Lister(), test.predicate) + get, err := listWithPredicates(fakeInformerFactory.Core().V1().Nodes().Lister(), test.predicate) sort.Slice(get, func(i, j int) bool { return get[i].Name < get[j].Name }) @@ -1658,7 +1655,7 @@ func TestListWithPredicate(t *testing.T) { } } -func Test_shouldSyncNode(t *testing.T) { +func Test_shouldSyncUpdatedNode_individualPredicates(t *testing.T) { testcases := []struct { name string oldNode *v1.Node @@ -1666,7 +1663,7 @@ func Test_shouldSyncNode(t *testing.T) { shouldSync bool }{ { - name: "spec.unschedable field changed", + name: "unschedulable F->T", oldNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "node", @@ -1674,6 +1671,14 @@ func Test_shouldSyncNode(t *testing.T) { Spec: v1.NodeSpec{ Unschedulable: false, }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, }, newNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ @@ -1682,16 +1687,380 @@ func Test_shouldSyncNode(t *testing.T) { Spec: v1.NodeSpec{ Unschedulable: true, }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, }, shouldSync: true, }, { - name: "labels changed", + name: "unschedable T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: false, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: true, + }, + { + name: "unschedable T->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "unschedable F->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: false, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: false, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "taint F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: true, + }, + { + name: "taint T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: true, + }, + { + name: "taint F->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "taint T->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "other taint F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: "other", + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "other taint T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: "other", + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "excluded F->T", oldNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "node", Labels: map[string]string{}, }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, }, newNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ @@ -1700,11 +2069,219 @@ func Test_shouldSyncNode(t *testing.T) { v1.LabelNodeExcludeBalancers: "", }, }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, }, shouldSync: true, }, { - name: "ready condition changed", + name: "excluded changed T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: true, + }, + { + name: "excluded changed T->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "excluded changed F->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "other label changed F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + "other": "", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "other label changed T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + "other": "", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "readiness changed F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: true, + }, + { + name: "readiness changed T->F", oldNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "node", @@ -1734,7 +2311,7 @@ func Test_shouldSyncNode(t *testing.T) { shouldSync: true, }, { - name: "not relevant condition changed and no ready condition", + name: "readiness changed T->T", oldNode: &v1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "node", @@ -1742,7 +2319,7 @@ func Test_shouldSyncNode(t *testing.T) { Status: v1.NodeStatus{ Conditions: []v1.NodeCondition{ { - Type: v1.NodeNetworkUnavailable, + Type: v1.NodeReady, Status: v1.ConditionTrue, }, }, @@ -1755,7 +2332,1307 @@ func Test_shouldSyncNode(t *testing.T) { Status: v1.NodeStatus{ Conditions: []v1.NodeCondition{ { - Type: v1.NodeNetworkUnavailable, + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "readiness changed F->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "readiness changed F->unset", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{}, + }, + }, + shouldSync: false, + }, + { + name: "readiness changed T->unset", + 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{}, + }, + }, + shouldSync: true, + }, + { + name: "readiness changed unset->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{}, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "readiness changed unset->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{}, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: true, + }, + { + name: "readiness changed unset->unset", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{}, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{}, + }, + }, + shouldSync: false, + }, + { + name: "ready F, other condition changed F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeDiskPressure, + Status: v1.ConditionFalse, + }, + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeDiskPressure, + Status: v1.ConditionTrue, + }, + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "ready F, other condition changed T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeDiskPressure, + Status: v1.ConditionTrue, + }, + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeDiskPressure, + Status: v1.ConditionFalse, + }, + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "ready T, other condition changed F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeDiskPressure, + Status: v1.ConditionFalse, + }, + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeDiskPressure, + Status: v1.ConditionTrue, + }, + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "ready T, other condition changed T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeDiskPressure, + Status: v1.ConditionTrue, + }, + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeDiskPressure, + Status: v1.ConditionFalse, + }, + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + } + for _, testcase := range testcases { + t.Run(testcase.name, func(t *testing.T) { + shouldSync := shouldSyncUpdatedNode(testcase.oldNode, testcase.newNode) + if shouldSync != testcase.shouldSync { + t.Errorf("unexpected result from shouldSyncNode, expected: %v, actual: %v", testcase.shouldSync, shouldSync) + } + }) + } +} + +func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) { + testcases := []struct { + name string + oldNode *v1.Node + newNode *v1.Node + shouldSync bool + }{ + { + name: "unschedable T, tainted F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + Taints: []v1.Taint{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "unschedable T, tainted T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + Taints: []v1.Taint{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "unschedable T, excluded T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{}, + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "unschedable T, excluded F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{}, + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "unschedable T, ready F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "unschedable T, ready T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "tainted T, unschedulable F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: false, + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "tainted T, unschedulable T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: false, + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "tainted T, excluded F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{}, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "tainted T, excluded T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{}, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "tainted T, ready F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "tainted T, ready T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "excluded T, unschedulable F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Spec: v1.NodeSpec{ + Unschedulable: false, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "excluded T, unschedulable T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Spec: v1.NodeSpec{ + Unschedulable: false, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "excluded T, tainted F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "excluded T, tainted T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "excluded T, ready F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "excluded T, ready T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionTrue, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "ready F, unschedulable F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: false, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "ready F, unschedulable T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: true, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Unschedulable: false, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "ready F, tainted F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "ready F, tainted T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{ + { + Key: ToBeDeletedTaint, + }, + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + }, + Spec: v1.NodeSpec{ + Taints: []v1.Taint{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "ready F, excluded F->T", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + shouldSync: false, + }, + { + name: "ready F, excluded T->F", + oldNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{ + v1.LabelNodeExcludeBalancers: "", + }, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, + Status: v1.ConditionFalse, + }, + }, + }, + }, + newNode: &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "node", + Labels: map[string]string{}, + }, + Status: v1.NodeStatus{ + Conditions: []v1.NodeCondition{ + { + Type: v1.NodeReady, Status: v1.ConditionFalse, }, }, @@ -1764,14 +3641,11 @@ func Test_shouldSyncNode(t *testing.T) { shouldSync: false, }, } - for _, testcase := range testcases { t.Run(testcase.name, func(t *testing.T) { - shouldSync := shouldSyncNode(testcase.oldNode, testcase.newNode) + shouldSync := shouldSyncUpdatedNode(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") + t.Errorf("unexpected result from shouldSyncNode, expected: %v, actual: %v", testcase.shouldSync, shouldSync) } }) } From bb3cba0d16409fa349f253b90e97fa220a4ec5e8 Mon Sep 17 00:00:00 2001 From: Alexander Constantinescu Date: Thu, 21 Jul 2022 12:11:54 +0200 Subject: [PATCH 2/2] Avoid re-syncing LBs for ETP=local svc This removes the CCM's LB sync for nodes which are experiencing a transitioning readiness state only. --- .../controllers/service/controller.go | 83 +++++-- .../controllers/service/controller_test.go | 215 +++++++++++++++++- 2 files changed, 277 insertions(+), 21 deletions(-) diff --git a/staging/src/k8s.io/cloud-provider/controllers/service/controller.go b/staging/src/k8s.io/cloud-provider/controllers/service/controller.go index f1944d1fd41..79a886500d5 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/service/controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/service/controller.go @@ -97,6 +97,9 @@ type Controller struct { nodeSyncCh chan interface{} // needFullSync indicates if the nodeSyncInternal will do a full node sync on all LB services. needFullSync bool + // lastSyncedNodes is used when reconciling node state and keeps track of the last synced set of + // nodes. Access to this attribute by multiple go-routines is protected by nodeSyncLock + lastSyncedNodes []*v1.Node } // New returns a new service controller to keep cloud provider service resources @@ -131,7 +134,8 @@ func New( nodeListerSynced: nodeInformer.Informer().HasSynced, queue: workqueue.NewNamedRateLimitingQueue(workqueue.NewItemExponentialFailureRateLimiter(minRetryDelay, maxRetryDelay), "service"), // nodeSyncCh has a size 1 buffer. Only one pending sync signal would be cached. - nodeSyncCh: make(chan interface{}, 1), + nodeSyncCh: make(chan interface{}, 1), + lastSyncedNodes: []*v1.Node{}, } serviceInformer.Informer().AddEventHandlerWithResyncPeriod( @@ -450,7 +454,7 @@ func (c *Controller) syncLoadBalancerIfNeeded(ctx context.Context, service *v1.S } func (c *Controller) ensureLoadBalancer(ctx context.Context, service *v1.Service) (*v1.LoadBalancerStatus, error) { - nodes, err := listWithPredicates(c.nodeLister, allNodePredicates...) + nodes, err := listWithPredicates(c.nodeLister, getNodePredicatesForService(service)...) if err != nil { return nil, err } @@ -664,6 +668,15 @@ func portEqualForLB(x, y *v1.ServicePort) bool { return true } +func serviceKeys(services []*v1.Service) sets.String { + ret := sets.NewString() + for _, service := range services { + key, _ := cache.MetaNamespaceKeyFunc(service) + ret.Insert(key) + } + return ret +} + func nodeNames(nodes []*v1.Node) sets.String { ret := sets.NewString() for _, node := range nodes { @@ -680,6 +693,19 @@ func nodeSlicesEqualForLB(x, y []*v1.Node) bool { } func shouldSyncUpdatedNode(oldNode, newNode *v1.Node) bool { + // Evaluate the individual node exclusion predicate before evaluating the + // compounded result of all predicates. We don't sync ETP=local services + // for changes on the readiness condition, hence if a node remains NotReady + // and a user adds the exclusion label we will need to sync as to make sure + // this change is reflected correctly on ETP=local services. The sync + // function compares lastSyncedNodes with the new (existing) set of nodes + // for each service, so services which are synced with the same set of nodes + // should be skipped internally in the sync function. This is needed as to + // trigger a global sync for all services and make sure no service gets + // skipped due to a changing node predicate. + if respectsPredicates(oldNode, nodeIncludedPredicate) != respectsPredicates(newNode, nodeIncludedPredicate) { + return true + } return respectsPredicates(oldNode, allNodePredicates...) != respectsPredicates(newNode, allNodePredicates...) } @@ -722,24 +748,29 @@ func (c *Controller) nodeSyncInternal(ctx context.Context, workers int) { numServices-len(c.servicesToUpdate), numServices) } -// nodeSyncService syncs the nodes for one load balancer type service -func (c *Controller) nodeSyncService(svc *v1.Service) bool { +// nodeSyncService syncs the nodes for one load balancer type service. The return value +// indicates if we should retry. Hence, this functions returns false if we've updated +// load balancers and finished doing it successfully, or didn't try to at all because +// there's no need. This function returns true if we tried to update load balancers and +// failed, indicating to the caller that we should try again. +func (c *Controller) nodeSyncService(svc *v1.Service, oldNodes, newNodes []*v1.Node) bool { + retSuccess := false + retNeedRetry := true if svc == nil || !wantsLoadBalancer(svc) { - return false + return retSuccess + } + newNodes = filterWithPredicates(newNodes, getNodePredicatesForService(svc)...) + oldNodes = filterWithPredicates(oldNodes, getNodePredicatesForService(svc)...) + if nodeNames(newNodes).Equal(nodeNames(oldNodes)) { + return retSuccess } klog.V(4).Infof("nodeSyncService started for service %s/%s", svc.Namespace, svc.Name) - hosts, err := listWithPredicates(c.nodeLister, allNodePredicates...) - if err != nil { - runtime.HandleError(fmt.Errorf("failed to retrieve node list: %v", err)) - return true - } - - if err := c.lockedUpdateLoadBalancerHosts(svc, hosts); err != nil { + if err := c.lockedUpdateLoadBalancerHosts(svc, newNodes); err != nil { runtime.HandleError(fmt.Errorf("failed to update load balancer hosts for service %s/%s: %v", svc.Namespace, svc.Name, err)) - return true + return retNeedRetry } klog.V(4).Infof("nodeSyncService finished successfully for service %s/%s", svc.Namespace, svc.Name) - return false + return retSuccess } // updateLoadBalancerHosts updates all existing load balancers so that @@ -748,11 +779,20 @@ func (c *Controller) nodeSyncService(svc *v1.Service) bool { func (c *Controller) updateLoadBalancerHosts(ctx context.Context, services []*v1.Service, workers int) (servicesToRetry sets.String) { klog.V(4).Infof("Running updateLoadBalancerHosts(len(services)==%d, workers==%d)", len(services), workers) + // Include all nodes and let nodeSyncService filter and figure out if + // the update is relevant for the service in question. + nodes, err := listWithPredicates(c.nodeLister) + if err != nil { + runtime.HandleError(fmt.Errorf("failed to retrieve node list: %v", err)) + return serviceKeys(services) + } + // lock for servicesToRetry servicesToRetry = sets.NewString() lock := sync.Mutex{} + doWork := func(piece int) { - if shouldRetry := c.nodeSyncService(services[piece]); !shouldRetry { + if shouldRetry := c.nodeSyncService(services[piece], c.lastSyncedNodes, nodes); !shouldRetry { return } lock.Lock() @@ -762,6 +802,7 @@ func (c *Controller) updateLoadBalancerHosts(ctx context.Context, services []*v1 } workqueue.ParallelizeUntil(ctx, workers, len(services), doWork) + c.lastSyncedNodes = nodes klog.V(4).Infof("Finished updateLoadBalancerHosts") return servicesToRetry } @@ -944,8 +985,20 @@ var ( nodeUnTaintedPredicate, nodeReadyPredicate, } + etpLocalNodePredicates []NodeConditionPredicate = []NodeConditionPredicate{ + nodeIncludedPredicate, + nodeSchedulablePredicate, + nodeUnTaintedPredicate, + } ) +func getNodePredicatesForService(service *v1.Service) []NodeConditionPredicate { + if service.Spec.ExternalTrafficPolicy == v1.ServiceExternalTrafficPolicyTypeLocal { + return etpLocalNodePredicates + } + return allNodePredicates +} + // We consider the node for load balancing only when the node is not labelled for exclusion. func nodeIncludedPredicate(node *v1.Node) bool { _, hasExcludeBalancerLabel := node.Labels[v1.LabelNodeExcludeBalancers] diff --git a/staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go b/staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go index 69452bb990e..b7877004b20 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go +++ b/staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go @@ -44,6 +44,7 @@ import ( "k8s.io/client-go/util/workqueue" fakecloud "k8s.io/cloud-provider/fake" servicehelper "k8s.io/cloud-provider/service/helpers" + utilpointer "k8s.io/utils/pointer" "github.com/stretchr/testify/assert" @@ -64,6 +65,20 @@ func newService(name string, uid types.UID, serviceType v1.ServiceType) *v1.Serv } } +func newETPLocalService(name string, serviceType v1.ServiceType) *v1.Service { + return &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + UID: "777", + }, + Spec: v1.ServiceSpec{ + Type: serviceType, + ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyTypeLocal, + }, + } +} + // Wrap newService so that you don't have to call default arguments again and again. func defaultExternalService() *v1.Service { return newService("external-balancer", types.UID("123"), v1.ServiceTypeLoadBalancer) @@ -96,6 +111,7 @@ func newController() (*Controller, *fakecloud.Cloud, *fake.Clientset) { nodeListerSynced: nodeInformer.Informer().HasSynced, queue: workqueue.NewNamedRateLimitingQueue(workqueue.NewItemExponentialFailureRateLimiter(minRetryDelay, maxRetryDelay), "service"), nodeSyncCh: make(chan interface{}, 1), + lastSyncedNodes: []*v1.Node{}, } balancer, _ := cloud.LoadBalancer() @@ -559,6 +575,193 @@ func TestUpdateNodesInExternalLoadBalancer(t *testing.T) { } } +func TestNodeChangesForExternalTrafficPolicyLocalServices(t *testing.T) { + node1 := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node0"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}} + node2 := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}} + node2NotReady := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionFalse}}}} + node2Tainted := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Spec: v1.NodeSpec{Taints: []v1.Taint{{Key: ToBeDeletedTaint}}}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionFalse}}}} + node2Unschedulable := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Spec: v1.NodeSpec{Unschedulable: true}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}} + node2SpuriousChange := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Status: v1.NodeStatus{Phase: v1.NodeTerminated, Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}} + node2Exclude := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1", Labels: map[string]string{v1.LabelNodeExcludeBalancers: ""}}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}} + node3 := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node73"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}} + + type stateChanges struct { + nodes []*v1.Node + syncCallErr bool + } + + etpLocalservice1 := newETPLocalService("s0", v1.ServiceTypeLoadBalancer) + etpLocalservice2 := newETPLocalService("s1", v1.ServiceTypeLoadBalancer) + service3 := defaultExternalService() + + services := []*v1.Service{etpLocalservice1, etpLocalservice2, service3} + + for _, tc := range []struct { + desc string + expectedUpdateCalls []fakecloud.UpdateBalancerCall + stateChanges []stateChanges + initialState []*v1.Node + }{ + { + desc: "No node changes", + initialState: []*v1.Node{node1, node2, node3}, + stateChanges: []stateChanges{ + { + nodes: []*v1.Node{node1, node2, node3}, + }, + }, + expectedUpdateCalls: []fakecloud.UpdateBalancerCall{}, + }, + { + desc: "1 new node gets added", + initialState: []*v1.Node{node1, node2}, + stateChanges: []stateChanges{ + { + nodes: []*v1.Node{node1, node2, node3}, + }, + }, + expectedUpdateCalls: []fakecloud.UpdateBalancerCall{ + {Service: etpLocalservice1, Hosts: []*v1.Node{node1, node2, node3}}, + {Service: etpLocalservice2, Hosts: []*v1.Node{node1, node2, node3}}, + {Service: service3, Hosts: []*v1.Node{node1, node2, node3}}, + }, + }, + { + desc: "1 new node gets added - with retries", + initialState: []*v1.Node{node1, node2}, + stateChanges: []stateChanges{ + { + nodes: []*v1.Node{node1, node2, node3}, + syncCallErr: true, + }, + { + nodes: []*v1.Node{node1, node2, node3}, + }, + }, + expectedUpdateCalls: []fakecloud.UpdateBalancerCall{ + {Service: etpLocalservice1, Hosts: []*v1.Node{node1, node2, node3}}, + {Service: etpLocalservice2, Hosts: []*v1.Node{node1, node2, node3}}, + {Service: service3, Hosts: []*v1.Node{node1, node2, node3}}, + }, + }, + { + desc: "1 node goes NotReady", + initialState: []*v1.Node{node1, node2, node3}, + stateChanges: []stateChanges{ + { + nodes: []*v1.Node{node1, node2NotReady, node3}, + }, + }, + expectedUpdateCalls: []fakecloud.UpdateBalancerCall{ + {Service: service3, Hosts: []*v1.Node{node1, node3}}, + }, + }, + { + desc: "1 node gets Tainted", + initialState: []*v1.Node{node1, node2, node3}, + stateChanges: []stateChanges{ + { + nodes: []*v1.Node{node1, node2Tainted, node3}, + }, + }, + expectedUpdateCalls: []fakecloud.UpdateBalancerCall{ + {Service: etpLocalservice1, Hosts: []*v1.Node{node1, node3}}, + {Service: etpLocalservice2, Hosts: []*v1.Node{node1, node3}}, + {Service: service3, Hosts: []*v1.Node{node1, node3}}, + }, + }, + { + desc: "1 node goes unschedulable", + initialState: []*v1.Node{node1, node2, node3}, + stateChanges: []stateChanges{ + { + nodes: []*v1.Node{node1, node2Unschedulable, node3}, + }, + }, + expectedUpdateCalls: []fakecloud.UpdateBalancerCall{ + {Service: etpLocalservice1, Hosts: []*v1.Node{node1, node3}}, + {Service: etpLocalservice2, Hosts: []*v1.Node{node1, node3}}, + {Service: service3, Hosts: []*v1.Node{node1, node3}}, + }, + }, + { + desc: "1 node goes Ready", + initialState: []*v1.Node{node1, node2NotReady, node3}, + stateChanges: []stateChanges{ + { + nodes: []*v1.Node{node1, node2, node3}, + }, + }, + expectedUpdateCalls: []fakecloud.UpdateBalancerCall{ + {Service: service3, Hosts: []*v1.Node{node1, node2, node3}}, + }, + }, + { + desc: "1 node get excluded", + initialState: []*v1.Node{node1, node2, node3}, + stateChanges: []stateChanges{ + { + nodes: []*v1.Node{node1, node2Exclude, node3}, + }, + }, + expectedUpdateCalls: []fakecloud.UpdateBalancerCall{ + {Service: etpLocalservice1, Hosts: []*v1.Node{node1, node3}}, + {Service: etpLocalservice2, Hosts: []*v1.Node{node1, node3}}, + {Service: service3, Hosts: []*v1.Node{node1, node3}}, + }, + }, + { + desc: "1 old node gets deleted", + initialState: []*v1.Node{node1, node2, node3}, + stateChanges: []stateChanges{ + { + nodes: []*v1.Node{node1, node2}, + }, + }, + expectedUpdateCalls: []fakecloud.UpdateBalancerCall{ + {Service: etpLocalservice1, Hosts: []*v1.Node{node1, node2}}, + {Service: etpLocalservice2, Hosts: []*v1.Node{node1, node2}}, + {Service: service3, Hosts: []*v1.Node{node1, node2}}, + }, + }, + { + desc: "1 spurious node update", + initialState: []*v1.Node{node1, node2, node3}, + stateChanges: []stateChanges{ + { + nodes: []*v1.Node{node1, node2SpuriousChange, node3}, + }, + }, + expectedUpdateCalls: []fakecloud.UpdateBalancerCall{}, + }, + } { + t.Run(tc.desc, func(t *testing.T) { + controller, cloud, _ := newController() + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + controller.lastSyncedNodes = tc.initialState + + for _, state := range tc.stateChanges { + setupState := func() { + controller.nodeLister = newFakeNodeLister(nil, state.nodes...) + if state.syncCallErr { + cloud.Err = fmt.Errorf("error please") + } + } + cleanupState := func() { + cloud.Err = nil + } + setupState() + controller.updateLoadBalancerHosts(ctx, services, 3) + cleanupState() + } + + compareUpdateCalls(t, tc.expectedUpdateCalls, cloud.UpdateCalls) + }) + } +} + func TestNodeChangesInExternalLoadBalancer(t *testing.T) { node1 := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node0"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}} node2 := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}} @@ -2782,7 +2985,7 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) { }, }, }, - shouldSync: false, + shouldSync: true, }, { name: "unschedable T, excluded F->T", @@ -2822,7 +3025,7 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) { }, }, }, - shouldSync: false, + shouldSync: true, }, { name: "unschedable T, ready F->T", @@ -3034,7 +3237,7 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) { }, }, }, - shouldSync: false, + shouldSync: true, }, { name: "tainted T, excluded T->F", @@ -3082,7 +3285,7 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) { }, }, }, - shouldSync: false, + shouldSync: true, }, { name: "tainted T, ready F->T", @@ -3604,7 +3807,7 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) { }, }, }, - shouldSync: false, + shouldSync: true, }, { name: "ready F, excluded T->F", @@ -3638,7 +3841,7 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) { }, }, }, - shouldSync: false, + shouldSync: true, }, } for _, testcase := range testcases {