Convert resource group name in Azure providerID to lower case

cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go

@@ -133,7 +133,11 @@ func (as *AgentPool) GetVMIndexes() ([]int, map[int]string, error) {
 		}
 
 		indexes = append(indexes, index)
-		indexToVM[index] = "azure://" + strings.ToLower(*instance.ID)
+		resourceID, err := convertResourceGroupNameToLower("azure://" + *instance.ID)
+		if err != nil {
+			return nil, nil, err
+		}
+		indexToVM[index] = resourceID
 	}
 
 	sortedIndexes := sort.IntSlice(indexes)
@@ -398,9 +402,13 @@ func (as *AgentPool) Nodes() ([]cloudprovider.Instance, error) {
 			continue
 		}
 
-		// To keep consistent with providerID from kubernetes cloud provider, do not convert ID to lower case.
-		name := "azure://" + strings.ToLower(*instance.ID)
-		nodes = append(nodes, cloudprovider.Instance{Id: name})
+		// To keep consistent with providerID from kubernetes cloud provider, convert
+		// resourceGroupName in the ID to lower case.
+		resourceID, err := convertResourceGroupNameToLower("azure://" + *instance.ID)
+		if err != nil {
+			return nil, err
+		}
+		nodes = append(nodes, cloudprovider.Instance{Id: resourceID})
 	}
 
 	return nodes, nil

cluster-autoscaler/cloudprovider/azure/azure_cache.go

