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) } }) }