Skip to content

Commit

Permalink
Fix IPv6/dual-stack EnsureBackendPoolDeleted() failure
Browse files Browse the repository at this point in the history 
* IP config of IPv6 is not primary, it should not be
skipped in EnsureBackendPoolDeleted(). Updated e2e code.
* Fix health probe e2e

Signed-off-by: Zhecheng Li <zhechengli@microsoft.com>
  • Loading branch information
@lzhecheng
lzhecheng committed Jul 14, 2023
1 parent 8f75b40 commit 61c67fa
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 63 deletions.
9 changes: 7 additions & 2 deletions pkg/provider/azure_standard.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1079,15 +1079,20 @@ func (as *availabilitySet) EnsureBackendPoolDeleted(service *v1.Service, backend
ipconfigPrefixToNicMap[ipConfigIDPrefix] = nic
}
}
v4Enabled, v6Enabled := getIPFamiliesEnabled(service)
isServiceIPv4 := v4Enabled && !v6Enabled
var nicUpdated atomic.Bool
for k := range ipconfigPrefixToNicMap {
nic := ipconfigPrefixToNicMap[k]
newIPConfigs := *nic.IPConfigurations
for j, ipConf := range newIPConfigs {
if !pointer.BoolDeref(ipConf.Primary, false) {
if isServiceIPv4 && !pointer.BoolDeref(ipConf.Primary, false) {
// found primary ip configuration
continue
}
// found primary ip configuration
// To support IPv6 only and dual-stack 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
108 changes: 66 additions & 42 deletions tests/e2e/network/ensureloadbalancer.go
View file
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 @@ -699,15 +700,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 @@ -809,7 +814,7 @@ var _ = Describe("Ensure LoadBalancer", Label(utils.TestSuiteLabelLB), func() {
utils.Logf("Checking LB rule %q", *lbRule.Name)
lbRuleSplit := strings.Split(*lbRule.Name, "-")
Expect(len(lbRuleSplit)).NotTo(Equal(0))
// id is like xxx-IPv4 or xxx-IPv6 and lbRuleSplit[0] is like xxx.
// id is like xxx or xxx-IPv6 and lbRuleSplit[0] is like xxx.
if !strings.Contains(configID, lbRuleSplit[0]) {
continue
}
Expand Down Expand Up @@ -1202,60 +1207,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 Expand Up @@ -1287,11 +1309,13 @@ func updateServicePIPNames(service *v1.Service, pipNames []string) *v1.Service {
service.Annotations = map[string]string{}
}

isDualStack := len(service.Spec.IPFamilies) == 2

for _, pipName := range pipNames {
if strings.HasSuffix(pipName, "-IPv6") {
service.Annotations[consts.ServiceAnnotationPIPNameDualStack[consts.IPVersionIPv6]] = pipName
} else {
if !isDualStack || !strings.HasSuffix(pipName, "-IPv6") {
service.Annotations[consts.ServiceAnnotationPIPNameDualStack[consts.IPVersionIPv4]] = pipName
} else {
service.Annotations[consts.ServiceAnnotationPIPNameDualStack[consts.IPVersionIPv6]] = pipName
}
}

Expand Down
48 changes: 29 additions & 19 deletions tests/e2e/network/service_annotations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,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 @@ -812,8 +813,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 @@ -823,6 +823,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 @@ -837,11 +838,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 @@ -869,7 +874,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 @@ -878,8 +883,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 @@ -889,22 +893,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

0 comments on commit 61c67fa

Please sign in to comment.