Merge pull request #77179 from feiskyer/vmss-reus

Reuse vmssIPConfigurationRE for getting the NIC's resource group
kubernetes · Apr 28, 2019 · f01c140 · f01c140
2 parents 2dc3509 + 1a6c283
commit f01c140
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 13 deletions.
diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_vmss.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_vmss.go
@@ -39,11 +39,10 @@ var (
 	// ErrorNotVmssInstance indicates an instance is not belongint to any vmss.
 	ErrorNotVmssInstance = errors.New("not a vmss instance")
 
-	scaleSetNameRE         = regexp.MustCompile(`.*/subscriptions/(?:.*)/Microsoft.Compute/virtualMachineScaleSets/(.+)/virtualMachines(?:.*)`)
-	resourceGroupRE        = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(.+)/providers/Microsoft.Compute/virtualMachineScaleSets/(?:.*)/virtualMachines(?:.*)`)
-	vmssNicResourceGroupRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(.+)/providers/Microsoft.Compute/virtualMachineScaleSets/(?:.*)/virtualMachines/(?:.*)/networkInterfaces/(?:.*)`)
-	vmssMachineIDTemplate  = "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/virtualMachineScaleSets/%s/virtualMachines/%s"
-	vmssIPConfigurationRE  = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(.+)/providers/Microsoft.Compute/virtualMachineScaleSets/(.+)/virtualMachines/(.+)/networkInterfaces(?:.*)`)
+	scaleSetNameRE        = regexp.MustCompile(`.*/subscriptions/(?:.*)/Microsoft.Compute/virtualMachineScaleSets/(.+)/virtualMachines(?:.*)`)
+	resourceGroupRE       = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(.+)/providers/Microsoft.Compute/virtualMachineScaleSets/(?:.*)/virtualMachines(?:.*)`)
+	vmssMachineIDTemplate = "/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/virtualMachineScaleSets/%s/virtualMachines/%s"
+	vmssIPConfigurationRE = regexp.MustCompile(`.*/subscriptions/(?:.*)/resourceGroups/(.+)/providers/Microsoft.Compute/virtualMachineScaleSets/(.+)/virtualMachines/(.+)/networkInterfaces(?:.*)`)
 )
 
 // scaleSet implements VMSet interface for Azure scale set.
@@ -515,8 +514,8 @@ func (ss *scaleSet) GetVMSetNames(service *v1.Service, nodes []*v1.Node) (vmSetN
 
 // extractResourceGroupByVMSSNicID extracts the resource group name by vmss nicID.
 func extractResourceGroupByVMSSNicID(nicID string) (string, error) {
-	matches := vmssNicResourceGroupRE.FindStringSubmatch(nicID)
-	if len(matches) != 2 {
+	matches := vmssIPConfigurationRE.FindStringSubmatch(nicID)
+	if len(matches) != 4 {
 		return "", fmt.Errorf("error of extracting resourceGroup from nicID %q", nicID)
 	}
 
@@ -579,8 +578,8 @@ func (ss *scaleSet) GetPrimaryInterface(nodeName string) (network.Interface, err
 	return nic, nil
 }
 
-// getPrimarynetworkInterfaceConfiguration gets primary network interface configuration for scale set virtual machine.
-func (ss *scaleSet) getPrimarynetworkInterfaceConfiguration(networkConfigurations []compute.VirtualMachineScaleSetNetworkConfiguration, nodeName string) (*compute.VirtualMachineScaleSetNetworkConfiguration, error) {
+// getPrimaryNetworkInterfaceConfiguration gets primary network interface configuration for scale set virtual machine.
+func (ss *scaleSet) getPrimaryNetworkInterfaceConfiguration(networkConfigurations []compute.VirtualMachineScaleSetNetworkConfiguration, nodeName string) (*compute.VirtualMachineScaleSetNetworkConfiguration, error) {
 	if len(networkConfigurations) == 1 {
 		return &networkConfigurations[0], nil
 	}
@@ -622,8 +621,7 @@ func (ss *scaleSet) EnsureHostInPool(service *v1.Service, nodeName types.NodeNam
 	}
 
 	// Check scale set name:
-	// - For basic SKU load balancer, errNotInVMSet should be returned if the node's
-	//   scale set is mismatched with vmSetName.
+	// - For basic SKU load balancer, return nil if the node's scale set is mismatched with vmSetName.
 	// - For standard SKU load balancer, backend could belong to multiple VMSS, so we
 	//   don't check vmSet for it.
 	if vmSetName != "" && !ss.useStandardLoadBalancer() && !strings.EqualFold(vmSetName, ssName) {
@@ -633,7 +631,7 @@ func (ss *scaleSet) EnsureHostInPool(service *v1.Service, nodeName types.NodeNam
 
 	// Find primary network interface configuration.
 	networkInterfaceConfigurations := *vm.NetworkProfileConfiguration.NetworkInterfaceConfigurations
-	primaryNetworkInterfaceConfiguration, err := ss.getPrimarynetworkInterfaceConfiguration(networkInterfaceConfigurations, vmName)
+	primaryNetworkInterfaceConfiguration, err := ss.getPrimaryNetworkInterfaceConfiguration(networkInterfaceConfigurations, vmName)
 	if err != nil {
 		return err
 	}
@@ -784,7 +782,7 @@ func (ss *scaleSet) ensureBackendPoolDeletedFromNode(service *v1.Service, nodeNa
 
 	// Find primary network interface configuration.
 	networkInterfaceConfigurations := *vm.NetworkProfileConfiguration.NetworkInterfaceConfigurations
-	primaryNetworkInterfaceConfiguration, err := ss.getPrimarynetworkInterfaceConfiguration(networkInterfaceConfigurations, nodeName)
+	primaryNetworkInterfaceConfiguration, err := ss.getPrimaryNetworkInterfaceConfiguration(networkInterfaceConfigurations, nodeName)
 	if err != nil {
 		return err
 	}

diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_vmss_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_vmss_test.go
@@ -374,3 +374,41 @@ func TestGetNodeNameByIPConfigurationID(t *testing.T) {
 		assert.Equal(t, test.expected, nodeName, test.description)
 	}
 }
+
+func TestExtractResourceGroupByVMSSNicID(t *testing.T) {
+	vmssNicIDTemplate := "/subscriptions/script/resourceGroups/%s/providers/Microsoft.Compute/virtualMachineScaleSets/%s/virtualMachines/%s/networkInterfaces/nic-0"
+
+	testCases := []struct {
+		description string
+		vmssNicID   string
+		expected    string
+		expectError bool
+	}{
+		{
+			description: "extractResourceGroupByVMSSNicID should get resource group name for vmss nic ID",
+			vmssNicID:   fmt.Sprintf(vmssNicIDTemplate, "rg1", "vmss1", "0"),
+			expected:    "rg1",
+		},
+		{
+			description: "extractResourceGroupByVMSSNicID should return error for VM nic ID",
+			vmssNicID:   "/subscriptions/script/resourceGroups/rg2/providers/Microsoft.Network/networkInterfaces/nic-0",
+			expectError: true,
+		},
+		{
+			description: "extractResourceGroupByVMSSNicID should return error for wrong vmss nic ID",
+			vmssNicID:   "wrong-nic-id",
+			expectError: true,
+		},
+	}
+
+	for _, test := range testCases {
+		resourceGroup, err := extractResourceGroupByVMSSNicID(test.vmssNicID)
+		if test.expectError {
+			assert.Error(t, err, test.description)
+			continue
+		}
+
+		assert.NoError(t, err, test.description)
+		assert.Equal(t, test.expected, resourceGroup, test.description)
+	}
+}