Fix custom backendPool not being used

In some places the default backendPool name was being used instead of the specified value.
kubernetes-sigs · Jun 29, 2023 · f947836 · f947836
1 parent 7059146
commit f947836
Show file tree

Hide file tree

Showing 10 changed files with 86 additions and 49 deletions.
diff --git a/azure/interfaces.go b/azure/interfaces.go
@@ -63,7 +63,7 @@ type NetworkDescriber interface {
 	ControlPlaneRouteTable() infrav1.RouteTable
 	APIServerLB() *infrav1.LoadBalancerSpec
 	APIServerLBName() string
-	APIServerLBPoolName(string) string
+	APIServerLBPoolName() string
 	IsAPIServerPrivate() bool
 	GetPrivateDNSZoneName() string
 	OutboundLBName(string) string

diff --git a/azure/mock_azure/azure_mock.go b/azure/mock_azure/azure_mock.go
diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go
@@ -711,33 +711,37 @@ func (s *ClusterScope) GetPrivateDNSZoneName() string {
 }
 
 // APIServerLBPoolName returns the API Server LB backend pool name.
-func (s *ClusterScope) APIServerLBPoolName(loadBalancerName string) string {
-	return azure.GenerateBackendAddressPoolName(loadBalancerName)
+func (s *ClusterScope) APIServerLBPoolName() string {
+	return s.APIServerLB().BackendPool.Name
 }
 
-// OutboundLBName returns the name of the outbound LB.
-func (s *ClusterScope) OutboundLBName(role string) string {
+// OutboundLB returns the outbound LB.
+func (s *ClusterScope) OutboundLB(role string) *infrav1.LoadBalancerSpec {
 	if role == infrav1.Node {
-		if s.NodeOutboundLB() == nil {
-			return ""
-		}
-		return s.NodeOutboundLB().Name
+		return s.NodeOutboundLB()
 	}
 	if s.IsAPIServerPrivate() {
-		if s.ControlPlaneOutboundLB() == nil {
-			return ""
-		}
-		return s.ControlPlaneOutboundLB().Name
+		return s.ControlPlaneOutboundLB()
+	}
+	return s.APIServerLB()
+}
+
+// OutboundLBName returns the name of the outbound LB.
+func (s *ClusterScope) OutboundLBName(role string) string {
+	lb := s.OutboundLB(role)
+	if lb == nil {
+		return ""
 	}
-	return s.APIServerLBName()
+	return lb.Name
 }
 
 // OutboundPoolName returns the outbound LB backend pool name.
-func (s *ClusterScope) OutboundPoolName(loadBalancerName string) string {
-	if loadBalancerName == "" {
+func (s *ClusterScope) OutboundPoolName(role string) string {
+	lb := s.OutboundLB(role)
+	if lb == nil {
 		return ""
 	}
-	return azure.GenerateOutboundBackendAddressPoolName(loadBalancerName)
+	return lb.BackendPool.Name
 }
 
 // ResourceGroup returns the cluster resource group.

diff --git a/azure/scope/cluster_test.go b/azure/scope/cluster_test.go
@@ -1968,6 +1968,11 @@ func TestAPIServerLBPoolName(t *testing.T) {
 					},
 				},
 				Spec: infrav1.AzureClusterSpec{
+					NetworkSpec: infrav1.NetworkSpec{
+						APIServerLB: infrav1.LoadBalancerSpec{
+							Name: tc.lbName,
+						},
+					},
 					AzureClusterClassSpec: infrav1.AzureClusterClassSpec{
 						SubscriptionID: "123",
 					},
@@ -1985,8 +1990,9 @@ func TestAPIServerLBPoolName(t *testing.T) {
 				AzureCluster: azureCluster,
 				Client:       fakeClient,
 			})
+			clusterScope.AzureCluster.SetBackendPoolNameDefault()
 			g.Expect(err).NotTo(HaveOccurred())
-			got := clusterScope.APIServerLBPoolName(tc.lbName)
+			got := clusterScope.APIServerLBPoolName()
 			g.Expect(tc.expectLBpoolName).Should(Equal(got))
 		})
 	}
@@ -2125,6 +2131,7 @@ func TestOutboundLBName(t *testing.T) {
 				AzureCluster: azureCluster,
 				Client:       fakeClient,
 			})
