Skip to content

Commit

Permalink
Merge pull request #2833 from nilo19/fix/cherry-pick-2829-1.25
Browse files Browse the repository at this point in the history 
[release-1.25] fix: decouple vmss from the lb if the backend pool is empty when usin…
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Nov 29, 2022
2 parents 80797f8 + a65610d commit db25a1e
Show file tree
Hide file tree
Showing 13 changed files with 167 additions and 129 deletions.
9 changes: 4 additions & 5 deletions pkg/provider/azure_loadbalancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ func (az *Cloud) cleanOrphanedLoadBalancer(lb *network.LoadBalancer, existingLBs
// safeDeleteLoadBalancer deletes the load balancer after decoupling it from the vmSet
func (az *Cloud) safeDeleteLoadBalancer(lb network.LoadBalancer, clusterName, vmSetName string, service *v1.Service) *retry.Error {
lbBackendPoolID := az.getBackendPoolID(to.String(lb.Name), az.getLoadBalancerResourceGroup(), getBackendPoolName(clusterName, service))
err := az.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, lb.BackendAddressPools, true)
_, err := az.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, lb.BackendAddressPools, true)
if err != nil {
return retry.NewError(false, fmt.Errorf("safeDeleteLoadBalancer: failed to EnsureBackendPoolDeleted: %w", err))
}
Expand Down Expand Up @@ -1650,7 +1650,7 @@ func (az *Cloud) reconcileLoadBalancer(clusterName string, service *v1.Service,
if !exist {
return nil, fmt.Errorf("load balancer %q not found", lbName)
}
lb = &newLB
lb = newLB
}
}

Expand Down Expand Up @@ -2422,7 +2422,7 @@ func (az *Cloud) reconcileSecurityGroup(clusterName string, service *v1.Service,
if !exist {
return nil, fmt.Errorf("unable to get lb %s", to.String(lbName))
}
backendPrivateIPv4s, backendPrivateIPv6s := az.LoadBalancerBackendPool.GetBackendPrivateIPs(clusterName, service, &lb)
backendPrivateIPv4s, backendPrivateIPv6s := az.LoadBalancerBackendPool.GetBackendPrivateIPs(clusterName, service, lb)
backendIPAddresses = backendPrivateIPv4s
if utilnet.IsIPv6String(*lbIP) {
backendIPAddresses = backendPrivateIPv6s
Expand Down Expand Up @@ -2941,11 +2941,10 @@ func (az *Cloud) reconcilePublicIP(clusterName string, service *v1.Service, lbNa
}

if lbName != "" {
loadBalancer, _, err := az.getAzureLoadBalancer(lbName, azcache.CacheReadTypeDefault)
lb, _, err = az.getAzureLoadBalancer(lbName, azcache.CacheReadTypeDefault)
if err != nil {
return nil, err
}
lb = &loadBalancer
}

discoveredDesiredPublicIP, pipsToBeDeleted, deletedDesiredPublicIP, pipsToBeUpdated, err := az.getPublicIPUpdates(
Expand Down
58 changes: 36 additions & 22 deletions pkg/provider/azure_loadbalancer_backendpool.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"k8s.io/klog/v2"
utilnet "k8s.io/utils/net"

"sigs.k8s.io/cloud-provider-azure/pkg/cache"
"sigs.k8s.io/cloud-provider-azure/pkg/consts"
)

Expand Down Expand Up @@ -118,15 +119,16 @@ func (bc *backendPoolTypeNodeIPConfig) CleanupVMSetFromBackendPoolByCondition(sl
},
}
// decouple the backendPool from the node
err := bc.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, backendpoolToBeDeleted, true)
shouldRefreshLB, err := bc.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, backendpoolToBeDeleted, true)
if err != nil {
return nil, err
}
slb.BackendAddressPools = &newBackendPools
// Proactively disable the etag to prevent etag mismatch error when putting lb later.
// This could happen because when we remove the hosts from the lb, the nrp
// would put the lb to remove the backend references as well.
slb.Etag = nil
if shouldRefreshLB {
slb, _, err = bc.getAzureLoadBalancer(to.String(slb.Name), cache.CacheReadTypeForceRefresh)
if err != nil {
return nil, fmt.Errorf("bc.CleanupVMSetFromBackendPoolByCondition: failed to get load balancer %s, err: %w", to.String(slb.Name), err)
}
}
}

return slb, nil
Expand All @@ -141,6 +143,7 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string,

foundBackendPool := false
changed := false
shouldRefreshLB := false
lbName := *lb.Name

serviceName := getServiceName(service)
Expand All @@ -167,14 +170,13 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string,
bp.LoadBalancerBackendAddresses != nil &&
len(*bp.LoadBalancerBackendAddresses) > 0 {
if removeNodeIPAddressesFromBackendPool(bp, []string{}, true) {
bp.Etag = nil
if err := bc.CreateOrUpdateLBBackendPool(lbName, bp); err != nil {
klog.Errorf("bc.ReconcileBackendPools for service (%s): failed to cleanup IP based backend pool %s: %s", serviceName, lbBackendPoolName, err.Error())
return false, false, fmt.Errorf("bc.ReconcileBackendPools for service (%s): failed to cleanup IP based backend pool %s: %w", serviceName, lbBackendPoolName, err)
}
newBackendPools[i] = bp
lb.BackendAddressPools = &newBackendPools
lb.Etag = nil
shouldRefreshLB = true
}
}

