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

fix: skip removing nics from lb if there will be no nics in the backe… #3212

Merged
merged 1 commit into from
Jan 28, 2023
Merged

fix: skip removing nics from lb if there will be no nics in the backe… #3212

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
92 changes: 75 additions & 17 deletions pkg/provider/azure_loadbalancer_backendpool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,15 +180,15 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string,
}
}

var backendIPConfigurationsToBeDeleted []network.InterfaceIPConfiguration
var backendIPConfigurationsToBeDeleted, bipConfigNotFound, bipConfigExclude []network.InterfaceIPConfiguration
if bp.BackendAddressPoolPropertiesFormat != nil && bp.BackendIPConfigurations != nil {
for _, ipConf := range *bp.BackendIPConfigurations {
ipConfID := pointer.StringDeref(ipConf.ID, "")
nodeName, _, err := bc.VMSet.GetNodeNameByIPConfigurationID(ipConfID)
if err != nil {
if errors.Is(err, cloudprovider.InstanceNotFound) {
klog.V(2).Infof("bc.ReconcileBackendPools for service (%s): vm not found for ipConfID %s", serviceName, ipConfID)
backendIPConfigurationsToBeDeleted = append(backendIPConfigurationsToBeDeleted, ipConf)
bipConfigNotFound = append(bipConfigNotFound, ipConf)
} else {
return false, false, err
}
Expand All @@ -206,10 +206,11 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string,
if shouldExcludeLoadBalancer {
klog.V(2).Infof("bc.ReconcileBackendPools for service (%s): lb backendpool - found unwanted node %s, decouple it from the LB %s", serviceName, nodeName, lbName)
// construct a backendPool that only contains the IP config of the node to be deleted
backendIPConfigurationsToBeDeleted = append(backendIPConfigurationsToBeDeleted, network.InterfaceIPConfiguration{ID: pointer.String(ipConfID)})
bipConfigExclude = append(bipConfigExclude, network.InterfaceIPConfiguration{ID: pointer.String(ipConfID)})
}
}
}
backendIPConfigurationsToBeDeleted = getBackendIPConfigurationsToBeDeleted(bp, bipConfigNotFound, bipConfigExclude)
if len(backendIPConfigurationsToBeDeleted) > 0 {
backendpoolToBeDeleted := &[]network.BackendAddressPool{
{
Expand Down Expand Up @@ -249,6 +250,47 @@ func (bc *backendPoolTypeNodeIPConfig) ReconcileBackendPools(clusterName string,
return isBackendPoolPreConfigured, changed, err
}

func getBackendIPConfigurationsToBeDeleted(
bp network.BackendAddressPool,
bipConfigNotFound, bipConfigExclude []network.InterfaceIPConfiguration,
) []network.InterfaceIPConfiguration {
if bp.BackendAddressPoolPropertiesFormat == nil || bp.BackendIPConfigurations == nil {
return []network.InterfaceIPConfiguration{}
}

bipConfigNotFoundIDSet := sets.NewString()
bipConfigExcludeIDSet := sets.NewString()
for _, ipConfig := range bipConfigNotFound {
bipConfigNotFoundIDSet.Insert(pointer.StringDeref(ipConfig.ID, ""))
}
for _, ipConfig := range bipConfigExclude {
bipConfigExcludeIDSet.Insert(pointer.StringDeref(ipConfig.ID, ""))
}

var bipConfigToBeDeleted []network.InterfaceIPConfiguration
ipConfigs := *bp.BackendIPConfigurations
for i := len(ipConfigs) - 1; i >= 0; i-- {
ipConfigID := pointer.StringDeref(ipConfigs[i].ID, "")
if bipConfigNotFoundIDSet.Has(ipConfigID) {
bipConfigToBeDeleted = append(bipConfigToBeDeleted, ipConfigs[i])
ipConfigs = append(ipConfigs[:i], ipConfigs[i+1:]...)
}
}

var unwantedIPConfigs []network.InterfaceIPConfiguration
for _, ipConfig := range ipConfigs {
ipConfigID := pointer.StringDeref(ipConfig.ID, "")
if bipConfigExcludeIDSet.Has(ipConfigID) {
unwantedIPConfigs = append(unwantedIPConfigs, ipConfig)
}
}
if len(unwantedIPConfigs) == len(ipConfigs) {
klog.V(2).Info("getBackendIPConfigurationsToBeDeleted: the pool is empty or will be empty after removing the unwanted IP addresses, skipping the removal")
return bipConfigToBeDeleted
}
return append(bipConfigToBeDeleted, unwantedIPConfigs...)
}

func (bc *backendPoolTypeNodeIPConfig) GetBackendPrivateIPs(clusterName string, service *v1.Service, lb *network.LoadBalancer) ([]string, []string) {
serviceName := getServiceName(service)
lbBackendPoolName := getBackendPoolName(clusterName, service)
Expand Down Expand Up @@ -574,23 +616,39 @@ func newBackendPool(lb *network.LoadBalancer, isBackendPoolPreConfigured bool, p
func removeNodeIPAddressesFromBackendPool(backendPool network.BackendAddressPool, nodeIPAddresses []string, removeAll bool) bool {
changed := false
nodeIPsSet := sets.NewString(nodeIPAddresses...)
if backendPool.BackendAddressPoolPropertiesFormat != nil &&
backendPool.LoadBalancerBackendAddresses != nil {
for i := len(*backendPool.LoadBalancerBackendAddresses) - 1; i >= 0; i-- {
if (*backendPool.LoadBalancerBackendAddresses)[i].LoadBalancerBackendAddressPropertiesFormat != nil {
ipAddress := pointer.StringDeref((*backendPool.LoadBalancerBackendAddresses)[i].IPAddress, "")
if ipAddress == "" {
klog.V(4).Infof("removeNodeIPAddressFromBackendPool: LoadBalancerBackendAddress %s is not IP-based, skipping", pointer.StringDeref((*backendPool.LoadBalancerBackendAddresses)[i].Name, ""))
continue
}
if removeAll || nodeIPsSet.Has(ipAddress) {
klog.V(4).Infof("removeNodeIPAddressFromBackendPool: removing %s from the backend pool %s", ipAddress, pointer.StringDeref(backendPool.Name, ""))
*backendPool.LoadBalancerBackendAddresses = append((*backendPool.LoadBalancerBackendAddresses)[:i], (*backendPool.LoadBalancerBackendAddresses)[i+1:]...)
changed = true
}

if backendPool.BackendAddressPoolPropertiesFormat == nil ||
backendPool.LoadBalancerBackendAddresses == nil {
return false
}

addresses := *backendPool.LoadBalancerBackendAddresses
for i := len(addresses) - 1; i >= 0; i-- {
if addresses[i].LoadBalancerBackendAddressPropertiesFormat != nil {
ipAddress := pointer.StringDeref((*backendPool.LoadBalancerBackendAddresses)[i].IPAddress, "")
if ipAddress == "" {
klog.V(4).Infof("removeNodeIPAddressFromBackendPool: LoadBalancerBackendAddress %s is not IP-based, skipping", pointer.StringDeref(addresses[i].Name, ""))
continue
}
if removeAll || nodeIPsSet.Has(ipAddress) {
klog.V(4).Infof("removeNodeIPAddressFromBackendPool: removing %s from the backend pool %s", ipAddress, pointer.StringDeref(backendPool.Name, ""))
addresses = append(addresses[:i], addresses[i+1:]...)
changed = true
}
}
}

if removeAll {
backendPool.LoadBalancerBackendAddresses = &addresses
return changed
}

if len(addresses) == 0 {
klog.V(2).Info("removeNodeIPAddressFromBackendPool: the pool is empty or will be empty after removing the unwanted IP addresses, skipping the removal")
changed = false
} else if changed {
backendPool.LoadBalancerBackendAddresses = &addresses
}

return changed
}
153 changes: 115 additions & 38 deletions pkg/provider/azure_loadbalancer_backendpool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,51 +521,59 @@ func TestReconcileBackendPoolsNodeIPConfigToIP(t *testing.T) {
assert.Empty(t, (*lb.BackendAddressPools)[0].LoadBalancerBackendAddresses)
}

func TestRemoveNodeIPAddressFromBackendPool(t *testing.T) {
nodeIPAddresses := []string{"1.2.3.4", "4.3.2.1"}
func buildTestLoadBalancerBackendPoolWithIPs(name string, ips []string) network.BackendAddressPool {
backendPool := network.BackendAddressPool{
Name: pointer.String("kubernetes"),
Name: &name,
BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{
LoadBalancerBackendAddresses: &[]network.LoadBalancerBackendAddress{
{
LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{
IPAddress: pointer.String("1.2.3.4"),
},
},
{
LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{
IPAddress: pointer.String("5.6.7.8"),
},
},
{
LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{
IPAddress: pointer.String("4.3.2.1"),
},
},
{
LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{},
},
},
LoadBalancerBackendAddresses: &[]network.LoadBalancerBackendAddress{},
},
}
expectedBackendPool := network.BackendAddressPool{
Name: pointer.String("kubernetes"),
BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{
LoadBalancerBackendAddresses: &[]network.LoadBalancerBackendAddress{
{
LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{
IPAddress: pointer.String("5.6.7.8"),
},
},
{
LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{},
},
for _, ip := range ips {
ip := ip
*backendPool.LoadBalancerBackendAddresses = append(*backendPool.LoadBalancerBackendAddresses, network.LoadBalancerBackendAddress{
LoadBalancerBackendAddressPropertiesFormat: &network.LoadBalancerBackendAddressPropertiesFormat{
IPAddress: &ip,
},
},
})
}

removeNodeIPAddressesFromBackendPool(backendPool, nodeIPAddresses, false)
assert.Equal(t, expectedBackendPool, backendPool)
return backendPool
}

func TestRemoveNodeIPAddressFromBackendPool(t *testing.T) {
for _, tc := range []struct {
description string
removeAll bool
unwantedIPs, existingIPs, expectedIPs []string
}{
{
description: "removeNodeIPAddressFromBackendPool should remove the unwanted IP addresses from the backend pool",
unwantedIPs: []string{"1.2.3.4", "4.3.2.1"},
existingIPs: []string{"1.2.3.4", "5.6.7.8", "4.3.2.1", ""},
expectedIPs: []string{"5.6.7.8", ""},
},
{
description: "removeNodeIPAddressFromBackendPool should not make the backend pool empty",
unwantedIPs: []string{"1.2.3.4", "4.3.2.1"},
existingIPs: []string{"1.2.3.4", "4.3.2.1"},
expectedIPs: []string{"1.2.3.4", "4.3.2.1"},
},
{
description: "removeNodeIPAddressFromBackendPool should remove all the IP addresses from the backend pool",
removeAll: true,
unwantedIPs: []string{"1.2.3.4", "4.3.2.1"},
existingIPs: []string{"1.2.3.4", "4.3.2.1", ""},
expectedIPs: []string{""},
},
} {
t.Run(tc.description, func(t *testing.T) {
backendPool := buildTestLoadBalancerBackendPoolWithIPs("kubernetes", tc.existingIPs)
expectedBackendPool := buildTestLoadBalancerBackendPoolWithIPs("kubernetes", tc.expectedIPs)

removeNodeIPAddressesFromBackendPool(backendPool, tc.unwantedIPs, tc.removeAll)
assert.Equal(t, expectedBackendPool, backendPool)
})
}
}

func TestGetBackendPrivateIPsNodeIPConfig(t *testing.T) {
Expand All @@ -588,3 +596,72 @@ func TestGetBackendPrivateIPsNodeIPConfig(t *testing.T) {
assert.Equal(t, []string{"1.2.3.4"}, ipv4)
assert.Equal(t, []string{"fe80::1"}, ipv6)
}

func TestGetBackendIPConfigurationsToBeDeleted(t *testing.T) {
for _, tc := range []struct {
description string
bipConfigNotFound, bipConfigExclude []network.InterfaceIPConfiguration
expected map[string]bool
}{
{
description: "should ignore excluded IP configurations if the backend pool will be empty after removing IP configurations of not found vms",
bipConfigNotFound: []network.InterfaceIPConfiguration{
{ID: pointer.String("ipconfig1")},
{ID: pointer.String("ipconfig2")},
},
bipConfigExclude: []network.InterfaceIPConfiguration{
{ID: pointer.String("ipconfig3")},
},
expected: map[string]bool{
"ipconfig1": true,
"ipconfig2": true,
},
},
{
description: "should remove both not found and excluded vms",
bipConfigNotFound: []network.InterfaceIPConfiguration{
{ID: pointer.String("ipconfig1")},
},
bipConfigExclude: []network.InterfaceIPConfiguration{
{ID: pointer.String("ipconfig3")},
},
expected: map[string]bool{
"ipconfig1": true,
"ipconfig3": true,
},
},
{
description: "should remove all not found vms even if the backend pool will be empty",
bipConfigNotFound: []network.InterfaceIPConfiguration{
{ID: pointer.String("ipconfig1")},
{ID: pointer.String("ipconfig2")},
{ID: pointer.String("ipconfig3")},
},
bipConfigExclude: []network.InterfaceIPConfiguration{
{ID: pointer.String("ipconfig4")},
},
expected: map[string]bool{
"ipconfig1": true,
"ipconfig2": true,
"ipconfig3": true,
},
},
} {
bp := network.BackendAddressPool{
BackendAddressPoolPropertiesFormat: &network.BackendAddressPoolPropertiesFormat{
BackendIPConfigurations: &[]network.InterfaceIPConfiguration{
{ID: pointer.String("ipconfig1")},
{ID: pointer.String("ipconfig2")},
{ID: pointer.String("ipconfig3")},
},
},
}

ipConfigs := getBackendIPConfigurationsToBeDeleted(bp, tc.bipConfigNotFound, tc.bipConfigExclude)
actual := make(map[string]bool)
for _, ipConfig := range ipConfigs {
actual[*ipConfig.ID] = true
}
assert.Equal(t, tc.expected, actual)
}
}
8 changes: 7 additions & 1 deletion tests/e2e/network/ensureloadbalancer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,13 @@ var _ = Describe("Ensure LoadBalancer", Label(utils.TestSuiteLabelLB), func() {
By("Labeling node")
node, err := utils.LabelNode(cs, &nodes[0], label, false)
Expect(err).NotTo(HaveOccurred())
err = waitForNodesInLBBackendPool(tc, publicIP, len(nodes)-1)
var expectedCount int
if len(nodes) == 1 {
expectedCount = 1
} else {
expectedCount = len(nodes) - 1
}
err = waitForNodesInLBBackendPool(tc, publicIP, expectedCount)
Expect(err).NotTo(HaveOccurred())

By("Unlabeling node")
Expand Down