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.26] Fix IPv6/dual-stack EnsureBackendPoolDeleted() failure #4321

Merged
merged 1 commit into from
Jul 20, 2023
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
7 changes: 5 additions & 2 deletions pkg/provider/azure_standard.go
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,7 @@ func (as *availabilitySet) EnsureBackendPoolDeleted(service *v1.Service, backend
nicUpdaters := make([]func() error, 0)
allErrs := make([]error, 0)
var nicUpdated bool
isServiceIPv4 := len(service.Spec.IPFamilies) == 1 && service.Spec.IPFamilies[0] == v1.IPv4Protocol
for i := range ipConfigurationIDs {
ipConfigurationID := ipConfigurationIDs[i]
nodeName, _, err := as.GetNodeNameByIPConfigurationID(ipConfigurationID)
Expand Down Expand Up @@ -1141,10 +1142,12 @@ func (as *availabilitySet) EnsureBackendPoolDeleted(service *v1.Service, backend
if nic.InterfacePropertiesFormat != nil && nic.InterfacePropertiesFormat.IPConfigurations != nil {
newIPConfigs := *nic.IPConfigurations
for j, ipConf := range newIPConfigs {
if !pointer.BoolDeref(ipConf.Primary, false) {
if isServiceIPv4 && !pointer.BoolDeref(ipConf.Primary, false) {
continue
}
// found primary ip configuration
// To support IPv6 only clusters, all IP configurations
// should be checked regardless of primary or not because IPv6 IP configurations
// are not marked as primary.
if ipConf.LoadBalancerBackendAddressPools != nil {
newLBAddressPools := *ipConf.LoadBalancerBackendAddressPools
for k := len(newLBAddressPools) - 1; k >= 0; k-- {
Expand Down
98 changes: 60 additions & 38 deletions tests/e2e/network/ensureloadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package network

import (
"context"
"errors"
"fmt"
"net"
"os"
Expand Down Expand Up @@ -628,15 +629,19 @@ var _ = Describe("Ensure LoadBalancer", Label(utils.TestSuiteLabelLB), func() {
lb := getAzureLoadBalancerFromPIP(tc, publicIP, tc.GetResourceGroup(), "")
if lb.Sku != nil && lb.Sku.Name == aznetwork.LoadBalancerSkuNameBasic {
// For a basic lb, not autoscaling pipeline
ipConfigs := (*lb.BackendAddressPools)[getLBBackendPoolIndex(lb)].BackendIPConfigurations
idxes := getLBBackendPoolIndex(lb)
Expect(idxes).NotTo(BeZero())
ipConfigs := (*lb.BackendAddressPools)[idxes[0]].BackendIPConfigurations
Expect(ipConfigs).NotTo(BeNil())
lbBackendPoolIPConfigCount := len(*ipConfigs)
Expect(lbBackendPoolIPConfigCount).To(Equal(len(nodes)))
utils.Logf("Initial node number in the LB backend pool is %d", lbBackendPoolIPConfigCount)
} else {
// SLB: Here we use BackendPool IP instead of IP config because this works for both NIC based LB and IP based LB.
lbBackendPoolIPCount := 0
lbBackendPoolIPs := (*lb.BackendAddressPools)[getLBBackendPoolIndex(lb)].LoadBalancerBackendAddresses
idxes := getLBBackendPoolIndex(lb)
Expect(idxes).NotTo(BeZero())
lbBackendPoolIPs := (*lb.BackendAddressPools)[idxes[0]].LoadBalancerBackendAddresses
Expect(lbBackendPoolIPs).NotTo(BeNil())
if utils.IsAutoscalingAKSCluster() {
for _, ip := range *lbBackendPoolIPs {
Expand Down Expand Up @@ -1066,60 +1071,77 @@ func waitForNodesInLBBackendPool(tc *utils.AzureTestClient, ip string, expectedN
lb := getAzureLoadBalancerFromPIP(tc, ip, tc.GetResourceGroup(), "")
if lb.Sku != nil && lb.Sku.Name == aznetwork.LoadBalancerSkuNameBasic {
// basic lb
lbBackendPoolIPConfigs := (*lb.BackendAddressPools)[getLBBackendPoolIndex(lb)].BackendIPConfigurations
ipConfigNum := 0
if lbBackendPoolIPConfigs != nil {
ipConfigNum = len(*lbBackendPoolIPConfigs)
idxes := getLBBackendPoolIndex(lb)
if len(idxes) == 0 {
return false, errors.New("no backend pool found")
}
if expectedNum == ipConfigNum {
utils.Logf("Number of IP configs matches expected number %d. Success", expectedNum)
return true, nil
failed := false
for _, idx := range idxes {
bp := (*lb.BackendAddressPools)[idx]
lbBackendPoolIPConfigs := bp.BackendIPConfigurations
ipConfigNum := 0
if lbBackendPoolIPConfigs != nil {
ipConfigNum = len(*lbBackendPoolIPConfigs)
}
if expectedNum == ipConfigNum {
utils.Logf("Number of IP configs in the LB backend pool %q matches expected number %d. Success", *bp.Name, expectedNum)
} else {
utils.Logf("Number of IP configs: %d in the LB backend pool %q, expected %d, will retry soon", ipConfigNum, *bp.Name, expectedNum)
failed = true
}
}
utils.Logf("Number of IP configs: %d in the LB backend pool, expected %d, will retry soon", ipConfigNum, expectedNum)
return false, nil
return !failed, nil
}
// SLB
lbBackendPoolIPs := (*lb.BackendAddressPools)[getLBBackendPoolIndex(lb)].LoadBalancerBackendAddresses
ipNum := 0
if lbBackendPoolIPs != nil {
if utils.IsAutoscalingAKSCluster() {
// Autoscaling tests don't include IP based LB.
for _, ip := range *lbBackendPoolIPs {
if ip.LoadBalancerBackendAddressPropertiesFormat == nil ||
ip.LoadBalancerBackendAddressPropertiesFormat.NetworkInterfaceIPConfiguration == nil {
return false, fmt.Errorf("LB backendPool address's NIC IP config ID is nil")
}
ipConfigID := pointer.StringDeref(ip.LoadBalancerBackendAddressPropertiesFormat.NetworkInterfaceIPConfiguration.ID, "")
if !strings.Contains(ipConfigID, utils.SystemPool) {
ipNum++
idxes := getLBBackendPoolIndex(lb)
if len(idxes) == 0 {
return false, errors.New("no backend pool found")
}
failed := false
for _, idx := range idxes {
bp := (*lb.BackendAddressPools)[idx]
lbBackendPoolIPs := bp.LoadBalancerBackendAddresses
ipNum := 0
if lbBackendPoolIPs != nil {
if utils.IsAutoscalingAKSCluster() {
// Autoscaling tests don't include IP based LB.
for _, ip := range *lbBackendPoolIPs {
if ip.LoadBalancerBackendAddressPropertiesFormat == nil ||
ip.LoadBalancerBackendAddressPropertiesFormat.NetworkInterfaceIPConfiguration == nil {
return false, fmt.Errorf("LB backendPool address's NIC IP config ID is nil")
}
ipConfigID := pointer.StringDeref(ip.LoadBalancerBackendAddressPropertiesFormat.NetworkInterfaceIPConfiguration.ID, "")
if !strings.Contains(ipConfigID, utils.SystemPool) {
ipNum++
}
}
} else {
ipNum = len(*lbBackendPoolIPs)
}
}
if ipNum == expectedNum {
utils.Logf("Number of IPs in the LB backend pool %q matches expected number %d. Success", *bp.Name, expectedNum)
} else {
ipNum = len(*lbBackendPoolIPs)
utils.Logf("Number of IPs: %d in the LB backend pool %q, expected %d, will retry soon", ipNum, *bp.Name, expectedNum)
failed = true
}
}
if ipNum == expectedNum {
utils.Logf("Number of IPs matches expected number %d. Success", expectedNum)
return true, nil
}
utils.Logf("Number of IPs: %d in the LB backend pool, expected %d, will retry soon", ipNum, expectedNum)
return false, nil
return !failed, nil
})
}

func judgeInternal(service v1.Service) bool {
return service.Annotations[consts.ServiceAnnotationLoadBalancerInternal] == utils.TrueValue
}

func getLBBackendPoolIndex(lb *aznetwork.LoadBalancer) int {
if os.Getenv(utils.AKSTestCCM) != "" {
for index, backendPool := range *lb.BackendAddressPools {
if *backendPool.Name != "aksOutboundBackendPool" {
return index
}
func getLBBackendPoolIndex(lb *aznetwork.LoadBalancer) []int {
idxes := []int{}
for index, backendPool := range *lb.BackendAddressPools {
if !strings.Contains(strings.ToLower(*backendPool.Name), "outboundbackendpool") {
idxes = append(idxes, index)
}
}
return 0
return idxes
}

func updateServiceLBIPs(service *v1.Service, isInternal bool, ips []string) (result *v1.Service) {
Expand Down
48 changes: 29 additions & 19 deletions tests/e2e/network/service_annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -805,6 +805,7 @@ var _ = Describe("Service with annotation", Label(utils.TestSuiteLabelServiceAnn
Expect(len(pipFrontendConfigIDSplit)).NotTo(Equal(0))
ids = append(ids, pipFrontendConfigIDSplit[len(pipFrontendConfigIDSplit)-1])
}
utils.Logf("PIP frontend config IDs %q", ids)

var lb *network.LoadBalancer
var targetProbes []*network.Probe
Expand All @@ -822,8 +823,7 @@ var _ = Describe("Service with annotation", Label(utils.TestSuiteLabelServiceAnn
probeSplit := strings.Split(*probe.Name, "-")
Expect(len(probeSplit)).NotTo(Equal(0))
probeSplitID := probeSplit[0]
if len(probeSplit) > 1 &&
(probeSplit[len(probeSplit)-1] == "IPv4" || probeSplit[len(probeSplit)-1] == "IPv6") {
if probeSplit[len(probeSplit)-1] == "IPv6" {
probeSplitID += "-" + probeSplit[len(probeSplit)-1]
}
for _, id := range ids {
Expand All @@ -833,6 +833,7 @@ var _ = Describe("Service with annotation", Label(utils.TestSuiteLabelServiceAnn
}
}

utils.Logf("targetProbes count %d, expectedTargetProbes count %d", len(targetProbes), expectedTargetProbesCount)
return len(targetProbes) == expectedTargetProbesCount, nil
})
Expect(err).NotTo(HaveOccurred())
Expand All @@ -847,11 +848,15 @@ var _ = Describe("Service with annotation", Label(utils.TestSuiteLabelServiceAnn
utils.Logf("Validating health probe config intervalInSeconds")
Expect(*probe.IntervalInSeconds).To(Equal(int32(10)))
}
utils.Logf("Validating health probe config ProbeProtocolHTTP")
Expect(probe.Protocol).To(Equal(network.ProbeProtocolHTTP))
}

By("Changing ExternalTrafficPolicy of the service to Local")
expectedTargetProbesCount = 1
expectedTargetProbesLocalCount := 1
if tc.IPFamily == utils.DualStack {
expectedTargetProbesLocalCount = 2
}
var service *v1.Service
utils.Logf("Updating service " + serviceName + " in namespace " + ns.Name)
retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
Expand Down Expand Up @@ -879,7 +884,7 @@ var _ = Describe("Service with annotation", Label(utils.TestSuiteLabelServiceAnn
})
Expect(retryErr).NotTo(HaveOccurred())

err = wait.PollImmediate(5*time.Second, 60*time.Second, func() (bool, error) {
err = wait.PollImmediate(5*time.Second, 300*time.Second, func() (bool, error) {
lb = getAzureLoadBalancerFromPIP(tc, publicIPs[0], tc.GetResourceGroup(), "")
targetProbes = []*network.Probe{}
for i := range *lb.LoadBalancerPropertiesFormat.Probes {
Expand All @@ -888,8 +893,7 @@ var _ = Describe("Service with annotation", Label(utils.TestSuiteLabelServiceAnn
probeSplit := strings.Split(*probe.Name, "-")
Expect(len(probeSplit)).NotTo(Equal(0))
probeSplitID := probeSplit[0]
if len(probeSplit) > 1 &&
(probeSplit[len(probeSplit)-1] == "IPv4" || probeSplit[len(probeSplit)-1] == "IPv6") {
if probeSplit[len(probeSplit)-1] == "IPv6" {
probeSplitID += "-" + probeSplit[len(probeSplit)-1]
}
for _, id := range ids {
Expand All @@ -899,22 +903,28 @@ var _ = Describe("Service with annotation", Label(utils.TestSuiteLabelServiceAnn
}
}

return len(targetProbes) == expectedTargetProbesCount, nil
})
Expect(err).NotTo(HaveOccurred())

By("Validating health probe configs")
for _, probe := range targetProbes {
if probe.ProbeThreshold != nil {
utils.Logf("Validating health probe config numberOfProbes")
Expect(*probe.ProbeThreshold).To(Equal(int32(5)))
utils.Logf("targetProbes count %d, expectedTargetProbesLocal count %d", len(targetProbes), expectedTargetProbesLocalCount)
if len(targetProbes) != expectedTargetProbesLocalCount {
return false, nil
}
if probe.IntervalInSeconds != nil {
By("Validating health probe configs")
for _, probe := range targetProbes {
utils.Logf("Validating health probe config numberOfProbes")
if probe.ProbeThreshold == nil || *probe.ProbeThreshold != int32(5) {
return false, nil
}
utils.Logf("Validating health probe config intervalInSeconds")
Expect(*probe.IntervalInSeconds).To(Equal(int32(15)))
if probe.IntervalInSeconds == nil || *probe.IntervalInSeconds != int32(15) {
return false, nil
}
utils.Logf("Validating health probe config ProbeProtocolHTTP")
if !strings.EqualFold(string(probe.Protocol), string(network.ProbeProtocolHTTP)) {
return false, nil
}
}
Expect(probe.Protocol).To(Equal(network.ProbeProtocolHTTP))
}
return true, nil
})
Expect(err).NotTo(HaveOccurred())
})

It("should generate health probe configs in multi-port scenario", func() {
Expand Down