Expand Down Expand Up @@ -213,17 +215,27 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string,
},
}
// decouple the backendPool from the node
err = bc.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, backendpoolToBeDeleted, false)
updated, err := bc.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, backendpoolToBeDeleted, false)
if err != nil {
return false, false, err
}
if updated {
shouldRefreshLB = true
}
}
break
} else {
klog.V(10).Infof("bc.ReconcileBackendPools for service (%s): lb backendpool - found unmanaged backendpool %s", serviceName, *bp.Name)
}
}

if shouldRefreshLB {
lb, _, err = bc.getAzureLoadBalancer(lbName, cache.CacheReadTypeForceRefresh)
if err != nil {
return false, false, fmt.Errorf("bc.ReconcileBackendPools for service (%s): failed to get loadbalancer %s: %w", serviceName, lbName, err)
}
}

if !foundBackendPool {
isBackendPoolPreConfigured = newBackendPool(lb, isBackendPoolPreConfigured, bc.PreConfiguredBackendPoolLoadBalancerTypes, getServiceName(service), getBackendPoolName(clusterName, service))
changed = true
Expand Down Expand Up @@ -434,13 +446,15 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi

foundBackendPool := false
changed := false
shouldRefreshLB := false
lbName := *lb.Name
serviceName := getServiceName(service)
lbBackendPoolName := getBackendPoolName(clusterName, service)
vmSetName := bi.mapLoadBalancerNameToVMSet(lbName, clusterName)
lbBackendPoolID := bi.getBackendPoolID(to.String(lb.Name), bi.getLoadBalancerResourceGroup(), getBackendPoolName(clusterName, service))
isBackendPoolPreConfigured := bi.isBackendPoolPreConfigured(service)

var err error
for i := len(newBackendPools) - 1; i >= 0; i-- {
bp := newBackendPools[i]
if strings.EqualFold(*bp.Name, lbBackendPoolName) {
Expand All @@ -455,19 +469,11 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi
// If the LB backend pool type is configured from nodeIPConfiguration
// to nodeIP, we need to decouple the VM NICs from the LB
// before attaching nodeIPs/podIPs to the LB backend pool.
if bp.BackendAddressPoolPropertiesFormat != nil &&
bp.BackendIPConfigurations != nil &&
len(*bp.BackendIPConfigurations) > 0 {
klog.V(2).Infof("bi.ReconcileBackendPools for service (%s): ensuring the LB is decoupled from the VMSet", serviceName)
if err := bi.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, lb.BackendAddressPools, true); err != nil {
klog.Errorf("bi.ReconcileBackendPools for service (%s): failed to EnsureBackendPoolDeleted: %s", serviceName, err.Error())
return false, false, err
}
newBackendPools[i].BackendAddressPoolPropertiesFormat.LoadBalancerBackendAddresses = &[]network.LoadBalancerBackendAddress{}
newBackendPools[i].BackendAddressPoolPropertiesFormat.BackendIPConfigurations = &[]network.InterfaceIPConfiguration{}
newBackendPools[i].Etag = nil
lb.Etag = nil
break
klog.V(2).Infof("bi.ReconcileBackendPools for service (%s) and vmSet (%s): ensuring the LB is decoupled from the VMSet", serviceName, vmSetName)
shouldRefreshLB, err = bi.VMSet.EnsureBackendPoolDeleted(service, lbBackendPoolID, vmSetName, lb.BackendAddressPools, true)
if err != nil {
klog.Errorf("bi.ReconcileBackendPools for service (%s): failed to EnsureBackendPoolDeleted: %s", serviceName, err.Error())
return false, false, err
}

var nodeIPAddressesToBeDeleted []string
Expand All @@ -484,6 +490,7 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi
if err := bi.CreateOrUpdateLBBackendPool(lbName, bp); err != nil {
return false, false, fmt.Errorf("bi.ReconcileBackendPools for service (%s): lb backendpool - failed to update backend pool %s for load balancer %s: %w", serviceName, lbBackendPoolName, lbName, err)
}
shouldRefreshLB = true
}
}
break
Expand All @@ -492,6 +499,13 @@ func (bi *backendPoolTypeNodeIP) ReconcileBackendPools(clusterName string, servi
}
}

if shouldRefreshLB {
lb, _, err = bi.getAzureLoadBalancer(lbName, cache.CacheReadTypeForceRefresh)
if err != nil {
return false, false, fmt.Errorf("bi.ReconcileBackendPools for service (%s): failed to get load balancer %s: %w", serviceName, lbName, err)
}
}

if !foundBackendPool {
isBackendPoolPreConfigured = newBackendPool(lb, isBackendPoolPreConfigured, bi.PreConfiguredBackendPoolLoadBalancerTypes, getServiceName(service), getBackendPoolName(clusterName, service))
changed = true
Expand Down

0 comments on commit db25a1e

Please sign in to comment.