From 0579d7139f550b432f4912d60f91daaa0f4490c0 Mon Sep 17 00:00:00 2001 From: Jonathan Tong Date: Fri, 5 Nov 2021 16:33:44 -0400 Subject: [PATCH] Make virtual network peerings reconcile/delete async --- api/v1beta1/conditions_consts.go | 2 + azure/scope/cluster.go | 14 +- azure/services/vnetpeerings/client.go | 123 +++- .../mock_vnetpeerings/client_mock.go | 67 +- .../mock_vnetpeerings/vnetpeerings_mock.go | 95 ++- azure/services/vnetpeerings/spec.go | 70 +++ azure/services/vnetpeerings/vnetpeerings.go | 76 ++- .../vnetpeerings/vnetpeerings_test.go | 575 +++++++----------- azure/types.go | 9 - 9 files changed, 556 insertions(+), 475 deletions(-) create mode 100644 azure/services/vnetpeerings/spec.go diff --git a/api/v1beta1/conditions_consts.go b/api/v1beta1/conditions_consts.go index cfa1af268e3..d11ffc8ec68 100644 --- a/api/v1beta1/conditions_consts.go +++ b/api/v1beta1/conditions_consts.go @@ -82,6 +82,8 @@ const ( ResourceGroupReadyCondition clusterv1.ConditionType = "ResourceGroupReady" // VNetReadyCondition means the virtual network exists and is ready to be used. VNetReadyCondition clusterv1.ConditionType = "VNetReady" + // VnetPeeringReadyCondition means the virtual network peerings exist and are ready to be used. + VnetPeeringReadyCondition clusterv1.ConditionType = "VnetPeeringReady" // SecurityGroupsReadyCondition means the security groups exist and are ready to be used. SecurityGroupsReadyCondition clusterv1.ConditionType = "SecurityGroupsReady" // RouteTablesReadyCondition means the route tables exist and are ready to be used. diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go index 160ff9bb914..c2760667a06 100644 --- a/azure/scope/cluster.go +++ b/azure/scope/cluster.go @@ -38,6 +38,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/vnetpeerings" "sigs.k8s.io/cluster-api-provider-azure/util/futures" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) @@ -303,23 +304,24 @@ func (s *ClusterScope) GroupSpec() azure.ResourceSpecGetter { } // VnetPeeringSpecs returns the virtual network peering specs. -func (s *ClusterScope) VnetPeeringSpecs() []azure.VnetPeeringSpec { - peeringSpecs := make([]azure.VnetPeeringSpec, 2*len(s.Vnet().Peerings)) - +func (s *ClusterScope) VnetPeeringSpecs() []azure.ResourceSpecGetter { + peeringSpecs := make([]azure.ResourceSpecGetter, 2*len(s.Vnet().Peerings)) for i, peering := range s.Vnet().Peerings { - forwardPeering := azure.VnetPeeringSpec{ + forwardPeering := &vnetpeerings.VnetPeeringSpec{ PeeringName: azure.GenerateVnetPeeringName(s.Vnet().Name, peering.RemoteVnetName), SourceVnetName: s.Vnet().Name, SourceResourceGroup: s.Vnet().ResourceGroup, RemoteVnetName: peering.RemoteVnetName, RemoteResourceGroup: peering.ResourceGroup, + SubscriptionID: s.SubscriptionID(), } - reversePeering := azure.VnetPeeringSpec{ + reversePeering := &vnetpeerings.VnetPeeringSpec{ PeeringName: azure.GenerateVnetPeeringName(peering.RemoteVnetName, s.Vnet().Name), SourceVnetName: peering.RemoteVnetName, SourceResourceGroup: peering.ResourceGroup, RemoteVnetName: s.Vnet().Name, RemoteResourceGroup: s.Vnet().ResourceGroup, + SubscriptionID: s.SubscriptionID(), } peeringSpecs[i*2] = forwardPeering peeringSpecs[i*2+1] = reversePeering @@ -597,6 +599,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error { conditions.WithConditions( infrav1.ResourceGroupReadyCondition, infrav1.NetworkInfrastructureReadyCondition, + infrav1.VnetPeeringReadyCondition, ), ) @@ -607,6 +610,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error { clusterv1.ReadyCondition, infrav1.ResourceGroupReadyCondition, infrav1.NetworkInfrastructureReadyCondition, + infrav1.VnetPeeringReadyCondition, }}) } diff --git a/azure/services/vnetpeerings/client.go b/azure/services/vnetpeerings/client.go index 45cd63c8798..0674cc7f541 100644 --- a/azure/services/vnetpeerings/client.go +++ b/azure/services/vnetpeerings/client.go @@ -18,19 +18,26 @@ package vnetpeerings 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.VirtualNetworkPeering, error) - CreateOrUpdate(context.Context, string, string, string, network.VirtualNetworkPeering) error - Delete(context.Context, string, string, string) error + CreateOrUpdateAsync(context.Context, azure.ResourceSpecGetter) (interface{}, azureautorest.FutureAPI, error) + DeleteAsync(context.Context, azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) + IsDone(context.Context, azureautorest.FutureAPI) (bool, error) + Result(context.Context, azureautorest.FutureAPI, string) (interface{}, error) } // AzureClient contains the Azure go-sdk Client. @@ -61,36 +68,120 @@ func (ac *AzureClient) Get(ctx context.Context, resourceGroupName, vnetName, pee return ac.peerings.Get(ctx, resourceGroupName, vnetName, peeringName) } -// CreateOrUpdate creates or updates a virtual network peering in the specified virtual network. -func (ac *AzureClient) CreateOrUpdate(ctx context.Context, resourceGroupName, vnetName, peeringName string, peering network.VirtualNetworkPeering) error { - ctx, span := tele.Tracer().Start(ctx, "vnetpeerings.AzureClient.CreateOrUpdate") +// CreateOrUpdateAsync creates or updates a virtual network peering asynchronously. +// It sends a PUT request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing +// progress of the operation. +func (ac *AzureClient) CreateOrUpdateAsync(ctx context.Context, spec azure.ResourceSpecGetter) (interface{}, azureautorest.FutureAPI, error) { + ctx, span := tele.Tracer().Start(ctx, "vnetpeerings.AzureClient.CreateOrUpdateAsync") defer span.End() - future, err := ac.peerings.CreateOrUpdate(ctx, resourceGroupName, vnetName, peeringName, peering, network.SyncRemoteAddressSpaceTrue) + var existingPeering interface{} + + if existing, err := ac.Get(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName()); err != nil && !azure.ResourceNotFound(err) { + return nil, nil, errors.Wrapf(err, "failed to get virtual network peering %s for %s in %s", spec.ResourceName(), spec.OwnerResourceName(), spec.ResourceGroupName()) + } else if err == nil { + existingPeering = existing + } + + params, err := spec.Parameters(existingPeering) if err != nil { - return err + return nil, nil, errors.Wrapf(err, "failed to get desired parameters for virtual network peering %s", spec.ResourceName()) } + + peering, ok := params.(network.VirtualNetworkPeering) + if !ok { + if params == nil { + // nothing to do here. + return existingPeering, nil, nil + } + return nil, nil, errors.Errorf("%T is not a network.VirtualNetworkPeering", params) + } + + future, err := ac.peerings.CreateOrUpdate(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName(), peering, network.SyncRemoteAddressSpaceTrue) + if err != nil { + return nil, nil, err + } + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + err = future.WaitForCompletionRef(ctx, ac.peerings.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, &future, err } - _, err = future.Result(ac.peerings) - return err + + result, err := future.Result(ac.peerings) + // if the operation completed, return a nil future + return result, nil, err } -// Delete deletes the specified virtual network peering. -func (ac *AzureClient) Delete(ctx context.Context, resourceGroupName, vnetName, peeringName string) error { +// DeleteAsync deletes a virtual network peering asynchronously. DeleteAsync sends a DELETE +// request to Azure and if accepted without error, the func will return a Future which can be used to track the ongoing +// progress of the operation. +func (ac *AzureClient) DeleteAsync(ctx context.Context, spec azure.ResourceSpecGetter) (azureautorest.FutureAPI, error) { ctx, span := tele.Tracer().Start(ctx, "vnetpeerings.AzureClient.Delete") defer span.End() - future, err := ac.peerings.Delete(ctx, resourceGroupName, vnetName, peeringName) + future, err := ac.peerings.Delete(ctx, spec.ResourceGroupName(), spec.OwnerResourceName(), spec.ResourceName()) if err != nil { - return err + return nil, err } + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureCallTimeout) + defer cancel() + err = future.WaitForCompletionRef(ctx, ac.peerings.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 &future, err } _, err = future.Result(ac.peerings) - return err + // 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, span := tele.Tracer().Start(ctx, "vnetpeerings.AzureClient.IsDone") + defer span.End() + + done, err := future.DoneWithContext(ctx, ac.peerings) + if err != nil { + return false, errors.Wrap(err, "failed checking if the operation was complete") + } + + return done, nil +} + +// Result fetches the result of a long-running operation future. +func (ac *AzureClient) Result(ctx context.Context, futureData azureautorest.FutureAPI, futureType string) (interface{}, error) { + if futureData == nil { + return nil, errors.Errorf("cannot get result from nil future") + } + var result func(client network.VirtualNetworkPeeringsClient) (peering network.VirtualNetworkPeering, err error) + + switch futureType { + case infrav1.PutFuture: + var future *network.VirtualNetworkPeeringsCreateOrUpdateFuture + jsonData, err := futureData.MarshalJSON() + if err != nil { + return nil, errors.Wrap(err, "failed to marshal future") + } + if err := json.Unmarshal(jsonData, &future); err != nil { + return nil, errors.Wrap(err, "failed to unmarshal future data") + } + result = (*future).Result + + case infrav1.DeleteFuture: + // Delete does not return a result virtual network peering + return nil, nil + + default: + return nil, errors.Errorf("unknown future type %q", futureType) + } + + return result(ac.peerings) } diff --git a/azure/services/vnetpeerings/mock_vnetpeerings/client_mock.go b/azure/services/vnetpeerings/mock_vnetpeerings/client_mock.go index d9d34bfdb56..ecd68a5d0fd 100644 --- a/azure/services/vnetpeerings/mock_vnetpeerings/client_mock.go +++ b/azure/services/vnetpeerings/mock_vnetpeerings/client_mock.go @@ -25,7 +25,9 @@ import ( reflect "reflect" network "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" + azure "github.com/Azure/go-autorest/autorest/azure" gomock "github.com/golang/mock/gomock" + azure0 "sigs.k8s.io/cluster-api-provider-azure/azure" ) // MockClient is a mock of Client interface. @@ -51,32 +53,35 @@ 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.VirtualNetworkPeering) error { +// CreateOrUpdateAsync mocks base method. +func (m *MockClient) CreateOrUpdateAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (interface{}, azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateOrUpdate", arg0, arg1, arg2, arg3, arg4) - ret0, _ := ret[0].(error) - return ret0 + ret := m.ctrl.Call(m, "CreateOrUpdateAsync", arg0, arg1) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(azure.FutureAPI) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } -// CreateOrUpdate indicates an expected call of CreateOrUpdate. -func (mr *MockClientMockRecorder) CreateOrUpdate(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { +// CreateOrUpdateAsync indicates an expected call of CreateOrUpdateAsync. +func (mr *MockClientMockRecorder) CreateOrUpdateAsync(arg0, arg1 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) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateOrUpdateAsync", reflect.TypeOf((*MockClient)(nil).CreateOrUpdateAsync), arg0, arg1) } -// Delete mocks base method. -func (m *MockClient) Delete(arg0 context.Context, arg1, arg2, arg3 string) error { +// DeleteAsync mocks base method. +func (m *MockClient) DeleteAsync(arg0 context.Context, arg1 azure0.ResourceSpecGetter) (azure.FutureAPI, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Delete", arg0, arg1, arg2, arg3) - ret0, _ := ret[0].(error) - return ret0 + ret := m.ctrl.Call(m, "DeleteAsync", arg0, arg1) + ret0, _ := ret[0].(azure.FutureAPI) + ret1, _ := ret[1].(error) + return ret0, ret1 } -// Delete indicates an expected call of Delete. -func (mr *MockClientMockRecorder) Delete(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { +// DeleteAsync indicates an expected call of DeleteAsync. +func (mr *MockClientMockRecorder) DeleteAsync(arg0, arg1 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) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAsync", reflect.TypeOf((*MockClient)(nil).DeleteAsync), arg0, arg1) } // Get mocks base method. @@ -93,3 +98,33 @@ func (mr *MockClientMockRecorder) Get(arg0, arg1, arg2, arg3 interface{}) *gomoc mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockClient)(nil).Get), arg0, arg1, arg2, arg3) } + +// IsDone mocks base method. +func (m *MockClient) IsDone(arg0 context.Context, arg1 azure.FutureAPI) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsDone", arg0, arg1) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// IsDone indicates an expected call of IsDone. +func (mr *MockClientMockRecorder) IsDone(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDone", reflect.TypeOf((*MockClient)(nil).IsDone), arg0, arg1) +} + +// Result mocks base method. +func (m *MockClient) Result(arg0 context.Context, arg1 azure.FutureAPI, arg2 string) (interface{}, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Result", arg0, arg1, arg2) + ret0, _ := ret[0].(interface{}) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Result indicates an expected call of Result. +func (mr *MockClientMockRecorder) Result(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Result", reflect.TypeOf((*MockClient)(nil).Result), arg0, arg1, arg2) +} diff --git a/azure/services/vnetpeerings/mock_vnetpeerings/vnetpeerings_mock.go b/azure/services/vnetpeerings/mock_vnetpeerings/vnetpeerings_mock.go index 01a07b5b165..c43ead8632e 100644 --- a/azure/services/vnetpeerings/mock_vnetpeerings/vnetpeerings_mock.go +++ b/azure/services/vnetpeerings/mock_vnetpeerings/vnetpeerings_mock.go @@ -28,6 +28,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" ) // MockVnetPeeringScope is a mock of VnetPeeringScope interface. @@ -123,18 +124,16 @@ func (mr *MockVnetPeeringScopeMockRecorder) CloudEnvironment() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CloudEnvironment", reflect.TypeOf((*MockVnetPeeringScope)(nil).CloudEnvironment)) } -// ClusterName mocks base method. -func (m *MockVnetPeeringScope) ClusterName() string { +// DeleteLongRunningOperationState mocks base method. +func (m *MockVnetPeeringScope) DeleteLongRunningOperationState(arg0, arg1 string) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ClusterName") - ret0, _ := ret[0].(string) - return ret0 + m.ctrl.Call(m, "DeleteLongRunningOperationState", arg0, arg1) } -// ClusterName indicates an expected call of ClusterName. -func (mr *MockVnetPeeringScopeMockRecorder) ClusterName() *gomock.Call { +// DeleteLongRunningOperationState indicates an expected call of DeleteLongRunningOperationState. +func (mr *MockVnetPeeringScopeMockRecorder) DeleteLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterName", reflect.TypeOf((*MockVnetPeeringScope)(nil).ClusterName)) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteLongRunningOperationState", reflect.TypeOf((*MockVnetPeeringScope)(nil).DeleteLongRunningOperationState), arg0, arg1) } // Enabled mocks base method. @@ -168,6 +167,20 @@ func (mr *MockVnetPeeringScopeMockRecorder) Error(err, msg interface{}, keysAndV return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Error", reflect.TypeOf((*MockVnetPeeringScope)(nil).Error), varargs...) } +// GetLongRunningOperationState mocks base method. +func (m *MockVnetPeeringScope) 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 *MockVnetPeeringScopeMockRecorder) GetLongRunningOperationState(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLongRunningOperationState", reflect.TypeOf((*MockVnetPeeringScope)(nil).GetLongRunningOperationState), arg0, arg1) +} + // HashKey mocks base method. func (m *MockVnetPeeringScope) HashKey() string { m.ctrl.T.Helper() @@ -199,6 +212,18 @@ func (mr *MockVnetPeeringScopeMockRecorder) Info(msg interface{}, keysAndValues return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Info", reflect.TypeOf((*MockVnetPeeringScope)(nil).Info), varargs...) } +// SetLongRunningOperationState mocks base method. +func (m *MockVnetPeeringScope) SetLongRunningOperationState(arg0 *v1beta1.Future) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetLongRunningOperationState", arg0) +} + +// SetLongRunningOperationState indicates an expected call of SetLongRunningOperationState. +func (mr *MockVnetPeeringScopeMockRecorder) SetLongRunningOperationState(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockVnetPeeringScope)(nil).SetLongRunningOperationState), arg0) +} + // SubscriptionID mocks base method. func (m *MockVnetPeeringScope) SubscriptionID() string { m.ctrl.T.Helper() @@ -227,6 +252,42 @@ func (mr *MockVnetPeeringScopeMockRecorder) TenantID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TenantID", reflect.TypeOf((*MockVnetPeeringScope)(nil).TenantID)) } +// UpdateDeleteStatus mocks base method. +func (m *MockVnetPeeringScope) 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 *MockVnetPeeringScopeMockRecorder) UpdateDeleteStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateDeleteStatus", reflect.TypeOf((*MockVnetPeeringScope)(nil).UpdateDeleteStatus), arg0, arg1, arg2) +} + +// UpdatePatchStatus mocks base method. +func (m *MockVnetPeeringScope) 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 *MockVnetPeeringScopeMockRecorder) UpdatePatchStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePatchStatus", reflect.TypeOf((*MockVnetPeeringScope)(nil).UpdatePatchStatus), arg0, arg1, arg2) +} + +// UpdatePutStatus mocks base method. +func (m *MockVnetPeeringScope) 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 *MockVnetPeeringScopeMockRecorder) UpdatePutStatus(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePutStatus", reflect.TypeOf((*MockVnetPeeringScope)(nil).UpdatePutStatus), arg0, arg1, arg2) +} + // V mocks base method. func (m *MockVnetPeeringScope) V(level int) logr.Logger { m.ctrl.T.Helper() @@ -241,25 +302,11 @@ func (mr *MockVnetPeeringScopeMockRecorder) V(level interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "V", reflect.TypeOf((*MockVnetPeeringScope)(nil).V), level) } -// Vnet mocks base method. -func (m *MockVnetPeeringScope) Vnet() *v1beta1.VnetSpec { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Vnet") - ret0, _ := ret[0].(*v1beta1.VnetSpec) - return ret0 -} - -// Vnet indicates an expected call of Vnet. -func (mr *MockVnetPeeringScopeMockRecorder) Vnet() *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Vnet", reflect.TypeOf((*MockVnetPeeringScope)(nil).Vnet)) -} - // VnetPeeringSpecs mocks base method. -func (m *MockVnetPeeringScope) VnetPeeringSpecs() []azure.VnetPeeringSpec { +func (m *MockVnetPeeringScope) VnetPeeringSpecs() []azure.ResourceSpecGetter { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "VnetPeeringSpecs") - ret0, _ := ret[0].([]azure.VnetPeeringSpec) + ret0, _ := ret[0].([]azure.ResourceSpecGetter) return ret0 } diff --git a/azure/services/vnetpeerings/spec.go b/azure/services/vnetpeerings/spec.go new file mode 100644 index 00000000000..2fcc0af3f51 --- /dev/null +++ b/azure/services/vnetpeerings/spec.go @@ -0,0 +1,70 @@ +/* +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 vnetpeerings + +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" + "sigs.k8s.io/cluster-api-provider-azure/azure" +) + +// VnetPeeringSpec defines the specification for a virtual network peering. +type VnetPeeringSpec struct { + SourceResourceGroup string + SourceVnetName string + RemoteResourceGroup string + RemoteVnetName string + PeeringName string + SubscriptionID string +} + +// ResourceName returns the name of the virtual network peering. +func (s *VnetPeeringSpec) ResourceName() string { + return s.PeeringName +} + +// ResourceGroupName returns the name of the resource group. +func (s *VnetPeeringSpec) ResourceGroupName() string { + return s.SourceResourceGroup +} + +// OwnerResourceName is a no-op for virtual network peerings. +func (s *VnetPeeringSpec) OwnerResourceName() string { + return s.SourceVnetName +} + +// Parameters returns the parameters for the virtual network peering. +func (s *VnetPeeringSpec) Parameters(existing interface{}) (interface{}, error) { + if existing != nil { + if _, ok := existing.(network.VirtualNetworkPeering); !ok { + return nil, errors.Errorf("%T is not a network.VnetPeering", existing) + } + // virtual network peering already exists + return nil, nil + } + vnetID := azure.VNetID(s.SubscriptionID, s.RemoteResourceGroup, s.RemoteVnetName) + peeringProperties := network.VirtualNetworkPeeringPropertiesFormat{ + RemoteVirtualNetwork: &network.SubResource{ + ID: to.StringPtr(vnetID), + }, + } + return network.VirtualNetworkPeering{ + Name: to.StringPtr(s.PeeringName), + VirtualNetworkPeeringPropertiesFormat: &peeringProperties, + }, nil +} diff --git a/azure/services/vnetpeerings/vnetpeerings.go b/azure/services/vnetpeerings/vnetpeerings.go index 3b16583dd9d..5601b7e5495 100644 --- a/azure/services/vnetpeerings/vnetpeerings.go +++ b/azure/services/vnetpeerings/vnetpeerings.go @@ -19,24 +19,23 @@ package vnetpeerings import ( "context" - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" - "github.com/Azure/go-autorest/autorest/to" "github.com/go-logr/logr" - "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 = "vnetpeerings" + // VnetPeeringScope defines the scope interface for a subnet service. type VnetPeeringScope interface { logr.Logger azure.Authorizer - Vnet() *infrav1.VnetSpec - ClusterName() string - SubscriptionID() string - VnetPeeringSpecs() []azure.VnetPeeringSpec + azure.AsyncStatusUpdater + VnetPeeringSpecs() []azure.ResourceSpecGetter } // Service provides operations on Azure resources. @@ -58,52 +57,45 @@ func (s *Service) Reconcile(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "vnetpeerings.Service.Reconcile") defer span.End() - for _, peeringSpec := range s.Scope.VnetPeeringSpecs() { - vnetID := azure.VNetID(s.Scope.SubscriptionID(), peeringSpec.RemoteResourceGroup, peeringSpec.RemoteVnetName) - peeringProperties := network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr(vnetID), - }, - } + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() - s.Scope.V(2).Info("creating peering", "peering", peeringSpec.PeeringName, "from", "vnet", peeringSpec.SourceVnetName, "to", "vnet", peeringSpec.RemoteVnetName) - err := s.Client.CreateOrUpdate( - ctx, - peeringSpec.SourceResourceGroup, - peeringSpec.SourceVnetName, - peeringSpec.PeeringName, - network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &peeringProperties, - }, - ) - - if err != nil { - return errors.Wrapf(err, "failed to create peering %s in resource group %s", peeringSpec.PeeringName, s.Scope.Vnet().ResourceGroup) + // We go through the list of VnetPeeringSpecs to reconcile each one, independently of the result of the previous one. + // If multiple erros occur, we return the most pressing one + // order of precedence is: error creating -> creating in progress -> created (no error) + var result error + for _, peeringSpec := range s.Scope.VnetPeeringSpecs() { + if _, err := async.CreateResource(ctx, s.Scope, s.Client, peeringSpec, serviceName); err != nil { + if !azure.IsOperationNotDoneError(err) || result == nil { + result = err + } } - - s.Scope.V(2).Info("successfully created peering", "peering", peeringSpec.PeeringName, "from", "vnet", peeringSpec.SourceVnetName, "to", "vnet", peeringSpec.RemoteVnetName) } - return nil + s.Scope.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, result) + return result } // Delete deletes the peering with the provided name. func (s *Service) Delete(ctx context.Context) error { ctx, span := tele.Tracer().Start(ctx, "vnetpeerings.Service.Delete") defer span.End() + + ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) + defer cancel() + + var result error + + // We go through the list of VnetPeeringSpecs to delete each one, independently of the result of the previous one. + // If multiple erros occur, we return the most pressing one + // order of precedence is: error deleting -> deleting in progress -> deleted (no error) for _, peeringSpec := range s.Scope.VnetPeeringSpecs() { - s.Scope.V(2).Info("deleting peering in vnets", "vnet1", peeringSpec.SourceVnetName, "and", "vnet2", peeringSpec.RemoteVnetName) - err := s.Client.Delete(ctx, peeringSpec.SourceResourceGroup, peeringSpec.SourceVnetName, peeringSpec.PeeringName) - if err != nil && azure.ResourceNotFound(err) { - // already deleted - continue - } - if err != nil { - return errors.Wrapf(err, "failed to delete peering %s in vnet %s and resource group %s", peeringSpec.PeeringName, peeringSpec.SourceVnetName, peeringSpec.SourceResourceGroup) + if err := async.DeleteResource(ctx, s.Scope, s.Client, peeringSpec, serviceName); err != nil { + if !azure.IsOperationNotDoneError(err) || result == nil { + result = err + } } - - s.Scope.V(2).Info("successfully deleted peering in vnet", "peering", peeringSpec.PeeringName, "vnet", peeringSpec.SourceVnetName) } - - return nil + s.Scope.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, result) + return result } diff --git a/azure/services/vnetpeerings/vnetpeerings_test.go b/azure/services/vnetpeerings/vnetpeerings_test.go index 8ae0533f3c8..c9ff8d4c3e3 100644 --- a/azure/services/vnetpeerings/vnetpeerings_test.go +++ b/azure/services/vnetpeerings/vnetpeerings_test.go @@ -18,13 +18,12 @@ package vnetpeerings import ( "context" + "errors" "fmt" "net/http" "testing" - "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-02-01/network" - "github.com/Azure/go-autorest/autorest" - "github.com/Azure/go-autorest/autorest/to" + azureautorest "github.com/Azure/go-autorest/autorest/azure" "github.com/golang/mock/gomock" . "github.com/onsi/gomega" "k8s.io/klog/v2/klogr" @@ -34,6 +33,60 @@ import ( gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" ) +var ( + fakePeering1To2 = VnetPeeringSpec{ + PeeringName: "vnet1-to-vnet2", + SourceVnetName: "vnet1", + SourceResourceGroup: "group1", + RemoteVnetName: "vnet2", + RemoteResourceGroup: "group2", + SubscriptionID: "sub1", + } + fakePeering2To1 = VnetPeeringSpec{ + PeeringName: "vnet2-to-vnet1", + SourceVnetName: "vnet2", + SourceResourceGroup: "group2", + RemoteVnetName: "vnet1", + RemoteResourceGroup: "group1", + SubscriptionID: "sub1", + } + fakePeering1To3 = VnetPeeringSpec{ + PeeringName: "vnet1-to-vnet3", + SourceVnetName: "vnet1", + SourceResourceGroup: "group1", + RemoteVnetName: "vnet3", + RemoteResourceGroup: "group3", + SubscriptionID: "sub1", + } + fakePeering3To1 = VnetPeeringSpec{ + PeeringName: "vnet3-to-vnet1", + SourceVnetName: "vnet3", + SourceResourceGroup: "group3", + RemoteVnetName: "vnet1", + RemoteResourceGroup: "group1", + SubscriptionID: "sub1", + } + fakePeeringExtra = VnetPeeringSpec{ + PeeringName: "extra-peering", + SourceVnetName: "vnet3", + SourceResourceGroup: "group3", + RemoteVnetName: "vnet4", + RemoteResourceGroup: "group4", + SubscriptionID: "sub1", + } + fakePeeringSpecs = []azure.ResourceSpecGetter{&fakePeering1To2, &fakePeering2To1, &fakePeering1To3, &fakePeering3To1} + fakePeeringExtraSpecs = []azure.ResourceSpecGetter{&fakePeering1To2, &fakePeering2To1, &fakePeeringExtra} + fakeFuture, _ = azureautorest.NewFutureFromResponse(&http.Response{ + Status: "200 OK", + StatusCode: 200, + Request: &http.Request{ + Method: http.MethodDelete, + }, + }) + errFake = errors.New("this is an error") + errFoo = errors.New("foo") +) + func TestReconcileVnetPeerings(t *testing.T) { testcases := []struct { name string @@ -45,24 +98,11 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - p.VnetPeeringSpecs().Return([]azure.VnetPeeringSpec{ - { - PeeringName: "vnet1-to-vnet2", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet2", - RemoteResourceGroup: "group2", - }, - }) - p.ClusterName().AnyTimes().Return("fake-cluster") - p.SubscriptionID().AnyTimes().Return("123") - m.CreateOrUpdate(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet2", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group2/providers/Microsoft.Network/virtualNetworks/vnet2"), - }, - }, - })) + p.VnetPeeringSpecs().Return(fakePeeringSpecs[:1]) + p.GetLongRunningOperationState("vnet1-to-vnet2", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering1To2).Return(nil, nil, nil) + + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) }, }, { @@ -70,9 +110,8 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - p.VnetPeeringSpecs().Return([]azure.VnetPeeringSpec{}) - p.ClusterName().AnyTimes().Return("fake-cluster") - p.SubscriptionID().AnyTimes().Return("123") + p.VnetPeeringSpecs().Return(fakePeeringSpecs[:0]) + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) }, }, { @@ -80,38 +119,14 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - p.VnetPeeringSpecs().Return([]azure.VnetPeeringSpec{ - { - PeeringName: "vnet1-to-vnet2", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet2", - RemoteResourceGroup: "group2", - }, - { - PeeringName: "vnet2-to-vnet1", - SourceVnetName: "vnet2", - SourceResourceGroup: "group2", - RemoteVnetName: "vnet1", - RemoteResourceGroup: "group1", - }, - }) - p.ClusterName().AnyTimes().Return("fake-cluster") - p.SubscriptionID().AnyTimes().Return("123") - m.CreateOrUpdate(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet2", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group2/providers/Microsoft.Network/virtualNetworks/vnet2"), - }, - }, - })) - m.CreateOrUpdate(gomockinternal.AContext(), "group2", "vnet2", "vnet2-to-vnet1", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group1/providers/Microsoft.Network/virtualNetworks/vnet1"), - }, - }, - })) + p.VnetPeeringSpecs().Return(fakePeeringSpecs[:2]) + p.GetLongRunningOperationState("vnet1-to-vnet2", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering1To2).Return(nil, nil, nil) + + p.GetLongRunningOperationState("vnet2-to-vnet1", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering2To1).Return(nil, nil, nil) + + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) }, }, { @@ -119,53 +134,17 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - p.VnetPeeringSpecs().Return([]azure.VnetPeeringSpec{ - { - PeeringName: "vnet1-to-vnet2", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet2", - RemoteResourceGroup: "group2", - }, - { - PeeringName: "vnet2-to-vnet1", - SourceVnetName: "vnet2", - SourceResourceGroup: "group2", - RemoteVnetName: "vnet1", - RemoteResourceGroup: "group1", - }, - { - PeeringName: "extra-peering", - SourceVnetName: "vnet3", - SourceResourceGroup: "group3", - RemoteVnetName: "vnet4", - RemoteResourceGroup: "group4", - }, - }) - p.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet2"}) - p.ClusterName().AnyTimes().Return("fake-cluster") - p.SubscriptionID().AnyTimes().Return("123") - m.CreateOrUpdate(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet2", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group2/providers/Microsoft.Network/virtualNetworks/vnet2"), - }, - }, - })) - m.CreateOrUpdate(gomockinternal.AContext(), "group2", "vnet2", "vnet2-to-vnet1", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group1/providers/Microsoft.Network/virtualNetworks/vnet1"), - }, - }, - })) - m.CreateOrUpdate(gomockinternal.AContext(), "group3", "vnet3", "extra-peering", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group4/providers/Microsoft.Network/virtualNetworks/vnet4"), - }, - }, - })) + p.VnetPeeringSpecs().Return(fakePeeringExtraSpecs) + p.GetLongRunningOperationState("vnet1-to-vnet2", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering1To2).Return(nil, nil, nil) + + p.GetLongRunningOperationState("vnet2-to-vnet1", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering2To1).Return(nil, nil, nil) + + p.GetLongRunningOperationState("extra-peering", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeeringExtra).Return(nil, nil, nil) + + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) }, }, { @@ -173,130 +152,63 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - p.VnetPeeringSpecs().Return([]azure.VnetPeeringSpec{ - { - PeeringName: "vnet1-to-vnet2", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet2", - RemoteResourceGroup: "group2", - }, - { - PeeringName: "vnet2-to-vnet1", - SourceVnetName: "vnet2", - SourceResourceGroup: "group2", - RemoteVnetName: "vnet1", - RemoteResourceGroup: "group1", - }, - { - PeeringName: "vnet1-to-vnet3", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet3", - RemoteResourceGroup: "group3", - }, - { - PeeringName: "vnet3-to-vnet1", - SourceVnetName: "vnet3", - SourceResourceGroup: "group3", - RemoteVnetName: "vnet1", - RemoteResourceGroup: "group1", - }, - }) - p.ClusterName().AnyTimes().Return("fake-cluster") - p.SubscriptionID().AnyTimes().Return("123") - m.CreateOrUpdate(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet2", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group2/providers/Microsoft.Network/virtualNetworks/vnet2"), - }, - }, - })) - m.CreateOrUpdate(gomockinternal.AContext(), "group2", "vnet2", "vnet2-to-vnet1", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group1/providers/Microsoft.Network/virtualNetworks/vnet1"), - }, - }, - })) - m.CreateOrUpdate(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet3", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group3/providers/Microsoft.Network/virtualNetworks/vnet3"), - }, - }, - })) - m.CreateOrUpdate(gomockinternal.AContext(), "group3", "vnet3", "vnet3-to-vnet1", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group1/providers/Microsoft.Network/virtualNetworks/vnet1"), - }, - }, - })) + p.VnetPeeringSpecs().Return(fakePeeringSpecs) + p.GetLongRunningOperationState("vnet1-to-vnet2", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering1To2).Return(nil, nil, nil) + + p.GetLongRunningOperationState("vnet2-to-vnet1", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering2To1).Return(nil, nil, nil) + + p.GetLongRunningOperationState("vnet1-to-vnet3", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering1To3).Return(nil, nil, nil) + + p.GetLongRunningOperationState("vnet3-to-vnet1", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering3To1).Return(nil, nil, nil) + + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) + }, + }, + { + name: "error in creating peering", + expectedError: "failed to create resource group1/vnet1-to-vnet3 (service: vnetpeerings): this is an error", + expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { + p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + p.VnetPeeringSpecs().Return(fakePeeringSpecs) + p.GetLongRunningOperationState("vnet1-to-vnet2", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering1To2).Return(nil, nil, nil) + + p.GetLongRunningOperationState("vnet2-to-vnet1", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering2To1).Return(nil, nil, nil) + + p.GetLongRunningOperationState("vnet1-to-vnet3", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering1To3).Return(nil, nil, errFake) + + p.GetLongRunningOperationState("vnet3-to-vnet1", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering3To1).Return(nil, nil, nil) + + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, gomockinternal.ErrStrEq("failed to create resource group1/vnet1-to-vnet3 (service: vnetpeerings): this is an error")) }, }, { - name: "error in creating peering where loop terminates prematurely", - expectedError: "failed to create peering vnet1-to-vnet3 in resource group group1: #: Internal Server Error: StatusCode=500", + name: "error in creating peering which is not done", + expectedError: "failed to create resource group1/vnet1-to-vnet3 (service: vnetpeerings): this is an error", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - p.VnetPeeringSpecs().Return([]azure.VnetPeeringSpec{ - { - PeeringName: "vnet1-to-vnet2", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet2", - RemoteResourceGroup: "group2", - }, - { - PeeringName: "vnet2-to-vnet1", - SourceVnetName: "vnet2", - SourceResourceGroup: "group2", - RemoteVnetName: "vnet1", - RemoteResourceGroup: "group1", - }, - { - PeeringName: "vnet1-to-vnet3", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet3", - RemoteResourceGroup: "group3", - }, - { - PeeringName: "vnet3-to-vnet1", - SourceVnetName: "vnet3", - SourceResourceGroup: "group3", - RemoteVnetName: "vnet1", - RemoteResourceGroup: "group1", - }, - }) - p.Vnet().AnyTimes().Return(&infrav1.VnetSpec{ - Name: "vnet1", - ResourceGroup: "group1", - }) - p.ClusterName().AnyTimes().Return("fake-cluster") - p.SubscriptionID().AnyTimes().Return("123") - m.CreateOrUpdate(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet2", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group2/providers/Microsoft.Network/virtualNetworks/vnet2"), - }, - }, - })) - m.CreateOrUpdate(gomockinternal.AContext(), "group2", "vnet2", "vnet2-to-vnet1", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group1/providers/Microsoft.Network/virtualNetworks/vnet1"), - }, - }, - })) - m.CreateOrUpdate(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet3", gomockinternal.DiffEq(network.VirtualNetworkPeering{ - VirtualNetworkPeeringPropertiesFormat: &network.VirtualNetworkPeeringPropertiesFormat{ - RemoteVirtualNetwork: &network.SubResource{ - ID: to.StringPtr("/subscriptions/123/resourceGroups/group3/providers/Microsoft.Network/virtualNetworks/vnet3"), - }, - }, - })).Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + p.VnetPeeringSpecs().Return(fakePeeringSpecs) + p.GetLongRunningOperationState("vnet1-to-vnet2", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering1To2).Return(nil, nil, nil) + + p.GetLongRunningOperationState("vnet2-to-vnet1", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering2To1).Return(nil, nil, nil) + + p.GetLongRunningOperationState("vnet1-to-vnet3", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering1To3).Return(nil, nil, errFake) + + p.GetLongRunningOperationState("vnet3-to-vnet1", serviceName).Return(nil) + m.CreateOrUpdateAsync(gomockinternal.AContext(), &fakePeering3To1).Return(nil, &fakeFuture, errFoo) + p.SetLongRunningOperationState(gomock.AssignableToTypeOf(&infrav1.Future{})) + + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, gomockinternal.ErrStrEq("failed to create resource group1/vnet1-to-vnet3 (service: vnetpeerings): this is an error")) }, }, } @@ -341,18 +253,11 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - p.VnetPeeringSpecs().Return([]azure.VnetPeeringSpec{ - { - PeeringName: "vnet1-to-vnet2", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet2", - RemoteResourceGroup: "group2", - }, - }) - p.ClusterName().AnyTimes().Return("fake-cluster") - p.SubscriptionID().AnyTimes().Return("123") - m.Delete(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet2") + p.VnetPeeringSpecs().Return(fakePeeringSpecs[:1]) + p.GetLongRunningOperationState("vnet1-to-vnet2", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering1To2).Return(nil, nil) + + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) }, }, { @@ -360,9 +265,8 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - p.VnetPeeringSpecs().Return([]azure.VnetPeeringSpec{}) - p.ClusterName().AnyTimes().Return("fake-cluster") - p.SubscriptionID().AnyTimes().Return("123") + p.VnetPeeringSpecs().Return(fakePeeringSpecs[:0]) + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) }, }, { @@ -370,26 +274,14 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - p.VnetPeeringSpecs().Return([]azure.VnetPeeringSpec{ - { - PeeringName: "vnet1-to-vnet2", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet2", - RemoteResourceGroup: "group2", - }, - { - PeeringName: "vnet2-to-vnet1", - SourceVnetName: "vnet2", - SourceResourceGroup: "group2", - RemoteVnetName: "vnet1", - RemoteResourceGroup: "group1", - }, - }) - p.ClusterName().AnyTimes().Return("fake-cluster") - p.SubscriptionID().AnyTimes().Return("123") - m.Delete(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet2") - m.Delete(gomockinternal.AContext(), "group2", "vnet2", "vnet2-to-vnet1") + p.VnetPeeringSpecs().Return(fakePeeringSpecs[:2]) + p.GetLongRunningOperationState("vnet1-to-vnet2", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering1To2).Return(nil, nil) + + p.GetLongRunningOperationState("vnet2-to-vnet1", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering2To1).Return(nil, nil) + + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) }, }, { @@ -397,35 +289,17 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - p.VnetPeeringSpecs().Return([]azure.VnetPeeringSpec{ - { - PeeringName: "vnet1-to-vnet2", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet2", - RemoteResourceGroup: "group2", - }, - { - PeeringName: "vnet2-to-vnet1", - SourceVnetName: "vnet2", - SourceResourceGroup: "group2", - RemoteVnetName: "vnet1", - RemoteResourceGroup: "group1", - }, - { - PeeringName: "extra-peering", - SourceVnetName: "vnet3", - SourceResourceGroup: "group3", - RemoteVnetName: "vnet4", - RemoteResourceGroup: "group4", - }, - }) - p.Vnet().AnyTimes().Return(&infrav1.VnetSpec{Name: "vnet2"}) - p.ClusterName().AnyTimes().Return("fake-cluster") - p.SubscriptionID().AnyTimes().Return("123") - m.Delete(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet2") - m.Delete(gomockinternal.AContext(), "group2", "vnet2", "vnet2-to-vnet1") - m.Delete(gomockinternal.AContext(), "group3", "vnet3", "extra-peering") + p.VnetPeeringSpecs().Return(fakePeeringExtraSpecs) + p.GetLongRunningOperationState("vnet1-to-vnet2", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering1To2).Return(nil, nil) + + p.GetLongRunningOperationState("vnet2-to-vnet1", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering2To1).Return(nil, nil) + + p.GetLongRunningOperationState("extra-peering", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeeringExtra).Return(nil, nil) + + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) }, }, { @@ -433,88 +307,63 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - p.VnetPeeringSpecs().Return([]azure.VnetPeeringSpec{ - { - PeeringName: "vnet1-to-vnet2", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet2", - RemoteResourceGroup: "group2", - }, - { - PeeringName: "vnet2-to-vnet1", - SourceVnetName: "vnet2", - SourceResourceGroup: "group2", - RemoteVnetName: "vnet1", - RemoteResourceGroup: "group1", - }, - { - PeeringName: "vnet1-to-vnet3", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet3", - RemoteResourceGroup: "group3", - }, - { - PeeringName: "vnet3-to-vnet1", - SourceVnetName: "vnet3", - SourceResourceGroup: "group3", - RemoteVnetName: "vnet1", - RemoteResourceGroup: "group1", - }, - }) - p.ClusterName().AnyTimes().Return("fake-cluster") - p.SubscriptionID().AnyTimes().Return("123") - m.Delete(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet2") - m.Delete(gomockinternal.AContext(), "group2", "vnet2", "vnet2-to-vnet1") - m.Delete(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet3") - m.Delete(gomockinternal.AContext(), "group3", "vnet3", "vnet3-to-vnet1") + p.VnetPeeringSpecs().Return(fakePeeringSpecs) + p.GetLongRunningOperationState("vnet1-to-vnet2", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering1To2).Return(nil, nil) + + p.GetLongRunningOperationState("vnet2-to-vnet1", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering2To1).Return(nil, nil) + + p.GetLongRunningOperationState("vnet1-to-vnet3", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering1To3).Return(nil, nil) + + p.GetLongRunningOperationState("vnet3-to-vnet1", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering3To1).Return(nil, nil) + + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) }, }, { - name: "error in deleting peering where loop terminates prematurely", - expectedError: "failed to delete peering vnet1-to-vnet3 in vnet vnet1 and resource group group1: #: Internal Server Error: StatusCode=500", + name: "error in deleting peering", + expectedError: "failed to delete resource group1/vnet1-to-vnet3 (service: vnetpeerings): this is an error", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) - p.VnetPeeringSpecs().Return([]azure.VnetPeeringSpec{ - { - PeeringName: "vnet1-to-vnet2", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet2", - RemoteResourceGroup: "group2", - }, - { - PeeringName: "vnet2-to-vnet1", - SourceVnetName: "vnet2", - SourceResourceGroup: "group2", - RemoteVnetName: "vnet1", - RemoteResourceGroup: "group1", - }, - { - PeeringName: "vnet1-to-vnet3", - SourceVnetName: "vnet1", - SourceResourceGroup: "group1", - RemoteVnetName: "vnet3", - RemoteResourceGroup: "group3", - }, - { - PeeringName: "vnet3-to-vnet1", - SourceVnetName: "vnet3", - SourceResourceGroup: "group3", - RemoteVnetName: "vnet1", - RemoteResourceGroup: "group1", - }, - }) - p.Vnet().AnyTimes().Return(&infrav1.VnetSpec{ - Name: "vnet1", - ResourceGroup: "group1", - }) - p.ClusterName().AnyTimes().Return("fake-cluster") - p.SubscriptionID().AnyTimes().Return("123") - m.Delete(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet2") - m.Delete(gomockinternal.AContext(), "group2", "vnet2", "vnet2-to-vnet1") - m.Delete(gomockinternal.AContext(), "group1", "vnet1", "vnet1-to-vnet3").Return(autorest.NewErrorWithResponse("", "", &http.Response{StatusCode: 500}, "Internal Server Error")) + p.VnetPeeringSpecs().Return(fakePeeringSpecs) + p.GetLongRunningOperationState("vnet1-to-vnet2", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering1To2).Return(nil, nil) + + p.GetLongRunningOperationState("vnet2-to-vnet1", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering2To1).Return(nil, nil) + + p.GetLongRunningOperationState("vnet1-to-vnet3", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering1To3).Return(nil, errFake) + + p.GetLongRunningOperationState("vnet3-to-vnet1", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering3To1).Return(nil, nil) + + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, gomockinternal.ErrStrEq("failed to delete resource group1/vnet1-to-vnet3 (service: vnetpeerings): this is an error")) + }, + }, + { + name: "error in deleting peering which is not done", + expectedError: "failed to delete resource group1/vnet1-to-vnet3 (service: vnetpeerings): this is an error", + expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, m *mock_vnetpeerings.MockClientMockRecorder) { + p.V(gomock.AssignableToTypeOf(2)).AnyTimes().Return(klogr.New()) + p.VnetPeeringSpecs().Return(fakePeeringSpecs) + p.GetLongRunningOperationState("vnet1-to-vnet2", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering1To2).Return(nil, nil) + + p.GetLongRunningOperationState("vnet2-to-vnet1", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering2To1).Return(nil, nil) + + p.GetLongRunningOperationState("vnet1-to-vnet3", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering1To3).Return(nil, errFake) + + p.GetLongRunningOperationState("vnet3-to-vnet1", serviceName).Return(nil) + m.DeleteAsync(gomockinternal.AContext(), &fakePeering3To1).Return(&fakeFuture, errFoo) + p.SetLongRunningOperationState(gomock.AssignableToTypeOf(&infrav1.Future{})) + + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, gomockinternal.ErrStrEq("failed to delete resource group1/vnet1-to-vnet3 (service: vnetpeerings): this is an error")) }, }, } diff --git a/azure/types.go b/azure/types.go index a4a0e62ce02..0cf5e361969 100644 --- a/azure/types.go +++ b/azure/types.go @@ -110,15 +110,6 @@ type VNetSpec struct { Peerings []infrav1.VnetPeeringSpec } -// VnetPeeringSpec defines the specification for a virtual network peering. -type VnetPeeringSpec struct { - SourceResourceGroup string - SourceVnetName string - RemoteResourceGroup string - RemoteVnetName string - PeeringName string -} - // RoleAssignmentSpec defines the specification for a Role Assignment. type RoleAssignmentSpec struct { MachineName string