Skip to content

Commit

Permalink
Merge pull request #4234 from alexanderConstantinescu/issue-4230
Browse files Browse the repository at this point in the history
Issue 4230: remove readiness check for cache exclusion
  • Loading branch information
k8s-ci-robot authored Jul 13, 2023
2 parents 570e938 + b27e994 commit 9813c3b
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 22 deletions.
19 changes: 0 additions & 19 deletions pkg/provider/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,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"

"sigs.k8s.io/yaml"
Expand Down Expand Up @@ -1270,15 +1269,6 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) {
az.excludeLoadBalancerNodes.Insert(newNode.ObjectMeta.Name)
klog.V(6).Infof("excluding Node %q from LoadBalancer because it has exclude-from-external-load-balancers label", 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)
klog.V(6).Infof("excluding Node %q from LoadBalancer because it is not in ready state or a newly created one", 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 @@ -1423,15 +1413,6 @@ 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
}

func (az *Cloud) getActiveNodesByLoadBalancerName(lbName string) sets.Set[string] {
az.multipleStandardLoadBalancersActiveNodesLock.Lock()
defer az.multipleStandardLoadBalancersActiveNodesLock.Unlock()
Expand Down
6 changes: 3 additions & 3 deletions pkg/provider/azure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3554,7 +3554,7 @@ func TestUpdateNodeCacheExcludeLoadBalancer(t *testing.T) {
az.nodeZones = map[string]sets.Set[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.New[string]()
az.excludeLoadBalancerNodes = sets.New[string]()
az.nodeNames = sets.New[string]()
Expand All @@ -3575,9 +3575,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 9813c3b

Please sign in to comment.