diff --git a/pkg/provider/azure_loadbalancer_backendpool.go b/pkg/provider/azure_loadbalancer_backendpool.go index 3f51e983d5..3893f8b3e4 100644 --- a/pkg/provider/azure_loadbalancer_backendpool.go +++ b/pkg/provider/azure_loadbalancer_backendpool.go @@ -231,6 +231,11 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools( } } + if nodeName == "" { + logger.V(2).Info("empty nodeName, skipping ipConfID", "serviceName", serviceName, "ipConfID", ipConfID) + continue + } + // If a node is not supposed to be included in the LB, it // would not be in the `nodes` slice. We need to check the nodes that // have been added to the LB's backendpool, find the unwanted ones and @@ -363,6 +368,10 @@ func (bc *backendPoolTypeNodeIPConfig) GetBackendPrivateIPs(ctx context.Context, logger.Error(err, "failed: GetNodeNameByIPConfigurationID", "service", serviceName) continue } + if nodeName == "" { + logger.V(2).Info("empty nodeName, skipping ipConfigID", "serviceName", serviceName, "ipConfigID", ipConfigID) + continue + } privateIPsSet, ok := bc.nodePrivateIPs[strings.ToLower(nodeName)] if !ok { klog.Warningf("bc.GetBackendPrivateIPs for service (%s): failed to get private IPs of node %s", serviceName, nodeName) diff --git a/pkg/provider/azure_vmss.go b/pkg/provider/azure_vmss.go index 0fbf35c3ba..648e7a4eea 100644 --- a/pkg/provider/azure_vmss.go +++ b/pkg/provider/azure_vmss.go @@ -1697,6 +1697,11 @@ func (ss *ScaleSet) GetNodeNameByIPConfigurationID(ctx context.Context, ipConfig return "", "", err } + if vmManagementType == ManagedByNoVM { + logger.V(2).Info("No VM attached, skipping node", "ipConfigurationID", ipConfigurationID) + return "", "", nil + } + if vmManagementType == ManagedByAvSet { // vm is managed by availability set. return ss.availabilitySet.GetNodeNameByIPConfigurationID(ctx, ipConfigurationID) @@ -1963,6 +1968,10 @@ func (ss *ScaleSet) ensureBackendPoolDeleted(ctx context.Context, service *v1.Se allErrs = append(allErrs, err) continue } + if nodeName == "" { + logger.V(2).Info("Empty nodeName, skipping node", "service", getServiceName(service), "ipConfigurationID", ipConfigurationID) + continue + } nodeResourceGroup, nodeVMSS, nodeInstanceID, nodeVMSSVM, err := ss.ensureBackendPoolDeletedFromNode(ctx, nodeName, backendPoolIDs) if err != nil { diff --git a/pkg/provider/azure_vmss_cache.go b/pkg/provider/azure_vmss_cache.go index 19d5a68e67..df3affa6c8 100644 --- a/pkg/provider/azure_vmss_cache.go +++ b/pkg/provider/azure_vmss_cache.go @@ -63,6 +63,7 @@ const ( ManagedByVmssFlex VMManagementType = "ManagedByVmssFlex" ManagedByAvSet VMManagementType = "ManagedByAvSet" ManagedByUnknownVMSet VMManagementType = "ManagedByUnknownVMSet" + ManagedByNoVM VMManagementType = "ManagedByNoVM" ) func (ss *ScaleSet) newVMSSCache() (azcache.Resource, error) { @@ -530,6 +531,9 @@ func (ss *ScaleSet) getVMManagementTypeByIPConfigurationID(ctx context.Context, if err != nil { return ManagedByUnknownVMSet, fmt.Errorf("failed to get vm name by ip config ID %s: %w", ipConfigurationID, err) } + if vmName == "" { + return ManagedByNoVM, nil + } if cachedAvSetVMs.Has(vmName) { return ManagedByAvSet, nil } @@ -542,10 +546,14 @@ func (az *Cloud) GetVMNameByIPConfigurationName(ctx context.Context, nicResource if rerr != nil { return "", fmt.Errorf("failed to get interface of name %s: %w", nicName, rerr) } + // Return empty when VM association is missing if nic.Properties == nil || nic.Properties.VirtualMachine == nil || nic.Properties.VirtualMachine.ID == nil { - return "", fmt.Errorf("failed to get vm ID of nic %s", ptr.Deref(nic.Name, "")) + return "", nil } vmID := ptr.Deref(nic.Properties.VirtualMachine.ID, "") + if vmID == "" { + return "", nil + } matches := vmIDRE.FindStringSubmatch(vmID) if len(matches) != 2 { return "", fmt.Errorf("invalid virtual machine ID %s", vmID) diff --git a/pkg/provider/azure_vmss_cache_test.go b/pkg/provider/azure_vmss_cache_test.go index 2037842c5c..0144711b8b 100644 --- a/pkg/provider/azure_vmss_cache_test.go +++ b/pkg/provider/azure_vmss_cache_test.go @@ -18,7 +18,6 @@ package provider import ( "context" - "errors" "fmt" "net/http" "testing" @@ -382,11 +381,11 @@ func TestGetVMManagementTypeByIPConfigurationID(t *testing.T) { expectedVMManagementType: ManagedByAvSet, }, { - description: "getVMManagementTypeByIPConfigurationID should return an error if nic.VirtualMachine.ID is empty", + description: "getVMManagementTypeByIPConfigurationID should return ManagedByNoVM if nic.VirtualMachine.ID is empty", ipConfigurationID: "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/testvm3-interface/ipConfigurations/pipConfig", expectedNIC: "testvm3", - expectedVMManagementType: ManagedByUnknownVMSet, - expectedErr: fmt.Errorf("failed to get vm name by ip config ID /subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/testvm3-interface/ipConfigurations/pipConfig: %w", errors.New("failed to get vm ID of nic testvm3")), + expectedVMManagementType: ManagedByNoVM, + expectedErr: nil, }, { description: "getVMManagementTypeByIPConfigurationID should return an error if failed to get nic", diff --git a/pkg/provider/azure_vmss_test.go b/pkg/provider/azure_vmss_test.go index d571fd29dd..a72453cd29 100644 --- a/pkg/provider/azure_vmss_test.go +++ b/pkg/provider/azure_vmss_test.go @@ -688,6 +688,7 @@ func TestGetNodeNameByIPConfigurationID(t *testing.T) { expectedNodeName string expectedScaleSetName string expectError bool + nicWithoutVM bool }{ { description: "GetNodeNameByIPConfigurationID should get node's Name when the node is existing", @@ -711,6 +712,15 @@ func TestGetNodeNameByIPConfigurationID(t *testing.T) { vmList: []string{"vmssee6c2000004", "vmssee6c2000005"}, expectError: true, }, + { + description: "GetNodeNameByIPConfigurationID should return empty strings for NIC without VM attachment", + scaleSet: "scaleset4", + ipConfigurationID: "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/orphaned-nic/ipConfigurations/ipconfig1", + vmList: []string{"vmssee6c2000006", "vmssee6c2000007"}, + expectedNodeName: "", + expectedScaleSetName: "", + nicWithoutVM: true, + }, } for _, test := range testCases { @@ -738,6 +748,17 @@ func TestGetNodeNameByIPConfigurationID(t *testing.T) { } mockVMsClient.EXPECT().List(gomock.Any(), gomock.Any()).Return([]*armcompute.VirtualMachine{}, nil).AnyTimes() + if test.nicWithoutVM { + mockNICClient := ss.ComputeClientFactory.GetInterfaceClient().(*mock_interfaceclient.MockInterface) + nicWithoutVM := &armnetwork.Interface{ + Name: ptr.To("orphaned-nic"), + Properties: &armnetwork.InterfacePropertiesFormat{ + VirtualMachine: nil, + }, + } + mockNICClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nicWithoutVM, nil).AnyTimes() + } + nodeName, scalesetName, err := ss.GetNodeNameByIPConfigurationID(context.TODO(), test.ipConfigurationID) if test.expectError { assert.Error(t, err, test.description) @@ -3667,6 +3688,31 @@ func TestEnsureBackendPoolDeleted(t *testing.T) { }, }, }, + { + description: "EnsureBackendPoolDeleted should skip IP configuration with no attached VM", + backendpoolID: testLBBackendpoolID0, + backendAddressPools: []*armnetwork.BackendAddressPool{ + { + ID: ptr.To(testLBBackendpoolID0), + Properties: &armnetwork.BackendAddressPoolPropertiesFormat{ + BackendIPConfigurations: []*armnetwork.InterfaceIPConfiguration{ + { + Name: ptr.To("ip-vmss"), + ID: ptr.To("/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/virtualMachineScaleSets/vmss/virtualMachines/0/networkInterfaces/nic"), + }, + { + Name: ptr.To("ip-orphan"), + ID: ptr.To("/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/orphan-nic/ipConfigurations/ipconfig1"), + }, + }, + }, + }, + { + ID: ptr.To(testLBBackendpoolID1), + }, + }, + expectedVMSSVMPutTimes: 1, + }, } for _, test := range testCases { @@ -3699,6 +3745,14 @@ func TestEnsureBackendPoolDeleted(t *testing.T) { mockVMsClient := ss.ComputeClientFactory.GetVirtualMachineClient().(*mock_virtualmachineclient.MockInterface) mockVMsClient.EXPECT().List(gomock.Any(), gomock.Any()).Return([]*armcompute.VirtualMachine{}, nil).AnyTimes() + mockInterfaceClient := ss.ComputeClientFactory.GetInterfaceClient().(*mock_interfaceclient.MockInterface) + orphanNIC := &armnetwork.Interface{ + Properties: &armnetwork.InterfacePropertiesFormat{ + VirtualMachine: nil, + }, + } + mockInterfaceClient.EXPECT().Get(gomock.Any(), "rg", "orphan-nic", nil).Return(orphanNIC, nil).AnyTimes() + updated, err := ss.EnsureBackendPoolDeleted(context.TODO(), &v1.Service{}, []string{test.backendpoolID}, testVMSSName, test.backendAddressPools, true) assert.Equal(t, test.expectedErr, err != nil, test.description+errMsgSuffix) if !test.expectedErr && test.expectedVMSSVMPutTimes > 0 { diff --git a/pkg/provider/azure_vmssflex.go b/pkg/provider/azure_vmssflex.go index 64fab31242..30adc42e58 100644 --- a/pkg/provider/azure_vmssflex.go +++ b/pkg/provider/azure_vmssflex.go @@ -408,6 +408,11 @@ func (fs *FlexScaleSet) getNodeInformationByIPConfigurationID(ctx context.Contex if err != nil { return "", "", "", fmt.Errorf("failed to get vm name of ip config ID %s: %w", ipConfigurationID, err) } + if vmName == "" { + // skip this node + logger.Info("Empty vmName, skipping node.", "ipConfigurationID", ipConfigurationID) + return "", "", "", nil + } nodeName, err := fs.getNodeNameByVMName(ctx, vmName) if err != nil { diff --git a/pkg/provider/azure_vmssflex_test.go b/pkg/provider/azure_vmssflex_test.go index 404723ff90..da5534a2a5 100644 --- a/pkg/provider/azure_vmssflex_test.go +++ b/pkg/provider/azure_vmssflex_test.go @@ -929,6 +929,21 @@ func TestGetNodeNameByIPConfigurationIDVmssFlex(t *testing.T) { expectedVMSetName: "", expectedErr: fmt.Errorf("failed to get resource group and name from ip config ID /subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces//ipConfigurations/pipConfig: %w", errors.New("invalid ip config ID /subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces//ipConfigurations/pipConfig")), }, + { + description: "GetNodeNameByIPConfigurationID should return empty strings when NIC has no attached VM", + ipConfigurationID: "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Network/networkInterfaces/orphan-nic/ipConfigurations/pipConfig", + testVMListWithoutInstanceView: testVMListWithoutInstanceView, + testVMListWithOnlyInstanceView: testVMListWithOnlyInstanceView, + vmListErr: nil, + nic: func() *armnetwork.Interface { + nic := generateTestNic("orphan-nic", false, to.Ptr(armnetwork.ProvisioningStateSucceeded), "") + nic.Properties.VirtualMachine = nil + return nic + }(), + expectedNodeName: "", + expectedVMSetName: "", + expectedErr: nil, + }, } for _, tc := range testCases {