Skip to content

Commit d352e7c

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #1865 from Jont828/async-natgateways
Make NAT Gateway reconcile/delete async
2 parents ff49c7a + 154bf95 commit d352e7c

File tree

21 files changed

+487
-567
lines changed

21 files changed

+487
-567
lines changed

‎api/v1beta1/azurecluster_default.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ func (c *AzureCluster) setNodeOutboundLBDefaults() {
219219
}
220220
}
221221

222-
// If we don't default the outbound LB when there are some subnets with nat gateway,
222+
// If we don't default the outbound LB when there are some subnets with NAT gateway,
223223
// and some without, those without wouldn't have outbound traffic. So taking the
224224
// safer route, we configure the outbound LB in that scenario.
225225
if !needsOutboundLB {
@@ -401,7 +401,7 @@ func generateControlPlaneOutboundIPName(clusterName string) string {
401401
return fmt.Sprintf("pip-%s-controlplane-outbound", clusterName)
402402
}
403403

404-
// generateNatGatewayIPName generates a nat gateway IP name.
404+
// generateNatGatewayIPName generates a NAT gateway IP name.
405405
func generateNatGatewayIPName(clusterName, subnetName string) string {
406406
return fmt.Sprintf("pip-%s-%s-natgw", clusterName, subnetName)
407407
}

‎api/v1beta1/azurecluster_default_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -978,7 +978,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) {
978978
},
979979
},
980980
{
981-
name: "NAT Gateway enabled - no LB",
981+
name: "NAT gateway enabled - no LB",
982982
cluster: &AzureCluster{
983983
ObjectMeta: metav1.ObjectMeta{
984984
Name: "cluster-test",
@@ -1037,7 +1037,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) {
10371037
},
10381038
},
10391039
{
1040-
name: "NAT Gateway enabled on 1 of 2 node subnets",
1040+
name: "NAT gateway enabled on 1 of 2 node subnets",
10411041
cluster: &AzureCluster{
10421042
ObjectMeta: metav1.ObjectMeta{
10431043
Name: "cluster-test",
@@ -1121,7 +1121,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) {
11211121
},
11221122
},
11231123
{
1124-
name: "multiple node subnets, NAT Gateway not enabled in any of them",
1124+
name: "multiple node subnets, NAT gateway not enabled in any of them",
11251125
cluster: &AzureCluster{
11261126
ObjectMeta: metav1.ObjectMeta{
11271127
Name: "cluster-test",
@@ -1211,7 +1211,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) {
12111211
},
12121212
},
12131213
{
1214-
name: "multiple node subnets, NAT Gateway enabled on all of them",
1214+
name: "multiple node subnets, NAT gateway enabled on all of them",
12151215
cluster: &AzureCluster{
12161216
ObjectMeta: metav1.ObjectMeta{
12171217
Name: "cluster-test",

‎api/v1beta1/types.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,10 @@ type RouteTable struct {
160160
Name string `json:"name"`
161161
}
162162

163-
// NatGateway defines an Azure Nat Gateway.
163+
// NatGateway defines an Azure NAT gateway.
164164
// NAT gateway resources are part of Vnet NAT and provide outbound Internet connectivity for subnets of a virtual network.
165165
type NatGateway struct {
166-
// ID is the Azure resource ID of the nat gateway.
166+
// ID is the Azure resource ID of the NAT gateway.
167167
// READ-ONLY
168168
// +optional
169169
ID string `json:"id,omitempty"`
@@ -574,7 +574,7 @@ func (n *NetworkSpec) UpdateNodeSubnet(subnet SubnetSpec) {
574574
}
575575
}
576576

577-
// IsNatGatewayEnabled returns whether or not a Nat Gateway is enabled on the subnet.
577+
// IsNatGatewayEnabled returns whether or not a NAT gateway is enabled on the subnet.
578578
func (s SubnetSpec) IsNatGatewayEnabled() bool {
579579
return s.NatGateway.Name != ""
580580
}

‎azure/defaults.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ func GenerateFrontendIPConfigName(lbName string) string {
107107
return fmt.Sprintf("%s-%s", lbName, "frontEnd")
108108
}
109109

110-
// GenerateNatGatewayIPName generates a nat gateway IP name.
110+
// GenerateNatGatewayIPName generates a NAT gateway IP name.
111111
func GenerateNatGatewayIPName(clusterName, subnetName string) string {
112112
return fmt.Sprintf("pip-%s-%s-natgw", clusterName, subnetName)
113113
}
@@ -220,7 +220,7 @@ func SecurityGroupID(subscriptionID, resourceGroup, nsgName string) string {
220220
return fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Network/networkSecurityGroups/%s", subscriptionID, resourceGroup, nsgName)
221221
}
222222

223-
// NatGatewayID returns the azure resource ID for a given nat gateway.
223+
// NatGatewayID returns the azure resource ID for a given NAT gateway.
224224
func NatGatewayID(subscriptionID, resourceGroup, natgatewayName string) string {
225225
return fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Network/natGateways/%s", subscriptionID, resourceGroup, natgatewayName)
226226
}

‎azure/scope/cluster.go‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
3737
"sigs.k8s.io/cluster-api-provider-azure/azure"
3838
"sigs.k8s.io/cluster-api-provider-azure/azure/services/groups"
39+
"sigs.k8s.io/cluster-api-provider-azure/azure/services/natgateways"
3940
"sigs.k8s.io/cluster-api-provider-azure/azure/services/vnetpeerings"
4041
"sigs.k8s.io/cluster-api-provider-azure/util/futures"
4142
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
@@ -138,7 +139,7 @@ func (s *ClusterScope) PublicIPSpecs() []azure.PublicIPSpec {
138139
publicIPSpecs = append(publicIPSpecs, nodeOutboundIPSpecs...)
139140
}
140141

141-
// Public IP specs for node nat gateways
142+
// Public IP specs for node NAT gateways
142143
var nodeNatGatewayIPSpecs []azure.PublicIPSpec
143144
for _, subnet := range s.NodeSubnets() {
144145
if subnet.IsNatGatewayEnabled() {
@@ -220,20 +221,26 @@ func (s *ClusterScope) RouteTableSpecs() []azure.RouteTableSpec {
220221
return routetables
221222
}
222223

223-
// NatGatewaySpecs returns the node nat gateway.
224-
func (s *ClusterScope) NatGatewaySpecs() []azure.NatGatewaySpec {
225-
var natGateways []azure.NatGatewaySpec
224+
// NatGatewaySpecs returns the node NAT gateway.
225+
func (s *ClusterScope) NatGatewaySpecs() []azure.ResourceSpecGetter {
226+
natGatewaySet := make(map[string]struct{})
227+
var natGateways []azure.ResourceSpecGetter
226228

227-
// We ignore the control plane nat gateway, as we will always use a LB to enable egress on the control plane.
229+
// We ignore the control plane NAT gateway, as we will always use a LB to enable egress on the control plane.
228230
for _, subnet := range s.NodeSubnets() {
229231
if subnet.IsNatGatewayEnabled() {
230-
natGateways = append(natGateways, azure.NatGatewaySpec{
231-
Name: subnet.NatGateway.Name,
232-
NatGatewayIP: infrav1.PublicIPSpec{
233-
Name: subnet.NatGateway.NatGatewayIP.Name,
234-
},
235-
Subnet: subnet,
236-
})
232+
if _, ok := natGatewaySet[subnet.NatGateway.Name]; !ok {
233+
natGatewaySet[subnet.NatGateway.Name] = struct{}{} // empty struct to represent hash set
234+
natGateways = append(natGateways, &natgateways.NatGatewaySpec{
235+
Name: subnet.NatGateway.Name,
236+
ResourceGroup: s.ResourceGroup(),
237+
SubscriptionID: s.SubscriptionID(),
238+
Location: s.Location(),
239+
NatGatewayIP: infrav1.PublicIPSpec{
240+
Name: subnet.NatGateway.NatGatewayIP.Name,
241+
},
242+
})
243+
}
237244
}
238245
}
239246

@@ -446,6 +453,15 @@ func (s *ClusterScope) SetSubnet(subnetSpec infrav1.SubnetSpec) {
446453
}
447454
}
448455

456+
func (s *ClusterScope) SetNatGatewayIDInSubnets(name string, id string) {
457+
for _, subnet := range s.Subnets() {
458+
if subnet.NatGateway.Name == name {
459+
subnet.NatGateway.ID = id
460+
s.SetSubnet(subnet)
461+
}
462+
}
463+
}
464+
449465
// ControlPlaneRouteTable returns the cluster controlplane routetable.
450466
func (s *ClusterScope) ControlPlaneRouteTable() infrav1.RouteTable {
451467
subnet, _ := s.AzureCluster.Spec.NetworkSpec.GetControlPlaneSubnet()
@@ -597,6 +613,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error {
597613
infrav1.NetworkInfrastructureReadyCondition,
598614
infrav1.VnetPeeringReadyCondition,
599615
infrav1.DisksReadyCondition,
616+
infrav1.NATGatewaysReadyCondition,
600617
),
601618
)
602619

@@ -609,6 +626,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error {
609626
infrav1.NetworkInfrastructureReadyCondition,
610627
infrav1.VnetPeeringReadyCondition,
611628
infrav1.DisksReadyCondition,
629+
infrav1.NATGatewaysReadyCondition,
612630
}})
613631
}
614632

‎azure/scope/machine.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ func (m *MachineScope) NICSpecs() []azure.NICSpec {
231231
}
232232
}
233233

234-
// If Nat Gateway is not enabled and node has no public IP, then the NIC needs to reference the LB to get outbound traffic.
234+
// If NAT gateway is not enabled and node has no public IP, then the NIC needs to reference the LB to get outbound traffic.
235235
if m.Role() == infrav1.Node && !m.Subnet().IsNatGatewayEnabled() && !m.AzureMachine.Spec.AllocatePublicIP {
236236
spec.PublicLBName = m.OutboundLBName(m.Role())
237237
spec.PublicLBAddressPoolName = m.OutboundPoolName(m.OutboundLBName(m.Role()))

‎azure/scope/machine_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1306,7 +1306,7 @@ func TestMachineScope_NICSpecs(t *testing.T) {
13061306
want []azure.NICSpec
13071307
}{
13081308
{
1309-
name: "Node Machine with no nat gateway and no public IP address",
1309+
name: "Node Machine with no NAT gateway and no public IP address",
13101310
machineScope: MachineScope{
13111311
ClusterScoper: &ClusterScope{
13121312
Cluster: &clusterv1.Cluster{
@@ -1385,7 +1385,7 @@ func TestMachineScope_NICSpecs(t *testing.T) {
13851385
},
13861386
},
13871387
{
1388-
name: "Node Machine with nat gateway",
1388+
name: "Node Machine with NAT gateway",
13891389
machineScope: MachineScope{
13901390
ClusterScoper: &ClusterScope{
13911391
Cluster: &clusterv1.Cluster{

‎azure/scope/managedcontrolplane.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ func (s *ManagedControlPlaneScope) NodeRouteTable() infrav1.RouteTable {
230230
return infrav1.RouteTable{}
231231
}
232232

233-
// NodeNatGateway returns the cluster node nat gateway.
233+
// NodeNatGateway returns the cluster node NAT gateway.
234234
func (s *ManagedControlPlaneScope) NodeNatGateway() infrav1.NatGateway {
235235
return infrav1.NatGateway{}
236236
}

‎azure/services/disks/client.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func NewDisksClient(subscriptionID string, baseURI string, authorizer autorest.A
5151
// request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing
5252
// progress of the operation.
5353
func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) {
54-
ctx, _, done := tele.StartSpanWithLogger(ctx, "disks.Service.DeleteAsync")
54+
ctx, _, done := tele.StartSpanWithLogger(ctx, "disks.azureClient.DeleteAsync")
5555
defer done()
5656

5757
future, err := ac.disks.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName())
@@ -80,7 +80,7 @@ func (ac *azureClient) Result(ctx context.Context, futureData azureautorest.Futu
8080

8181
// IsDone returns true if the long-running operation has completed.
8282
func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) {
83-
ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.Service.IsDone")
83+
ctx, _, done := tele.StartSpanWithLogger(ctx, "disks.azureClient.IsDone")
8484
defer done()
8585

8686
isDone, err := future.DoneWithContext(ctx, ac.disks)

0 commit comments

Comments
 (0)