Trigger re-sync on any change to providerID

This commit is contained in:
Alexander Constantinescu 2023-05-02 15:32:02 +02:00
parent 3c45b61b64
commit d257d5dfdc
2 changed files with 70 additions and 37 deletions

View File

@ -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]

View File

@ -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,
},