Skip to content

Commit

Permalink
Make NAT gateway reconcile/delete async
Browse files Browse the repository at this point in the history
  • Loading branch information
Jont828 committed Dec 14, 2021
1 parent ff49c7a commit 32788e8
Show file tree
Hide file tree
Showing 22 changed files with 491 additions and 568 deletions.
2 changes: 1 addition & 1 deletion api/v1alpha4/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ type RouteTable struct {
}

// NatGateway defines an Azure Nat Gateway.
// NAT gateway resources are part of Vnet NAT and provide outbound Internet connectivity for subnets of a virtual network.
// Nat Gateway resources are part of Vnet NAT and provide outbound Internet connectivity for subnets of a virtual network.
type NatGateway struct {
ID string `json:"id,omitempty"`
Name string `json:"name,omitempty"`
Expand Down
4 changes: 2 additions & 2 deletions api/v1beta1/azurecluster_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func (c *AzureCluster) setNodeOutboundLBDefaults() {
}
}

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

// generateNatGatewayIPName generates a nat gateway IP name.
// generateNatGatewayIPName generates a NAT gateway IP name.
func generateNatGatewayIPName(clusterName, subnetName string) string {
return fmt.Sprintf("pip-%s-%s-natgw", clusterName, subnetName)
}
Expand Down
8 changes: 4 additions & 4 deletions api/v1beta1/azurecluster_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -978,7 +978,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) {
},
},
{
name: "NAT Gateway enabled - no LB",
name: "NAT gateway enabled - no LB",
cluster: &AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
Expand Down Expand Up @@ -1037,7 +1037,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) {
},
},
{
name: "NAT Gateway enabled on 1 of 2 node subnets",
name: "NAT gateway enabled on 1 of 2 node subnets",
cluster: &AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
Expand Down Expand Up @@ -1121,7 +1121,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) {
},
},
{
name: "multiple node subnets, NAT Gateway not enabled in any of them",
name: "multiple node subnets, NAT gateway not enabled in any of them",
cluster: &AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
Expand Down Expand Up @@ -1211,7 +1211,7 @@ func TestNodeOutboundLBDefaults(t *testing.T) {
},
},
{
name: "multiple node subnets, NAT Gateway enabled on all of them",
name: "multiple node subnets, NAT gateway enabled on all of them",
cluster: &AzureCluster{
ObjectMeta: metav1.ObjectMeta{
Name: "cluster-test",
Expand Down
6 changes: 3 additions & 3 deletions api/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,10 @@ type RouteTable struct {
Name string `json:"name"`
}

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

// IsNatGatewayEnabled returns whether or not a Nat Gateway is enabled on the subnet.
// IsNatGatewayEnabled returns whether or not a NAT gateway is enabled on the subnet.
func (s SubnetSpec) IsNatGatewayEnabled() bool {
return s.NatGateway.Name != ""
}
Expand Down
4 changes: 2 additions & 2 deletions azure/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func GenerateFrontendIPConfigName(lbName string) string {
return fmt.Sprintf("%s-%s", lbName, "frontEnd")
}

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

// NatGatewayID returns the azure resource ID for a given nat gateway.
// NatGatewayID returns the azure resource ID for a given NAT gateway.
func NatGatewayID(subscriptionID, resourceGroup, natgatewayName string) string {
return fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Network/natGateways/%s", subscriptionID, resourceGroup, natgatewayName)
}
Expand Down
42 changes: 30 additions & 12 deletions azure/scope/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
"sigs.k8s.io/cluster-api-provider-azure/azure"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/groups"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/natgateways"
"sigs.k8s.io/cluster-api-provider-azure/azure/services/vnetpeerings"
"sigs.k8s.io/cluster-api-provider-azure/util/futures"
"sigs.k8s.io/cluster-api-provider-azure/util/tele"
Expand Down Expand Up @@ -138,7 +139,7 @@ func (s *ClusterScope) PublicIPSpecs() []azure.PublicIPSpec {
publicIPSpecs = append(publicIPSpecs, nodeOutboundIPSpecs...)
}

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

// NatGatewaySpecs returns the node nat gateway.
func (s *ClusterScope) NatGatewaySpecs() []azure.NatGatewaySpec {
var natGateways []azure.NatGatewaySpec
// NatGatewaySpecs returns the node NAT gateway.
func (s *ClusterScope) NatGatewaySpecs() []azure.ResourceSpecGetter {
natGatewaySet := make(map[string]struct{})
var natGateways []azure.ResourceSpecGetter

// We ignore the control plane nat gateway, as we will always use a LB to enable egress on the control plane.
// We ignore the control plane NAT gateway, as we will always use a LB to enable egress on the control plane.
for _, subnet := range s.NodeSubnets() {
if subnet.IsNatGatewayEnabled() {
natGateways = append(natGateways, azure.NatGatewaySpec{
Name: subnet.NatGateway.Name,
NatGatewayIP: infrav1.PublicIPSpec{
Name: subnet.NatGateway.NatGatewayIP.Name,
},
Subnet: subnet,
})
if _, ok := natGatewaySet[subnet.NatGateway.Name]; !ok {
natGatewaySet[subnet.NatGateway.Name] = struct{}{} // empty struct to represent hash set
natGateways = append(natGateways, &natgateways.NatGatewaySpec{
Name: subnet.NatGateway.Name,
ResourceGroup: s.ResourceGroup(),
SubscriptionID: s.SubscriptionID(),
Location: s.Location(),
NatGatewayIP: infrav1.PublicIPSpec{
Name: subnet.NatGateway.NatGatewayIP.Name,
},
})
}
}
}

Expand Down Expand Up @@ -446,6 +453,15 @@ func (s *ClusterScope) SetSubnet(subnetSpec infrav1.SubnetSpec) {
}
}

func (s *ClusterScope) SetNatGatewayIDInSubnets(name string, id string) {
for _, subnet := range s.Subnets() {
if subnet.NatGateway.Name == name {
subnet.NatGateway.ID = id
s.SetSubnet(subnet)
}
}
}

// ControlPlaneRouteTable returns the cluster controlplane routetable.
func (s *ClusterScope) ControlPlaneRouteTable() infrav1.RouteTable {
subnet, _ := s.AzureCluster.Spec.NetworkSpec.GetControlPlaneSubnet()
Expand Down Expand Up @@ -597,6 +613,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error {
infrav1.NetworkInfrastructureReadyCondition,
infrav1.VnetPeeringReadyCondition,
infrav1.DisksReadyCondition,
infrav1.NATGatewaysReadyCondition,
),
)

Expand All @@ -609,6 +626,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error {
infrav1.NetworkInfrastructureReadyCondition,
infrav1.VnetPeeringReadyCondition,
infrav1.DisksReadyCondition,
infrav1.NATGatewaysReadyCondition,
}})
}

Expand Down
5 changes: 4 additions & 1 deletion azure/scope/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func (m *MachineScope) NICSpecs() []azure.NICSpec {
}
}

