Merge pull request #4371 from marwanad/fix-1.20-buld

Fix 1.20 build after Azure cherry-picks

cluster-autoscaler/cloudprovider/azure/azure_config_test.go

@@ -235,11 +235,3 @@ func TestOverrideDefaultRateLimitConfig(t *testing.T) {
 	newconfig = overrideDefaultRateLimitConfig(&defaultConfigWithRateLimits.RateLimitConfig, &falseCloudProviderRateLimit.RateLimitConfig)
 	assert.Equal(t, &falseCloudProviderRateLimit.RateLimitConfig, newconfig)
 }
-
-func TestGetSubscriptionIdFromInstanceMetadata(t *testing.T) {
-	// metadataURL in azure_manager.go is not available for our tests, expect fail.
-	result, err := getSubscriptionIdFromInstanceMetadata()
-	expected := ""
-	assert.NotNil(t, err.Error())
-	assert.Equal(t, expected, result, "Verify return result failed, expected: %v, actual: %v", expected, result)
-}

cluster-autoscaler/cloudprovider/azure/azure_scale_set_test.go

@@ -337,7 +337,7 @@ func TestDeleteNodeUnregistered(t *testing.T) {
 
 	manager := newTestAzureManager(t)
 	vmssName := "test-asg"
-	var vmssCapacity int64 = 2
+	var vmssCapacity int64 = 3
 
 	expectedScaleSets := []compute.VirtualMachineScaleSet{
 		{
@@ -347,42 +347,39 @@ func TestDeleteNodeUnregistered(t *testing.T) {
 			},
 		},
 	}
-	expectedVMSSVMs := newTestVMSSVMList(2)
+	expectedVMSSVMs := newTestVMSSVMList(3)
 
 	mockVMSSClient := mockvmssclient.NewMockInterface(ctrl)
-	mockVMSSClient.EXPECT().List(gomock.Any(), manager.config.ResourceGroup).Return(expectedScaleSets, nil).Times(2)
+	mockVMSSClient.EXPECT().List(gomock.Any(), manager.config.ResourceGroup).Return(expectedScaleSets, nil).AnyTimes()
 	mockVMSSClient.EXPECT().DeleteInstancesAsync(gomock.Any(), manager.config.ResourceGroup, gomock.Any(), gomock.Any()).Return(nil, nil)
 	mockVMSSClient.EXPECT().WaitForAsyncOperationResult(gomock.Any(), gomock.Any()).Return(&http.Response{StatusCode: http.StatusOK}, nil).AnyTimes()
 	manager.azClient.virtualMachineScaleSetsClient = mockVMSSClient
 	mockVMSSVMClient := mockvmssvmclient.NewMockInterface(ctrl)
 	mockVMSSVMClient.EXPECT().List(gomock.Any(), manager.config.ResourceGroup, "test-asg", gomock.Any()).Return(expectedVMSSVMs, nil).AnyTimes()
 	manager.azClient.virtualMachineScaleSetVMsClient = mockVMSSVMClient
-	err := manager.forceRefresh()
-	assert.NoError(t, err)
+	manager.regenerateCache()
 
 	resourceLimiter := cloudprovider.NewResourceLimiter(
 		map[string]int64{cloudprovider.ResourceNameCores: 1, cloudprovider.ResourceNameMemory: 10000000},
 		map[string]int64{cloudprovider.ResourceNameCores: 10, cloudprovider.ResourceNameMemory: 100000000})
 	provider, err := BuildAzureCloudProvider(manager, resourceLimiter)
 	assert.NoError(t, err)
 
-	registered := manager.RegisterNodeGroup(
-		newTestScaleSet(manager, "test-asg"))
-	manager.explicitlyConfigured["test-asg"] = true
+	registered := manager.RegisterAsg(newTestScaleSet(manager, "test-asg"))
 	assert.True(t, registered)
-	err = manager.forceRefresh()
-	assert.NoError(t, err)
+	manager.regenerateCache()
 
 	scaleSet, ok := provider.NodeGroups()[0].(*ScaleSet)
 	assert.True(t, ok)
 
 	targetSize, err := scaleSet.TargetSize()
 	assert.NoError(t, err)
-	assert.Equal(t, 2, targetSize)
+	assert.Equal(t, 3, targetSize)
 
 	// annotate node with unregistered annotation
 	annotations := make(map[string]string)
 	annotations[cloudprovider.FakeNodeReasonAnnotation] = cloudprovider.FakeNodeUnregistered
+	// Perform the delete operation
 	nodesToDelete := []*apiv1.Node{
 		{
 			ObjectMeta: metav1.ObjectMeta{
@@ -399,12 +396,7 @@ func TestDeleteNodeUnregistered(t *testing.T) {
 	// Ensure the the cached size has NOT been proactively decremented
 	targetSize, err = scaleSet.TargetSize()
 	assert.NoError(t, err)
-	assert.Equal(t, 2, targetSize)
-
-	// Ensure that the status for the instances is Deleting
-	instance0, found := scaleSet.getInstanceByProviderID("azure://" + fmt.Sprintf(fakeVirtualMachineScaleSetVMID, 0))
-	assert.True(t, found, true)
-	assert.Equal(t, instance0.Status.State, cloudprovider.InstanceDeleting)
+	assert.Equal(t, 3, targetSize)
 }
 
 func TestDeleteNoConflictRequest(t *testing.T) {

cluster-autoscaler/cloudprovider/cloud_provider.go

@@ -240,6 +240,16 @@ func (c InstanceErrorClass) String() string {
 	}
 }
 
+const (
+	// FakeNodeReasonAnnotation is an annotation added to the fake placeholder nodes CA has created
+	// Note that this don't map to real nodes in k8s and are merely used for error handling
+	FakeNodeReasonAnnotation = "k8s.io/cluster-autoscaler/fake-node-reason"
+	// FakeNodeUnregistered represents a node that is identified by CA as unregistered
+	FakeNodeUnregistered = "unregistered"
+	// FakeNodeCreateError represents a node that is identified by CA as a created node with errors
+	FakeNodeCreateError = "create-error"
+)
+
 // PricingModel contains information about the node price and how it changes in time.
 type PricingModel interface {
 	// NodePrice returns a price of running the given node for a given period of time.

cluster-autoscaler/clusterstate/clusterstate.go

@@ -944,7 +944,7 @@ func getNotRegisteredNodes(allNodes []*apiv1.Node, cloudProviderNodeInstances ma
 		for _, instance := range instances {
 			if !registered.Has(instance.Id) {
 				notRegistered = append(notRegistered, UnregisteredNode{
-					Node:              fakeNode(instance),
+					Node:              fakeNode(instance, cloudprovider.FakeNodeUnregistered),
 					UnregisteredSince: time,
 				})
 			}
@@ -1091,7 +1091,7 @@ func (csr *ClusterStateRegistry) GetCreatedNodesWithErrors() []*apiv1.Node {
 		_, _, instancesByErrorCode := csr.buildInstanceToErrorCodeMappings(nodeGroupInstances)
 		for _, instances := range instancesByErrorCode {
 			for _, instance := range instances {
-				nodesWithCreateErrors = append(nodesWithCreateErrors, fakeNode(instance))
+				nodesWithCreateErrors = append(nodesWithCreateErrors, fakeNode(instance, cloudprovider.FakeNodeCreateError))
 			}
 		}
 	}
@@ -1108,10 +1108,13 @@ func (csr *ClusterStateRegistry) InvalidateNodeInstancesCacheEntry(nodeGroup clo
 	csr.cloudProviderNodeInstancesCache.InvalidateCacheEntry(nodeGroup)
 }
 
-func fakeNode(instance cloudprovider.Instance) *apiv1.Node {
+func fakeNode(instance cloudprovider.Instance, reason string) *apiv1.Node {
 	return &apiv1.Node{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: instance.Id,
+			Annotations: map[string]string{
+				cloudprovider.FakeNodeReasonAnnotation: reason,
+			},
 		},
 		Spec: apiv1.NodeSpec{
 			ProviderID: instance.Id,