@@ -118,7 +118,11 @@ func (m *asgCache) FindForInstance(instance *azureRef, vmType string) (cloudprov
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 
-	inst := azureRef{Name: strings.ToLower(instance.Name)}
+	resourceID, err := convertResourceGroupNameToLower(instance.Name)
+	if err != nil {
+		return nil, err
+	}
+	inst := azureRef{Name: resourceID}
 	if m.notInRegisteredAsg[inst] {
 		// We already know we don't own this instance. Return early and avoid
 		// additional calls.

cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go

@@ -19,7 +19,6 @@ package azure
 import (
 	"io"
 	"os"
-	"strings"
 
 	"k8s.io/klog"
 
@@ -111,11 +110,6 @@ func (azure *AzureCloudProvider) Refresh() error {
 	return azure.azureManager.Refresh()
 }
 
-// GetInstanceID gets the instance ID for the specified node.
-func (azure *AzureCloudProvider) GetInstanceID(node *apiv1.Node) string {
-	return strings.ToLower(node.Spec.ProviderID)
-}
-
 // azureRef contains a reference to some entity in Azure world.
 type azureRef struct {
 	Name string

cluster-autoscaler/cloudprovider/azure/azure_container_service_pool.go

@@ -270,7 +270,7 @@ func (agentPool *ContainerServiceAgentPool) SetNodeCount(count int) (err error)
 func (agentPool *ContainerServiceAgentPool) GetProviderID(name string) string {
 	//TODO: come with a generic way to make it work with provider id formats
 	// in different version of k8s.
-	return "azure://" + strings.ToLower(name)
+	return "azure://" + name
 }
 
 //GetName extracts the name of the node (a format which underlying cloud service understands)
@@ -419,7 +419,13 @@ func (agentPool *ContainerServiceAgentPool) GetNodes() ([]string, error) {
 	for _, node := range vmList {
 		klog.V(5).Infof("Node Name: %s, ID: %s", *node.Name, *node.ID)
 		if agentPool.IsContainerServiceNode(node.Tags) {
-			providerID := agentPool.GetProviderID(*node.ID)
+			providerID, err := convertResourceGroupNameToLower(agentPool.GetProviderID(*node.ID))
+			if err != nil {
+				// This shouldn't happen. Log a waring message for tracking.
+				klog.Warningf("GetNodes.convertResourceGroupNameToLower failed with error: %v", err)
+				continue
+			}
+
 			klog.V(5).Infof("Returning back the providerID: %s", providerID)
 			nodeArray = append(nodeArray, providerID)
 		}

cluster-autoscaler/cloudprovider/azure/azure_fakes.go

@@ -30,7 +30,7 @@ import (
 )
 
 const (
-	fakeVirtualMachineScaleSetVMID = "/subscriptions/test-subscription-id/resourcegroups/test-asg/providers/microsoft.compute/virtualmachinescalesets/agents/virtualmachines/0"
+	fakeVirtualMachineScaleSetVMID = "/subscriptions/test-subscription-id/resourceGroups/test-asg/providers/Microsoft.Compute/virtualMachineScaleSets/agents/virtualMachines/0"
 )
 
 // VirtualMachineScaleSetsClientMock mocks for VirtualMachineScaleSetsClient.

cluster-autoscaler/cloudprovider/azure/azure_scale_set.go

@@ -211,7 +211,14 @@ func (scaleSet *ScaleSet) GetScaleSetVms() ([]string, error) {
 			continue
 		}
 
-		allVMs = append(allVMs, *vm.ID)
+		resourceID, err := convertResourceGroupNameToLower(*vm.ID)
+		if err != nil {
+			// This shouldn't happen. Log a waring message for tracking.
+			klog.Warningf("GetScaleSetVms.convertResourceGroupNameToLower failed with error: %v", err)
+			continue
+		}
+
+		allVMs = append(allVMs, resourceID)
 	}
 
 	return allVMs, nil
@@ -456,7 +463,7 @@ func (scaleSet *ScaleSet) Nodes() ([]cloudprovider.Instance, error) {
 
 	instances := make([]cloudprovider.Instance, 0, len(vms))
 	for i := range vms {
-		name := "azure://" + strings.ToLower(vms[i])
+		name := "azure://" + vms[i]
 		instances = append(instances, cloudprovider.Instance{Id: name})
 	}
 

cluster-autoscaler/cloudprovider/azure/azure_util.go

@@ -78,9 +78,10 @@ const (
 )
 
 var (
-	vmnameLinuxRegexp      = regexp.MustCompile(k8sLinuxVMNamingFormat)
-	vmnameWindowsRegexp    = regexp.MustCompile(k8sWindowsVMNamingFormat)
-	oldvmnameWindowsRegexp = regexp.MustCompile(k8sWindowsOldVMNamingFormat)
+	vmnameLinuxRegexp        = regexp.MustCompile(k8sLinuxVMNamingFormat)
+	vmnameWindowsRegexp      = regexp.MustCompile(k8sWindowsVMNamingFormat)
+	oldvmnameWindowsRegexp   = regexp.MustCompile(k8sWindowsOldVMNamingFormat)
+	azureResourceGroupNameRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(.+)/providers/(?:.*)`)
 )
 
 //AzUtil consists of utility functions which utilizes clients to different services.
@@ -628,3 +629,14 @@ func isSuccessHTTPResponse(resp *http.Response, err error) (isSuccess bool, real
 	// This shouldn't happen, it only ensures all exceptions are handled.
 	return false, fmt.Errorf("failed with unknown error")
 }
+
+// convertResourceGroupNameToLower converts the resource group name in the resource ID to be lowered.
+func convertResourceGroupNameToLower(resourceID string) (string, error) {
+	matches := azureResourceGroupNameRE.FindStringSubmatch(resourceID)
+	if len(matches) != 2 {
+		return "", fmt.Errorf("%q isn't in Azure resource ID format", resourceID)
+	}
+
+	resourceGroup := matches[1]
+	return strings.Replace(resourceID, resourceGroup, strings.ToLower(resourceGroup), 1), nil
+}

cluster-autoscaler/cloudprovider/azure/azure_util_test.go

@@ -189,3 +189,58 @@ func TestIsSuccessResponse(t *testing.T) {
 		assert.Equal(t, test.expectedError, realError, "[%s] expected: %v, saw: %v", test.name, realError, test.expectedError)
 	}
 }
+func TestConvertResourceGroupNameToLower(t *testing.T) {
+	tests := []struct {
+		desc        string
+		resourceID  string
+		expected    string
+		expectError bool
+	}{
+		{
+			desc:        "empty string should report error",
+			resourceID:  "",
+			expectError: true,
+		},
+		{
+			desc:        "resourceID not in Azure format should report error",
+			resourceID:  "invalid-id",
+			expectError: true,
+		},
+		{
+			desc:        "providerID not in Azure format should report error",
+			resourceID:  "azure://invalid-id",
+			expectError: true,
+		},
+		{
+			desc:       "resource group name in VM providerID should be converted",
+			resourceID: "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/virtualMachines/k8s-agent-AAAAAAAA-0",
+			expected:   "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroupname/providers/Microsoft.Compute/virtualMachines/k8s-agent-AAAAAAAA-0",
+		},
+		{
+			desc:       "resource group name in VM resourceID should be converted",
+			resourceID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/virtualMachines/k8s-agent-AAAAAAAA-0",
+			expected:   "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroupname/providers/Microsoft.Compute/virtualMachines/k8s-agent-AAAAAAAA-0",
+		},
+		{
+			desc:       "resource group name in VMSS providerID should be converted",
+			resourceID: "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/156",
+			expected:   "azure:///subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroupname/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/156",
+		},
+		{
+			desc:       "resource group name in VMSS resourceID should be converted",
+			resourceID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myResourceGroupName/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/156",
+			expected:   "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/myresourcegroupname/providers/Microsoft.Compute/virtualMachineScaleSets/myScaleSetName/virtualMachines/156",
+		},
+	}
+
+	for _, test := range tests {
+		real, err := convertResourceGroupNameToLower(test.resourceID)
+		if test.expectError {
+			assert.NotNil(t, err, test.desc)
+			continue
+		}
+
+		assert.Nil(t, err, test.desc)
+		assert.Equal(t, test.expected, real, test.desc)
+	}
+}