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

Make NAT Gateway reconcile/delete async #1865

Merged
merged 1 commit into from
Dec 15, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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() {
Jont828 marked this conversation as resolved.
Show resolved Hide resolved
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
2 changes: 1 addition & 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
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