From f4daf4bc717277687c8e4fa37d48bf5d4632b442 Mon Sep 17 00:00:00 2001 From: Jonathan Tong Date: Tue, 23 Nov 2021 17:16:21 -0500 Subject: [PATCH] Make subnet reconcile/delete async --- azure/scope/cluster.go | 36 +- azure/scope/managedcontrolplane.go | 30 +- azure/services/subnets/client.go | 124 ++- .../subnets/mock_subnets/client_mock.go | 95 --- azure/services/subnets/mock_subnets/doc.go | 2 - .../subnets/mock_subnets/subnets_mock.go | 103 ++- azure/services/subnets/spec.go | 104 +++ azure/services/subnets/spec_test.go | 213 +++++ azure/services/subnets/subnets.go | 155 ++-- azure/services/subnets/subnets_test.go | 766 +++++------------- azure/types.go | 11 - 11 files changed, 847 insertions(+), 792 deletions(-) delete mode 100644 azure/services/subnets/mock_subnets/client_mock.go create mode 100644 azure/services/subnets/spec.go create mode 100644 azure/services/subnets/spec_test.go diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index 5118733daad..c948b54aee0 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -35,6 +35,7 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/azure/services/loadbalancers" "sigs.k8s.io/cluster-api-provider-azure/azure/services/natgateways" "sigs.k8s.io/cluster-api-provider-azure/azure/services/routetables" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/subnets" "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualnetworks" "sigs.k8s.io/cluster-api-provider-azure/azure/services/vnetpeerings" "sigs.k8s.io/cluster-api-provider-azure/util/futures" @@ -289,20 +290,25 @@ func (s *ClusterScope) NSGSpecs() []azure.NSGSpec { } // SubnetSpecs returns the subnets specs. -func (s *ClusterScope) SubnetSpecs() []azure.SubnetSpec { +func (s *ClusterScope) SubnetSpecs() []azure.ResourceSpecGetter { numberOfSubnets := len(s.AzureCluster.Spec.NetworkSpec.Subnets) if s.IsAzureBastionEnabled() { numberOfSubnets++ } - subnetSpecs := make([]azure.SubnetSpec, 0, numberOfSubnets) + subnetSpecs := make([]azure.ResourceSpecGetter, 0, numberOfSubnets) + for _, subnet := range s.AzureCluster.Spec.NetworkSpec.Subnets { - subnetSpec := azure.SubnetSpec{ + subnetSpec := &subnets.SubnetSpec{ Name: subnet.Name, + ResourceGroup: s.ResourceGroup(), + SubscriptionID: s.SubscriptionID(), CIDRs: subnet.CIDRBlocks, VNetName: s.Vnet().Name, - SecurityGroupName: subnet.SecurityGroup.Name, + VNetResourceGroup: s.Vnet().ResourceGroup, + IsVNetManaged: s.IsVnetManaged(), RouteTableName: subnet.RouteTable.Name, + SecurityGroupName: subnet.SecurityGroup.Name, Role: subnet.Role, NatGatewayName: subnet.NatGateway.Name, } @@ -311,10 +317,14 @@ func (s *ClusterScope) SubnetSpecs() []azure.SubnetSpec { if s.IsAzureBastionEnabled() { azureBastionSubnet := s.AzureCluster.Spec.BastionSpec.AzureBastion.Subnet - subnetSpecs = append(subnetSpecs, azure.SubnetSpec{ + subnetSpecs = append(subnetSpecs, &subnets.SubnetSpec{ Name: azureBastionSubnet.Name, + ResourceGroup: s.ResourceGroup(), + SubscriptionID: s.SubscriptionID(), CIDRs: azureBastionSubnet.CIDRBlocks, VNetName: s.Vnet().Name, + VNetResourceGroup: s.Vnet().ResourceGroup, + IsVNetManaged: s.IsVnetManaged(), SecurityGroupName: azureBastionSubnet.SecurityGroup.Name, RouteTableName: azureBastionSubnet.RouteTable.Name, Role: azureBastionSubnet.Role, @@ -507,6 +517,20 @@ func (s *ClusterScope) SetNatGatewayIDInSubnets(name string, id string) { } } +// UpdateSubnetCIDRs updates the subnet CIDRs for the subnet with the same name. +func (s *ClusterScope) UpdateSubnetCIDRs(name string, cidrBlocks []string) { + subnetSpecInfra := s.Subnet(name) + subnetSpecInfra.CIDRBlocks = cidrBlocks + s.SetSubnet(subnetSpecInfra) +} + +// UpdateSubnetIDs updates the subnet IDs for the subnet with the same name. +func (s *ClusterScope) UpdateSubnetID(name string, id string) { + subnetSpecInfra := s.Subnet(name) + subnetSpecInfra.ID = id + s.SetSubnet(subnetSpecInfra) +} + // ControlPlaneRouteTable returns the cluster controlplane routetable. func (s *ClusterScope) ControlPlaneRouteTable() infrav1.RouteTable { subnet, _ := s.AzureCluster.Spec.NetworkSpec.GetControlPlaneSubnet() @@ -666,6 +690,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error { infrav1.LoadBalancersReadyCondition, infrav1.BastionHostReadyCondition, infrav1.VNetReadyCondition, + infrav1.SubnetsReadyCondition, ), ) @@ -683,6 +708,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error { infrav1.LoadBalancersReadyCondition, infrav1.BastionHostReadyCondition, infrav1.VNetReadyCondition, + infrav1.SubnetsReadyCondition, }}) } diff --git a/azure/scope/managedcontrolplane.go b/azure/scope/managedcontrolplane.go index 2049436dddb..b5f152efe3e 100644 --- a/azure/scope/managedcontrolplane.go +++ b/azure/scope/managedcontrolplane.go @@ -33,6 +33,7 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/services/groups" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/subnets" "sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualnetworks" infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/util/futures" @@ -255,12 +256,17 @@ func (s *ManagedControlPlaneScope) NodeNatGateway() infrav1.NatGateway { } // SubnetSpecs returns the subnets specs. -func (s *ManagedControlPlaneScope) SubnetSpecs() []azure.SubnetSpec { - return []azure.SubnetSpec{ - { - Name: s.NodeSubnet().Name, - CIDRs: s.NodeSubnet().CIDRBlocks, - VNetName: s.Vnet().Name, +func (s *ManagedControlPlaneScope) SubnetSpecs() []azure.ResourceSpecGetter { + return []azure.ResourceSpecGetter{ + &subnets.SubnetSpec{ + Name: s.NodeSubnet().Name, + ResourceGroup: s.ResourceGroup(), + SubscriptionID: s.SubscriptionID(), + CIDRs: s.NodeSubnet().CIDRBlocks, + VNetName: s.Vnet().Name, + VNetResourceGroup: s.Vnet().ResourceGroup, + IsVNetManaged: s.IsVnetManaged(), + Role: infrav1.SubnetNode, }, } } @@ -286,6 +292,18 @@ func (s *ManagedControlPlaneScope) SetSubnet(_ infrav1.SubnetSpec) { // no-op } +// UpdateSubnetCIDRs updates the subnet CIDRs for the subnet with the same name. +// This is not used when using a managed control plane. +func (s *ManagedControlPlaneScope) UpdateSubnetCIDRs(_ string, _ []string) { + // no-op +} + +// UpdateSubnetIDs updates the subnet IDs for the subnet with the same name. +// This is not used when using a managed control plane. +func (s *ManagedControlPlaneScope) UpdateSubnetID(_ string, _ string) { + // no-op +} + // ControlPlaneSubnet returns the cluster control plane subnet. func (s *ManagedControlPlaneScope) ControlPlaneSubnet() infrav1.SubnetSpec { return infrav1.SubnetSpec{} diff --git a/azure/services/subnets/client.go b/azure/services/subnets/client.go index a2b27b06429..917ee94e2a9 100644 --- a/azure/services/subnets/client.go +++ b/azure/services/subnets/client.go @@ -18,27 +18,23 @@ package subnets import ( "context" + "encoding/json" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" "github.com/Azure/go-autorest/autorest" + azureautorest "github.com/Azure/go-autorest/autorest/azure" + "github.com/pkg/errors" + 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/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) -// Client wraps go-sdk. -type Client interface { - Get(context.Context, string, string, string) (network.Subnet, error) - CreateOrUpdate(context.Context, string, string, string, network.Subnet) error - Delete(context.Context, string, string, string) error -} - // AzureClient contains the Azure go-sdk Client. type AzureClient struct { subnets network.SubnetsClient } -var _ Client = &AzureClient{} - // NewClient creates a new subnets client from subscription ID. func NewClient(auth azure.Authorizer) *AzureClient { c := newSubnetsClient(auth.SubscriptionID(), auth.BaseURI(), auth.Authorizer()) @@ -53,43 +49,113 @@ func newSubnetsClient(subscriptionID string, baseURI string, authorizer autorest } // Get gets the specified subnet by virtual network and resource group. -func (ac *AzureClient) Get(ctx context.Context, resourceGroupName, vnetName, snName string) (network.Subnet, error) { +func (ac *AzureClient) Get(ctx context.Context, spec azure.ResourceSpecGetter) (result interface{}, err error) { ctx, _, done := tele.StartSpanWithLogger(ctx, "subnets.AzureClient.Get") defer done() - return ac.subnets.Get(ctx, resourceGroupName, vnetName, snName, "") + return ac.subnets.Get(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName(), "") } -// CreateOrUpdate creates or updates a subnet in the specified virtual network. -func (ac *AzureClient) CreateOrUpdate(ctx context.Context, resourceGroupName, vnetName, snName string, sn network.Subnet) error { - ctx, _, done := tele.StartSpanWithLogger(ctx, "subnets.AzureClient.CreateOrUpdate") +// CreateOrUpdateAsync creates or updates a subnet 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, parameters interface{}) (result interface{}, future azureautorest.FutureAPI, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "subnets.AzureClient.CreateOrUpdateAsync") defer done() - future, err := ac.subnets.CreateOrUpdate(ctx, resourceGroupName, vnetName, snName, sn) + subnet, ok := parameters.(network.Subnet) + if !ok { + return nil, nil, errors.Errorf("%T is not a network.Subnet", parameters) + } + + createFuture, err := ac.subnets.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName(), subnet) if err != nil { - return err + return nil, nil, err } - err = future.WaitForCompletionRef(ctx, ac.subnets.Client) + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + + err = createFuture.WaitForCompletionRef(ctx, ac.subnets.Client) if err != nil { - return err + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return nil, &createFuture, err } - _, err = future.Result(ac.subnets) - return err + + result, err = createFuture.Result(ac.subnets) + // if the operation completed, return a nil future + return result, nil, err } -// Delete deletes the specified subnet. -func (ac *AzureClient) Delete(ctx context.Context, resourceGroupName, vnetName, snName string) error { - ctx, _, done := tele.StartSpanWithLogger(ctx, "subnets.AzureClient.Delete") +// DeleteAsync deletes a subnet 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) (future azureautorest.FutureAPI, err error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "subnets.AzureClient.DeleteAsync") defer done() - future, err := ac.subnets.Delete(ctx, resourceGroupName, vnetName, snName) + deleteFuture, err := ac.subnets.Delete(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName()) + if err != nil { + return nil, err + } + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + + err = deleteFuture.WaitForCompletionRef(ctx, ac.subnets.Client) if err != nil { - return err + // if an error occurs, return the future. + // this means the long-running operation didn't finish in the specified timeout. + return &deleteFuture, err } - err = future.WaitForCompletionRef(ctx, ac.subnets.Client) + _, err = deleteFuture.Result(ac.subnets) + // if the operation completed, return a nil future. + return nil, err +} + +// IsDone returns true if the long-running operation has completed. +func (ac *AzureClient) IsDone(ctx context.Context, future azureautorest.FutureAPI) (bool, error) { + ctx, _, done := tele.StartSpanWithLogger(ctx, "subnets.AzureClient.IsDone") + defer done() + + isDone, err := future.DoneWithContext(ctx, ac.subnets) if err != nil { - return err + return false, errors.Wrap(err, "failed checking if the operation was complete") + } + + return isDone, nil +} + +// Result fetches the result of a long-running operation future. +func (ac *AzureClient) Result(ctx context.Context, future azureautorest.FutureAPI, futureType string) (result interface{}, err error) { + _, _, done := tele.StartSpanWithLogger(ctx, "subnets.AzureClient.Result") + defer done() + + if future == nil { + return nil, errors.Errorf("cannot get result from nil future") + } + + switch futureType { + case infrav1.PutFuture: + // Marshal and Unmarshal the future to put it into the correct future type so we can access the Result function. + // Unfortunately the FutureAPI can't be casted directly to SubnetsCreateOrUpdateFuture because it is a azureautorest.Future, which doesn't implement the Result function. See PR #1686 for discussion on alternatives. + // It was converted back to a generic azureautorest.Future from the CAPZ infrav1.Future type stored in Status: https://github.com/kubernetes-sigs/cluster-api-provider-azure/blob/main/azure/converters/futures.go#L49. + var createFuture *network.SubnetsCreateOrUpdateFuture + jsonData, err := future.MarshalJSON() + if err != nil { + return nil, errors.Wrap(err, "failed to marshal future") + } + if err := json.Unmarshal(jsonData, &createFuture); err != nil { + return nil, errors.Wrap(err, "failed to unmarshal future data") + } + return (*createFuture).Result(ac.subnets) + + case infrav1.DeleteFuture: + // Delete does not return a result subnet + return nil, nil + + default: + return nil, errors.Errorf("unknown future type %q", futureType) } - _, err = future.Result(ac.subnets) - return err } diff --git a/azure/services/subnets/mock_subnets/client_mock.go b/azure/services/subnets/mock_subnets/client_mock.go deleted file mode 100644 index c5226a3c7db..00000000000 --- a/azure/services/subnets/mock_subnets/client_mock.go +++ /dev/null @@ -1,95 +0,0 @@ -/* -Copyright 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. -*/ - -// Code generated by MockGen. DO NOT EDIT. -// Source: ../client.go - -// Package mock_subnets is a generated GoMock package. -package mock_subnets - -import ( - context "context" - reflect "reflect" - - network "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" - gomock "github.com/golang/mock/gomock" -) - -// MockClient is a mock of Client interface. -type MockClient struct { - ctrl *gomock.Controller - recorder *MockClientMockRecorder -} - -// MockClientMockRecorder is the mock recorder for MockClient. -type MockClientMockRecorder struct { - mock *MockClient -} - -// NewMockClient creates a new mock instance. -func NewMockClient(ctrl *gomock.Controller) *MockClient { - mock := &MockClient{ctrl: ctrl} - mock.recorder = &MockClientMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockClient) EXPECT() *MockClientMockRecorder { - return m.recorder -} - -// CreateOrUpdate mocks base method. -func (m *MockClient) CreateOrUpdate(arg0 context.Context, arg1, arg2, arg3 string, arg4 network.Subnet) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdate", arg0, arg1, arg2, arg3, arg4) - ret0, _ := ret[0].(error) - return ret0 -} - -// CreateOrUpdate indicates an expected call of CreateOrUpdate. -func (mr *MockClientMockRecorder) CreateOrUpdate(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdate", reflect.TypeOf((*MockClient)(nil).CreateOrUpdate), arg0, arg1, arg2, arg3, arg4) -} - -// Delete mocks base method. -func (m *MockClient) Delete(arg0 context.Context, arg1, arg2, arg3 string) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Delete", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(error) - return ret0 -} - -// Delete indicates an expected call of Delete. -func (mr *MockClientMockRecorder) Delete(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockClient)(nil).Delete), arg0, arg1, arg2, arg3) -} - -// Get mocks base method. -func (m *MockClient) Get(arg0 context.Context, arg1, arg2, arg3 string) (network.Subnet, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Get", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(network.Subnet) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Get indicates an expected call of Get. -func (mr *MockClientMockRecorder) Get(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), arg0, arg1, arg2, arg3) -} diff --git a/azure/services/subnets/mock_subnets/doc.go b/azure/services/subnets/mock_subnets/doc.go index 06eff0f5dc8..81978dd5ddf 100644 --- a/azure/services/subnets/mock_subnets/doc.go +++ b/azure/services/subnets/mock_subnets/doc.go @@ -15,8 +15,6 @@ limitations under the License. */ // Run go generate to regenerate this mock. -//go:generate ../../../../hack/tools/bin/mockgen -destination client_mock.go -package mock_subnets -source ../client.go Client //go:generate ../../../../hack/tools/bin/mockgen -destination subnets_mock.go -package mock_subnets -source ../subnets.go SubnetScope -//go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt client_mock.go > _client_mock.go && mv _client_mock.go client_mock.go" //go:generate /usr/bin/env bash -c "cat ../../../../hack/boilerplate/boilerplate.generatego.txt subnets_mock.go > _subnets_mock.go && mv _subnets_mock.go subnets_mock.go" package mock_subnets //nolint diff --git a/azure/services/subnets/mock_subnets/subnets_mock.go b/azure/services/subnets/mock_subnets/subnets_mock.go index 9026f657dfc..8d787dd8e43 100644 --- a/azure/services/subnets/mock_subnets/subnets_mock.go +++ b/azure/services/subnets/mock_subnets/subnets_mock.go @@ -27,6 +27,7 @@ import ( gomock "github.com/golang/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" ) // MockSubnetScope is a mock of SubnetScope interface. @@ -248,6 +249,18 @@ func (mr *MockSubnetScopeMockRecorder) ControlPlaneSubnet() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ControlPlaneSubnet", reflect.TypeOf((*MockSubnetScope)(nil).ControlPlaneSubnet)) } +// DeleteLongRunningOperationState mocks base method. +func (m *MockSubnetScope) DeleteLongRunningOperationState(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) +} + +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockSubnetScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockSubnetScope)(nil).DeleteLongRunningOperationState), arg0, arg1) +} + // FailureDomains mocks base method. func (m *MockSubnetScope) FailureDomains() []string { m.ctrl.T.Helper() @@ -262,6 +275,20 @@ func (mr *MockSubnetScopeMockRecorder) FailureDomains() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FailureDomains", reflect.TypeOf((*MockSubnetScope)(nil).FailureDomains)) } +// GetLongRunningOperationState mocks base method. +func (m *MockSubnetScope) GetLongRunningOperationState(arg0, arg1 string) *v1beta1.Future { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetLongRunningOperationState", arg0, arg1) + ret0, _ := ret[0].(*v1beta1.Future) + return ret0 +} + +// GetLongRunningOperationState indicates an expected call of GetLongRunningOperationState. +func (mr *MockSubnetScopeMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockSubnetScope)(nil).GetLongRunningOperationState), arg0, arg1) +} + // GetPrivateDNSZoneName mocks base method. func (m *MockSubnetScope) GetPrivateDNSZoneName() string { m.ctrl.T.Helper() @@ -402,6 +429,18 @@ func (mr *MockSubnetScopeMockRecorder) ResourceGroup() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResourceGroup", reflect.TypeOf((*MockSubnetScope)(nil).ResourceGroup)) } +// SetLongRunningOperationState mocks base method. +func (m *MockSubnetScope) SetLongRunningOperationState(arg0 *v1beta1.Future) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) +} + +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockSubnetScopeMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockSubnetScope)(nil).SetLongRunningOperationState), arg0) +} + // SetSubnet mocks base method. func (m *MockSubnetScope) SetSubnet(arg0 v1beta1.SubnetSpec) { m.ctrl.T.Helper() @@ -429,10 +468,10 @@ func (mr *MockSubnetScopeMockRecorder) Subnet(arg0 interface{}) *gomock.Call { } // SubnetSpecs mocks base method. -func (m *MockSubnetScope) SubnetSpecs() []azure.SubnetSpec { +func (m *MockSubnetScope) SubnetSpecs() []azure.ResourceSpecGetter { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "SubnetSpecs") - ret0, _ := ret[0].([]azure.SubnetSpec) + ret0, _ := ret[0].([]azure.ResourceSpecGetter) return ret0 } @@ -484,6 +523,66 @@ func (mr *MockSubnetScopeMockRecorder) TenantID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockSubnetScope)(nil).TenantID)) } +// UpdateDeleteStatus mocks base method. +func (m *MockSubnetScope) UpdateDeleteStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdateDeleteStatus", arg0, arg1, arg2) +} + +// UpdateDeleteStatus indicates an expected call of UpdateDeleteStatus. +func (mr *MockSubnetScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockSubnetScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) +} + +// UpdatePatchStatus mocks base method. +func (m *MockSubnetScope) UpdatePatchStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdatePatchStatus", arg0, arg1, arg2) +} + +// UpdatePatchStatus indicates an expected call of UpdatePatchStatus. +func (mr *MockSubnetScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockSubnetScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockSubnetScope) UpdatePutStatus(arg0 v1beta10.ConditionType, arg1 string, arg2 error) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdatePutStatus", arg0, arg1, arg2) +} + +// UpdatePutStatus indicates an expected call of UpdatePutStatus. +func (mr *MockSubnetScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockSubnetScope)(nil).UpdatePutStatus), arg0, arg1, arg2) +} + +// UpdateSubnetCIDRs mocks base method. +func (m *MockSubnetScope) UpdateSubnetCIDRs(arg0 string, arg1 []string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdateSubnetCIDRs", arg0, arg1) +} + +// UpdateSubnetCIDRs indicates an expected call of UpdateSubnetCIDRs. +func (mr *MockSubnetScopeMockRecorder) UpdateSubnetCIDRs(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateSubnetCIDRs", reflect.TypeOf((*MockSubnetScope)(nil).UpdateSubnetCIDRs), arg0, arg1) +} + +// UpdateSubnetID mocks base method. +func (m *MockSubnetScope) UpdateSubnetID(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "UpdateSubnetID", arg0, arg1) +} + +// UpdateSubnetID indicates an expected call of UpdateSubnetID. +func (mr *MockSubnetScopeMockRecorder) UpdateSubnetID(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateSubnetID", reflect.TypeOf((*MockSubnetScope)(nil).UpdateSubnetID), arg0, arg1) +} + // Vnet mocks base method. func (m *MockSubnetScope) Vnet() *v1beta1.VnetSpec { m.ctrl.T.Helper() diff --git a/azure/services/subnets/spec.go b/azure/services/subnets/spec.go new file mode 100644 index 00000000000..e281412bead --- /dev/null +++ b/azure/services/subnets/spec.go @@ -0,0 +1,104 @@ +/* +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 subnets + +import ( + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" + "github.com/Azure/go-autorest/autorest/to" + "github.com/pkg/errors" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" + "sigs.k8s.io/cluster-api-provider-azure/azure" +) + +// SubnetSpec defines the specification for a Subnet. +type SubnetSpec struct { + Name string + ResourceGroup string + SubscriptionID string + CIDRs []string + VNetName string + VNetResourceGroup string + IsVNetManaged bool + RouteTableName string + SecurityGroupName string + Role infrav1.SubnetRole + NatGatewayName string +} + +// ResourceName returns the name of the subnet. +func (s *SubnetSpec) ResourceName() string { + return s.Name +} + +// ResourceGroupName returns the name of the resource group of the VNet that owns this subnet. +func (s *SubnetSpec) ResourceGroupName() string { + return s.VNetResourceGroup +} + +// OwnerResourceName returns the name of the VNet that owns this subnet. +func (s *SubnetSpec) OwnerResourceName() string { + return s.VNetName +} + +// Parameters returns the parameters for the subnet. +func (s *SubnetSpec) Parameters(existing interface{}) (parameters interface{}, err error) { + if existing != nil { + _, ok := existing.(network.Subnet) + if !ok { + return nil, errors.Errorf("%T is not a network.Subnet", existing) + } + + return nil, nil + } + + if !s.IsVNetManaged { + // TODO: change this to terminal error once we add support for handling them + return nil, errors.Errorf("custom vnet was provided but subnet %s is missing", s.Name) + } + subnetProperties := network.SubnetPropertiesFormat{ + AddressPrefixes: &s.CIDRs, + } + + // workaround needed to avoid SubscriptionNotRegisteredForFeature for feature Microsoft.Network/AllowMultipleAddressPrefixesOnSubnet. + if len(s.CIDRs) == 1 { + subnetProperties = network.SubnetPropertiesFormat{ + AddressPrefix: &s.CIDRs[0], + } + } + + if s.RouteTableName != "" { + subnetProperties.RouteTable = &network.RouteTable{ + ID: to.StringPtr(azure.RouteTableID(s.SubscriptionID, s.ResourceGroup, s.RouteTableName)), + } + } + + if s.NatGatewayName != "" { + subnetProperties.NatGateway = &network.SubResource{ + ID: to.StringPtr(azure.NatGatewayID(s.SubscriptionID, s.ResourceGroup, s.NatGatewayName)), + } + } + + if s.SecurityGroupName != "" { + subnetProperties.NetworkSecurityGroup = &network.SecurityGroup{ + ID: to.StringPtr(azure.SecurityGroupID(s.SubscriptionID, s.ResourceGroup, s.SecurityGroupName)), + } + } + + return network.Subnet{ + SubnetPropertiesFormat: &subnetProperties, + }, nil +} diff --git a/azure/services/subnets/spec_test.go b/azure/services/subnets/spec_test.go new file mode 100644 index 00000000000..750a7b755bc --- /dev/null +++ b/azure/services/subnets/spec_test.go @@ -0,0 +1,213 @@ +/* +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 subnets + +import ( + "testing" + + "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" + "github.com/Azure/go-autorest/autorest/to" + . "github.com/onsi/gomega" + infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" +) + +var ( + fakeSubnetOneCidrSpec = SubnetSpec{ + Name: "my-subnet-1", + ResourceGroup: "my-rg", + SubscriptionID: "123", + CIDRs: []string{"10.0.0.0/16"}, + IsVNetManaged: true, + VNetName: "my-vnet", + VNetResourceGroup: "my-rg", + RouteTableName: "my-subnet_route_table", + SecurityGroupName: "my-sg", + NatGatewayName: "my-nat-gateway", + Role: infrav1.SubnetNode, + } + + fakeSubnetOneCidrParams = network.Subnet{ + SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ + AddressPrefix: to.StringPtr("10.0.0.0/16"), + RouteTable: &network.RouteTable{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/routeTables/my-subnet_route_table")}, + NetworkSecurityGroup: &network.SecurityGroup{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkSecurityGroups/my-sg")}, + NatGateway: &network.SubResource{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/natGateways/my-nat-gateway")}, + }, + } + + fakeSubnetMultipleCidrSpec = SubnetSpec{ + Name: "my-subnet-1", + ResourceGroup: "my-rg", + SubscriptionID: "123", + CIDRs: []string{"10.0.0.0/16", "10.1.0.0/16", "10.2.0.0/16"}, + IsVNetManaged: true, + VNetName: "my-vnet", + VNetResourceGroup: "my-rg", + RouteTableName: "my-subnet_route_table", + SecurityGroupName: "my-sg", + NatGatewayName: "my-nat-gateway", + Role: infrav1.SubnetNode, + } + + fakeSubnetMultipleCidrParams = network.Subnet{ + SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ + AddressPrefixes: &[]string{ + "10.0.0.0/16", + "10.1.0.0/16", + "10.2.0.0/16", + }, + RouteTable: &network.RouteTable{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/routeTables/my-subnet_route_table")}, + NetworkSecurityGroup: &network.SecurityGroup{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkSecurityGroups/my-sg")}, + NatGateway: &network.SubResource{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/natGateways/my-nat-gateway")}, + }, + } + + fakeSubnetSpecNotManaged = SubnetSpec{ + Name: "my-subnet-1", + ResourceGroup: "my-rg", + SubscriptionID: "123", + CIDRs: []string{"10.0.0.0/16"}, + IsVNetManaged: false, + VNetName: "my-vnet", + VNetResourceGroup: "my-vnet-rg", + RouteTableName: "my-subnet_route_table", + SecurityGroupName: "my-sg-1", + Role: infrav1.SubnetNode, + } + fakeSubnetNotManaged = network.Subnet{ + ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/virtualNetworks/my-vnet/subnets/my-subnet-1"), + Name: to.StringPtr("my-subnet-1"), + SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ + AddressPrefix: to.StringPtr("10.0.0.0/16"), + RouteTable: &network.RouteTable{ + ID: to.StringPtr("rt-id"), + Name: to.StringPtr("my-subnet_route_table"), + }, + NetworkSecurityGroup: &network.SecurityGroup{ + ID: to.StringPtr("sg-id-1"), + Name: to.StringPtr("my-sg-1"), + }, + }, + } + + fakeIpv6SubnetSpecNotManaged = SubnetSpec{ + Name: "my-ipv6-subnet", + ResourceGroup: "my-rg", + SubscriptionID: "123", + CIDRs: []string{"10.0.0.0/16", "2001:1234:5678:9abd::/64"}, + IsVNetManaged: false, + VNetName: "my-vnet", + VNetResourceGroup: "my-vnet-rg", + RouteTableName: "my-subnet_route_table", + SecurityGroupName: "my-sg", + Role: infrav1.SubnetNode, + } + + fakeIpv6SubnetNotManaged = network.Subnet{ + ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/virtualNetworks/my-vnet/subnets/my-ipv6-subnet"), + Name: to.StringPtr("my-ipv6-subnet"), + SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ + AddressPrefixes: &[]string{ + "10.0.0.0/16", + "2001:1234:5678:9abd::/64", + }, + RouteTable: &network.RouteTable{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/routeTables/my-subnet_route_table")}, + NetworkSecurityGroup: &network.SecurityGroup{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkSecurityGroups/my-sg")}, + }, + } +) + +func TestParameters(t *testing.T) { + testcases := []struct { + name string + spec *SubnetSpec + existing interface{} + expect func(g *WithT, result interface{}) + expectedError string + }{ + { + name: "get parameters for subnet with one cidr block", + spec: &fakeSubnetOneCidrSpec, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(Equal(fakeSubnetOneCidrParams)) + }, + expectedError: "", + }, + { + name: "get parameters for subnet with multiple cidr blocks", + spec: &fakeSubnetMultipleCidrSpec, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(Equal(fakeSubnetMultipleCidrParams)) + }, + expectedError: "", + }, + { + name: "error vnet is not managed but subnet is missing", + spec: &fakeSubnetSpecNotManaged, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "custom vnet was provided but subnet my-subnet-1 is missing", + }, + { + name: "vnet is not managed and subnet is present", + spec: &fakeSubnetSpecNotManaged, + existing: fakeSubnetNotManaged, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "", + }, + { + name: "error vnet is not managed but ipv6 subnet is missing", + spec: &fakeIpv6SubnetSpecNotManaged, + existing: nil, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "custom vnet was provided but subnet my-ipv6-subnet is missing", + }, + { + name: "vnet is not managed and ipv6 subnet is present", + spec: &fakeIpv6SubnetSpecNotManaged, + existing: fakeIpv6SubnetNotManaged, + expect: func(g *WithT, result interface{}) { + g.Expect(result).To(BeNil()) + }, + expectedError: "", + }, + } + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + t.Parallel() + + result, err := tc.spec.Parameters(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) + }) + } +} diff --git a/azure/services/subnets/subnets.go b/azure/services/subnets/subnets.go index 162f70b7f04..d00d4511ac7 100644 --- a/azure/services/subnets/subnets.go +++ b/azure/services/subnets/subnets.go @@ -18,103 +18,80 @@ package subnets import ( "context" - "fmt" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" "github.com/Azure/go-autorest/autorest/to" "github.com/pkg/errors" 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" ) +const serviceName = "subnets" + // SubnetScope defines the scope interface for a subnet service. type SubnetScope interface { azure.ClusterScoper - SubnetSpecs() []azure.SubnetSpec + azure.AsyncStatusUpdater + UpdateSubnetID(string, string) + UpdateSubnetCIDRs(string, []string) + SubnetSpecs() []azure.ResourceSpecGetter } // Service provides operations on Azure resources. type Service struct { Scope SubnetScope - Client + async.Reconciler } // New creates a new service. func New(scope SubnetScope) *Service { + Client := NewClient(scope) return &Service{ - Scope: scope, - Client: NewClient(scope), + Scope: scope, + Reconciler: async.New(scope, Client, Client), } } // Reconcile gets/creates/updates a subnet. func (s *Service) Reconcile(ctx context.Context) error { - ctx, log, done := tele.StartSpanWithLogger(ctx, "subnets.Service.Reconcile") + ctx, _, done := tele.StartSpanWithLogger(ctx, "subnets.Service.Reconcile") defer done() - for _, subnetSpec := range s.Scope.SubnetSpecs() { - existingSubnet, err := s.getExisting(ctx, s.Scope.Vnet().ResourceGroup, subnetSpec) - switch { - case err != nil && !azure.ResourceNotFound(err): - return errors.Wrapf(err, "failed to get subnet %s", subnetSpec.Name) - case err == nil: - // subnet already exists, update the spec and skip creation - s.Scope.SetSubnet(*existingSubnet) - continue - - case !s.Scope.IsVnetManaged(): - return fmt.Errorf("vnet was provided but subnet %s is missing", subnetSpec.Name) - - default: - - subnetProperties := network.SubnetPropertiesFormat{ - AddressPrefixes: &subnetSpec.CIDRs, - } - - // workaround needed to avoid SubscriptionNotRegisteredForFeature for feature Microsoft.Network/AllowMultipleAddressPrefixesOnSubnet. - if len(subnetSpec.CIDRs) == 1 { - subnetProperties = network.SubnetPropertiesFormat{ - AddressPrefix: &subnetSpec.CIDRs[0], - } - } - - if subnetSpec.RouteTableName != "" { - subnetProperties.RouteTable = &network.RouteTable{ - ID: to.StringPtr(azure.RouteTableID(s.Scope.SubscriptionID(), s.Scope.ResourceGroup(), subnetSpec.RouteTableName)), - } - } + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() - if subnetSpec.NatGatewayName != "" { - subnetProperties.NatGateway = &network.SubResource{ - ID: to.StringPtr(azure.NatGatewayID(s.Scope.SubscriptionID(), s.Scope.ResourceGroup(), subnetSpec.NatGatewayName)), - } + // We go through the list of SubnetSpecs to reconcile each one, independently of the result 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 (i.e. error creating) -> operationNotDoneError (i.e. creating in progress) -> no error (i.e. created) + var resultErr error + for _, subnetSpec := range s.Scope.SubnetSpecs() { + result, err := s.CreateResource(ctx, subnetSpec, serviceName) + if err != nil { + if !azure.IsOperationNotDoneError(err) || resultErr == nil { + resultErr = err } - - if subnetSpec.SecurityGroupName != "" { - subnetProperties.NetworkSecurityGroup = &network.SecurityGroup{ - ID: to.StringPtr(azure.SecurityGroupID(s.Scope.SubscriptionID(), s.Scope.ResourceGroup(), subnetSpec.SecurityGroupName)), - } + } else { + subnet, ok := result.(network.Subnet) + if !ok { + return errors.Errorf("%T is not a network.Subnet", result) } - - log.V(2).Info("creating subnet in vnet", "subnet", subnetSpec.Name, "vnet", subnetSpec.VNetName) - err = s.Client.CreateOrUpdate( - ctx, - s.Scope.Vnet().ResourceGroup, - subnetSpec.VNetName, - subnetSpec.Name, - network.Subnet{ - SubnetPropertiesFormat: &subnetProperties, - }, - ) - if err != nil { - return errors.Wrapf(err, "failed to create subnet %s in resource group %s", subnetSpec.Name, s.Scope.Vnet().ResourceGroup) + var addresses []string + if subnet.SubnetPropertiesFormat != nil && subnet.SubnetPropertiesFormat.AddressPrefix != nil { + addresses = []string{to.String(subnet.SubnetPropertiesFormat.AddressPrefix)} + } else if subnet.SubnetPropertiesFormat != nil && subnet.SubnetPropertiesFormat.AddressPrefixes != nil { + addresses = to.StringSlice(subnet.SubnetPropertiesFormat.AddressPrefixes) } - log.V(2).Info("successfully created subnet in vnet", "subnet", subnetSpec.Name, "vnet", subnetSpec.VNetName) + s.Scope.UpdateSubnetID(subnetSpec.ResourceName(), to.String(subnet.ID)) + s.Scope.UpdateSubnetCIDRs(subnetSpec.ResourceName(), addresses) } } - return nil + + s.Scope.UpdatePutStatus(infrav1.SubnetsReadyCondition, serviceName, resultErr) + return resultErr } // Delete deletes the subnet with the provided name. @@ -122,46 +99,28 @@ func (s *Service) Delete(ctx context.Context) error { ctx, log, done := tele.StartSpanWithLogger(ctx, "subnets.Service.Delete") defer done() - for _, subnetSpec := range s.Scope.SubnetSpecs() { - if !s.Scope.Vnet().IsManaged(s.Scope.ClusterName()) { - log.V(4).Info("Skipping subnets deletion in custom vnet mode") - continue - } - log.V(2).Info("deleting subnet in vnet", "subnet", subnetSpec.Name, "vnet", subnetSpec.VNetName) - err := s.Client.Delete(ctx, s.Scope.Vnet().ResourceGroup, subnetSpec.VNetName, subnetSpec.Name) - if err != nil && azure.ResourceNotFound(err) { - // already deleted - continue - } - if err != nil { - return errors.Wrapf(err, "failed to delete subnet %s in resource group %s", subnetSpec.Name, s.Scope.Vnet().ResourceGroup) - } + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() - log.V(2).Info("successfully deleted subnet in vnet", "subnet", subnetSpec.Name, "vnet", subnetSpec.VNetName) - } - return nil -} - -// getExisting provides information about an existing subnet. -func (s *Service) getExisting(ctx context.Context, rgName string, spec azure.SubnetSpec) (*infrav1.SubnetSpec, error) { - ctx, _, done := tele.StartSpanWithLogger(ctx, "subnets.Service.getExisting") - defer done() + if !s.Scope.IsVnetManaged() { + log.V(4).Info("Skipping subnets deletion in custom vnet mode") - subnet, err := s.Client.Get(ctx, rgName, spec.VNetName, spec.Name) - if err != nil { - return nil, errors.Wrapf(err, "failed to fetch subnet named %s in vnet %s", spec.VNetName, spec.Name) + s.Scope.UpdateDeleteStatus(infrav1.SubnetsReadyCondition, serviceName, nil) + return nil } - var addresses []string - if subnet.SubnetPropertiesFormat != nil && subnet.SubnetPropertiesFormat.AddressPrefix != nil { - addresses = []string{to.String(subnet.SubnetPropertiesFormat.AddressPrefix)} - } else if subnet.SubnetPropertiesFormat != nil && subnet.SubnetPropertiesFormat.AddressPrefixes != nil { - addresses = to.StringSlice(subnet.SubnetPropertiesFormat.AddressPrefixes) - } - - subnetSpec := s.Scope.Subnet(spec.Name) - subnetSpec.ID = to.String(subnet.ID) - subnetSpec.CIDRBlocks = addresses + var result error - return &subnetSpec, nil + // We go through the list of SubnetSpecs to delete each one, independently of the result 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 (i.e. error deleting) -> operationNotDoneError (i.e. deleting in progress) -> no error (i.e. deleted) + for _, subnetSpec := range s.Scope.SubnetSpecs() { + if err := s.DeleteResource(ctx, subnetSpec, serviceName); err != nil { + if !azure.IsOperationNotDoneError(err) || result == nil { + result = err + } + } + } + s.Scope.UpdateDeleteStatus(infrav1.SubnetsReadyCondition, serviceName, result) + return result } diff --git a/azure/services/subnets/subnets_test.go b/azure/services/subnets/subnets_test.go index 1a73c83fd60..32ddb495993 100644 --- a/azure/services/subnets/subnets_test.go +++ b/azure/services/subnets/subnets_test.go @@ -28,468 +28,225 @@ import ( . "github.com/onsi/gomega" 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/subnets/mock_subnets" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" ) +var ( + fakeSubnetSpec1 = SubnetSpec{ + Name: "my-subnet-1", + ResourceGroup: "my-rg", + SubscriptionID: "123", + CIDRs: []string{"10.0.0.0/16"}, + IsVNetManaged: true, + VNetName: "my-vnet", + VNetResourceGroup: "my-rg", + RouteTableName: "my-subnet_route_table", + SecurityGroupName: "my-sg-1", + Role: infrav1.SubnetNode, + } + + fakeSubnet1 = network.Subnet{ + ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/virtualNetworks/my-vnet/subnets/my-subnet-1"), + Name: to.StringPtr("my-subnet-1"), + SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ + AddressPrefix: to.StringPtr("10.0.0.0/16"), + RouteTable: &network.RouteTable{ + ID: to.StringPtr("rt-id"), + Name: to.StringPtr("my-subnet_route_table"), + }, + NetworkSecurityGroup: &network.SecurityGroup{ + ID: to.StringPtr("sg-id-1"), + Name: to.StringPtr("my-sg-1"), + }, + }, + } + + fakeSubnetSpec2 = SubnetSpec{ + Name: "my-subnet-2", + ResourceGroup: "my-rg", + SubscriptionID: "123", + CIDRs: []string{"10.2.0.0/16"}, + IsVNetManaged: true, + VNetName: "my-vnet", + VNetResourceGroup: "my-rg", + RouteTableName: "my-subnet_route_table", + SecurityGroupName: "my-sg-2", + Role: infrav1.SubnetNode, + } + + fakeSubnet2 = network.Subnet{ + ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/virtualNetworks/my-vnet/subnets/my-subnet-2"), + Name: to.StringPtr("my-subnet-2"), + SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ + AddressPrefix: to.StringPtr("10.2.0.0/16"), + RouteTable: &network.RouteTable{ + ID: to.StringPtr("rt-id"), + Name: to.StringPtr("my-subnet_route_table"), + }, + NetworkSecurityGroup: &network.SecurityGroup{ + ID: to.StringPtr("sg-id-2"), + Name: to.StringPtr("my-sg-2"), + }, + }, + } + + fakeCtrlPlaneSubnetSpec = SubnetSpec{ + Name: "my-subnet-ctrl-plane", + ResourceGroup: "my-rg", + SubscriptionID: "123", + CIDRs: []string{"10.1.0.0/16"}, + IsVNetManaged: true, + VNetName: "my-vnet", + VNetResourceGroup: "my-rg", + RouteTableName: "my-subnet_route_table", + SecurityGroupName: "my-sg", + Role: infrav1.SubnetControlPlane, + } + + fakeIpv6SubnetSpec = SubnetSpec{ + Name: "my-ipv6-subnet", + ResourceGroup: "my-rg", + SubscriptionID: "123", + CIDRs: []string{"10.0.0.0/16", "2001:1234:5678:9abd::/64"}, + IsVNetManaged: true, + VNetName: "my-vnet", + VNetResourceGroup: "my-rg", + RouteTableName: "my-subnet_route_table", + SecurityGroupName: "my-sg", + Role: infrav1.SubnetNode, + } + + fakeIpv6Subnet = network.Subnet{ + ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/virtualNetworks/my-vnet/subnets/my-ipv6-subnet"), + Name: to.StringPtr("my-ipv6-subnet"), + SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ + AddressPrefixes: &[]string{ + "10.0.0.0/16", + "2001:1234:5678:9abd::/64", + }, + RouteTable: &network.RouteTable{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/routeTables/my-subnet_route_table")}, + NetworkSecurityGroup: &network.SecurityGroup{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkSecurityGroups/my-sg")}, + }, + } + + fakeIpv6SubnetSpecCP = SubnetSpec{ + Name: "my-ipv6-subnet-cp", + ResourceGroup: "my-rg", + SubscriptionID: "123", + CIDRs: []string{"10.2.0.0/16", "2001:1234:5678:9abc::/64"}, + IsVNetManaged: true, + VNetName: "my-vnet", + VNetResourceGroup: "my-rg", + RouteTableName: "my-subnet_route_table", + SecurityGroupName: "my-sg", + Role: infrav1.SubnetNode, + } + + fakeIpv6SubnetCP = network.Subnet{ + ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/virtualNetworks/my-vnet/subnets/my-ipv6-subnet-cp"), + Name: to.StringPtr("my-ipv6-subnet-cp"), + SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ + AddressPrefixes: &[]string{ + "10.2.0.0/16", + "2001:1234:5678:9abc::/64", + }, + RouteTable: &network.RouteTable{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/routeTables/my-subnet_route_table")}, + NetworkSecurityGroup: &network.SecurityGroup{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkSecurityGroups/my-sg")}, + }, + } + + internalError = autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error") +) + func TestReconcileSubnets(t *testing.T) { testcases := []struct { name string expectedError string - expect func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) + expect func(s *mock_subnets.MockSubnetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) }{ { - name: "subnet does not exist", + name: "create subnet", expectedError: "", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.SubnetSpecs().Return([]azure.SubnetSpec{ - { - Name: "my-subnet", - CIDRs: []string{"10.0.0.0/16"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "my-vnet"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.IsIPv6Enabled().AnyTimes().Return(false) - s.IsVnetManaged().Return(true) - m.Get(gomockinternal.AContext(), "", "my-vnet", "my-subnet"). - Return(network.Subnet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - m.CreateOrUpdate(gomockinternal.AContext(), "", "my-vnet", "my-subnet", gomockinternal.DiffEq(network.Subnet{ - SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ - AddressPrefix: to.StringPtr("10.0.0.0/16"), - NetworkSecurityGroup: &network.SecurityGroup{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkSecurityGroups/my-sg")}, - RouteTable: &network.RouteTable{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/routeTables/my-subnet_route_table")}, - }, - })) + expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.SubnetSpecs().Return([]azure.ResourceSpecGetter{&fakeSubnetSpec1}) + + r.CreateResource(gomockinternal.AContext(), &fakeSubnetSpec1, serviceName).Return(fakeSubnet1, nil) + s.UpdateSubnetID(fakeSubnetSpec1.Name, to.String(fakeSubnet1.ID)) + s.UpdateSubnetCIDRs(fakeSubnetSpec1.Name, []string{to.String(fakeSubnet1.AddressPrefix)}) + + s.UpdatePutStatus(infrav1.SubnetsReadyCondition, serviceName, nil) }, }, { - name: "subnet ipv6 does not exist", + name: "create multiple subnets", expectedError: "", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.SubnetSpecs().Return([]azure.SubnetSpec{ - { - Name: "my-ipv6-subnet", - CIDRs: []string{"10.0.0.0/16", "2001:1234:5678:9abd::/64"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "my-vnet", ResourceGroup: "my-rg"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.SubscriptionID().AnyTimes().Return("123") - s.IsVnetManaged().Return(true) - m.Get(gomockinternal.AContext(), "my-rg", "my-vnet", "my-ipv6-subnet"). - Return(network.Subnet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - m.CreateOrUpdate(gomockinternal.AContext(), "my-rg", "my-vnet", "my-ipv6-subnet", gomockinternal.DiffEq(network.Subnet{ - SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ - AddressPrefixes: &[]string{ - "10.0.0.0/16", - "2001:1234:5678:9abd::/64", - }, - RouteTable: &network.RouteTable{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/routeTables/my-subnet_route_table")}, - NetworkSecurityGroup: &network.SecurityGroup{ID: to.StringPtr("/subscriptions/123/resourceGroups/my-rg/providers/Microsoft.Network/networkSecurityGroups/my-sg")}, - }, - })) - }, - }, - { - name: "fail to create subnet", - expectedError: "failed to create subnet my-subnet in resource group : #: Internal Server Error: StatusCode=500", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.SubnetSpecs().Return([]azure.SubnetSpec{ - { - Name: "my-subnet", - CIDRs: []string{"10.0.0.0/16"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "my-vnet"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - s.IsIPv6Enabled().AnyTimes().Return(false) - s.IsVnetManaged().Return(true) - m.Get(gomockinternal.AContext(), "", "my-vnet", "my-subnet"). - Return(network.Subnet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) - m.CreateOrUpdate(gomockinternal.AContext(), "", "my-vnet", "my-subnet", gomock.AssignableToTypeOf(network.Subnet{})).Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) - }, - }, - { - name: "fail to get existing subnet", - expectedError: "failed to get subnet my-subnet: failed to fetch subnet named my-vnet in vnet my-subnet: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.SubnetSpecs().Return([]azure.SubnetSpec{ - { - Name: "my-subnet", - CIDRs: []string{"10.0.0.0/16"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "my-vnet"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Get(gomockinternal.AContext(), "", "my-vnet", "my-subnet"). - Return(network.Subnet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) - }, - }, - { - name: "vnet was provided but subnet is missing", - expectedError: "vnet was provided but subnet my-subnet is missing", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.SubnetSpecs().Return([]azure.SubnetSpec{ - { - Name: "my-subnet", - CIDRs: []string{"10.0.0.0/16"}, - VNetName: "custom-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{ResourceGroup: "custom-vnet-rg", Name: "custom-vnet", ID: "id1", - VnetClassSpec: infrav1.VnetClassSpec{ - Tags: infrav1.Tags{ - "Name": "vnet-exists", - "sigs.k8s.io_cluster-api-provider-azure_cluster_test-cluster": "not", - "sigs.k8s.io_cluster-api-provider-azure_role": "dd", - }, - }, - }) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("custom-vnet-rg") - s.IsVnetManaged().Return(false) - m.Get(gomockinternal.AContext(), "custom-vnet-rg", "custom-vnet", "my-subnet"). - Return(network.Subnet{}, autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 404}, "Not found")) + expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.SubnetSpecs().Return([]azure.ResourceSpecGetter{&fakeSubnetSpec1, &fakeSubnetSpec2}) + + r.CreateResource(gomockinternal.AContext(), &fakeSubnetSpec1, serviceName).Return(fakeSubnet1, nil) + s.UpdateSubnetID(fakeSubnetSpec1.Name, to.String(fakeSubnet1.ID)) + s.UpdateSubnetCIDRs(fakeSubnetSpec1.Name, []string{to.String(fakeSubnet1.AddressPrefix)}) + + r.CreateResource(gomockinternal.AContext(), &fakeSubnetSpec2, serviceName).Return(fakeSubnet2, nil) + s.UpdateSubnetID(fakeSubnetSpec2.Name, to.String(fakeSubnet2.ID)) + s.UpdateSubnetCIDRs(fakeSubnetSpec2.Name, []string{to.String(fakeSubnet2.AddressPrefix)}) + + s.UpdatePutStatus(infrav1.SubnetsReadyCondition, serviceName, nil) }, }, { - name: "vnet was provided and subnet exists", + name: "create ipv6 subnet", expectedError: "", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.Subnet("my-subnet").AnyTimes().Return(infrav1.SubnetSpec{ - ID: "subnet-id", - Name: "my-subnet", - SubnetClassSpec: infrav1.SubnetClassSpec{ - Role: infrav1.SubnetNode, - CIDRBlocks: []string{"10.0.0.0/16"}, - }, - }) - s.Subnet("my-subnet-1").AnyTimes().Return(infrav1.SubnetSpec{ - ID: "subnet-id-1", - Name: "my-subnet-1", - SubnetClassSpec: infrav1.SubnetClassSpec{ - Role: infrav1.SubnetControlPlane, - CIDRBlocks: []string{"10.2.0.0/16"}, - }, - }) - s.SubnetSpecs().AnyTimes().Return([]azure.SubnetSpec{ - { - Name: "my-subnet", - CIDRs: []string{"10.0.0.0/16"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - { - Name: "my-subnet-1", - CIDRs: []string{"10.2.0.0/16"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg-1", - Role: infrav1.SubnetControlPlane, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "my-vnet"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Get(gomockinternal.AContext(), "", "my-vnet", "my-subnet"). - Return(network.Subnet{ - ID: to.StringPtr("subnet-id"), - Name: to.StringPtr("my-subnet"), - SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ - AddressPrefix: to.StringPtr("10.0.0.0/16"), - RouteTable: &network.RouteTable{ - ID: to.StringPtr("rt-id"), - Name: to.StringPtr("my-subnet_route_table"), - }, - NetworkSecurityGroup: &network.SecurityGroup{ - ID: to.StringPtr("sg-id"), - Name: to.StringPtr("my-sg"), - }, - }, - }, nil) - s.SetSubnet(infrav1.SubnetSpec{ - ID: "subnet-id", - Name: "my-subnet", - SubnetClassSpec: infrav1.SubnetClassSpec{ - Role: infrav1.SubnetNode, - CIDRBlocks: []string{"10.0.0.0/16"}, - }, - }).Times(1) - m.Get(gomockinternal.AContext(), "", "my-vnet", "my-subnet-1"). - Return(network.Subnet{ - ID: to.StringPtr("subnet-id-1"), - Name: to.StringPtr("my-subnet-1"), - SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ - AddressPrefix: to.StringPtr("10.2.0.0/16"), - RouteTable: &network.RouteTable{ - ID: to.StringPtr("rt-id"), - Name: to.StringPtr("my-subnet_route_table"), - }, - NetworkSecurityGroup: &network.SecurityGroup{ - ID: to.StringPtr("sg-id"), - Name: to.StringPtr("my-sg-1"), - }, - }, - }, nil) - s.SetSubnet(infrav1.SubnetSpec{ - ID: "subnet-id-1", - Name: "my-subnet-1", - SubnetClassSpec: infrav1.SubnetClassSpec{ - Role: infrav1.SubnetControlPlane, - CIDRBlocks: []string{"10.2.0.0/16"}, - }, - }).Times(1) + expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.SubnetSpecs().Return([]azure.ResourceSpecGetter{&fakeIpv6SubnetSpec}) + + r.CreateResource(gomockinternal.AContext(), &fakeIpv6SubnetSpec, serviceName).Return(fakeIpv6Subnet, nil) + s.UpdateSubnetID(fakeIpv6SubnetSpec.Name, to.String(fakeIpv6Subnet.ID)) + s.UpdateSubnetCIDRs(fakeIpv6SubnetSpec.Name, to.StringSlice(fakeIpv6Subnet.AddressPrefixes)) + + s.UpdatePutStatus(infrav1.SubnetsReadyCondition, serviceName, nil) }, }, { - name: "vnet for ipv6 is provided", + name: "create multiple ipv6 subnets", expectedError: "", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.Subnet("my-ipv6-subnet").AnyTimes().Return(infrav1.SubnetSpec{ - ID: "subnet-id", - Name: "my-ipv6-subnet", - SubnetClassSpec: infrav1.SubnetClassSpec{ - Role: infrav1.SubnetNode, - CIDRBlocks: []string{"10.0.0.0/16", "2001:1234:5678:9abd::/64"}, - }, - }) - s.Subnet("my-ipv6-subnet-cp").AnyTimes().Return(infrav1.SubnetSpec{ - ID: "subnet-id-1", - Name: "my-ipv6-subnet-cp", - SubnetClassSpec: infrav1.SubnetClassSpec{ - Role: infrav1.SubnetControlPlane, - CIDRBlocks: []string{"10.2.0.0/16", "2001:1234:5678:9abc::/64"}, - }, - }) - s.SubnetSpecs().AnyTimes().Return([]azure.SubnetSpec{ - { - Name: "my-ipv6-subnet", - CIDRs: []string{"10.0.0.0/16", "2001:1234:5678:9abd::/64"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - { - Name: "my-ipv6-subnet-cp", - CIDRs: []string{"10.2.0.0/16", "2001:1234:5678:9abc::/64"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg-1", - Role: infrav1.SubnetControlPlane, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "my-vnet"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.IsIPv6Enabled().AnyTimes().Return(true) - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Get(gomockinternal.AContext(), "", "my-vnet", "my-ipv6-subnet"). - Return(network.Subnet{ - ID: to.StringPtr("subnet-id"), - Name: to.StringPtr("my-ipv6-subnet"), - SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ - AddressPrefixes: &[]string{ - "10.0.0.0/16", - "2001:1234:5678:9abd::/64", - }, - RouteTable: &network.RouteTable{ - ID: to.StringPtr("rt-id"), - Name: to.StringPtr("my-subnet_route_table"), - }, - NetworkSecurityGroup: &network.SecurityGroup{ - ID: to.StringPtr("sg-id"), - Name: to.StringPtr("my-sg"), - }, - }, - }, nil) - s.SetSubnet(infrav1.SubnetSpec{ - ID: "subnet-id", - Name: "my-ipv6-subnet", - SubnetClassSpec: infrav1.SubnetClassSpec{ - Role: infrav1.SubnetNode, - CIDRBlocks: []string{"10.0.0.0/16", "2001:1234:5678:9abd::/64"}, - }, - }).Times(1) - m.Get(gomockinternal.AContext(), "", "my-vnet", "my-ipv6-subnet-cp"). - Return(network.Subnet{ - ID: to.StringPtr("subnet-id-1"), - Name: to.StringPtr("my-ipv6-subnet-cp"), - SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ - AddressPrefixes: &[]string{ - "10.2.0.0/16", - "2001:1234:5678:9abc::/64", - }, - RouteTable: &network.RouteTable{ - ID: to.StringPtr("rt-id"), - Name: to.StringPtr("my-subnet_route_table"), - }, - NetworkSecurityGroup: &network.SecurityGroup{ - ID: to.StringPtr("sg-id"), - Name: to.StringPtr("my-sg-1"), - }, - }, - }, nil) - s.SetSubnet(infrav1.SubnetSpec{ - ID: "subnet-id-1", - Name: "my-ipv6-subnet-cp", - SubnetClassSpec: infrav1.SubnetClassSpec{ - Role: infrav1.SubnetControlPlane, - CIDRBlocks: []string{"10.2.0.0/16", "2001:1234:5678:9abc::/64"}, - }, - }).Times(1) + expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.SubnetSpecs().Return([]azure.ResourceSpecGetter{&fakeIpv6SubnetSpec, &fakeIpv6SubnetSpecCP}) + + r.CreateResource(gomockinternal.AContext(), &fakeIpv6SubnetSpec, serviceName).Return(fakeIpv6Subnet, nil) + s.UpdateSubnetID(fakeIpv6SubnetSpec.Name, to.String(fakeIpv6Subnet.ID)) + s.UpdateSubnetCIDRs(fakeIpv6SubnetSpec.Name, to.StringSlice(fakeIpv6Subnet.AddressPrefixes)) + + r.CreateResource(gomockinternal.AContext(), &fakeIpv6SubnetSpecCP, serviceName).Return(fakeIpv6SubnetCP, nil) + s.UpdateSubnetID(fakeIpv6SubnetSpecCP.Name, to.String(fakeIpv6SubnetCP.ID)) + s.UpdateSubnetCIDRs(fakeIpv6SubnetSpecCP.Name, to.StringSlice(fakeIpv6SubnetCP.AddressPrefixes)) + + s.UpdatePutStatus(infrav1.SubnetsReadyCondition, serviceName, nil) }, }, { - name: "doesn't overwrite existing NAT gateway", - expectedError: "", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.Subnet("my-subnet").AnyTimes().Return(infrav1.SubnetSpec{ - ID: "subnet-id", - Name: "my-subnet", - SubnetClassSpec: infrav1.SubnetClassSpec{ - Role: infrav1.SubnetNode, - CIDRBlocks: []string{"10.0.0.0/16"}, - }, - NatGateway: infrav1.NatGateway{ - ID: azure.NatGatewayID("123", "my-rg", "existing-natgateway"), - Name: "existing-natgateway", - NatGatewayIP: infrav1.PublicIPSpec{ - Name: "existing-natgateway-ip-name", - }, - }, - }) - s.SubnetSpecs().AnyTimes().Return([]azure.SubnetSpec{ - { - Name: "my-subnet", - CIDRs: []string{"10.0.0.0/16"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "my-vnet"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Get(gomockinternal.AContext(), "", "my-vnet", "my-subnet"). - Return(network.Subnet{ - ID: to.StringPtr("subnet-id"), - Name: to.StringPtr("my-subnet"), - SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ - AddressPrefix: to.StringPtr("10.0.0.0/16"), - RouteTable: &network.RouteTable{ - ID: to.StringPtr("rt-id"), - Name: to.StringPtr("my-subnet_route_table"), - }, - NetworkSecurityGroup: &network.SecurityGroup{ - ID: to.StringPtr("sg-id"), - Name: to.StringPtr("my-sg"), - }, - NatGateway: &network.SubResource{ - ID: to.StringPtr(azure.NatGatewayID("123", "my-rg", "existing-natgateway")), - }, - }, - }, nil) - s.SetSubnet(infrav1.SubnetSpec{ - ID: "subnet-id", - Name: "my-subnet", - SubnetClassSpec: infrav1.SubnetClassSpec{ - Role: infrav1.SubnetNode, - CIDRBlocks: []string{"10.0.0.0/16"}, - }, - NatGateway: infrav1.NatGateway{ - ID: azure.NatGatewayID("123", "my-rg", "existing-natgateway"), - Name: "existing-natgateway", - NatGatewayIP: infrav1.PublicIPSpec{ - Name: "existing-natgateway-ip-name", - }, - }, - }).Times(1) + name: "fail to create subnet", + expectedError: "#: Internal Server Error: StatusCode=500", + expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.SubnetSpecs().Return([]azure.ResourceSpecGetter{&fakeSubnetSpec1}) + r.CreateResource(gomockinternal.AContext(), &fakeSubnetSpec1, serviceName).Return(nil, internalError) + s.UpdatePutStatus(infrav1.SubnetsReadyCondition, serviceName, internalError) }, }, { - name: "spec has empty CIDR and ID data but GET from Azure has the values", - expectedError: "", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.Subnet("my-subnet").AnyTimes().Return(infrav1.SubnetSpec{ - ID: "", - Name: "my-subnet", - SubnetClassSpec: infrav1.SubnetClassSpec{ - Role: infrav1.SubnetNode, - CIDRBlocks: []string{}, - }, - }) - s.SubnetSpecs().AnyTimes().Return([]azure.SubnetSpec{ - { - Name: "my-subnet", - CIDRs: []string{}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "my-vnet"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.SubscriptionID().AnyTimes().Return("123") - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Get(gomockinternal.AContext(), "", "my-vnet", "my-subnet"). - Return(network.Subnet{ - ID: to.StringPtr("subnet-id"), - Name: to.StringPtr("my-subnet"), - SubnetPropertiesFormat: &network.SubnetPropertiesFormat{ - AddressPrefix: to.StringPtr("10.0.0.0/16"), - RouteTable: &network.RouteTable{ - ID: to.StringPtr("rt-id"), - Name: to.StringPtr("my-subnet_route_table"), - }, - NetworkSecurityGroup: &network.SecurityGroup{ - ID: to.StringPtr("sg-id"), - Name: to.StringPtr("my-sg"), - }, - }, - }, nil) - s.SetSubnet(infrav1.SubnetSpec{ - ID: "subnet-id", - Name: "my-subnet", - SubnetClassSpec: infrav1.SubnetClassSpec{ - Role: infrav1.SubnetNode, - CIDRBlocks: []string{"10.0.0.0/16"}, - }, - }).Times(1) + name: "fail to create subnets", + expectedError: "#: Internal Server Error: StatusCode=500", + expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.SubnetSpecs().Return([]azure.ResourceSpecGetter{&fakeSubnetSpec1, &fakeSubnetSpec2}) + r.CreateResource(gomockinternal.AContext(), &fakeSubnetSpec1, serviceName).Return(nil, internalError) + + r.CreateResource(gomockinternal.AContext(), &fakeSubnetSpec2, serviceName).Return(fakeSubnet2, nil) + s.UpdateSubnetID(fakeSubnetSpec2.Name, to.String(fakeSubnet2.ID)) + s.UpdateSubnetCIDRs(fakeSubnetSpec2.Name, []string{to.String(fakeSubnet2.AddressPrefix)}) + + s.UpdatePutStatus(infrav1.SubnetsReadyCondition, serviceName, internalError) }, }, } @@ -503,13 +260,13 @@ func TestReconcileSubnets(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() scopeMock := mock_subnets.NewMockSubnetScope(mockCtrl) - clientMock := mock_subnets.NewMockClient(mockCtrl) + asyncMock := mock_async.NewMockReconciler(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), asyncMock.EXPECT()) s := &Service{ - Scope: scopeMock, - Client: clientMock, + Scope: scopeMock, + Reconciler: asyncMock, } err := s.Reconcile(context.TODO()) @@ -527,125 +284,46 @@ func TestDeleteSubnets(t *testing.T) { testcases := []struct { name string expectedError string - expect func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) + expect func(s *mock_subnets.MockSubnetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) }{ { - name: "subnet deleted successfully", + name: "subnets deleted successfully", expectedError: "", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.SubnetSpecs().Return([]azure.SubnetSpec{ - { - Name: "my-subnet", - CIDRs: []string{"10.0.0.0/16"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - { - Name: "my-subnet-1", - CIDRs: []string{"10.1.0.0/16"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetControlPlane, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "my-vnet"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Delete(gomockinternal.AContext(), "", "my-vnet", "my-subnet") - m.Delete(gomockinternal.AContext(), "", "my-vnet", "my-subnet-1") - }, - }, - { - name: "subnet already deleted", - expectedError: "", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.SubnetSpecs().Return([]azure.SubnetSpec{ - { - Name: "my-subnet", - CIDRs: []string{"10.0.0.0/16"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "my-vnet"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Delete(gomockinternal.AContext(), "", "my-vnet", "my-subnet"). - Return(autorest.NewErrorWithResponse("", "my-vnet", &http.Response{StatusCode: 404}, "Not found")) + expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.IsVnetManaged().Return(true) + s.SubnetSpecs().Return([]azure.ResourceSpecGetter{&fakeSubnetSpec1, &fakeSubnetSpec2}) + r.DeleteResource(gomockinternal.AContext(), &fakeSubnetSpec1, serviceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakeSubnetSpec2, serviceName).Return(nil) + s.UpdateDeleteStatus(infrav1.SubnetsReadyCondition, serviceName, nil) }, }, { - name: "node subnet already deleted and controlplane subnet deleted successfully", + name: "node subnet and controlplane subnet deleted successfully", expectedError: "", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.SubnetSpecs().Return([]azure.SubnetSpec{ - { - Name: "my-subnet", - CIDRs: []string{"10.0.0.0/16"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - { - Name: "my-subnet-1", - CIDRs: []string{"10.1.0.0/16"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetControlPlane, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "my-vnet"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Delete(gomockinternal.AContext(), "", "my-vnet", "my-subnet"). - Return(autorest.NewErrorWithResponse("", "my-vnet", &http.Response{StatusCode: 404}, "Not found")) - m.Delete(gomockinternal.AContext(), "", "my-vnet", "my-subnet-1") + expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.IsVnetManaged().Return(true) + s.SubnetSpecs().Return([]azure.ResourceSpecGetter{&fakeSubnetSpec1, &fakeCtrlPlaneSubnetSpec}) + r.DeleteResource(gomockinternal.AContext(), &fakeSubnetSpec1, serviceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakeCtrlPlaneSubnetSpec, serviceName).Return(nil) + s.UpdateDeleteStatus(infrav1.SubnetsReadyCondition, serviceName, nil) }, }, { name: "skip delete if vnet is managed", expectedError: "", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.SubnetSpecs().Return([]azure.SubnetSpec{ - { - Name: "my-subnet", - CIDRs: []string{"10.0.0.0/16"}, - VNetName: "custom-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{ResourceGroup: "custom-vnet-rg", Name: "custom-vnet", ID: "id1"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.ResourceGroup().AnyTimes().Return("my-rg") + expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.IsVnetManaged().Return(false) + s.UpdateDeleteStatus(infrav1.SubnetsReadyCondition, serviceName, nil) }, }, { name: "fail delete subnet", - expectedError: "failed to delete subnet my-subnet in resource group my-rg: #: Internal Server Error: StatusCode=500", - expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, m *mock_subnets.MockClientMockRecorder) { - s.SubnetSpecs().Return([]azure.SubnetSpec{ - { - Name: "my-subnet", - CIDRs: []string{"10.0.0.0/16"}, - VNetName: "my-vnet", - RouteTableName: "my-subnet_route_table", - SecurityGroupName: "my-sg", - Role: infrav1.SubnetNode, - }, - }) - s.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "my-vnet", ResourceGroup: "my-rg"}) - s.ClusterName().AnyTimes().Return("fake-cluster") - s.ResourceGroup().AnyTimes().Return("my-rg") - m.Delete(gomockinternal.AContext(), "my-rg", "my-vnet", "my-subnet").Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + expectedError: "#: Internal Server Error: StatusCode=500", + expect: func(s *mock_subnets.MockSubnetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + s.IsVnetManaged().Return(true) + s.SubnetSpecs().Return([]azure.ResourceSpecGetter{&fakeSubnetSpec1}) + r.DeleteResource(gomockinternal.AContext(), &fakeSubnetSpec1, serviceName).Return(internalError) + s.UpdateDeleteStatus(infrav1.SubnetsReadyCondition, serviceName, internalError) }, }, } @@ -658,13 +336,13 @@ func TestDeleteSubnets(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() scopeMock := mock_subnets.NewMockSubnetScope(mockCtrl) - clientMock := mock_subnets.NewMockClient(mockCtrl) + asyncMock := mock_async.NewMockReconciler(mockCtrl) - tc.expect(scopeMock.EXPECT(), clientMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), asyncMock.EXPECT()) s := &Service{ - Scope: scopeMock, - Client: clientMock, + Scope: scopeMock, + Reconciler: asyncMock, } err := s.Delete(context.TODO()) diff --git a/azure/types.go b/azure/types.go index aa283ca4589..06a271b2da3 100644 --- a/azure/types.go +++ b/azure/types.go @@ -30,17 +30,6 @@ type PublicIPSpec struct { IsIPv6 bool } -// SubnetSpec defines the specification for a Subnet. -type SubnetSpec struct { - Name string - CIDRs []string - VNetName string - RouteTableName string - SecurityGroupName string - Role infrav1.SubnetRole - NatGatewayName string -} - // RoleAssignmentSpec defines the specification for a Role Assignment. type RoleAssignmentSpec struct { MachineName string