+			clusterScope.AzureCluster.SetBackendPoolNameDefault()
 			g.Expect(err).NotTo(HaveOccurred())
 			got := clusterScope.OutboundLBName(tc.role)
 			g.Expect(tc.expected).Should(Equal(got))
@@ -2253,6 +2260,7 @@ func TestBackendPoolName(t *testing.T) {
 				AzureCluster: azureCluster,
 				Client:       fakeClient,
 			})
+			clusterScope.AzureCluster.SetBackendPoolNameDefault()
 			g.Expect(err).NotTo(HaveOccurred())
 			got := clusterScope.LBSpecs()
 			g.Expect(len(got)).To(Equal(3))
@@ -2325,7 +2333,15 @@ func TestOutboundPoolName(t *testing.T) {
 				},
 			}
 
+			if tc.loadBalancerName != "" {
+				azureCluster.Spec.NetworkSpec.NodeOutboundLB = &infrav1.LoadBalancerSpec{
+					Name: tc.loadBalancerName,
+				}
+			}
+
 			initObjects := []runtime.Object{cluster, azureCluster}
+			azureCluster.Default()
+
 			fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(initObjects...).Build()
 
 			clusterScope, err := NewClusterScope(context.TODO(), ClusterScopeParams{
@@ -2336,8 +2352,9 @@ func TestOutboundPoolName(t *testing.T) {
 				AzureCluster: azureCluster,
 				Client:       fakeClient,
 			})
+			clusterScope.AzureCluster.SetBackendPoolNameDefault()
 			g.Expect(err).NotTo(HaveOccurred())
-			got := clusterScope.OutboundPoolName(tc.loadBalancerName)
+			got := clusterScope.OutboundPoolName(infrav1.Node)
 			g.Expect(tc.expectOutboundPoolName).Should(Equal(got))
 		})
 	}

diff --git a/azure/scope/machine.go b/azure/scope/machine.go
@@ -277,13 +277,13 @@ func (m *MachineScope) BuildNICSpec(nicName string, infrav1NetworkInterface infr
 
 		if m.Role() == infrav1.ControlPlane {
 			spec.PublicLBName = m.OutboundLBName(m.Role())
-			spec.PublicLBAddressPoolName = m.OutboundPoolName(m.OutboundLBName(m.Role()))
+			spec.PublicLBAddressPoolName = m.OutboundPoolName(m.Role())
 			if m.IsAPIServerPrivate() {
 				spec.InternalLBName = m.APIServerLBName()
-				spec.InternalLBAddressPoolName = m.APIServerLBPoolName(m.APIServerLBName())
+				spec.InternalLBAddressPoolName = m.APIServerLBPoolName()
 			} else {
 				spec.PublicLBNATRuleName = m.Name()
-				spec.PublicLBAddressPoolName = m.APIServerLBPoolName(m.APIServerLBName())
+				spec.PublicLBAddressPoolName = m.APIServerLBPoolName()
 			}
 		}
 
@@ -293,12 +293,7 @@ func (m *MachineScope) BuildNICSpec(nicName string, infrav1NetworkInterface infr
 		// If the NAT gateway is not enabled and node has no public IP, then the NIC needs to reference the LB to get outbound traffic.
 		if m.Role() == infrav1.Node && !m.Subnet().IsNatGatewayEnabled() && !m.AzureMachine.Spec.AllocatePublicIP {
 			spec.PublicLBName = m.OutboundLBName(m.Role())
-			spec.PublicLBAddressPoolName = m.OutboundPoolName(m.OutboundLBName(m.Role()))
-		}
-		// If the NAT gateway is not enabled and node has no public IP, then the NIC needs to reference the LB to get outbound traffic.
-		if m.Role() == infrav1.Node && !m.Subnet().IsNatGatewayEnabled() && !m.AzureMachine.Spec.AllocatePublicIP {
-			spec.PublicLBName = m.OutboundLBName(m.Role())
-			spec.PublicLBAddressPoolName = m.OutboundPoolName(m.OutboundLBName(m.Role()))
+			spec.PublicLBAddressPoolName = m.OutboundPoolName(m.Role())
 		}
 	}
 

diff --git a/azure/scope/machine_test.go b/azure/scope/machine_test.go
@@ -1715,6 +1715,9 @@ func TestMachineScope_NICSpecs(t *testing.T) {
 								},
 								NodeOutboundLB: &infrav1.LoadBalancerSpec{
 									Name: "outbound-lb",
+									BackendPool: infrav1.BackendPool{
+										Name: "outbound-lb-outboundBackendPool",
+									},
 								},
 							},
 						},
