Issue 4230: remove readiness check for cache exclusion
This commit is contained in:
		@@ -41,7 +41,6 @@ import (
 | 
				
			|||||||
	"k8s.io/client-go/tools/cache"
 | 
						"k8s.io/client-go/tools/cache"
 | 
				
			||||||
	"k8s.io/client-go/tools/record"
 | 
						"k8s.io/client-go/tools/record"
 | 
				
			||||||
	cloudprovider "k8s.io/cloud-provider"
 | 
						cloudprovider "k8s.io/cloud-provider"
 | 
				
			||||||
	cloudproviderapi "k8s.io/cloud-provider/api"
 | 
					 | 
				
			||||||
	"k8s.io/klog/v2"
 | 
						"k8s.io/klog/v2"
 | 
				
			||||||
	"k8s.io/legacy-cloud-providers/azure/auth"
 | 
						"k8s.io/legacy-cloud-providers/azure/auth"
 | 
				
			||||||
	azcache "k8s.io/legacy-cloud-providers/azure/cache"
 | 
						azcache "k8s.io/legacy-cloud-providers/azure/cache"
 | 
				
			||||||
@@ -858,14 +857,6 @@ func (az *Cloud) updateNodeCaches(prevNode, newNode *v1.Node) {
 | 
				
			|||||||
		case hasExcludeBalancerLabel:
 | 
							case hasExcludeBalancerLabel:
 | 
				
			||||||
			az.excludeLoadBalancerNodes.Insert(newNode.ObjectMeta.Name)
 | 
								az.excludeLoadBalancerNodes.Insert(newNode.ObjectMeta.Name)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
		case !isNodeReady(newNode) && 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:
 | 
							default:
 | 
				
			||||||
			// Nodes not falling into the three cases above are valid backends and
 | 
								// Nodes not falling into the three cases above are valid backends and
 | 
				
			||||||
			// should not appear in excludeLoadBalancerNodes cache.
 | 
								// should not appear in excludeLoadBalancerNodes cache.
 | 
				
			||||||
@@ -995,21 +986,3 @@ func (az *Cloud) ShouldNodeExcludedFromLoadBalancer(nodeName string) (bool, erro
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
	return az.excludeLoadBalancerNodes.Has(nodeName), nil
 | 
						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 getCloudTaint(taints []v1.Taint) *v1.Taint {
 | 
					 | 
				
			||||||
	for _, taint := range taints {
 | 
					 | 
				
			||||||
		if taint.Key == cloudproviderapi.TaintExternalCloudProvider {
 | 
					 | 
				
			||||||
			return &taint
 | 
					 | 
				
			||||||
		}
 | 
					 | 
				
			||||||
	}
 | 
					 | 
				
			||||||
	return nil
 | 
					 | 
				
			||||||
}
 | 
					 | 
				
			||||||
 
 | 
				
			|||||||
@@ -3313,7 +3313,7 @@ func TestUpdateNodeCacheExcludeLoadBalancer(t *testing.T) {
 | 
				
			|||||||
	az.updateNodeCaches(&prevNode, &newNode)
 | 
						az.updateNodeCaches(&prevNode, &newNode)
 | 
				
			||||||
	assert.Equal(t, 1, len(az.excludeLoadBalancerNodes))
 | 
						assert.Equal(t, 1, len(az.excludeLoadBalancerNodes))
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	// a non-ready node should be excluded
 | 
						// a non-ready node should be included
 | 
				
			||||||
	az.unmanagedNodes = sets.NewString()
 | 
						az.unmanagedNodes = sets.NewString()
 | 
				
			||||||
	az.excludeLoadBalancerNodes = sets.NewString()
 | 
						az.excludeLoadBalancerNodes = sets.NewString()
 | 
				
			||||||
	az.nodeNames = sets.NewString()
 | 
						az.nodeNames = sets.NewString()
 | 
				
			||||||
@@ -3334,9 +3334,9 @@ func TestUpdateNodeCacheExcludeLoadBalancer(t *testing.T) {
 | 
				
			|||||||
		},
 | 
							},
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	az.updateNodeCaches(nil, &nonReadyNode)
 | 
						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{
 | 
						readyNode := v1.Node{
 | 
				
			||||||
		ObjectMeta: metav1.ObjectMeta{
 | 
							ObjectMeta: metav1.ObjectMeta{
 | 
				
			||||||
			Labels: map[string]string{
 | 
								Labels: map[string]string{
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user