// If Nat Gateway is not enabled and node has no public IP, then the NIC needs to reference the LB to get outbound traffic.
// If 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()))
Expand Down Expand Up @@ -534,9 +534,11 @@ func (m *MachineScope) PatchObject(ctx context.Context) error {
conditions.SetSummary(m.AzureMachine,
conditions.WithConditions(
infrav1.VMRunningCondition,
infrav1.AvailabilitySetReadyCondition,
),
conditions.WithStepCounterIfOnly(
infrav1.VMRunningCondition,
infrav1.AvailabilitySetReadyCondition,
),
)

Expand All @@ -546,6 +548,7 @@ func (m *MachineScope) PatchObject(ctx context.Context) error {
patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
clusterv1.ReadyCondition,
infrav1.VMRunningCondition,
infrav1.AvailabilitySetReadyCondition,
}})
}

Expand Down
4 changes: 2 additions & 2 deletions azure/scope/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1306,7 +1306,7 @@ func TestMachineScope_NICSpecs(t *testing.T) {
want []azure.NICSpec
}{
{
name: "Node Machine with no nat gateway and no public IP address",
name: "Node Machine with no NAT gateway and no public IP address",
machineScope: MachineScope{
ClusterScoper: &ClusterScope{
Cluster: &clusterv1.Cluster{
Expand Down Expand Up @@ -1385,7 +1385,7 @@ func TestMachineScope_NICSpecs(t *testing.T) {
},
},
{
name: "Node Machine with nat gateway",
name: "Node Machine with NAT gateway",
machineScope: MachineScope{
ClusterScoper: &ClusterScope{
Cluster: &clusterv1.Cluster{
Expand Down
2 changes: 1 addition & 1 deletion azure/scope/managedcontrolplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func (s *ManagedControlPlaneScope) NodeRouteTable() infrav1.RouteTable {
return infrav1.RouteTable{}
}

// NodeNatGateway returns the cluster node nat gateway.
// NodeNatGateway returns the cluster node NAT gateway.
func (s *ManagedControlPlaneScope) NodeNatGateway() infrav1.NatGateway {
return infrav1.NatGateway{}
}
Expand Down
4 changes: 2 additions & 2 deletions azure/services/disks/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func NewDisksClient(subscriptionID string, baseURI string, authorizer autorest.A
// request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing
// progress of the operation.
func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) {
ctx, _, done := tele.StartSpanWithLogger(ctx, "disks.Service.DeleteAsync")
ctx, _, done := tele.StartSpanWithLogger(ctx, "disks.azureClient.DeleteAsync")
defer done()

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

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

isDone, err := future.DoneWithContext(ctx, ac.disks)
Expand Down
Loading

0 comments on commit 32788e8

Please sign in to comment.