@@ -1819,6 +1822,9 @@ func TestMachineScope_NICSpecs(t *testing.T) {
 								},
 								NodeOutboundLB: &infrav1.LoadBalancerSpec{
 									Name: "outbound-lb",
+									BackendPool: infrav1.BackendPool{
+										Name: "outbound-lb-outboundBackendPool",
+									},
 								},
 							},
 						},
@@ -2147,6 +2153,9 @@ func TestMachineScope_NICSpecs(t *testing.T) {
 									LoadBalancerClassSpec: infrav1.LoadBalancerClassSpec{
 										Type: infrav1.Internal,
 									},
+									BackendPool: infrav1.BackendPool{
+										Name: "api-lb-backendPool",
+									},
 								},
 								NodeOutboundLB: &infrav1.LoadBalancerSpec{
 									Name: "outbound-lb",
@@ -2254,6 +2263,9 @@ func TestMachineScope_NICSpecs(t *testing.T) {
 								},
 								APIServerLB: infrav1.LoadBalancerSpec{
 									Name: "api-lb",
+									BackendPool: infrav1.BackendPool{
+										Name: "api-lb-backendPool",
+									},
 								},
 								NodeOutboundLB: &infrav1.LoadBalancerSpec{
 									Name: "outbound-lb",
@@ -2361,6 +2373,9 @@ func TestMachineScope_NICSpecs(t *testing.T) {
 								},
 								APIServerLB: infrav1.LoadBalancerSpec{
 									Name: "api-lb",
+									BackendPool: infrav1.BackendPool{
+										Name: "api-lb-backendPool",
+									},
 								},
 								NodeOutboundLB: &infrav1.LoadBalancerSpec{
 									Name: "outbound-lb",
@@ -2472,6 +2487,9 @@ func TestMachineScope_NICSpecs(t *testing.T) {
 								},
 								NodeOutboundLB: &infrav1.LoadBalancerSpec{
 									Name: "outbound-lb",
+									BackendPool: infrav1.BackendPool{
+										Name: "outbound-lb-outboundBackendPool",
+									},
 								},
 							},
 						},
@@ -2747,6 +2765,9 @@ func TestMachineScope_NICSpecs(t *testing.T) {
 								},
 								NodeOutboundLB: &infrav1.LoadBalancerSpec{
 									Name: "outbound-lb",
+									BackendPool: infrav1.BackendPool{
+										Name: "outbound-lb-outboundBackendPool",
+									},
 								},
 							},
 						},

diff --git a/azure/scope/machinepool.go b/azure/scope/machinepool.go
@@ -129,7 +129,7 @@ func (m *MachinePoolScope) ScaleSetSpec() azure.ScaleSetSpec {
 		VNetName:                     m.Vnet().Name,
 		VNetResourceGroup:            m.Vnet().ResourceGroup,
 		PublicLBName:                 m.OutboundLBName(infrav1.Node),
-		PublicLBAddressPoolName:      azure.GenerateOutboundBackendAddressPoolName(m.OutboundLBName(infrav1.Node)),
+		PublicLBAddressPoolName:      m.OutboundPoolName(infrav1.Node),
 		AcceleratedNetworking:        m.AzureMachinePool.Spec.Template.NetworkInterfaces[0].AcceleratedNetworking,
 		Identity:                     m.AzureMachinePool.Spec.Identity,
 		UserAssignedIdentities:       m.AzureMachinePool.Spec.UserAssignedIdentities,

diff --git a/azure/services/bastionhosts/mocks_bastionhosts/bastionhosts_mock.go b/azure/services/bastionhosts/mocks_bastionhosts/bastionhosts_mock.go
diff --git a/azure/services/loadbalancers/mock_loadbalancers/loadbalancers_mock.go b/azure/services/loadbalancers/mock_loadbalancers/loadbalancers_mock.go
diff --git a/azure/services/natgateways/mock_natgateways/natgateways_mock.go b/azure/services/natgateways/mock_natgateways/natgateways_mock.go