From 3c45b61b6432ccd1701fa70d69bf820dcb5a565b Mon Sep 17 00:00:00 2001 From: Alexander Constantinescu Date: Wed, 26 Apr 2023 01:43:11 +0200 Subject: [PATCH 1/2] [KCCM]: have providerID trigger re-sync, but not be required for nodes --- .../controllers/service/controller.go | 14 +- .../controllers/service/controller_test.go | 297 +++++++++++------- 2 files changed, 182 insertions(+), 129 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 a8b4bdf477f..50b19acaf0f 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/service/controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/service/controller.go @@ -948,17 +948,14 @@ var ( nodeIncludedPredicate, nodeUnTaintedPredicate, nodeReadyPredicate, - nodeHasProviderIDPredicate, } etpLocalNodePredicates []NodeConditionPredicate = []NodeConditionPredicate{ nodeIncludedPredicate, nodeUnTaintedPredicate, - nodeHasProviderIDPredicate, } stableNodeSetPredicates []NodeConditionPredicate = []NodeConditionPredicate{ nodeNotDeletedPredicate, nodeIncludedPredicate, - nodeHasProviderIDPredicate, // This is not perfect, but probably good enough. We won't update the // LBs just because the taint was added (see shouldSyncUpdatedNode) but // if any other situation causes an LB sync, tainted nodes will be @@ -978,16 +975,19 @@ func getNodePredicatesForService(service *v1.Service) []NodeConditionPredicate { return allNodePredicates } +// This predicate just validates if the providerID has been set and triggers a +// node sync. It is _not_ used when determining which nodes to use when +// configuring the load balancer's backend pool. +func nodeHasProviderIDPredicate(node *v1.Node) bool { + return node.Spec.ProviderID != "" +} + // 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 } -func nodeHasProviderIDPredicate(node *v1.Node) bool { - return node.Spec.ProviderID != "" -} - // 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 { 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 8018b7bd0f4..abfb1af73e1 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 @@ -1779,7 +1779,7 @@ func Test_respectsPredicates(t *testing.T) { }{ {want: false, input: &v1.Node{}}, {want: true, input: &v1.Node{Spec: v1.NodeSpec{ProviderID: providerID}, 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.ConditionTrue}}}}}, + {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{ProviderID: providerID}, 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: ""}}}}, @@ -2144,143 +2144,196 @@ func Test_shouldSyncUpdatedNode_individualPredicates(t *testing.T) { } func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) { + type testCase struct { + name string + oldNode *v1.Node + newNode *v1.Node + shouldSync bool + fgEnabled bool + } + testcases := []testCase{} for _, fgEnabled := range []bool{true, false} { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StableLoadBalancerNodeSet, fgEnabled)() - testcases := []struct { - name string - oldNode *v1.Node - newNode *v1.Node - shouldSync bool - }{{ - name: "tainted T, excluded F->T", - oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), - newNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - shouldSync: true, - }, { - name: "tainted T, excluded T->F", - oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - newNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), - shouldSync: true, - }, { - name: "tainted T, providerID set F->T", - oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakUnsetProviderID()), - newNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), - shouldSync: true, - }, { - name: "tainted T, providerID set T->F", - oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), - newNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakUnsetProviderID()), - shouldSync: true, - }, { - name: "tainted T, ready F->T", - oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakSetReady(false)), - newNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), - shouldSync: false, - }, { - name: "tainted T, ready T->F", - oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), - newNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakSetReady(false)), - shouldSync: false, - }, { - name: "excluded T, tainted F->T", - oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakAddTaint(ToBeDeletedTaint)), - shouldSync: false, - }, { - name: "excluded T, tainted T->F", - oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakAddTaint(ToBeDeletedTaint)), - newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - shouldSync: false, - }, { - name: "excluded T, ready F->T", - oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakSetReady(false)), - newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - shouldSync: false, - }, { - name: "excluded T, ready T->F", - oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakSetReady(false)), - shouldSync: false, - }, { - name: "excluded T, providerID set F->T", - oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakUnsetProviderID()), - newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - shouldSync: true, - }, { - name: "excluded T, providerID set T->F", - oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakUnsetProviderID()), - shouldSync: true, - }, { - name: "ready F, tainted F->T", - oldNode: makeNode(tweakSetReady(false)), - newNode: makeNode(tweakSetReady(false), tweakAddTaint(ToBeDeletedTaint)), - shouldSync: false, - }, { - name: "ready F, tainted T->F", - oldNode: makeNode(tweakSetReady(false), tweakAddTaint(ToBeDeletedTaint)), - newNode: makeNode(tweakSetReady(false)), - shouldSync: false, - }, { - name: "ready F, excluded F->T", - oldNode: makeNode(tweakSetReady(false)), - newNode: makeNode(tweakSetReady(false), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - shouldSync: true, - }, { - name: "ready F, excluded T->F", - oldNode: makeNode(tweakSetReady(false), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - newNode: makeNode(tweakSetReady(false)), - shouldSync: true, - }, { - name: "ready F, providerID set F->T", - oldNode: makeNode(tweakSetReady(false), tweakUnsetProviderID()), - newNode: makeNode(tweakSetReady(false)), - shouldSync: true, - }, { - name: "ready F, providerID set T->F", - oldNode: makeNode(tweakSetReady(false)), - newNode: makeNode(tweakSetReady(false), tweakUnsetProviderID()), - shouldSync: true, - }, { - name: "providerID unset, tainted F->T", + testcases = append(testcases, []testCase{ + { + name: "tainted T, excluded F->T", + oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), + newNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "tainted T, excluded T->F", + oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + newNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "tainted T, providerID set F->T", + oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakUnsetProviderID()), + newNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "tainted T, providerID set T->F", + oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), + newNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakUnsetProviderID()), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "tainted T, ready F->T", + oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakSetReady(false)), + newNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), + shouldSync: false, + fgEnabled: fgEnabled, + }, { + name: "tainted T, ready T->F", + oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), + newNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakSetReady(false)), + shouldSync: false, + fgEnabled: fgEnabled, + }, { + name: "excluded T, tainted F->T", + oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakAddTaint(ToBeDeletedTaint)), + shouldSync: false, + fgEnabled: fgEnabled, + }, { + name: "excluded T, tainted T->F", + oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakAddTaint(ToBeDeletedTaint)), + newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + shouldSync: false, + fgEnabled: fgEnabled, + }, { + name: "excluded T, ready F->T", + oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakSetReady(false)), + newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + shouldSync: false, + fgEnabled: fgEnabled, + }, { + name: "excluded T, ready T->F", + oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakSetReady(false)), + shouldSync: false, + fgEnabled: fgEnabled, + }, { + name: "excluded T, providerID set F->T", + oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakUnsetProviderID()), + newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "excluded T, providerID set T->F", + oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakUnsetProviderID()), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "ready F, tainted F->T", + oldNode: makeNode(tweakSetReady(false)), + newNode: makeNode(tweakSetReady(false), tweakAddTaint(ToBeDeletedTaint)), + shouldSync: false, + fgEnabled: fgEnabled, + }, { + name: "ready F, tainted T->F", + oldNode: makeNode(tweakSetReady(false), tweakAddTaint(ToBeDeletedTaint)), + newNode: makeNode(tweakSetReady(false)), + shouldSync: false, + fgEnabled: fgEnabled, + }, { + name: "ready F, excluded F->T", + oldNode: makeNode(tweakSetReady(false)), + newNode: makeNode(tweakSetReady(false), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "ready F, excluded T->F", + oldNode: makeNode(tweakSetReady(false), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + newNode: makeNode(tweakSetReady(false)), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "ready F, providerID set F->T", + oldNode: makeNode(tweakSetReady(false), tweakUnsetProviderID()), + newNode: makeNode(tweakSetReady(false)), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "ready F, providerID set T->F", + oldNode: makeNode(tweakSetReady(false)), + newNode: makeNode(tweakSetReady(false), tweakUnsetProviderID()), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "providerID unset, excluded F->T", + oldNode: makeNode(tweakUnsetProviderID()), + newNode: makeNode(tweakUnsetProviderID(), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "providerID unset, excluded T->F", + oldNode: makeNode(tweakUnsetProviderID(), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + newNode: makeNode(tweakUnsetProviderID()), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "providerID unset, ready T->F", + oldNode: makeNode(tweakUnsetProviderID()), + newNode: makeNode(tweakUnsetProviderID(), tweakSetReady(true)), + shouldSync: false, + fgEnabled: fgEnabled, + }, + }...) + } + testcases = append(testcases, []testCase{ + { + name: "providerID unset, ready F->T", oldNode: makeNode(tweakUnsetProviderID()), - newNode: makeNode(tweakUnsetProviderID(), tweakAddTaint(ToBeDeletedTaint)), + newNode: makeNode(tweakUnsetProviderID(), tweakSetReady(false)), shouldSync: false, + fgEnabled: true, + }, + { + name: "providerID unset, ready F->T", + oldNode: makeNode(tweakUnsetProviderID()), + newNode: makeNode(tweakUnsetProviderID(), tweakSetReady(false)), + shouldSync: true, + fgEnabled: false, }, { name: "providerID unset, tainted T->F", oldNode: makeNode(tweakUnsetProviderID(), tweakAddTaint(ToBeDeletedTaint)), newNode: makeNode(tweakUnsetProviderID()), shouldSync: false, - }, { - name: "providerID unset, excluded F->T", - oldNode: makeNode(tweakUnsetProviderID()), - newNode: makeNode(tweakUnsetProviderID(), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - shouldSync: true, - }, { - name: "providerID unset, excluded T->F", - oldNode: makeNode(tweakUnsetProviderID(), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + fgEnabled: true, + }, + { + name: "providerID unset, tainted T->F", + oldNode: makeNode(tweakUnsetProviderID(), tweakAddTaint(ToBeDeletedTaint)), newNode: makeNode(tweakUnsetProviderID()), shouldSync: true, + fgEnabled: false, }, { - name: "providerID unset, ready F->T", + name: "providerID unset, tainted F->T", oldNode: makeNode(tweakUnsetProviderID()), - newNode: makeNode(tweakUnsetProviderID(), tweakSetReady(false)), + newNode: makeNode(tweakUnsetProviderID(), tweakAddTaint(ToBeDeletedTaint)), shouldSync: false, + fgEnabled: true, }, { - name: "providerID unset, ready T->F", + name: "providerID unset, tainted F->T", oldNode: makeNode(tweakUnsetProviderID()), - newNode: makeNode(tweakUnsetProviderID(), tweakSetReady(true)), - shouldSync: false, - }} - for _, testcase := range testcases { - t.Run(fmt.Sprintf("%s - StableLoadBalancerNodeSet: %v", testcase.name, fgEnabled), 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) - } - }) - } + newNode: makeNode(tweakUnsetProviderID(), tweakAddTaint(ToBeDeletedTaint)), + shouldSync: true, + fgEnabled: false, + }, + }...) + for _, testcase := range testcases { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StableLoadBalancerNodeSet, testcase.fgEnabled)() + t.Run(fmt.Sprintf("%s - StableLoadBalancerNodeSet: %v", testcase.name, testcase.fgEnabled), 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) + } + }) } + } type fakeNodeLister struct { From d257d5dfdc4c9bdec9e7876104f58e0cebfa45e1 Mon Sep 17 00:00:00 2001 From: Alexander Constantinescu Date: Tue, 2 May 2023 15:32:02 +0200 Subject: [PATCH 2/2] Trigger re-sync on any change to providerID --- .../controllers/service/controller.go | 11 +-- .../controllers/service/controller_test.go | 96 +++++++++++++------ 2 files changed, 70 insertions(+), 37 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 50b19acaf0f..5d63ce8ceb1 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/service/controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/service/controller.go @@ -683,8 +683,8 @@ func shouldSyncUpdatedNode(oldNode, newNode *v1.Node) bool { if respectsPredicates(oldNode, nodeIncludedPredicate) != respectsPredicates(newNode, nodeIncludedPredicate) { return true } - // For the same reason as above, also check for changes to the providerID - if respectsPredicates(oldNode, nodeHasProviderIDPredicate) != respectsPredicates(newNode, nodeHasProviderIDPredicate) { + // For the same reason as above, also check for any change to the providerID + if oldNode.Spec.ProviderID != newNode.Spec.ProviderID { return true } if !utilfeature.DefaultFeatureGate.Enabled(features.StableLoadBalancerNodeSet) { @@ -975,13 +975,6 @@ func getNodePredicatesForService(service *v1.Service) []NodeConditionPredicate { return allNodePredicates } -// This predicate just validates if the providerID has been set and triggers a -// node sync. It is _not_ used when determining which nodes to use when -// configuring the load balancer's backend pool. -func nodeHasProviderIDPredicate(node *v1.Node) bool { - return node.Spec.ProviderID != "" -} - // 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 abfb1af73e1..43d880e177c 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 @@ -1957,9 +1957,9 @@ func tweakDeleted() nodeTweak { } } -func tweakUnsetProviderID() nodeTweak { +func tweakProviderID(id string) nodeTweak { return func(n *v1.Node) { - n.Spec.ProviderID = "" + n.Spec.ProviderID = id } } @@ -2122,15 +2122,37 @@ func Test_shouldSyncUpdatedNode_individualPredicates(t *testing.T) { stableNodeSetEnabled: true, }, { name: "providerID set F -> T", - oldNode: makeNode(tweakUnsetProviderID()), + oldNode: makeNode(tweakProviderID("")), newNode: makeNode(), shouldSync: true, }, { name: "providerID set F -> T", - oldNode: makeNode(tweakUnsetProviderID()), + oldNode: makeNode(tweakProviderID("")), newNode: makeNode(), shouldSync: true, stableNodeSetEnabled: true, + }, { + name: "providerID set T-> F", + oldNode: makeNode(), + newNode: makeNode(tweakProviderID("")), + shouldSync: true, + }, { + name: "providerID set T-> F", + oldNode: makeNode(), + newNode: makeNode(tweakProviderID("")), + shouldSync: true, + stableNodeSetEnabled: true, + }, { + name: "providerID change", + oldNode: makeNode(), + newNode: makeNode(tweakProviderID(providerID + "-2")), + shouldSync: true, + }, { + name: "providerID change", + oldNode: makeNode(), + newNode: makeNode(tweakProviderID(providerID + "-2")), + shouldSync: true, + stableNodeSetEnabled: true, }} for _, testcase := range testcases { t.Run(fmt.Sprintf("%s - StableLoadBalancerNodeSet: %v", testcase.name, testcase.stableNodeSetEnabled), func(t *testing.T) { @@ -2168,14 +2190,20 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) { fgEnabled: fgEnabled, }, { name: "tainted T, providerID set F->T", - oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakUnsetProviderID()), + oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakProviderID("")), newNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), shouldSync: true, fgEnabled: fgEnabled, }, { name: "tainted T, providerID set T->F", oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), - newNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakUnsetProviderID()), + newNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakProviderID("")), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "tainted T, providerID change", + oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint)), + newNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakProviderID(providerID+"-2")), shouldSync: true, fgEnabled: fgEnabled, }, { @@ -2216,14 +2244,20 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) { fgEnabled: fgEnabled, }, { name: "excluded T, providerID set F->T", - oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakUnsetProviderID()), + oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakProviderID("")), newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), shouldSync: true, fgEnabled: fgEnabled, }, { name: "excluded T, providerID set T->F", oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakUnsetProviderID()), + newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakProviderID("")), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "excluded T, providerID change", + oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakProviderID(providerID+"-2")), shouldSync: true, fgEnabled: fgEnabled, }, { @@ -2252,32 +2286,38 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) { fgEnabled: fgEnabled, }, { name: "ready F, providerID set F->T", - oldNode: makeNode(tweakSetReady(false), tweakUnsetProviderID()), + oldNode: makeNode(tweakSetReady(false), tweakProviderID("")), newNode: makeNode(tweakSetReady(false)), shouldSync: true, fgEnabled: fgEnabled, }, { name: "ready F, providerID set T->F", oldNode: makeNode(tweakSetReady(false)), - newNode: makeNode(tweakSetReady(false), tweakUnsetProviderID()), + newNode: makeNode(tweakSetReady(false), tweakProviderID("")), + shouldSync: true, + fgEnabled: fgEnabled, + }, { + name: "ready F, providerID change", + oldNode: makeNode(tweakSetReady(false)), + newNode: makeNode(tweakSetReady(false), tweakProviderID(providerID+"-2")), shouldSync: true, fgEnabled: fgEnabled, }, { name: "providerID unset, excluded F->T", - oldNode: makeNode(tweakUnsetProviderID()), - newNode: makeNode(tweakUnsetProviderID(), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + oldNode: makeNode(tweakProviderID("")), + newNode: makeNode(tweakProviderID(""), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), shouldSync: true, fgEnabled: fgEnabled, }, { name: "providerID unset, excluded T->F", - oldNode: makeNode(tweakUnsetProviderID(), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), - newNode: makeNode(tweakUnsetProviderID()), + oldNode: makeNode(tweakProviderID(""), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")), + newNode: makeNode(tweakProviderID("")), shouldSync: true, fgEnabled: fgEnabled, }, { name: "providerID unset, ready T->F", - oldNode: makeNode(tweakUnsetProviderID()), - newNode: makeNode(tweakUnsetProviderID(), tweakSetReady(true)), + oldNode: makeNode(tweakProviderID("")), + newNode: makeNode(tweakProviderID(""), tweakSetReady(true)), shouldSync: false, fgEnabled: fgEnabled, }, @@ -2286,40 +2326,40 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) { testcases = append(testcases, []testCase{ { name: "providerID unset, ready F->T", - oldNode: makeNode(tweakUnsetProviderID()), - newNode: makeNode(tweakUnsetProviderID(), tweakSetReady(false)), + oldNode: makeNode(tweakProviderID("")), + newNode: makeNode(tweakProviderID(""), tweakSetReady(false)), shouldSync: false, fgEnabled: true, }, { name: "providerID unset, ready F->T", - oldNode: makeNode(tweakUnsetProviderID()), - newNode: makeNode(tweakUnsetProviderID(), tweakSetReady(false)), + oldNode: makeNode(tweakProviderID("")), + newNode: makeNode(tweakProviderID(""), tweakSetReady(false)), shouldSync: true, fgEnabled: false, }, { name: "providerID unset, tainted T->F", - oldNode: makeNode(tweakUnsetProviderID(), tweakAddTaint(ToBeDeletedTaint)), - newNode: makeNode(tweakUnsetProviderID()), + oldNode: makeNode(tweakProviderID(""), tweakAddTaint(ToBeDeletedTaint)), + newNode: makeNode(tweakProviderID("")), shouldSync: false, fgEnabled: true, }, { name: "providerID unset, tainted T->F", - oldNode: makeNode(tweakUnsetProviderID(), tweakAddTaint(ToBeDeletedTaint)), - newNode: makeNode(tweakUnsetProviderID()), + oldNode: makeNode(tweakProviderID(""), tweakAddTaint(ToBeDeletedTaint)), + newNode: makeNode(tweakProviderID("")), shouldSync: true, fgEnabled: false, }, { name: "providerID unset, tainted F->T", - oldNode: makeNode(tweakUnsetProviderID()), - newNode: makeNode(tweakUnsetProviderID(), tweakAddTaint(ToBeDeletedTaint)), + oldNode: makeNode(tweakProviderID("")), + newNode: makeNode(tweakProviderID(""), tweakAddTaint(ToBeDeletedTaint)), shouldSync: false, fgEnabled: true, }, { name: "providerID unset, tainted F->T", - oldNode: makeNode(tweakUnsetProviderID()), - newNode: makeNode(tweakUnsetProviderID(), tweakAddTaint(ToBeDeletedTaint)), + oldNode: makeNode(tweakProviderID("")), + newNode: makeNode(tweakProviderID(""), tweakAddTaint(ToBeDeletedTaint)), shouldSync: true, fgEnabled: false, },