Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-1.25] fix: decouple vmss with 0 instance from lb when deleting the service #2528

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
84 changes: 67 additions & 17 deletions pkg/provider/azure_vmss.go
Original file line number Diff line number Diff line change
Expand Up @@ -1518,22 +1518,72 @@ func getScaleSetAndResourceGroupNameByIPConfigurationID(ipConfigurationID string
return scaleSetName, resourceGroup, nil
}

func (ss *ScaleSet) ensureBackendPoolDeletedFromVMSS(service *v1.Service, backendPoolID, vmSetName string, ipConfigurationIDs []string) error {
func (ss *ScaleSet) ensureBackendPoolDeletedFromVMSS(backendPoolID, vmSetName string) error {
vmssNamesMap := make(map[string]bool)

// the standard load balancer supports multiple vmss in its backend while the basic sku doesn't
if ss.useStandardLoadBalancer() {
for _, ipConfigurationID := range ipConfigurationIDs {
// in this scenario the vmSetName is an empty string and the name of vmss should be obtained from the provider IDs of nodes
vmssName, resourceGroupName, err := getScaleSetAndResourceGroupNameByIPConfigurationID(ipConfigurationID)
cachedUniform, err := ss.vmssCache.Get(consts.VMSSKey, azcache.CacheReadTypeDefault)
if err != nil {
klog.Errorf("ensureBackendPoolDeletedFromVMSS: failed to get vmss uniform from cache: %v", err)
return err
}
vmssUniformMap := cachedUniform.(*sync.Map)

var errorList []error
walk := func(key, value interface{}) bool {
var vmss *compute.VirtualMachineScaleSet
if vmssEntry, ok := value.(*vmssEntry); ok {
vmss = vmssEntry.vmss
} else if v, ok := value.(*compute.VirtualMachineScaleSet); ok {
vmss = v
}

// When vmss is being deleted, CreateOrUpdate API would report "the vmss is being deleted" error.
// Since it is being deleted, we shouldn't send more CreateOrUpdate requests for it.
if vmss.ProvisioningState != nil && strings.EqualFold(*vmss.ProvisioningState, consts.VirtualMachineScaleSetsDeallocating) {
klog.V(3).Infof("ensureBackendPoolDeletedFromVMSS: found vmss %s being deleted, skipping", to.String(vmss.Name))
return true
}
if vmss.VirtualMachineProfile == nil {
klog.V(4).Infof("ensureBackendPoolDeletedFromVMSS: vmss %s has no VirtualMachineProfile, skipping", to.String(vmss.Name))
return true
}
if vmss.VirtualMachineProfile.NetworkProfile.NetworkInterfaceConfigurations == nil {
klog.V(4).Infof("ensureBackendPoolDeletedFromVMSS: cannot obtain the primary network interface configuration, of vmss %s", to.String(vmss.Name))
return true
}
vmssNIC := *vmss.VirtualMachineProfile.NetworkProfile.NetworkInterfaceConfigurations
primaryNIC, err := getPrimaryNetworkInterfaceConfigurationForScaleSet(vmssNIC, to.String(vmss.Name))
if err != nil {
klog.V(4).Infof("ensureBackendPoolDeletedFromVMSS: found VMAS ipcConfigurationID %s, will skip checking and continue", ipConfigurationID)
continue
klog.Errorf("ensureBackendPoolDeletedFromVMSS: failed to get the primary network interface config of the VMSS %s: %v", to.String(vmss.Name), err)
errorList = append(errorList, err)
return true
}
// only vmsses in the resource group same as it's in azure config are included
if strings.EqualFold(resourceGroupName, ss.ResourceGroup) {
vmssNamesMap[vmssName] = true
primaryIPConfig, err := getPrimaryIPConfigFromVMSSNetworkConfig(primaryNIC)
if err != nil {
klog.Errorf("ensureBackendPoolDeletedFromVMSS: failed to the primary IP config from the VMSS %s's network config : %v", to.String(vmss.Name), err)
errorList = append(errorList, err)
return true
}
loadBalancerBackendAddressPools := make([]compute.SubResource, 0)
if primaryIPConfig.LoadBalancerBackendAddressPools != nil {
loadBalancerBackendAddressPools = *primaryIPConfig.LoadBalancerBackendAddressPools
}
for _, loadBalancerBackendAddressPool := range loadBalancerBackendAddressPools {
if strings.EqualFold(to.String(loadBalancerBackendAddressPool.ID), backendPoolID) {
klog.V(4).Infof("ensureBackendPoolDeletedFromVMSS: found vmss %s with backend pool %s, removing it", to.String(vmss.Name), backendPoolID)
vmssNamesMap[to.String(vmss.Name)] = true
}
}

return true
}

// Walk through all cached vmss, and find the vmss that contains the backendPoolID.
vmssUniformMap.Range(walk)
if len(errorList) > 0 {
return utilerrors.Flatten(utilerrors.NewAggregate(errorList))
}
} else {
vmssNamesMap[vmSetName] = true
Expand Down Expand Up @@ -1570,7 +1620,7 @@ func (ss *ScaleSet) EnsureBackendPoolDeleted(service *v1.Service, backendPoolID,

// 1. Ensure the backendPoolID is deleted from the VMSS.
if deleteFromVMSet {
err := ss.ensureBackendPoolDeletedFromVMSS(service, backendPoolID, vmSetName, ipConfigurationIDs)
err := ss.ensureBackendPoolDeletedFromVMSS(backendPoolID, vmSetName)
if err != nil {
return err
}
Expand Down Expand Up @@ -1729,23 +1779,23 @@ func (ss *ScaleSet) EnsureBackendPoolDeletedFromVMSets(vmssNamesMap map[string]b
// When vmss is being deleted, CreateOrUpdate API would report "the vmss is being deleted" error.
// Since it is being deleted, we shouldn't send more CreateOrUpdate requests for it.
if vmss.ProvisioningState != nil && strings.EqualFold(*vmss.ProvisioningState, consts.VirtualMachineScaleSetsDeallocating) {
klog.V(3).Infof("ensureVMSSInPool: found vmss %s being deleted, skipping", vmssName)
klog.V(3).Infof("EnsureBackendPoolDeletedFromVMSets: found vmss %s being deleted, skipping", vmssName)
continue
}
if vmss.VirtualMachineProfile.NetworkProfile.NetworkInterfaceConfigurations == nil {
klog.V(4).Infof("EnsureHostInPool: cannot obtain the primary network interface configuration, of vmss %s", vmssName)
klog.V(4).Infof("EnsureBackendPoolDeletedFromVMSets: cannot obtain the primary network interface configuration, of vmss %s", vmssName)
continue
}
vmssNIC := *vmss.VirtualMachineProfile.NetworkProfile.NetworkInterfaceConfigurations
primaryNIC, err := getPrimaryNetworkInterfaceConfigurationForScaleSet(vmssNIC, vmssName)
if err != nil {
klog.Errorf("ensureBackendPoolDeletedFromVMSS: failed to get the primary network interface config of the VMSS %s: %v", vmssName, err)
klog.Errorf("EnsureBackendPoolDeletedFromVMSets: failed to get the primary network interface config of the VMSS %s: %v", vmssName, err)
errors = append(errors, err)
continue
}
primaryIPConfig, err := getPrimaryIPConfigFromVMSSNetworkConfig(primaryNIC)
if err != nil {
klog.Errorf("ensureBackendPoolDeletedFromVMSS: failed to the primary IP config from the VMSS %s's network config : %v", vmssName, err)
klog.Errorf("EnsureBackendPoolDeletedFromVMSets: failed to the primary IP config from the VMSS %s's network config : %v", vmssName, err)
errors = append(errors, err)
continue
}
Expand All @@ -1759,7 +1809,7 @@ func (ss *ScaleSet) EnsureBackendPoolDeletedFromVMSets(vmssNamesMap map[string]b
for i := len(loadBalancerBackendAddressPools) - 1; i >= 0; i-- {
curPool := loadBalancerBackendAddressPools[i]
if strings.EqualFold(backendPoolID, *curPool.ID) {
klog.V(10).Infof("ensureBackendPoolDeletedFromVMSS gets unwanted backend pool %q for VMSS %s", backendPoolID, vmssName)
klog.V(10).Infof("EnsureBackendPoolDeletedFromVMSets gets unwanted backend pool %q for VMSS %s", backendPoolID, vmssName)
found = true
newBackendPools = append(loadBalancerBackendAddressPools[:i], loadBalancerBackendAddressPools[i+1:]...)
}
Expand All @@ -1782,10 +1832,10 @@ func (ss *ScaleSet) EnsureBackendPoolDeletedFromVMSets(vmssNamesMap map[string]b
},
}

klog.V(2).Infof("ensureBackendPoolDeletedFromVMSS begins to update vmss(%s) with backendPoolID %s", vmssName, backendPoolID)
klog.V(2).Infof("EnsureBackendPoolDeletedFromVMSets begins to update vmss(%s) with backendPoolID %s", vmssName, backendPoolID)
rerr := ss.CreateOrUpdateVMSS(ss.ResourceGroup, vmssName, newVMSS)
if rerr != nil {
klog.Errorf("ensureBackendPoolDeletedFromVMSS CreateOrUpdateVMSS(%s) with new backendPoolID %s, err: %v", vmssName, backendPoolID, rerr)
klog.Errorf("EnsureBackendPoolDeletedFromVMSets CreateOrUpdateVMSS(%s) with new backendPoolID %s, err: %v", vmssName, backendPoolID, rerr)
return rerr.Error()
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/provider/azure_vmss_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ type availabilitySetNodeEntry struct {

func (ss *ScaleSet) newVMSSCache() (*azcache.TimedCache, error) {
getter := func(key string) (interface{}, error) {
localCache := &sync.Map{} // [vmasName]*vmssEntry
localCache := &sync.Map{} // [vmssName]*vmssEntry

allResourceGroups, err := ss.GetResourceGroups()
if err != nil {
Expand Down
47 changes: 23 additions & 24 deletions pkg/provider/azure_vmss_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2408,42 +2408,38 @@ func TestEnsureBackendPoolDeletedFromVMSS(t *testing.T) {
defer ctrl.Finish()

testCases := []struct {
description string
backendPoolID string
ipConfigurationIDs []string
isVMSSDeallocating bool
isVMSSNilNICConfig bool
expectedPutVMSS bool
vmssClientErr *retry.Error
expectedErr error
description string
backendPoolID string
ipConfigurationIDs []string
isVMSSDeallocating bool
isVMSSNilNICConfig bool
isVMSSNilVirtualMachineProfile bool
expectedPutVMSS bool
vmssClientErr *retry.Error
expectedErr error
}{
{
description: "ensureBackendPoolDeletedFromVMSS should skip the IP config if it's ID is invalid",
ipConfigurationIDs: []string{"invalid/id"},
},
{
description: "ensureBackendPoolDeletedFromVMSS should skip the VMSS if it's being deleting",
ipConfigurationIDs: []string{"/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/virtualMachineScaleSets/vmss/virtualMachines/vmss-vm-000000/networkInterfaces/nic"},
isVMSSDeallocating: true,
},
{
description: "ensureBackendPoolDeletedFromVMSS should skip the VMSS if it's NIC config is nil",
ipConfigurationIDs: []string{"/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/virtualMachineScaleSets/vmss/virtualMachines/vmss-vm-000000/networkInterfaces/nic"},
isVMSSNilNICConfig: true,
},
{
description: "ensureBackendPoolDeletedFromVMSS should delete the corresponding LB backendpool ID",
ipConfigurationIDs: []string{"/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/virtualMachineScaleSets/vmss/virtualMachines/vmss-vm-000000/networkInterfaces/nic"},
backendPoolID: testLBBackendpoolID0,
expectedPutVMSS: true,
description: "ensureBackendPoolDeletedFromVMSS should delete the corresponding LB backendpool ID",
backendPoolID: testLBBackendpoolID0,
expectedPutVMSS: true,
},
{
description: "ensureBackendPoolDeletedFromVMSS should skip the VMSS if there's no wanted LB backendpool ID",
ipConfigurationIDs: []string{"/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/virtualMachineScaleSets/vmss/virtualMachines/vmss-vm-000000/networkInterfaces/nic"},
backendPoolID: testLBBackendpoolID1,
description: "ensureBackendPoolDeletedFromVMSS should skip the VMSS if there's no wanted LB backendpool ID",
backendPoolID: testLBBackendpoolID1,
},
{
description: "ensureBackendPoolDeletedFromVMSS should skip the VMSS if there's no vm profile",
isVMSSNilVirtualMachineProfile: true,
},
{
description: "ensureBackendPoolDeletedFromVMSS should report the error that occurs during VMSS client's call",
ipConfigurationIDs: []string{"/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Compute/virtualMachineScaleSets/vmss/virtualMachines/vmss-vm-000000/networkInterfaces/nic"},
backendPoolID: testLBBackendpoolID0,
expectedPutVMSS: true,
Expand All @@ -2465,8 +2461,11 @@ func TestEnsureBackendPoolDeletedFromVMSS(t *testing.T) {
if test.isVMSSNilNICConfig {
expectedVMSS.VirtualMachineProfile.NetworkProfile.NetworkInterfaceConfigurations = nil
}
if test.isVMSSNilVirtualMachineProfile {
expectedVMSS.VirtualMachineProfile = nil
}
mockVMSSClient := ss.cloud.VirtualMachineScaleSetsClient.(*mockvmssclient.MockInterface)
mockVMSSClient.EXPECT().List(gomock.Any(), ss.ResourceGroup).Return([]compute.VirtualMachineScaleSet{expectedVMSS}, nil).AnyTimes()
mockVMSSClient.EXPECT().List(gomock.Any(), ss.ResourceGroup).Return([]compute.VirtualMachineScaleSet{expectedVMSS}, nil)
vmssPutTimes := 0
if test.expectedPutVMSS {
vmssPutTimes = 1
Expand All @@ -2478,7 +2477,7 @@ func TestEnsureBackendPoolDeletedFromVMSS(t *testing.T) {
mockVMSSVMClient := ss.cloud.VirtualMachineScaleSetVMsClient.(*mockvmssvmclient.MockInterface)
mockVMSSVMClient.EXPECT().List(gomock.Any(), ss.ResourceGroup, testVMSSName, gomock.Any()).Return(expectedVMSSVMs, nil).AnyTimes()

err = ss.ensureBackendPoolDeletedFromVMSS(&v1.Service{}, test.backendPoolID, testVMSSName, test.ipConfigurationIDs)
err = ss.ensureBackendPoolDeletedFromVMSS(test.backendPoolID, testVMSSName)
if test.expectedErr != nil {
assert.EqualError(t, test.expectedErr, err.Error(), test.description+", but an error occurs")
}
Expand Down