From 53da7020e15239273aacbaf15c12db82fbe74ff7 Mon Sep 17 00:00:00 2001 From: Nawaz Hussain Khazielakha Date: Sat, 30 Sep 2023 00:27:19 -0700 Subject: [PATCH] update natgateways to use aso framework --- Makefile | 2 +- azure/scope/cluster.go | 8 +- azure/scope/cluster_test.go | 88 ++- azure/services/natgateways/client.go | 122 ----- .../natgateways/mock_natgateways/doc.go | 2 +- .../mock_natgateways/natgateways_mock.go | 20 +- azure/services/natgateways/natgateways.go | 129 +---- .../services/natgateways/natgateways_test.go | 219 +------- azure/services/natgateways/spec.go | 98 ++-- azure/services/natgateways/spec_test.go | 171 +++--- config/aso/crds.yaml | 517 ++++++++++++++++++ config/rbac/role.yaml | 20 + controllers/azurecluster_controller.go | 2 + controllers/azurecluster_reconciler.go | 6 +- main.go | 2 + 15 files changed, 828 insertions(+), 578 deletions(-) delete mode 100644 azure/services/natgateways/client.go diff --git a/Makefile b/Makefile index 4e0e30dc0da..8f3f81f51b4 100644 --- a/Makefile +++ b/Makefile @@ -158,7 +158,7 @@ WEBHOOK_ROOT ?= $(MANIFEST_ROOT)/webhook RBAC_ROOT ?= $(MANIFEST_ROOT)/rbac ASO_CRDS_PATH := $(MANIFEST_ROOT)/aso/crds.yaml ASO_VERSION := v2.3.0 -ASO_CRDS := resourcegroups.resources.azure.com +ASO_CRDS := resourcegroups.resources.azure.com natgateways.network.azure.com # Allow overriding the imagePullPolicy PULL_POLICY ?= Always diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index 13d895cce3a..d03f73e0b18 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -25,6 +25,7 @@ import ( "strconv" "strings" + asonetworkv1 "github.com/Azure/azure-service-operator/v2/api/network/v1api20220701" asoresourcesv1 "github.com/Azure/azure-service-operator/v2/api/resources/v1api20200601" "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -320,9 +321,9 @@ func (s *ClusterScope) RouteTableSpecs() []azure.ResourceSpecGetter { } // NatGatewaySpecs returns the node NAT gateway. -func (s *ClusterScope) NatGatewaySpecs() []azure.ResourceSpecGetter { +func (s *ClusterScope) NatGatewaySpecs() []azure.ASOResourceSpecGetter[*asonetworkv1.NatGateway] { natGatewaySet := make(map[string]struct{}) - var natGateways []azure.ResourceSpecGetter + var natGateways []azure.ASOResourceSpecGetter[*asonetworkv1.NatGateway] // 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() { @@ -331,6 +332,7 @@ func (s *ClusterScope) NatGatewaySpecs() []azure.ResourceSpecGetter { natGatewaySet[subnet.NatGateway.Name] = struct{}{} // empty struct to represent hash set natGateways = append(natGateways, &natgateways.NatGatewaySpec{ Name: subnet.NatGateway.Name, + Namespace: s.Namespace(), ResourceGroup: s.ResourceGroup(), SubscriptionID: s.SubscriptionID(), Location: s.Location(), @@ -339,6 +341,8 @@ func (s *ClusterScope) NatGatewaySpecs() []azure.ResourceSpecGetter { Name: subnet.NatGateway.NatGatewayIP.Name, }, AdditionalTags: s.AdditionalTags(), + // We need to know if the VNet is managed to decide if this NAT Gateway was-managed or not. + IsVnetManaged: s.IsVnetManaged(), }) } } diff --git a/azure/scope/cluster_test.go b/azure/scope/cluster_test.go index b726a93f1b2..195ecd1d2f4 100644 --- a/azure/scope/cluster_test.go +++ b/azure/scope/cluster_test.go @@ -23,6 +23,7 @@ import ( "strings" "testing" + asonetworkv1 "github.com/Azure/azure-service-operator/v2/api/network/v1api20220701" "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/azure/auth" "github.com/google/go-cmp/cmp" @@ -44,7 +45,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -func specToString(spec azure.ResourceSpecGetter) string { +func specToString(spec any) string { var sb strings.Builder sb.WriteString("{ ") sb.WriteString(fmt.Sprintf("%+v ", spec)) @@ -52,7 +53,7 @@ func specToString(spec azure.ResourceSpecGetter) string { return sb.String() } -func specArrayToString(specs []azure.ResourceSpecGetter) string { +func specArrayToString[T any](specs []T) string { var sb strings.Builder sb.WriteString("[\n") for _, spec := range specs { @@ -867,7 +868,7 @@ func TestNatGatewaySpecs(t *testing.T) { tests := []struct { name string clusterScope ClusterScope - want []azure.ResourceSpecGetter + want []azure.ASOResourceSpecGetter[*asonetworkv1.NatGateway] }{ { name: "returns nil if no subnets are specified", @@ -929,7 +930,7 @@ func TestNatGatewaySpecs(t *testing.T) { }, cache: &ClusterCache{}, }, - want: []azure.ResourceSpecGetter{ + want: []azure.ASOResourceSpecGetter[*asonetworkv1.NatGateway]{ &natgateways.NatGatewaySpec{ Name: "fake-nat-gateway-1", ResourceGroup: "my-rg", @@ -940,6 +941,7 @@ func TestNatGatewaySpecs(t *testing.T) { Name: "44.78.67.90", }, AdditionalTags: make(infrav1.Tags), + IsVnetManaged: true, }, }, }, @@ -1007,7 +1009,7 @@ func TestNatGatewaySpecs(t *testing.T) { }, cache: &ClusterCache{}, }, - want: []azure.ResourceSpecGetter{ + want: []azure.ASOResourceSpecGetter[*asonetworkv1.NatGateway]{ &natgateways.NatGatewaySpec{ Name: "fake-nat-gateway-1", ResourceGroup: "my-rg", @@ -1018,6 +1020,7 @@ func TestNatGatewaySpecs(t *testing.T) { Name: "44.78.67.90", }, AdditionalTags: make(infrav1.Tags), + IsVnetManaged: true, }, }, }, @@ -1084,7 +1087,7 @@ func TestNatGatewaySpecs(t *testing.T) { }, cache: &ClusterCache{}, }, - want: []azure.ResourceSpecGetter{ + want: []azure.ASOResourceSpecGetter[*asonetworkv1.NatGateway]{ &natgateways.NatGatewaySpec{ Name: "fake-nat-gateway-1", ResourceGroup: "my-rg", @@ -1095,6 +1098,7 @@ func TestNatGatewaySpecs(t *testing.T) { Name: "44.78.67.90", }, AdditionalTags: make(infrav1.Tags), + IsVnetManaged: true, }, }, }, @@ -1111,6 +1115,78 @@ func TestNatGatewaySpecs(t *testing.T) { } } +func TestSetNatGatewayIDInSubnets(t *testing.T) { + tests := []struct { + name string + clusterScope ClusterScope + asoNatgateway *asonetworkv1.NatGateway + }{ + { + name: "sets nat gateway id in the matching subnet", + clusterScope: ClusterScope{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-cluster", + }, + }, + AzureCluster: &infrav1.AzureCluster{ + Spec: infrav1.AzureClusterSpec{ + NetworkSpec: infrav1.NetworkSpec{ + Subnets: infrav1.Subnets{ + { + SubnetClassSpec: infrav1.SubnetClassSpec{ + Name: "fake-subnet-1", + }, + NatGateway: infrav1.NatGateway{ + NatGatewayClassSpec: infrav1.NatGatewayClassSpec{ + Name: "fake-nat-gateway-1", + }, + }, + }, + { + SubnetClassSpec: infrav1.SubnetClassSpec{ + Name: "fake-subnet-2", + }, + NatGateway: infrav1.NatGateway{ + NatGatewayClassSpec: infrav1.NatGatewayClassSpec{ + Name: "fake-nat-gateway-2", + }, + }, + }, + }, + }, + }, + }, + cache: &ClusterCache{}, + }, + asoNatgateway: &asonetworkv1.NatGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "fake-nat-gateway-1", + }, + Status: asonetworkv1.NatGateway_STATUS{ + Id: ptr.To("dummy-id-1"), + }, + }, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + t.Parallel() + tt.clusterScope.SetNatGatewayIDInSubnets(tt.asoNatgateway.Name, *tt.asoNatgateway.Status.Id) + for _, subnet := range tt.clusterScope.AzureCluster.Spec.NetworkSpec.Subnets { + if subnet.NatGateway.Name == tt.asoNatgateway.Name { + g.Expect(subnet.NatGateway.ID).To(Equal(*tt.asoNatgateway.Status.Id)) + } else { + g.Expect(subnet.NatGateway.ID).To(Equal("")) + } + } + }) + } +} + func TestNSGSpecs(t *testing.T) { tests := []struct { name string diff --git a/azure/services/natgateways/client.go b/azure/services/natgateways/client.go deleted file mode 100644 index 3f3599b0557..00000000000 --- a/azure/services/natgateways/client.go +++ /dev/null @@ -1,122 +0,0 @@ -/* -Copyright 2021 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package natgateways - -import ( - "context" - - "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4" - "github.com/pkg/errors" - "sigs.k8s.io/cluster-api-provider-azure/azure" - "sigs.k8s.io/cluster-api-provider-azure/azure/services/async" - "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" - "sigs.k8s.io/cluster-api-provider-azure/util/tele" -) - -// azureClient contains the Azure go-sdk Client. -type azureClient struct { - natgateways *armnetwork.NatGatewaysClient -} - -// newClient creates a new nat gateways client from an authorizer. -func newClient(auth azure.Authorizer) (*azureClient, error) { - opts, err := azure.ARMClientOptions(auth.CloudEnvironment()) - if err != nil { - return nil, errors.Wrap(err, "failed to create natgateways client options") - } - factory, err := armnetwork.NewClientFactory(auth.SubscriptionID(), auth.Token(), opts) - if err != nil { - return nil, errors.Wrap(err, "failed to create armnetwork client factory") - } - return &azureClient{factory.NewNatGatewaysClient()}, nil -} - -// Get gets the specified nat gateway. -func (ac *azureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) { - ctx, _, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.Get") - defer done() - - resp, err := ac.natgateways.Get(ctx, spec.ResourceGroupName(), spec.ResourceName(), nil) - if err != nil { - return nil, err - } - return resp.NatGateway, nil -} - -// 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 Poller which can be used to track the ongoing -// progress of the operation. -func (ac *azureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter, resumeToken string, parameters interface{}) (result interface{}, poller *runtime.Poller[armnetwork.NatGatewaysClientCreateOrUpdateResponse], err error) { - ctx, log, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.CreateOrUpdateAsync") - defer done() - - natGateway, ok := parameters.(armnetwork.NatGateway) - if !ok && parameters != nil { - return nil, nil, errors.Errorf("%T is not an armnetwork.NatGateway", parameters) - } - - opts := &armnetwork.NatGatewaysClientBeginCreateOrUpdateOptions{ResumeToken: resumeToken} - log.V(4).Info("sending request", "resumeToken", resumeToken) - poller, err = ac.natgateways.BeginCreateOrUpdate(ctx, spec.ResourceGroupName(), spec.ResourceName(), natGateway, opts) - if err != nil { - return nil, nil, err - } - - ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) - defer cancel() - - pollOpts := &runtime.PollUntilDoneOptions{Frequency: async.DefaultPollerFrequency} - resp, err := poller.PollUntilDone(ctx, pollOpts) - if err != nil { - // if an error occurs, return the poller. - // this means the long-running operation didn't finish in the specified timeout. - return nil, poller, err - } - - // if the operation completed, return a nil poller - return resp.NatGateway, nil, err -} - -// DeleteAsync deletes a Nat Gateway asynchronously. DeleteAsync sends a DELETE -// request to Azure and if accepted without error, the func will return a Poller which can be used to track the ongoing -// progress of the operation. -func (ac *azureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter, resumeToken string) (poller *runtime.Poller[armnetwork.NatGatewaysClientDeleteResponse], err error) { - ctx, log, done := tele.StartSpanWithLogger(ctx, "natgateways.azureClient.DeleteAsync") - defer done() - - opts := &armnetwork.NatGatewaysClientBeginDeleteOptions{ResumeToken: resumeToken} - log.V(4).Info("sending request", "resumeToken", resumeToken) - poller, err = ac.natgateways.BeginDelete(ctx, spec.ResourceGroupName(), spec.ResourceName(), opts) - if err != nil { - return nil, err - } - - ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) - defer cancel() - - pollOpts := &runtime.PollUntilDoneOptions{Frequency: async.DefaultPollerFrequency} - _, err = poller.PollUntilDone(ctx, pollOpts) - if err != nil { - // if an error occurs, return the Poller. - // this means the long-running operation didn't finish in the specified timeout. - return poller, err - } - - // if the operation completed, return a nil poller. - return nil, err -} diff --git a/azure/services/natgateways/mock_natgateways/doc.go b/azure/services/natgateways/mock_natgateways/doc.go index 96bc5cd0e8c..ae39c69693b 100644 --- a/azure/services/natgateways/mock_natgateways/doc.go +++ b/azure/services/natgateways/mock_natgateways/doc.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Kubernetes Authors. +Copyright 2023 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. diff --git a/azure/services/natgateways/mock_natgateways/natgateways_mock.go b/azure/services/natgateways/mock_natgateways/natgateways_mock.go index 02f126f2ec0..75b9ae1cdef 100644 --- a/azure/services/natgateways/mock_natgateways/natgateways_mock.go +++ b/azure/services/natgateways/mock_natgateways/natgateways_mock.go @@ -28,10 +28,12 @@ import ( reflect "reflect" azcore "github.com/Azure/azure-sdk-for-go/sdk/azcore" + v1api20220701 "github.com/Azure/azure-service-operator/v2/api/network/v1api20220701" gomock "go.uber.org/mock/gomock" v1beta1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" azure "sigs.k8s.io/cluster-api-provider-azure/azure" v1beta10 "sigs.k8s.io/cluster-api/api/v1beta1" + client "sigs.k8s.io/controller-runtime/pkg/client" ) // MockNatGatewayScope is a mock of NatGatewayScope interface. @@ -307,6 +309,20 @@ func (mr *MockNatGatewayScopeMockRecorder) FailureDomains() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FailureDomains", reflect.TypeOf((*MockNatGatewayScope)(nil).FailureDomains)) } +// GetClient mocks base method. +func (m *MockNatGatewayScope) GetClient() client.Client { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetClient") + ret0, _ := ret[0].(client.Client) + return ret0 +} + +// GetClient indicates an expected call of GetClient. +func (mr *MockNatGatewayScopeMockRecorder) GetClient() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetClient", reflect.TypeOf((*MockNatGatewayScope)(nil).GetClient)) +} + // GetLongRunningOperationState mocks base method. func (m *MockNatGatewayScope) GetLongRunningOperationState(arg0, arg1, arg2 string) *v1beta1.Future { m.ctrl.T.Helper() @@ -406,10 +422,10 @@ func (mr *MockNatGatewayScopeMockRecorder) Location() *gomock.Call { } // NatGatewaySpecs mocks base method. -func (m *MockNatGatewayScope) NatGatewaySpecs() []azure.ResourceSpecGetter { +func (m *MockNatGatewayScope) NatGatewaySpecs() []azure.ASOResourceSpecGetter[*v1api20220701.NatGateway] { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "NatGatewaySpecs") - ret0, _ := ret[0].([]azure.ResourceSpecGetter) + ret0, _ := ret[0].([]azure.ASOResourceSpecGetter[*v1api20220701.NatGateway]) return ret0 } diff --git a/azure/services/natgateways/natgateways.go b/azure/services/natgateways/natgateways.go index ea97fd83328..d3b78ec3521 100644 --- a/azure/services/natgateways/natgateways.go +++ b/azure/services/natgateways/natgateways.go @@ -17,15 +17,10 @@ limitations under the License. package natgateways import ( - "context" - - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4" - "github.com/pkg/errors" + asonetworkv1 "github.com/Azure/azure-service-operator/v2/api/network/v1api20220701" 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/async" - "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" - "sigs.k8s.io/cluster-api-provider-azure/util/tele" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/aso" ) const serviceName = "natgateways" @@ -33,123 +28,33 @@ const serviceName = "natgateways" // NatGatewayScope defines the scope interface for NAT gateway service. type NatGatewayScope interface { azure.ClusterScoper - azure.AsyncStatusUpdater + aso.Scope SetNatGatewayIDInSubnets(natGatewayName string, natGatewayID string) - NatGatewaySpecs() []azure.ResourceSpecGetter + NatGatewaySpecs() []azure.ASOResourceSpecGetter[*asonetworkv1.NatGateway] } // Service provides operations on azure resources. type Service struct { Scope NatGatewayScope - async.Reconciler + *aso.Service[*asonetworkv1.NatGateway, NatGatewayScope] } // New creates a new service. -func New(scope NatGatewayScope) (*Service, error) { - client, err := newClient(scope) - if err != nil { - return nil, err - } +func New(scope NatGatewayScope) *Service { + svc := aso.NewService[*asonetworkv1.NatGateway, NatGatewayScope](serviceName, scope) + svc.Specs = scope.NatGatewaySpecs() + svc.ConditionType = infrav1.NATGatewaysReadyCondition + svc.PostCreateOrUpdateResourceHook = postCreateOrUpdateResourceHook return &Service{ - Scope: scope, - Reconciler: async.New[armnetwork.NatGatewaysClientCreateOrUpdateResponse, - armnetwork.NatGatewaysClientDeleteResponse](scope, client, client), - }, nil -} - -// Name returns the service name. -func (s *Service) Name() string { - return serviceName -} - -// Reconcile idempotently creates or 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, log, done := tele.StartSpanWithLogger(ctx, "natgateways.Service.Reconcile") - defer done() - - ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) - defer cancel() - - if managed, err := s.IsManaged(ctx); err == nil && !managed { - log.V(4).Info("Skipping nat gateways reconcile in custom vnet mode") - return nil - } else if err != nil { - return errors.Wrap(err, "failed to check if NAT gateways are managed") - } - - // We go through the list of NatGatewaySpecs to reconcile each one, independently of the resultingErr of the previous one. - specs := s.Scope.NatGatewaySpecs() - if len(specs) == 0 { - return nil + Scope: scope, + Service: svc, } - - // If multiple errors occur, we return the most pressing one. - // Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (ie. error creating) -> operationNotDoneError (ie. creating in progress) -> no error (ie. created) - var resultingErr error - for _, natGatewaySpec := range specs { - result, err := s.CreateOrUpdateResource(ctx, natGatewaySpec, serviceName) - if err != nil { - if !azure.IsOperationNotDoneError(err) || resultingErr == nil { - resultingErr = err - } - } - if err == nil { - natGateway, ok := result.(armnetwork.NatGateway) - if !ok { - // Return out of loop since this would be an unexpected fatal error - resultingErr = errors.Errorf("created resource %T is not an armnetwork.NatGateway", result) - break - } - - // TODO: ideally we wouldn't need to set the subnet spec based on the result of the create operation - s.Scope.SetNatGatewayIDInSubnets(natGatewaySpec.ResourceName(), *natGateway.ID) - } - } - - s.Scope.UpdatePutStatus(infrav1.NATGatewaysReadyCondition, serviceName, resultingErr) - return resultingErr } -// Delete deletes the NAT gateway with the provided name. -func (s *Service) Delete(ctx context.Context) error { - ctx, log, done := tele.StartSpanWithLogger(ctx, "natgateways.Service.Delete") - defer done() - - ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) - defer cancel() - - if managed, err := s.IsManaged(ctx); err == nil && !managed { - log.V(4).Info("Skipping nat gateway deletion in custom vnet mode") - return nil - } else if err != nil { - return errors.Wrap(err, "failed to check if NAT gateways are managed") - } - - specs := s.Scope.NatGatewaySpecs() - if len(specs) == 0 { - return nil +func postCreateOrUpdateResourceHook(scope NatGatewayScope, result *asonetworkv1.NatGateway, err error) { + // TODO: ideally we wouldn't need to set the subnet spec based on the result of the create operation + // result only gets populated when the resource is created or if it already exists + if result != nil && result.Status.Id != nil { + scope.SetNatGatewayIDInSubnets(result.Name, *result.Status.Id) } - - // We go through the list of NatGatewaySpecs to delete each one, independently of the resultingErr of the previous one. - // If multiple errors occur, we return the most pressing one. - // Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (ie. error creating) -> operationNotDoneError (ie. creating in progress) -> no error (ie. created) - var resultingErr error - for _, natGatewaySpec := range specs { - if err := s.DeleteResource(ctx, natGatewaySpec, serviceName); err != nil { - if !azure.IsOperationNotDoneError(err) || resultingErr == nil { - resultingErr = err - } - } - } - s.Scope.UpdateDeleteStatus(infrav1.NATGatewaysReadyCondition, serviceName, resultingErr) - return resultingErr -} - -// IsManaged returns true if the NAT gateways' lifecycles are managed. -func (s *Service) IsManaged(ctx context.Context) (bool, error) { - _, _, done := tele.StartSpanWithLogger(ctx, "natgateways.Service.IsManaged") - defer done() - - return s.Scope.IsVnetManaged(), nil } diff --git a/azure/services/natgateways/natgateways_test.go b/azure/services/natgateways/natgateways_test.go index 3ad663b422a..6d036ae2658 100644 --- a/azure/services/natgateways/natgateways_test.go +++ b/azure/services/natgateways/natgateways_test.go @@ -1,5 +1,5 @@ /* -Copyright 2020 The Kubernetes Authors. +Copyright 2023 The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -17,213 +17,40 @@ limitations under the License. package natgateways import ( - "context" - "net/http" "testing" - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4" - "github.com/Azure/go-autorest/autorest" - . "github.com/onsi/gomega" + asonetworkv1 "github.com/Azure/azure-service-operator/v2/api/network/v1api20220701" + "github.com/pkg/errors" "go.uber.org/mock/gomock" - "k8s.io/client-go/kubernetes/scheme" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" - 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/async/mock_async" "sigs.k8s.io/cluster-api-provider-azure/azure/services/natgateways/mock_natgateways" - gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" - clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) -func init() { - _ = clusterv1.AddToScheme(scheme.Scheme) -} - -var ( - natGatewaySpec1 = NatGatewaySpec{ - Name: "my-node-natgateway-1", - ResourceGroup: "my-rg", - SubscriptionID: "my-sub", - Location: "westus", - ClusterName: "my-cluster", - NatGatewayIP: infrav1.PublicIPSpec{Name: "pip-node-subnet"}, - } - natGateway1 = armnetwork.NatGateway{ - ID: ptr.To("/subscriptions/my-sub/resourceGroups/my-rg/providers/Microsoft.Network/natGateways/my-node-natgateway-1"), - } - customVNetTags = infrav1.Tags{ - "Name": "my-vnet", - "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "shared", - "sigs.k8s.io_cluster-api-provider-azure_role": "common", - } - ownedVNetTags = 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", - } - internalError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: http.StatusInternalServerError}, "Internal Server Error") -) - -func TestReconcileNatGateways(t *testing.T) { - testcases := []struct { - name string - tags infrav1.Tags - expectedError string - expect func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) - }{ - { - name: "noop if no NAT gateways specs are found", - tags: customVNetTags, - expectedError: "", - expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { - s.IsVnetManaged().Return(true) - s.NatGatewaySpecs().Return([]azure.ResourceSpecGetter{}) - }, - }, - { - name: "NAT gateways in custom vnet mode", - tags: customVNetTags, - expectedError: "", - expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { - s.IsVnetManaged().Return(false) - }, - }, - { - name: "NAT gateway create successfully", - tags: ownedVNetTags, - expectedError: "", - expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { - s.IsVnetManaged().Return(true) - s.NatGatewaySpecs().Return([]azure.ResourceSpecGetter{&natGatewaySpec1}) - r.CreateOrUpdateResource(gomockinternal.AContext(), &natGatewaySpec1, serviceName).Return(natGateway1, nil) - s.SetNatGatewayIDInSubnets(natGatewaySpec1.Name, *natGateway1.ID) - s.UpdatePutStatus(infrav1.NATGatewaysReadyCondition, serviceName, nil) - }, - }, - { - name: "fail to create a NAT gateway", - tags: ownedVNetTags, - expectedError: "#: Internal Server Error: StatusCode=500", - expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { - s.IsVnetManaged().Return(true) - s.NatGatewaySpecs().Return([]azure.ResourceSpecGetter{&natGatewaySpec1}) - r.CreateOrUpdateResource(gomockinternal.AContext(), &natGatewaySpec1, serviceName).Return(nil, internalError) - s.UpdatePutStatus(infrav1.NATGatewaysReadyCondition, serviceName, internalError) - }, - }, - { - name: "result is not a NAT gateway", - tags: ownedVNetTags, - expectedError: "created resource string is not an armnetwork.NatGateway", - expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { - s.IsVnetManaged().Return(true) - s.NatGatewaySpecs().Return([]azure.ResourceSpecGetter{&natGatewaySpec1}) - r.CreateOrUpdateResource(gomockinternal.AContext(), &natGatewaySpec1, serviceName).Return("not a nat gateway", nil) - s.UpdatePutStatus(infrav1.NATGatewaysReadyCondition, serviceName, gomockinternal.ErrStrEq("created resource string is not an armnetwork.NatGateway")) - }, - }, - } - - for _, tc := range testcases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - g := NewWithT(t) - t.Parallel() - mockCtrl := gomock.NewController(t) - defer mockCtrl.Finish() - scopeMock := mock_natgateways.NewMockNatGatewayScope(mockCtrl) - asyncMock := mock_async.NewMockReconciler(mockCtrl) +func TestPostCreateOrUpdateResourceHook(t *testing.T) { + t.Run("error creating or updating", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + scope := mock_natgateways.NewMockNatGatewayScope(mockCtrl) - tc.expect(scopeMock.EXPECT(), asyncMock.EXPECT()) + postCreateOrUpdateResourceHook(scope, nil, errors.New("an error")) + }) - s := &Service{ - Scope: scopeMock, - Reconciler: asyncMock, - } + t.Run("successful create or update", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + scope := mock_natgateways.NewMockNatGatewayScope(mockCtrl) - err := s.Reconcile(context.TODO()) - if tc.expectedError != "" { - g.Expect(err).To(HaveOccurred()) - g.Expect(err).To(MatchError(tc.expectedError)) - } else { - g.Expect(err).NotTo(HaveOccurred()) - } - }) - } -} + scope.EXPECT().SetNatGatewayIDInSubnets("dummy-natgateway-name", "dummy-natgateway-id") -func TestDeleteNatGateway(t *testing.T) { - testcases := []struct { - name string - tags infrav1.Tags - expectedError string - expect func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) - }{ - { - name: "noop if no NAT gateways specs are found", - tags: ownedVNetTags, - expectedError: "", - expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { - s.IsVnetManaged().Return(true) - s.NatGatewaySpecs().Return([]azure.ResourceSpecGetter{}) - }, - }, - { - name: "NAT gateways in custom vnet mode", - tags: customVNetTags, - expectedError: "", - expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { - s.IsVnetManaged().Return(false) + natGateway := &asonetworkv1.NatGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy-natgateway-name", + Namespace: "dummy", }, - }, - { - name: "NAT gateway deleted successfully", - tags: ownedVNetTags, - expectedError: "", - expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { - s.IsVnetManaged().Return(true) - s.NatGatewaySpecs().Return([]azure.ResourceSpecGetter{&natGatewaySpec1}) - r.DeleteResource(gomockinternal.AContext(), &natGatewaySpec1, serviceName).Return(nil) - s.UpdateDeleteStatus(infrav1.NATGatewaysReadyCondition, serviceName, nil) + Status: asonetworkv1.NatGateway_STATUS{ + Id: ptr.To("dummy-natgateway-id"), }, - }, - { - name: "NAT gateway deletion fails", - tags: ownedVNetTags, - expectedError: "#: Internal Server Error: StatusCode=500", - expect: func(s *mock_natgateways.MockNatGatewayScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { - s.IsVnetManaged().Return(true) - s.NatGatewaySpecs().Return([]azure.ResourceSpecGetter{&natGatewaySpec1}) - r.DeleteResource(gomockinternal.AContext(), &natGatewaySpec1, serviceName).Return(internalError) - s.UpdateDeleteStatus(infrav1.NATGatewaysReadyCondition, serviceName, internalError) - }, - }, - } - - for _, tc := range testcases { - tc := tc - t.Run(tc.name, func(t *testing.T) { - g := NewWithT(t) - t.Parallel() - mockCtrl := gomock.NewController(t) - defer mockCtrl.Finish() - scopeMock := mock_natgateways.NewMockNatGatewayScope(mockCtrl) - asyncMock := mock_async.NewMockReconciler(mockCtrl) - - tc.expect(scopeMock.EXPECT(), asyncMock.EXPECT()) - - s := &Service{ - Scope: scopeMock, - Reconciler: asyncMock, - } + } - err := s.Delete(context.TODO()) - if tc.expectedError != "" { - g.Expect(err).To(HaveOccurred()) - g.Expect(err).To(MatchError(tc.expectedError)) - } else { - g.Expect(err).NotTo(HaveOccurred()) - } - }) - } + postCreateOrUpdateResourceHook(scope, natGateway, nil) + }) } diff --git a/azure/services/natgateways/spec.go b/azure/services/natgateways/spec.go index 6f0323126ac..a782ffbe270 100644 --- a/azure/services/natgateways/spec.go +++ b/azure/services/natgateways/spec.go @@ -19,88 +19,72 @@ package natgateways import ( "context" - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4" - "github.com/pkg/errors" + asonetworkv1 "github.com/Azure/azure-service-operator/v2/api/network/v1api20220701" + "github.com/Azure/azure-service-operator/v2/pkg/genruntime" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" 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/converters" - azureutil "sigs.k8s.io/cluster-api-provider-azure/util/azure" ) // NatGatewaySpec defines the specification for a NAT gateway. type NatGatewaySpec struct { Name string + Namespace string ResourceGroup string SubscriptionID string Location string NatGatewayIP infrav1.PublicIPSpec ClusterName string AdditionalTags infrav1.Tags + IsVnetManaged bool } -// ResourceName returns the name of the NAT gateway. -func (s *NatGatewaySpec) ResourceName() string { - return s.Name -} - -// ResourceGroupName returns the name of the resource group. -func (s *NatGatewaySpec) ResourceGroupName() string { - return s.ResourceGroup -} - -// OwnerResourceName is a no-op for NAT gateways. -func (s *NatGatewaySpec) OwnerResourceName() string { - return "" +// ResourceRef implements azure.ASOResourceSpecGetter. +func (s *NatGatewaySpec) ResourceRef() *asonetworkv1.NatGateway { + return &asonetworkv1.NatGateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: s.Name, + Namespace: s.Namespace, + }, + } } -// Parameters returns the parameters for the NAT gateway. -func (s *NatGatewaySpec) Parameters(ctx context.Context, existing interface{}) (params interface{}, err error) { - if existing != nil { - existingNatGateway, ok := existing.(armnetwork.NatGateway) - if !ok { - return nil, errors.Errorf("%T is not an armnetwork.NatGateway", existing) - } +// Parameters implements azure.ASOResourceSpecGetter. +func (s *NatGatewaySpec) Parameters(ctx context.Context, existingNatGateway *asonetworkv1.NatGateway) (params *asonetworkv1.NatGateway, err error) { + natGateway := &asonetworkv1.NatGateway{} + natGateway.Spec = asonetworkv1.NatGateway_Spec{} - if hasPublicIP(existingNatGateway, s.NatGatewayIP.Name) { - // Skip update for NAT gateway as it exists with expected values - return nil, nil - } + if existingNatGateway != nil { + natGateway = existingNatGateway } - natGatewayToCreate := armnetwork.NatGateway{ - Name: ptr.To(s.Name), - Location: ptr.To(s.Location), - SKU: &armnetwork.NatGatewaySKU{Name: ptr.To(armnetwork.NatGatewaySKUNameStandard)}, - Properties: &armnetwork.NatGatewayPropertiesFormat{ - PublicIPAddresses: []*armnetwork.SubResource{ - { - ID: ptr.To(azure.PublicIPID(s.SubscriptionID, s.ResourceGroupName(), s.NatGatewayIP.Name)), - }, + natGateway.Spec.AzureName = s.Name + natGateway.Spec.Owner = &genruntime.KnownResourceReference{ + Name: s.ResourceGroup, + } + natGateway.Spec.Location = ptr.To(s.Location) + natGateway.Spec.Sku = &asonetworkv1.NatGatewaySku{ + Name: ptr.To(asonetworkv1.NatGatewaySku_Name_Standard), + } + natGateway.Spec.PublicIpAddresses = []asonetworkv1.ApplicationGatewaySubResource{ + { + Reference: &genruntime.ResourceReference{ + ARMID: azure.PublicIPID(s.SubscriptionID, s.ResourceGroup, s.NatGatewayIP.Name), }, }, - Tags: converters.TagsToMap(infrav1.Build(infrav1.BuildParams{ - ClusterName: s.ClusterName, - Lifecycle: infrav1.ResourceLifecycleOwned, - Name: ptr.To(s.Name), - Additional: s.AdditionalTags, - })), } + natGateway.Spec.Tags = infrav1.Build(infrav1.BuildParams{ + ClusterName: s.ClusterName, + Lifecycle: infrav1.ResourceLifecycleOwned, + Name: ptr.To(s.Name), + Additional: s.AdditionalTags, + }) - return natGatewayToCreate, nil + return natGateway, nil } -func hasPublicIP(natGateway armnetwork.NatGateway, publicIPName string) bool { - for _, publicIP := range natGateway.Properties.PublicIPAddresses { - if publicIP != nil && publicIP.ID != nil { - resource, err := azureutil.ParseResourceID(*publicIP.ID) - if err != nil { - continue - } - if resource.Name == publicIPName { - return true - } - } - } - return false +// WasManaged implements azure.ASOResourceSpecGetter. +func (s *NatGatewaySpec) WasManaged(resource *asonetworkv1.NatGateway) bool { + return s.IsVnetManaged } diff --git a/azure/services/natgateways/spec_test.go b/azure/services/natgateways/spec_test.go index aa1bae7d7f1..0377e5d33dd 100644 --- a/azure/services/natgateways/spec_test.go +++ b/azure/services/natgateways/spec_test.go @@ -20,104 +20,127 @@ import ( "context" "testing" - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork/v4" + asonetworkv1 "github.com/Azure/azure-service-operator/v2/api/network/v1api20220701" + "github.com/Azure/azure-service-operator/v2/pkg/genruntime" + "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" - "sigs.k8s.io/cluster-api-provider-azure/azure" ) var ( - fakeNatGateway = armnetwork.NatGateway{ - ID: ptr.To("/subscriptions/my-sub/resourceGroups/my-rg/providers/Microsoft.Network/natGateways/my-node-natgateway-1"), - Properties: &armnetwork.NatGatewayPropertiesFormat{ - PublicIPAddresses: []*armnetwork.SubResource{ - {ID: ptr.To("/subscriptions/my-sub/resourceGroups/my-rg/providers/Microsoft.Network/natGateways/pip-node-subnet")}, - }, - }, - } - fakeNatGatewaySpec = NatGatewaySpec{ - Name: "my-node-natgateway-1", + fakeNatGatewaySpec = &NatGatewaySpec{ + Name: "my-natgateway", + Namespace: "dummy-ns", ResourceGroup: "my-rg", - SubscriptionID: "my-sub", - Location: "westus", - ClusterName: "cluster", - NatGatewayIP: infrav1.PublicIPSpec{Name: "pip-node-subnet"}, - } - fakeNatGatewaysTags = map[string]*string{ - "sigs.k8s.io_cluster-api-provider-azure_cluster_cluster": ptr.To("owned"), - "Name": ptr.To("my-node-natgateway-1"), + SubscriptionID: "123", + Location: "eastus", + NatGatewayIP: infrav1.PublicIPSpec{ + Name: "my-natgateway-ip", + DNSName: "Standard", + }, + ClusterName: "my-cluster", + IsVnetManaged: true, + AdditionalTags: infrav1.Tags{}, } -) - -func TestNatGatewaySpec_Parameters(t *testing.T) { - testCases := []struct { - name string - spec *NatGatewaySpec - existing interface{} - expect func(g *WithT, result interface{}) - expectedError string - }{ - { - name: "error when existing is not of NatGateway type", - spec: &NatGatewaySpec{}, - existing: struct{}{}, - expect: func(g *WithT, result interface{}) { - g.Expect(result).To(BeNil()) + locationPtr = ptr.To("eastus") + standardSKUPtr = ptr.To(asonetworkv1.NatGatewaySku_Name_Standard) + existingNatGateway = &asonetworkv1.NatGateway{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "network.azure.com/v1api20220701", + Kind: "NatGateway", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "my-natgateway", + Namespace: "dummy-ns", + }, + Spec: asonetworkv1.NatGateway_Spec{ + AzureName: "my-natgateway", + IdleTimeoutInMinutes: ptr.To(6), + Location: locationPtr, + Owner: &genruntime.KnownResourceReference{ + Name: "my-rg", + }, + PublicIpAddresses: []asonetworkv1.ApplicationGatewaySubResource{ + { + Reference: &genruntime.ResourceReference{ + ARMID: "/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/publicIPAddresses/my-natgateway-ip", + }, + }, + }, + Sku: &asonetworkv1.NatGatewaySku{ + Name: ptr.To(asonetworkv1.NatGatewaySku_Name_Standard), + }, + Tags: map[string]string{ + "sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster": "owned", + "Name": "my-natgateway", }, - expectedError: "struct {} is not an armnetwork.NatGateway", }, - { - name: "get result as nil when existing NatGateway is present", - spec: &fakeNatGatewaySpec, - existing: fakeNatGateway, - expect: func(g *WithT, result interface{}) { - g.Expect(result).To(BeNil()) + Status: asonetworkv1.NatGateway_STATUS{ + Id: ptr.To("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/natGateways/test-1111-node-natgw-1"), + IdleTimeoutInMinutes: ptr.To(4), + Location: locationPtr, + Name: ptr.To("my-natgateway"), + ProvisioningState: ptr.To(asonetworkv1.ApplicationGatewayProvisioningState_STATUS_Succeeded), + PublicIpAddresses: []asonetworkv1.ApplicationGatewaySubResource_STATUS{ + { + Id: ptr.To("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/publicIPAddresses/my-natgateway-ip"), + }, + }, + Sku: &asonetworkv1.NatGatewaySku_STATUS{ + Name: ptr.To(asonetworkv1.NatGatewaySku_Name_STATUS_Standard), }, - expectedError: "", }, + } +) + +func TestParameters(t *testing.T) { + testcases := []struct { + name string + spec *NatGatewaySpec + existingSpec *asonetworkv1.NatGateway + expect func(g *WithT, existing *asonetworkv1.NatGateway, parameters *asonetworkv1.NatGateway) + }{ { - name: "get NatGateway when existing NatGateway is present but PublicIPAddresses is empty", - spec: &fakeNatGatewaySpec, - existing: armnetwork.NatGateway{ - Properties: &armnetwork.NatGatewayPropertiesFormat{PublicIPAddresses: []*armnetwork.SubResource{}}, - }, - expect: func(g *WithT, result interface{}) { - g.Expect(result).To(BeAssignableToTypeOf(armnetwork.NatGateway{})) - g.Expect(result.(armnetwork.NatGateway).Location).To(Equal(ptr.To[string](fakeNatGatewaySpec.Location))) - g.Expect(result.(armnetwork.NatGateway).Name).To(Equal(ptr.To[string](fakeNatGatewaySpec.ResourceName()))) + name: "create a new NAT Gateway spec when existing aso resource is nil", + spec: fakeNatGatewaySpec, + existingSpec: nil, + expect: func(g *WithT, existing *asonetworkv1.NatGateway, parameters *asonetworkv1.NatGateway) { + g.Expect(parameters).NotTo(BeNil()) + g.Expect(parameters.Spec.AzureName).NotTo(BeNil()) + g.Expect(parameters.Spec.AzureName).To(Equal("my-natgateway")) + g.Expect(parameters.Spec.Owner).NotTo(BeNil()) + g.Expect(parameters.Spec.Owner.Name).To(Equal("my-rg")) + g.Expect(parameters.Spec.Location).NotTo(BeNil()) + g.Expect(parameters.Spec.Location).To(Equal(locationPtr)) + g.Expect(parameters.Spec.Sku.Name).NotTo(BeNil()) + g.Expect(parameters.Spec.Sku.Name).To(Equal(standardSKUPtr)) + g.Expect(parameters.Spec.PublicIpAddresses).To(HaveLen(1)) + g.Expect(parameters.Spec.PublicIpAddresses[0].Reference.ARMID).To(Equal("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/publicIPAddresses/my-natgateway-ip")) + g.Expect(parameters.Spec.Tags).NotTo(BeEmpty()) + g.Expect(parameters.Spec.Tags).To(HaveKeyWithValue("Name", "my-natgateway")) + g.Expect(parameters.Spec.Tags).To(HaveKeyWithValue("sigs.k8s.io_cluster-api-provider-azure_cluster_my-cluster", "owned")) }, - expectedError: "", }, { - name: "get NatGateway when all values are present", - spec: &fakeNatGatewaySpec, - existing: nil, - expect: func(g *WithT, result interface{}) { - g.Expect(result).To(BeAssignableToTypeOf(armnetwork.NatGateway{})) - g.Expect(result.(armnetwork.NatGateway).Location).To(Equal(ptr.To[string](fakeNatGatewaySpec.Location))) - g.Expect(result.(armnetwork.NatGateway).Name).To(Equal(ptr.To[string](fakeNatGatewaySpec.ResourceName()))) - g.Expect(result.(armnetwork.NatGateway).Properties.PublicIPAddresses[0].ID).To(Equal(ptr.To[string](azure.PublicIPID(fakeNatGatewaySpec.SubscriptionID, - fakeNatGatewaySpec.ResourceGroupName(), fakeNatGatewaySpec.NatGatewayIP.Name)))) - g.Expect(result.(armnetwork.NatGateway).Tags).To(Equal(fakeNatGatewaysTags)) + name: "reconcile a NAT Gateway spec when there is an existing aso resource. User added extra spec fields", + spec: fakeNatGatewaySpec, + existingSpec: existingNatGateway, + expect: func(g *WithT, existing *asonetworkv1.NatGateway, parameters *asonetworkv1.NatGateway) { + diff := cmp.Diff(existing, parameters) + g.Expect(diff).To(BeEmpty()) }, - expectedError: "", }, } - for _, tc := range testCases { + for _, tc := range testcases { tc := tc t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) t.Parallel() - result, err := tc.spec.Parameters(context.TODO(), tc.existing) - if tc.expectedError != "" { - g.Expect(err).To(HaveOccurred()) - g.Expect(err).To(MatchError(tc.expectedError)) - } else { - g.Expect(err).NotTo(HaveOccurred()) - } - tc.expect(g, result) + result, _ := tc.spec.Parameters(context.TODO(), tc.existingSpec.DeepCopy()) + tc.expect(g, tc.existingSpec, result) }) } } diff --git a/config/aso/crds.yaml b/config/aso/crds.yaml index dc266c89512..de1cc5ba697 100644 --- a/config/aso/crds.yaml +++ b/config/aso/crds.yaml @@ -1,5 +1,522 @@ apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition +metadata: + annotations: + cert-manager.io/inject-ca-from: azureserviceoperator-system/azureserviceoperator-serving-cert + controller-gen.kubebuilder.io/version: v0.13.0 + labels: + app.kubernetes.io/name: azure-service-operator + app.kubernetes.io/version: v2.3.0 + name: natgateways.network.azure.com +spec: + conversion: + strategy: Webhook + webhook: + clientConfig: + service: + name: azureserviceoperator-webhook-service + namespace: azureserviceoperator-system + path: /convert + port: 443 + conversionReviewVersions: + - v1 + group: network.azure.com + names: + kind: NatGateway + listKind: NatGatewayList + plural: natgateways + singular: natgateway + preserveUnknownFields: false + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .status.conditions[?(@.type=='Ready')].status + name: Ready + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].severity + name: Severity + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].reason + name: Reason + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].message + name: Message + type: string + name: v1api20220701 + schema: + openAPIV3Schema: + description: 'Generator information: - Generated from: /network/resource-manager/Microsoft.Network/stable/2022-07-01/natGateway.json - ARM URI: /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/natGateways/{natGatewayName}' + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + properties: + azureName: + description: 'AzureName: The name of the resource in Azure. This is often the same as the name of the resource in Kubernetes but it doesn''t have to be.' + type: string + idleTimeoutInMinutes: + description: 'IdleTimeoutInMinutes: The idle timeout of the nat gateway.' + type: integer + location: + description: 'Location: Resource location.' + type: string + owner: + description: 'Owner: The owner of the resource. The owner controls where the resource goes when it is deployed. The owner also controls the resources lifecycle. When the owner is deleted the resource will also be deleted. Owner is expected to be a reference to a resources.azure.com/ResourceGroup resource' + properties: + armId: + pattern: (?i)(^(/subscriptions/([^/]+)(/resourcegroups/([^/]+))?)?/providers/([^/]+)/([^/]+/[^/]+)(/([^/]+/[^/]+))*$|^/subscriptions/([^/]+)(/resourcegroups/([^/]+))?$) + type: string + name: + description: This is the name of the Kubernetes resource to reference. + type: string + type: object + publicIpAddresses: + description: 'PublicIpAddresses: An array of public ip addresses associated with the nat gateway resource.' + items: + description: Reference to another subresource. + properties: + reference: + description: 'Reference: Resource ID.' + properties: + armId: + description: ARMID is a string of the form /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/{resourceProviderNamespace}/{resourceType}/{resourceName}. The /resourcegroups/{resourceGroupName} bit is optional as some resources are scoped at the subscription level ARMID is mutually exclusive with Group, Kind, Namespace and Name. + pattern: (?i)(^(/subscriptions/([^/]+)(/resourcegroups/([^/]+))?)?/providers/([^/]+)/([^/]+/[^/]+)(/([^/]+/[^/]+))*$|^/subscriptions/([^/]+)(/resourcegroups/([^/]+))?$) + type: string + group: + description: Group is the Kubernetes group of the resource. + type: string + kind: + description: Kind is the Kubernetes kind of the resource. + type: string + name: + description: Name is the Kubernetes name of the resource. + type: string + type: object + type: object + type: array + publicIpPrefixes: + description: 'PublicIpPrefixes: An array of public ip prefixes associated with the nat gateway resource.' + items: + description: Reference to another subresource. + properties: + reference: + description: 'Reference: Resource ID.' + properties: + armId: + description: ARMID is a string of the form /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/{resourceProviderNamespace}/{resourceType}/{resourceName}. The /resourcegroups/{resourceGroupName} bit is optional as some resources are scoped at the subscription level ARMID is mutually exclusive with Group, Kind, Namespace and Name. + pattern: (?i)(^(/subscriptions/([^/]+)(/resourcegroups/([^/]+))?)?/providers/([^/]+)/([^/]+/[^/]+)(/([^/]+/[^/]+))*$|^/subscriptions/([^/]+)(/resourcegroups/([^/]+))?$) + type: string + group: + description: Group is the Kubernetes group of the resource. + type: string + kind: + description: Kind is the Kubernetes kind of the resource. + type: string + name: + description: Name is the Kubernetes name of the resource. + type: string + type: object + type: object + type: array + sku: + description: 'Sku: The nat gateway SKU.' + properties: + name: + description: 'Name: Name of Nat Gateway SKU.' + enum: + - Standard + type: string + type: object + tags: + additionalProperties: + type: string + description: 'Tags: Resource tags.' + type: object + zones: + description: 'Zones: A list of availability zones denoting the zone in which Nat Gateway should be deployed.' + items: + type: string + type: array + required: + - owner + type: object + status: + description: Nat Gateway resource. + properties: + conditions: + description: 'Conditions: The observed state of the resource' + items: + description: Condition defines an extension to status (an observation) of a resource + properties: + lastTransitionTime: + description: LastTransitionTime is the last time the condition transitioned from one status to another. + format: date-time + type: string + message: + description: Message is a human readable message indicating details about the transition. This field may be empty. + type: string + observedGeneration: + description: ObservedGeneration is the .metadata.generation that the condition was set based upon. For instance, if .metadata.generation is currently 12, but the .status.condition[x].observedGeneration is 9, the condition is out of date with respect to the current state of the instance. + format: int64 + type: integer + reason: + description: Reason for the condition's last transition. Reasons are upper CamelCase (PascalCase) with no spaces. A reason is always provided, this field will not be empty. + type: string + severity: + description: Severity with which to treat failures of this type of condition. For conditions which have positive polarity (Status == True is their normal/healthy state), this will be omitted when Status == True For conditions which have negative polarity (Status == False is their normal/healthy state), this will be omitted when Status == False. This is omitted in all cases when Status == Unknown + type: string + status: + description: Status of the condition, one of True, False, or Unknown. + type: string + type: + description: Type of condition. + type: string + required: + - lastTransitionTime + - reason + - status + - type + type: object + type: array + etag: + description: 'Etag: A unique read-only string that changes whenever the resource is updated.' + type: string + id: + description: 'Id: Resource ID.' + type: string + idleTimeoutInMinutes: + description: 'IdleTimeoutInMinutes: The idle timeout of the nat gateway.' + type: integer + location: + description: 'Location: Resource location.' + type: string + name: + description: 'Name: Resource name.' + type: string + provisioningState: + description: 'ProvisioningState: The provisioning state of the NAT gateway resource.' + type: string + publicIpAddresses: + description: 'PublicIpAddresses: An array of public ip addresses associated with the nat gateway resource.' + items: + description: Reference to another subresource. + properties: + id: + description: 'Id: Resource ID.' + type: string + type: object + type: array + publicIpPrefixes: + description: 'PublicIpPrefixes: An array of public ip prefixes associated with the nat gateway resource.' + items: + description: Reference to another subresource. + properties: + id: + description: 'Id: Resource ID.' + type: string + type: object + type: array + resourceGuid: + description: 'ResourceGuid: The resource GUID property of the NAT gateway resource.' + type: string + sku: + description: 'Sku: The nat gateway SKU.' + properties: + name: + description: 'Name: Name of Nat Gateway SKU.' + type: string + type: object + subnets: + description: 'Subnets: An array of references to the subnets using this nat gateway resource.' + items: + description: Reference to another subresource. + properties: + id: + description: 'Id: Resource ID.' + type: string + type: object + type: array + tags: + additionalProperties: + type: string + description: 'Tags: Resource tags.' + type: object + type: + description: 'Type: Resource type.' + type: string + zones: + description: 'Zones: A list of availability zones denoting the zone in which Nat Gateway should be deployed.' + items: + type: string + type: array + type: object + type: object + served: true + storage: false + subresources: + status: {} + - additionalPrinterColumns: + - jsonPath: .status.conditions[?(@.type=='Ready')].status + name: Ready + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].severity + name: Severity + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].reason + name: Reason + type: string + - jsonPath: .status.conditions[?(@.type=='Ready')].message + name: Message + type: string + name: v1api20220701storage + schema: + openAPIV3Schema: + description: 'Storage version of v1api20220701.NatGateway Generator information: - Generated from: /network/resource-manager/Microsoft.Network/stable/2022-07-01/natGateway.json - ARM URI: /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Network/natGateways/{natGatewayName}' + properties: + apiVersion: + description: 'APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources' + type: string + kind: + description: 'Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds' + type: string + metadata: + type: object + spec: + description: Storage version of v1api20220701.NatGateway_Spec + properties: + $propertyBag: + additionalProperties: + type: string + description: PropertyBag is an unordered set of stashed information that used for properties not directly supported by storage resources, allowing for full fidelity round trip conversions + type: object + azureName: + description: 'AzureName: The name of the resource in Azure. This is often the same as the name of the resource in Kubernetes but it doesn''t have to be.' + type: string + idleTimeoutInMinutes: + type: integer + location: + type: string + originalVersion: + type: string + owner: + description: 'Owner: The owner of the resource. The owner controls where the resource goes when it is deployed. The owner also controls the resources lifecycle. When the owner is deleted the resource will also be deleted. Owner is expected to be a reference to a resources.azure.com/ResourceGroup resource' + properties: + armId: + pattern: (?i)(^(/subscriptions/([^/]+)(/resourcegroups/([^/]+))?)?/providers/([^/]+)/([^/]+/[^/]+)(/([^/]+/[^/]+))*$|^/subscriptions/([^/]+)(/resourcegroups/([^/]+))?$) + type: string + name: + description: This is the name of the Kubernetes resource to reference. + type: string + type: object + publicIpAddresses: + items: + description: Storage version of v1api20220701.ApplicationGatewaySubResource Reference to another subresource. + properties: + $propertyBag: + additionalProperties: + type: string + description: PropertyBag is an unordered set of stashed information that used for properties not directly supported by storage resources, allowing for full fidelity round trip conversions + type: object + reference: + description: 'Reference: Resource ID.' + properties: + armId: + description: ARMID is a string of the form /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/{resourceProviderNamespace}/{resourceType}/{resourceName}. The /resourcegroups/{resourceGroupName} bit is optional as some resources are scoped at the subscription level ARMID is mutually exclusive with Group, Kind, Namespace and Name. + pattern: (?i)(^(/subscriptions/([^/]+)(/resourcegroups/([^/]+))?)?/providers/([^/]+)/([^/]+/[^/]+)(/([^/]+/[^/]+))*$|^/subscriptions/([^/]+)(/resourcegroups/([^/]+))?$) + type: string + group: + description: Group is the Kubernetes group of the resource. + type: string + kind: + description: Kind is the Kubernetes kind of the resource. + type: string + name: + description: Name is the Kubernetes name of the resource. + type: string + type: object + type: object + type: array + publicIpPrefixes: + items: + description: Storage version of v1api20220701.ApplicationGatewaySubResource Reference to another subresource. + properties: + $propertyBag: + additionalProperties: + type: string + description: PropertyBag is an unordered set of stashed information that used for properties not directly supported by storage resources, allowing for full fidelity round trip conversions + type: object + reference: + description: 'Reference: Resource ID.' + properties: + armId: + description: ARMID is a string of the form /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/{resourceProviderNamespace}/{resourceType}/{resourceName}. The /resourcegroups/{resourceGroupName} bit is optional as some resources are scoped at the subscription level ARMID is mutually exclusive with Group, Kind, Namespace and Name. + pattern: (?i)(^(/subscriptions/([^/]+)(/resourcegroups/([^/]+))?)?/providers/([^/]+)/([^/]+/[^/]+)(/([^/]+/[^/]+))*$|^/subscriptions/([^/]+)(/resourcegroups/([^/]+))?$) + type: string + group: + description: Group is the Kubernetes group of the resource. + type: string + kind: + description: Kind is the Kubernetes kind of the resource. + type: string + name: + description: Name is the Kubernetes name of the resource. + type: string + type: object + type: object + type: array + sku: + description: Storage version of v1api20220701.NatGatewaySku SKU of nat gateway. + properties: + $propertyBag: + additionalProperties: + type: string + description: PropertyBag is an unordered set of stashed information that used for properties not directly supported by storage resources, allowing for full fidelity round trip conversions + type: object + name: + type: string + type: object + tags: + additionalProperties: + type: string + type: object + zones: + items: + type: string + type: array + required: + - owner + type: object + status: + description: Storage version of v1api20220701.NatGateway_STATUS Nat Gateway resource. + properties: + $propertyBag: + additionalProperties: + type: string + description: PropertyBag is an unordered set of stashed information that used for properties not directly supported by storage resources, allowing for full fidelity round trip conversions + type: object + conditions: + items: + description: Condition defines an extension to status (an observation) of a resource + properties: + lastTransitionTime: + description: LastTransitionTime is the last time the condition transitioned from one status to another. + format: date-time + type: string + message: + description: Message is a human readable message indicating details about the transition. This field may be empty. + type: string + observedGeneration: + description: ObservedGeneration is the .metadata.generation that the condition was set based upon. For instance, if .metadata.generation is currently 12, but the .status.condition[x].observedGeneration is 9, the condition is out of date with respect to the current state of the instance. + format: int64 + type: integer + reason: + description: Reason for the condition's last transition. Reasons are upper CamelCase (PascalCase) with no spaces. A reason is always provided, this field will not be empty. + type: string + severity: + description: Severity with which to treat failures of this type of condition. For conditions which have positive polarity (Status == True is their normal/healthy state), this will be omitted when Status == True For conditions which have negative polarity (Status == False is their normal/healthy state), this will be omitted when Status == False. This is omitted in all cases when Status == Unknown + type: string + status: + description: Status of the condition, one of True, False, or Unknown. + type: string + type: + description: Type of condition. + type: string + required: + - lastTransitionTime + - reason + - status + - type + type: object + type: array + etag: + type: string + id: + type: string + idleTimeoutInMinutes: + type: integer + location: + type: string + name: + type: string + provisioningState: + type: string + publicIpAddresses: + items: + description: Storage version of v1api20220701.ApplicationGatewaySubResource_STATUS Reference to another subresource. + properties: + $propertyBag: + additionalProperties: + type: string + description: PropertyBag is an unordered set of stashed information that used for properties not directly supported by storage resources, allowing for full fidelity round trip conversions + type: object + id: + type: string + type: object + type: array + publicIpPrefixes: + items: + description: Storage version of v1api20220701.ApplicationGatewaySubResource_STATUS Reference to another subresource. + properties: + $propertyBag: + additionalProperties: + type: string + description: PropertyBag is an unordered set of stashed information that used for properties not directly supported by storage resources, allowing for full fidelity round trip conversions + type: object + id: + type: string + type: object + type: array + resourceGuid: + type: string + sku: + description: Storage version of v1api20220701.NatGatewaySku_STATUS SKU of nat gateway. + properties: + $propertyBag: + additionalProperties: + type: string + description: PropertyBag is an unordered set of stashed information that used for properties not directly supported by storage resources, allowing for full fidelity round trip conversions + type: object + name: + type: string + type: object + subnets: + items: + description: Storage version of v1api20220701.ApplicationGatewaySubResource_STATUS Reference to another subresource. + properties: + $propertyBag: + additionalProperties: + type: string + description: PropertyBag is an unordered set of stashed information that used for properties not directly supported by storage resources, allowing for full fidelity round trip conversions + type: object + id: + type: string + type: object + type: array + tags: + additionalProperties: + type: string + type: object + type: + type: string + zones: + items: + type: string + type: array + type: object + type: object + served: true + storage: true + subresources: + status: {} +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition metadata: annotations: cert-manager.io/inject-ca-from: azureserviceoperator-system/azureserviceoperator-serving-cert diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index ce557be6b40..3cf9633c4c2 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -261,6 +261,26 @@ rules: - get - patch - update +- apiGroups: + - network.azure.com + resources: + - natgateways + verbs: + - create + - delete + - get + - list + - patch + - update + - watch +- apiGroups: + - network.azure.com + resources: + - natgateways/status + verbs: + - get + - list + - watch - apiGroups: - resources.azure.com resources: diff --git a/controllers/azurecluster_controller.go b/controllers/azurecluster_controller.go index eb20215cb01..a4c894a6127 100644 --- a/controllers/azurecluster_controller.go +++ b/controllers/azurecluster_controller.go @@ -113,6 +113,8 @@ func (acr *AzureClusterReconciler) SetupWithManager(ctx context.Context, mgr ctr // +kubebuilder:rbac:groups="",resources=namespaces,verbs=list; // +kubebuilder:rbac:groups=resources.azure.com,resources=resourcegroups,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=resources.azure.com,resources=resourcegroups/status,verbs=get;list;watch +// +kubebuilder:rbac:groups=network.azure.com,resources=natgateways,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=network.azure.com,resources=natgateways/status,verbs=get;list;watch // Reconcile idempotently gets, creates, and updates a cluster. func (acr *AzureClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) { diff --git a/controllers/azurecluster_reconciler.go b/controllers/azurecluster_reconciler.go index 551cb7a0b88..8d45b62765f 100644 --- a/controllers/azurecluster_reconciler.go +++ b/controllers/azurecluster_reconciler.go @@ -70,10 +70,6 @@ func newAzureClusterService(scope *scope.ClusterScope) (*azureClusterService, er if err != nil { return nil, err } - natGatewaysSvc, err := natgateways.New(scope) - if err != nil { - return nil, err - } publicIPsSvc, err := publicips.New(scope) if err != nil { return nil, err @@ -106,7 +102,7 @@ func newAzureClusterService(scope *scope.ClusterScope) (*azureClusterService, er securityGroupsSvc, routeTablesSvc, publicIPsSvc, - natGatewaysSvc, + natgateways.New(scope), subnetsSvc, vnetPeeringsSvc, loadbalancersSvc, diff --git a/main.go b/main.go index b81da918138..3f79e50bad6 100644 --- a/main.go +++ b/main.go @@ -26,6 +26,7 @@ import ( // +kubebuilder:scaffold:imports aadpodv1 "github.com/Azure/aad-pod-identity/pkg/apis/aadpodidentity/v1" + asonetworkv1 "github.com/Azure/azure-service-operator/v2/api/network/v1api20220701" asoresourcesv1 "github.com/Azure/azure-service-operator/v2/api/resources/v1api20200601" "github.com/spf13/pflag" corev1 "k8s.io/api/core/v1" @@ -74,6 +75,7 @@ func init() { _ = expv1.AddToScheme(scheme) _ = kubeadmv1.AddToScheme(scheme) _ = asoresourcesv1.AddToScheme(scheme) + _ = asonetworkv1.AddToScheme(scheme) // +kubebuilder:scaffold:scheme // Add aadpodidentity v1 to the scheme.