From 2626bcd2fe271cb823ec0debfd380cadca9ad5cd Mon Sep 17 00:00:00 2001 From: Qi Ni Date: Thu, 29 Jul 2021 10:52:58 +0800 Subject: [PATCH] Fix: ignore not a VMSS error for VMAS nodes in reconcileBackendPools --- .../azure/azure_vmss.go | 11 +++- .../azure/azure_vmss_test.go | 53 +++++++++++-------- 2 files changed, 42 insertions(+), 22 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 index 8c63b3baa81ad..ea36cb8f607e6 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_vmss.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_vmss.go @@ -1378,8 +1378,17 @@ func (ss *scaleSet) ensureBackendPoolDeletedFromNode(nodeName, backendPoolID str func (ss *scaleSet) GetNodeNameByIPConfigurationID(ipConfigurationID string) (string, string, error) { matches := vmssIPConfigurationRE.FindStringSubmatch(ipConfigurationID) if len(matches) != 4 { + if ss.DisableAvailabilitySetNodes { + return "", "", ErrorNotVmssInstance + } + klog.V(4).Infof("Can not extract scale set name from ipConfigurationID (%s), assuming it is managed by availability set", ipConfigurationID) - return "", "", ErrorNotVmssInstance + name, rg, err := ss.availabilitySet.GetNodeNameByIPConfigurationID(ipConfigurationID) + if err != nil { + klog.Errorf("GetNodeNameByIPConfigurationID: failed to invoke availabilitySet.GetNodeNameByIPConfigurationID: %s", err.Error()) + return "", "", err + } + return name, rg, nil } resourceGroup := matches[1] 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 index d0887eac0b045..80c71a3d94d1b 100644 --- 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 @@ -577,9 +577,10 @@ func TestGetNodeNameByIPConfigurationID(t *testing.T) { scaleSet string vmList []string ipConfigurationID string + disableVMAS bool expectedNodeName string expectedScaleSetName string - expectError bool + expectError error }{ { description: "GetNodeNameByIPConfigurationID should get node's Name when the node is existing", @@ -594,41 +595,50 @@ func TestGetNodeNameByIPConfigurationID(t *testing.T) { scaleSet: "scaleset2", ipConfigurationID: fmt.Sprintf(ipConfigurationIDTemplate, "scaleset2", "3", "scaleset1"), vmList: []string{"vmssee6c2000002", "vmssee6c2000003"}, - expectError: true, + expectError: fmt.Errorf("instance not found"), }, { description: "GetNodeNameByIPConfigurationID should return error for wrong ipConfigurationID", scaleSet: "scaleset3", ipConfigurationID: "invalid-configuration-id", vmList: []string{"vmssee6c2000004", "vmssee6c2000005"}, - expectError: true, + expectError: fmt.Errorf("invalid ip config ID invalid-configuration-id"), + }, + { + description: "GetNodeNameByIPConfigurationID should return ErrorNotVmssInstance if the vmas is disabled", + scaleSet: "scaleset1", + ipConfigurationID: "invalid-configuration-id", + vmList: []string{"vmssee6c2000004", "vmssee6c2000005"}, + disableVMAS: true, + expectError: ErrorNotVmssInstance, }, } for _, test := range testCases { - ss, err := newTestScaleSet(ctrl) - assert.NoError(t, err, test.description) + t.Run(test.description, func(t *testing.T) { + ss, err := newTestScaleSet(ctrl) + assert.NoError(t, err, test.description) - mockVMSSClient := mockvmssclient.NewMockInterface(ctrl) - mockVMSSVMClient := mockvmssvmclient.NewMockInterface(ctrl) - ss.cloud.VirtualMachineScaleSetsClient = mockVMSSClient - ss.cloud.VirtualMachineScaleSetVMsClient = mockVMSSVMClient + if test.disableVMAS { + ss.DisableAvailabilitySetNodes = true + } - expectedScaleSet := buildTestVMSS(test.scaleSet, "vmssee6c2") - mockVMSSClient.EXPECT().List(gomock.Any(), gomock.Any()).Return([]compute.VirtualMachineScaleSet{expectedScaleSet}, nil).AnyTimes() + mockVMSSClient := mockvmssclient.NewMockInterface(ctrl) + mockVMSSVMClient := mockvmssvmclient.NewMockInterface(ctrl) + ss.cloud.VirtualMachineScaleSetsClient = mockVMSSClient + ss.cloud.VirtualMachineScaleSetVMsClient = mockVMSSVMClient - expectedVMs, _, _ := buildTestVirtualMachineEnv(ss.cloud, test.scaleSet, "", 0, test.vmList, "", false) - mockVMSSVMClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(expectedVMs, nil).AnyTimes() + expectedScaleSet := buildTestVMSS(test.scaleSet, "vmssee6c2") + mockVMSSClient.EXPECT().List(gomock.Any(), gomock.Any()).Return([]compute.VirtualMachineScaleSet{expectedScaleSet}, nil).AnyTimes() - nodeName, scalesetName, err := ss.GetNodeNameByIPConfigurationID(test.ipConfigurationID) - if test.expectError { - assert.Error(t, err, test.description) - continue - } + expectedVMs, _, _ := buildTestVirtualMachineEnv(ss.cloud, test.scaleSet, "", 0, test.vmList, "", false) + mockVMSSVMClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(expectedVMs, nil).AnyTimes() - assert.NoError(t, err, test.description) - assert.Equal(t, test.expectedNodeName, nodeName, test.description) - assert.Equal(t, test.expectedScaleSetName, scalesetName, test.description) + nodeName, scalesetName, err := ss.GetNodeNameByIPConfigurationID(test.ipConfigurationID) + assert.Equal(t, err, test.expectError) + assert.Equal(t, test.expectedNodeName, nodeName, test.description) + assert.Equal(t, test.expectedScaleSetName, scalesetName, test.description) + }) } } @@ -2378,6 +2388,7 @@ func TestEnsureBackendPoolDeleted(t *testing.T) { for _, test := range testCases { ss, err := newTestScaleSet(ctrl) assert.NoError(t, err, test.description) + ss.DisableAvailabilitySetNodes = true expectedVMSS := buildTestVMSSWithLB(testVMSSName, "vmss-vm-", []string{testLBBackendpoolID0}, false) mockVMSSClient := ss.cloud.VirtualMachineScaleSetsClient.(*mockvmssclient.MockInterface)