Skip to content

Commit

Permalink
Issue 4230: remove readiness check for cache exclusion
Browse files Browse the repository at this point in the history
  • Loading branch information
alexanderConstantinescu authored and feiskyer committed Jul 15, 2023
1 parent d67587b commit 8a4489f
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 21 deletions.
18 changes: 0 additions & 18 deletions pkg/provider/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ import (
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/zoneclient"
azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
nodemanager "sigs.k8s.io/cloud-provider-azure/pkg/nodemanager"
"sigs.k8s.io/cloud-provider-azure/pkg/retry"

// ensure the newly added package from azure-sdk-for-go is in vendor/
Expand Down Expand Up @@ -1129,14 +1128,6 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) {
case hasExcludeBalancerLabel:
az.excludeLoadBalancerNodes.Insert(newNode.ObjectMeta.Name)

case !isNodeReady(newNode) && nodemanager.GetCloudTaint(newNode.Spec.Taints) == nil:
// If not in ready state and not a newly created node, add to excludeLoadBalancerNodes cache.
// New nodes (tainted with "node.cloudprovider.kubernetes.io/uninitialized") should not be
// excluded from load balancers regardless of their state, so as to reduce the number of
// VMSS API calls and not provoke VMScaleSetActiveModelsCountLimitReached.
// (https://github.com/kubernetes-sigs/cloud-provider-azure/issues/851)
az.excludeLoadBalancerNodes.Insert(newNode.ObjectMeta.Name)

default:
// Nodes not falling into the three cases above are valid backends and
// should not appear in excludeLoadBalancerNodes cache.
Expand Down Expand Up @@ -1276,12 +1267,3 @@ func (az *Cloud) ShouldNodeExcludedFromLoadBalancer(nodeName string) (bool, erro

return az.excludeLoadBalancerNodes.Has(nodeName), nil
}

func isNodeReady(node *v1.Node) bool {
for _, cond := range node.Status.Conditions {
if cond.Type == v1.NodeReady && cond.Status == v1.ConditionTrue {
return true
}
}
return false
}
6 changes: 3 additions & 3 deletions pkg/provider/azure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3513,7 +3513,7 @@ func TestUpdateNodeCacheExcludeLoadBalancer(t *testing.T) {
az.nodeZones = map[string]sets.String{zone: nodesInZone}
az.nodeResourceGroups = map[string]string{"aNode": "rg"}

// a non-ready node should be excluded
// a non-ready node should be included
az.unmanagedNodes = sets.NewString()
az.excludeLoadBalancerNodes = sets.NewString()
az.nodeNames = sets.NewString()
Expand All @@ -3534,9 +3534,9 @@ func TestUpdateNodeCacheExcludeLoadBalancer(t *testing.T) {
},
}
az.updateNodeCaches(nil, &nonReadyNode)
assert.Equal(t, 1, len(az.excludeLoadBalancerNodes))
assert.Equal(t, 0, len(az.excludeLoadBalancerNodes))

// node becomes ready, it should be removed from az.excludeLoadBalancerNodes
// node becomes ready => no impact on az.excludeLoadBalancerNodes
readyNode := v1.Node{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
Expand Down

0 comments on commit 8a4489f

Please sign in to comment.