From e1bbcd8f06801c7e38d75a5fcbb36a42c11489ce Mon Sep 17 00:00:00 2001 From: Pengfei Ni Date: Sat, 4 Jul 2020 19:19:30 +0800 Subject: [PATCH] Fix throttling issues when Azure VM computer name prefix is different from VMSS name --- .../azure/azure_instances.go | 25 ++++++---- .../azure/azure_instances_test.go | 50 +------------------ 2 files changed, 17 insertions(+), 58 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances.go index 53c58276126..b06c16a618c 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances.go @@ -21,7 +21,7 @@ package azure import ( "context" "fmt" - "strconv" + "os" "strings" v1 "k8s.io/api/core/v1" @@ -36,6 +36,10 @@ const ( vmPowerStateStopped = "stopped" vmPowerStateDeallocated = "deallocated" vmPowerStateDeallocating = "deallocating" + + // nodeNameEnvironmentName is the environment variable name for getting node name. + // It is only used for out-of-tree cloud provider. + nodeNameEnvironmentName = "NODE_NAME" ) var ( @@ -310,17 +314,20 @@ func (az *Cloud) InstanceMetadataByProviderID(ctx context.Context, providerID st } func (az *Cloud) isCurrentInstance(name types.NodeName, metadataVMName string) (bool, error) { + var err error nodeName := mapNodeNameToVMName(name) + // VMSS vmName is not same with hostname, use hostname instead. if az.VMType == vmTypeVMSS { - // VMSS vmName is not same with hostname, construct the node name "{computer-name-prefix}{base-36-instance-id}". - // Refer https://docs.microsoft.com/en-us/azure/virtual-machine-scale-sets/virtual-machine-scale-sets-instance-ids#scale-set-vm-computer-name. - if ssName, instanceID, err := extractVmssVMName(metadataVMName); err == nil { - instance, err := strconv.ParseInt(instanceID, 10, 64) - if err != nil { - return false, fmt.Errorf("failed to parse VMSS instanceID %q: %v", instanceID, err) - } - metadataVMName = fmt.Sprintf("%s%06s", ssName, strconv.FormatInt(instance, 36)) + metadataVMName, err = os.Hostname() + if err != nil { + return false, err + } + + // Use name from env variable "NODE_NAME" if it is set. + nodeNameEnv := os.Getenv(nodeNameEnvironmentName) + if nodeNameEnv != "" { + metadataVMName = nodeNameEnv } } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances_test.go index 3f5b02415ef..b2a7a3a46b9 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_instances_test.go @@ -173,14 +173,6 @@ func TestInstanceID(t *testing.T) { useInstanceMetadata: true, expectedErrMsg: fmt.Errorf("failure of getting instance metadata"), }, - { - name: "NodeAddresses should report error if VMSS instanceID is invalid", - nodeName: "vm123456", - metadataName: "vmss_$123", - vmType: vmTypeVMSS, - useInstanceMetadata: true, - expectedErrMsg: fmt.Errorf("failed to parse VMSS instanceID %q: strconv.ParseInt: parsing %q: invalid syntax", "$123", "$123"), - }, { name: "NodeAddresses should report error if cloud.vmSet is nil", nodeName: "vm1", @@ -452,14 +444,6 @@ func TestNodeAddresses(t *testing.T) { useInstanceMetadata: true, expectedErrMsg: fmt.Errorf("getError"), }, - { - name: "NodeAddresses should report error if VMSS instanceID is invalid", - nodeName: "vm123456", - metadataName: "vmss_$123", - vmType: vmTypeVMSS, - useInstanceMetadata: true, - expectedErrMsg: fmt.Errorf("failed to parse VMSS instanceID %q: strconv.ParseInt: parsing %q: invalid syntax", "$123", "$123"), - }, { name: "NodeAddresses should report error if cloud.vmSet is nil", nodeName: "vm1", @@ -769,14 +753,6 @@ func TestInstanceMetadataByProviderID(t *testing.T) { useCustomImsCache: true, expectedErrMsg: fmt.Errorf("getError"), }, - { - name: "InstanceMetadataByProviderID should report error if VMSS instanceID is invalid", - metadataName: "vmss_$123", - providerID: providerID, - vmType: vmTypeVMSS, - useInstanceMetadata: true, - expectedErrMsg: fmt.Errorf("failed to parse VMSS instanceID %q: strconv.ParseInt: parsing %q: invalid syntax", "$123", "$123"), - }, { name: "InstanceMetadataByProviderID should get metadata from Azure API if cloud.UseInstanceMetadata is false", metadataName: "vm1", @@ -920,7 +896,7 @@ func TestInstanceMetadataByProviderID(t *testing.T) { func TestIsCurrentInstance(t *testing.T) { cloud := &Cloud{ Config: Config{ - VMType: vmTypeVMSS, + VMType: vmTypeStandard, }, } testcases := []struct { @@ -939,22 +915,6 @@ func TestIsCurrentInstance(t *testing.T) { metadataVMName: "node2", expected: false, }, - { - nodeName: "vmss000001", - metadataVMName: "vmss_1", - expected: true, - }, - { - nodeName: "vmss_2", - metadataVMName: "vmss000000", - expected: false, - }, - { - nodeName: "vmss123456", - metadataVMName: "vmss_$123", - expected: false, - expectedErrMsg: fmt.Errorf("failed to parse VMSS instanceID %q: strconv.ParseInt: parsing %q: invalid syntax", "$123", "$123"), - }, } for _, test := range testcases { @@ -1048,14 +1008,6 @@ func TestInstanceTypeByProviderID(t *testing.T) { useCustomImsCache: true, expectedErrMsg: fmt.Errorf("getError"), }, - { - name: "NodeAddresses should report error if VMSS instanceID is invalid", - nodeName: "vm123456", - metadataName: "vmss_$123", - providerID: "azure:///subscriptions/subscription/resourceGroups/rg/providers/Microsoft.Compute/virtualMachines/vm1", - vmType: vmTypeVMSS, - expectedErrMsg: fmt.Errorf("failed to parse VMSS instanceID %q: strconv.ParseInt: parsing %q: invalid syntax", "$123", "$123"), - }, } for _, test := range testcases {