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.

Sign up for GitHub

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

Jump to bottom

[release-1.25] fix: decouple vmss from the lb if the backend pool is empty when usin… #2833

Merged
merged 1 commit into from
Nov 29, 2022
Merged

[release-1.25] fix: decouple vmss from the lb if the backend pool is empty when usin… #2833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 4 additions & 5 deletions pkg/provider/azure_loadbalancer.go
View file
Open in desktop
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
Open in desktop
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