From c9049092c30cc077c40d3db7eedb887d9b239f86 Mon Sep 17 00:00:00 2001 From: Jonathan Tong Date: Wed, 17 Nov 2021 13:02:00 -0500 Subject: [PATCH] Refactor spec and fix naming convention for NAT gateway --- api/v1alpha4/types.go | 4 +- api/v1beta1/azurecluster_default.go | 4 +- api/v1beta1/azurecluster_default_test.go | 8 ++-- api/v1beta1/types.go | 6 +-- azure/defaults.go | 4 +- azure/scope/cluster.go | 7 ++- azure/scope/machine.go | 2 +- azure/scope/machine_test.go | 4 +- azure/scope/managedcontrolplane.go | 2 +- azure/services/natgateways/client.go | 26 +++++------ azure/services/natgateways/natgateways.go | 46 +++++++++++-------- .../services/natgateways/natgateways_test.go | 26 +++++------ azure/services/natgateways/spec.go | 17 ++++--- azure/services/subnets/subnets_test.go | 2 +- controllers/azurecluster_reconciler.go | 4 +- docs/book/src/topics/node-outbound-lb.md | 10 ++-- 16 files changed, 90 insertions(+), 82 deletions(-) diff --git a/api/v1alpha4/types.go b/api/v1alpha4/types.go index 1ea9430e7d45..68f2090442c0 100644 --- a/api/v1alpha4/types.go +++ b/api/v1alpha4/types.go @@ -133,7 +133,7 @@ type RouteTable struct { Name string `json:"name,omitempty"` } -// 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 string `json:"id,omitempty"` @@ -545,7 +545,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 != "" } diff --git a/api/v1beta1/azurecluster_default.go b/api/v1beta1/azurecluster_default.go index f4374e3329da..6d4f8211a43b 100644 --- a/api/v1beta1/azurecluster_default.go +++ b/api/v1beta1/azurecluster_default.go @@ -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 { @@ -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) } diff --git a/api/v1beta1/azurecluster_default_test.go b/api/v1beta1/azurecluster_default_test.go index 4e5c03749e1d..6a30b28abc94 100644 --- a/api/v1beta1/azurecluster_default_test.go +++ b/api/v1beta1/azurecluster_default_test.go @@ -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", @@ -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", @@ -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", @@ -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", diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index fe04ad2c4666..eb952d06436f 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -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"` @@ -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 != "" } diff --git a/azure/defaults.go b/azure/defaults.go index 4a818f590d7f..ad23dce2a9e9 100644 --- a/azure/defaults.go +++ b/azure/defaults.go @@ -109,7 +109,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) } @@ -222,7 +222,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) } diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index 70e3ce45036c..176a4b4eb52d 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -148,7 +148,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() { @@ -230,11 +230,11 @@ func (s *ClusterScope) RouteTableSpecs() []azure.RouteTableSpec { return routetables } -// NatGatewaySpecs returns the node nat gateway. +// NatGatewaySpecs returns the node NAT gateway. func (s *ClusterScope) NatGatewaySpecs() []azure.ResourceSpecGetter { 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. // TODO: do we need to use setSubnet() in natgateways.go, since it doesn't add/remove any NodeSubnets() or change IsNatGatewayEnabled() for _, subnet := range s.NodeSubnets() { if subnet.IsNatGatewayEnabled() { @@ -246,7 +246,6 @@ func (s *ClusterScope) NatGatewaySpecs() []azure.ResourceSpecGetter { NatGatewayIP: infrav1.PublicIPSpec{ Name: subnet.NatGateway.NatGatewayIP.Name, }, - SubnetName: subnet.Name, }) } } diff --git a/azure/scope/machine.go b/azure/scope/machine.go index e2216f634998..ce9460ae9c81 100644 --- a/azure/scope/machine.go +++ b/azure/scope/machine.go @@ -229,7 +229,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())) diff --git a/azure/scope/machine_test.go b/azure/scope/machine_test.go index 5db946275c81..762b1280151f 100644 --- a/azure/scope/machine_test.go +++ b/azure/scope/machine_test.go @@ -1313,7 +1313,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{ @@ -1392,7 +1392,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{ diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index bbcb9bd90053..207a495dc513 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -228,7 +228,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{} } diff --git a/azure/services/natgateways/client.go b/azure/services/natgateways/client.go index 961de703eb12..1f246b2820c4 100644 --- a/azure/services/natgateways/client.go +++ b/azure/services/natgateways/client.go @@ -53,14 +53,14 @@ func newClient(auth azure.Authorizer) *azureClient { return &azureClient{c} } -// netNatGatewaysClient creates a new nat gateways client from subscription ID. +// netNatGatewaysClient creates a new NAT gateways client from subscription ID. func netNatGatewaysClient(subscriptionID string, baseURI string, authorizer autorest.Authorizer) network.NatGatewaysClient { natGatewaysClient := network.NewNatGatewaysClientWithBaseURI(baseURI, subscriptionID) azure.SetAutoRestClientDefaults(&natGatewaysClient.Client, authorizer) return natGatewaysClient } -// Get gets the specified nat gateway. +// Get gets the specified NAT gateway. func (ac *azureClient) Get(ctx context.Context, resourceGroupName, natGatewayName string) (network.NatGateway, error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.Get") defer done() @@ -68,24 +68,24 @@ func (ac *azureClient) Get(ctx context.Context, resourceGroupName, natGatewayNam return ac.natgateways.Get(ctx, resourceGroupName, natGatewayName, "") } -// CreateOrUpdateAsync creates or updates a Nat Gateway asynchronously. +// CreateOrUpdateAsync creates or updates a NAT gateway asynchronously. // It sends a PUT 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) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, azureautorest.FutureAPI, error) { - ctx, span := tele.Tracer().Start(ctx, "natgateways.azureClient.CreateOrUpdateAsync") - defer span.End() + ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.Service.CreateOrUpdateAsync") + defer done() var existingNatGateway interface{} if existing, err := ac.Get(ctx, spec.ResourceGroupName(), spec.ResourceName()); err != nil && !azure.ResourceNotFound(err) { - return nil, nil, errors.Wrapf(err, "failed to get Nat Gateway %s for %s in %s", spec.ResourceName(), spec.OwnerResourceName(), spec.ResourceGroupName()) + return nil, nil, errors.Wrapf(err, "failed to get NAT gateway %s for %s in %s", spec.ResourceName(), spec.OwnerResourceName(), spec.ResourceGroupName()) } else if err == nil { existingNatGateway = existing } params, err := spec.Parameters(existingNatGateway) if err != nil { - return nil, nil, errors.Wrapf(err, "failed to get desired parameters for Nat Gateway %s", spec.ResourceName()) + return nil, nil, errors.Wrapf(err, "failed to get desired parameters for NAT gateway %s", spec.ResourceName()) } natGateway, ok := params.(network.NatGateway) @@ -118,12 +118,12 @@ func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.Resou return result, nil, err } -// DeleteAsync deletes a Nat Gateway asynchronously. DeleteAsync sends a DELETE +// DeleteAsync deletes a NAT gateway asynchronously. DeleteAsync sends a DELETE // 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, span := tele.Tracer().Start(ctx, "natgateways.azureClient.Delete") - defer span.End() + ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.Service.DeleteAsync") + defer done() future, err := ac.natgateways.Delete(ctx, spec.ResourceGroupName(), spec.ResourceName()) if err != nil { @@ -146,8 +146,8 @@ func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecG // IsDone returns true if the long-running operation has completed. func (ac *azureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) { - ctx, span := tele.Tracer().Start(ctx, "natgateways.azureClient.IsDone") - defer span.End() + ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.Service.DeleteAsync") + defer done() done, err := future.DoneWithContext(ctx, ac.natgateways) if err != nil { @@ -177,7 +177,7 @@ func (ac *azureClient) Result(ctx context.Context, futureData azureautorest.Futu result = (*future).Result case infrav1.DeleteFuture: - // Delete does not return a result Nat Gateway + // Delete does not return a result NAT gateway return nil, nil default: diff --git a/azure/services/natgateways/natgateways.go b/azure/services/natgateways/natgateways.go index ea65e712edd6..492758571913 100644 --- a/azure/services/natgateways/natgateways.go +++ b/azure/services/natgateways/natgateways.go @@ -32,7 +32,7 @@ import ( const serviceName = "natgateways" -// NatGatewayScope defines the scope interface for nat gateway service. +// NatGatewayScope defines the scope interface for NAT gateway service. type NatGatewayScope interface { logr.Logger azure.ClusterScoper @@ -54,33 +54,33 @@ func New(scope NatGatewayScope) *Service { } } -// Reconcile gets/creates/updates a nat gateway. -// Only when the Nat Gateway 'Name' property is defined we create the Nat Gateway: it's opt-in. +// Reconcile gets/creates/updates a NAT gateway. +// Only when the NAT gateway 'Name' property is defined we create the NAT gateway: it's opt-in. func (s *Service) Reconcile(ctx context.Context) error { - ctx, span := tele.Tracer().Start(ctx, "natgateways.Service.Reconcile") - defer span.End() + ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.Service.Reconcile") + defer done() ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) defer cancel() - // We go through the list of NatGatewaySpecs to reconcile each one, independently of the result of the previous one. + // We go through the list of NatGatewaySpecs to reconcile each one, independently of the resultingErr of the previous one. // If multiple errors occur, we return the most pressing one // order of precedence is: error creating -> creating in progress -> created (no error) - var result error + var resultingErr error for _, natGatewaySpec := range s.Scope.NatGatewaySpecs() { natGateway, err := async.CreateResource(ctx, s.Scope, s.client, natGatewaySpec, serviceName) if err != nil { - if !azure.IsOperationNotDoneError(err) || result == nil { - result = err + if !azure.IsOperationNotDoneError(err) || resultingErr == nil { + resultingErr = err } } - if err == nil && result != nil { + if err == nil && resultingErr != nil { // TODO: consider making OwnerResourceName() a no-op subnetSpec := s.Scope.Subnet(natGatewaySpec.OwnerResourceName()) networkNatGateway, ok := natGateway.(network.NatGateway) if !ok { - // TODO: maybe this error shouldn't take priority - result = errors.Errorf("created resource %T is not a network.NatGateway", natGateway) + // Return out of loop since this would be an unexpcted fatal error + return errors.Errorf("created resource %T is not a network.NatGateway", natGateway) } // TODO: is it necessary to set the spec since it doesn't appear to change any behavior subnetSpec.NatGateway.ID = *networkNatGateway.ID @@ -88,14 +88,14 @@ func (s *Service) Reconcile(ctx context.Context) error { } } - s.Scope.UpdatePutStatus(infrav1.NATGatewaysReadyCondition, serviceName, result) - return result + s.Scope.UpdatePutStatus(infrav1.NATGatewaysReadyCondition, serviceName, resultingErr) + return resultingErr } -// Delete deletes the nat gateway with the provided name. +// Delete deletes the NAT gateway with the provided name. func (s *Service) Delete(ctx context.Context) error { - ctx, span := tele.Tracer().Start(ctx, "natgateways.Service.Delete") - defer span.End() + ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.Service.Delete") + defer done() ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) defer cancel() @@ -103,7 +103,7 @@ func (s *Service) Delete(ctx context.Context) error { var result error // We go through the list of NatGatewaySpecs to delete each one, independently of the result of the previous one. - // If multiple erros occur, we return the most pressing one + // If multiple errors occur, we return the most pressing one // order of precedence is: error deleting -> deleting in progress -> deleted (no error) for _, natGatewaySpec := range s.Scope.NatGatewaySpecs() { if err := async.DeleteResource(ctx, s.Scope, s.client, natGatewaySpec, serviceName); err != nil { @@ -115,3 +115,13 @@ func (s *Service) Delete(ctx context.Context) error { s.Scope.UpdateDeleteStatus(infrav1.NATGatewaysReadyCondition, serviceName, result) return result } + +func (s *Service) SetNatGatewayID(name string, id string) { + for _, subnet := range s.Scope.Subnets() { + if subnet.NatGateway.Name == name { + subnet.NatGateway.ID = id + s.Scope.SetSubnet(subnet) + // s.AzureCluster.Spec.NetworkSpec.Subnets[i] = id + } + } +} diff --git a/azure/services/natgateways/natgateways_test.go b/azure/services/natgateways/natgateways_test.go index 64a67d54a528..f83e87320c6a 100644 --- a/azure/services/natgateways/natgateways_test.go +++ b/azure/services/natgateways/natgateways_test.go @@ -48,7 +48,7 @@ func TestReconcileNatGateways(t *testing.T) { expect func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, m *mock_natgateways.MockclientMockRecorder) }{ { - name: "nat gateways in custom vnet mode", + name: "NAT gateways in custom vnet mode", tags: infrav1.Tags{ "Name": "my-vnet", "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "shared", @@ -65,7 +65,7 @@ func TestReconcileNatGateways(t *testing.T) { }, }, { - name: "nat gateway create successfully", + name: "NAT gateway create successfully", tags: infrav1.Tags{ "Name": "my-vnet", "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", @@ -108,7 +108,7 @@ func TestReconcileNatGateways(t *testing.T) { }, }, { - name: "update nat gateway if actual state does not match desired state", + name: "update NAT gateway if actual state does not match desired state", tags: infrav1.Tags{ "Name": "my-vnet", "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", @@ -159,7 +159,7 @@ func TestReconcileNatGateways(t *testing.T) { }, }, { - name: "nat gateway is not updated if it's up to date", + name: "NAT gateway is not updated if it's up to date", tags: infrav1.Tags{ "Name": "my-vnet", "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", @@ -212,13 +212,13 @@ func TestReconcileNatGateways(t *testing.T) { }, }, { - name: "fail when getting existing nat gateway", + name: "fail when getting existing NAT gateway", tags: infrav1.Tags{ "Name": "my-vnet", "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", "sigs.k8s.io_cluster-api-provider-azure_role": "common", }, - expectedError: "failed to get nat gateway my-node-natgateway in my-rg: #: Internal Server Error: StatusCode=500", + expectedError: "failed to get NAT gateway my-node-natgateway in my-rg: #: Internal Server Error: StatusCode=500", expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, m *mock_natgateways.MockclientMockRecorder) { s.Vnet().Return(&infrav1.VnetSpec{ Name: "my-vnet", @@ -240,13 +240,13 @@ func TestReconcileNatGateways(t *testing.T) { }, }, { - name: "fail to create a nat gateway", + name: "fail to create a NAT gateway", tags: infrav1.Tags{ "Name": "my-vnet", "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", "sigs.k8s.io_cluster-api-provider-azure_role": "common", }, - expectedError: "failed to create nat gateway my-node-natgateway in resource group my-rg: #: Internal Server Error: StatusCode=500", + expectedError: "failed to create NAT gateway my-node-natgateway in resource group my-rg: #: Internal Server Error: StatusCode=500", expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, m *mock_natgateways.MockclientMockRecorder) { s.Vnet().Return(&infrav1.VnetSpec{ Name: "my-vnet", @@ -307,7 +307,7 @@ func TestDeleteNatGateway(t *testing.T) { expect func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, m *mock_natgateways.MockclientMockRecorder) }{ { - name: "nat gateways in custom vnet mode", + name: "NAT gateways in custom vnet mode", tags: infrav1.Tags{ "Name": "my-vnet", "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "shared", @@ -324,7 +324,7 @@ func TestDeleteNatGateway(t *testing.T) { }, }, { - name: "nat gateway deleted successfully", + name: "NAT gateway deleted successfully", tags: infrav1.Tags{ "Name": "my-vnet", "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", @@ -351,7 +351,7 @@ func TestDeleteNatGateway(t *testing.T) { }, }, { - name: "nat gateway already deleted", + name: "NAT gateway already deleted", tags: infrav1.Tags{ "Name": "my-vnet", "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", @@ -380,13 +380,13 @@ func TestDeleteNatGateway(t *testing.T) { }, }, { - name: "nat gateway deletion fails", + name: "NAT gateway deletion fails", tags: infrav1.Tags{ "Name": "my-vnet", "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "owned", "sigs.k8s.io_cluster-api-provider-azure_role": "common", }, - expectedError: "failed to delete nat gateway my-node-natgateway in resource group my-rg: #: Internal Server Error: StatusCode=500", + expectedError: "failed to delete NAT gateway my-node-natgateway in resource group my-rg: #: Internal Server Error: StatusCode=500", expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, m *mock_natgateways.MockclientMockRecorder) { s.Vnet().Return(&infrav1.VnetSpec{ Name: "my-vnet", diff --git a/azure/services/natgateways/spec.go b/azure/services/natgateways/spec.go index 1733c8d7454b..c7c97618536a 100644 --- a/azure/services/natgateways/spec.go +++ b/azure/services/natgateways/spec.go @@ -27,17 +27,16 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/azure" ) -// NatGatewaySpec defines the specification for a Nat Gateway. +// NatGatewaySpec defines the specification for a NAT gateway. type NatGatewaySpec struct { Name string ResourceGroup string SubscriptionID string Location string - SubnetName string NatGatewayIP infrav1.PublicIPSpec } -// ResourceName returns the name of the Nat Gateway. +// ResourceName returns the name of the NAT gateway. func (s *NatGatewaySpec) ResourceName() string { return s.Name } @@ -47,25 +46,25 @@ func (s *NatGatewaySpec) ResourceGroupName() string { return s.ResourceGroup } -// OwnerResourceName returns the name of the associated subnet. +// OwnerResourceName is a no-op for NAT gateways func (s *NatGatewaySpec) OwnerResourceName() string { - return s.SubnetName + return "" } -// Parameters returns the parameters for the Nat Gateway. +// Parameters returns the parameters for the NAT gateway. func (s *NatGatewaySpec) Parameters(existing interface{}) (interface{}, error) { if existing != nil { existingNatGateway, ok := existing.(network.NatGateway) if !ok { return nil, errors.Errorf("%T is not a network.NatGateway", existing) } - // Nat Gateway already exists, parse existing spec + // NAT gateway already exists, parse existing spec publicIP, err := GetPublicIP(existingNatGateway) if err != nil { return nil, err } if s.NatGatewayIP.Name == publicIP { - // Skip update for Nat Gateway as it exists with expected values + // Skip update for NAT gateway as it exists with expected values return nil, nil } } @@ -90,7 +89,7 @@ func GetPublicIP(natGateway network.NatGateway) (string, error) { if !(natGateway.PublicIPAddresses != nil && len(*natGateway.PublicIPAddresses) > 0) { return "", errors.New("failed to parse PublicIPAddresses") } - // TODO: do we need to handle NatGateway resources w/ more than one public IP address? + // TODO: do we need to handle NAT gateway resources w/ more than one public IP address? publicIPAddressID := to.String((*natGateway.PublicIPAddresses)[0].ID) resource, err := autorest.ParseResourceID(publicIPAddressID) if err != nil { diff --git a/azure/services/subnets/subnets_test.go b/azure/services/subnets/subnets_test.go index 427bda0c66be..b09b73ac1d48 100644 --- a/azure/services/subnets/subnets_test.go +++ b/azure/services/subnets/subnets_test.go @@ -363,7 +363,7 @@ func TestReconcileSubnets(t *testing.T) { }, }, { - name: "doesn't overwrite existing NAT Gateway", + name: "doesn't overwrite existing NAT gateway", expectedError: "", expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { s.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) diff --git a/controllers/azurecluster_reconciler.go b/controllers/azurecluster_reconciler.go index c4445db3a447..9dc297716c2c 100644 --- a/controllers/azurecluster_reconciler.go +++ b/controllers/azurecluster_reconciler.go @@ -118,7 +118,7 @@ func (s *azureClusterService) Reconcile(ctx context.Context) error { } if err := s.natGatewaySvc.Reconcile(ctx); err != nil { - return errors.Wrapf(err, "failed to reconcile nat gateway") + return errors.Wrapf(err, "failed to reconcile NAT gateway") } if err := s.subnetsSvc.Reconcile(ctx); err != nil { @@ -176,7 +176,7 @@ func (s *azureClusterService) Delete(ctx context.Context) error { } if err := s.natGatewaySvc.Delete(ctx); err != nil { - return errors.Wrapf(err, "failed to delete nat gateway") + return errors.Wrapf(err, "failed to delete NAT gateway") } if err := s.publicIPSvc.Delete(ctx); err != nil { diff --git a/docs/book/src/topics/node-outbound-lb.md b/docs/book/src/topics/node-outbound-lb.md index 8e80f3cceeae..082671c893b8 100644 --- a/docs/book/src/topics/node-outbound-lb.md +++ b/docs/book/src/topics/node-outbound-lb.md @@ -69,10 +69,10 @@ spec: frontendIPsCount: 1 ``` -## Node Outbound Nat Gateway +## Node Outbound NAT gateway -You can configure a [Nat Gateway](https://docs.microsoft.com/en-us/azure/virtual-network/nat-gateway-resource) in a subnet to enable outbound traffic in the cluster nodes by setting the Nat Gateway's name in the subnet configuration. -A Public IP will also be created for the Nat Gateway. +You can configure a [NAT gateway](https://docs.microsoft.com/en-us/azure/virtual-network/nat-gateway-resource) in a subnet to enable outbound traffic in the cluster nodes by setting the NAT gateway's name in the subnet configuration. +A Public IP will also be created for the NAT gateway. Using this configuration, [a Load Balancer for the nodes outbound traffic](./node-outbound-lb.md) won't be created. @@ -80,7 +80,7 @@ Using this configuration, [a Load Balancer for the nodes outbound traffic](./nod

Warning

-CAPZ will ignore the Nat Gateway configuration in the control plane subnet because we always create a load balancer for the control plane, which we use for outbound traffic. +CAPZ will ignore the NAT gateway configuration in the control plane subnet because we always create a load balancer for the control plane, which we use for outbound traffic. @@ -107,5 +107,5 @@ spec: resourceGroup: cluster-natgw ``` -You can also define the Public IP name that should be used when creating the Public IP for the Nat Gateway. +You can also define the Public IP name that should be used when creating the Public IP for the NAT gateway. If you don't specify it, CAPZ will automatically generate a name for it.