diff --git a/azure/services/vnetpeerings/vnetpeerings.go b/azure/services/vnetpeerings/vnetpeerings.go index 322dea9ac57..fcd2bfa8b6d 100644 --- a/azure/services/vnetpeerings/vnetpeerings.go +++ b/azure/services/vnetpeerings/vnetpeerings.go @@ -26,7 +26,8 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) -const serviceName = "vnetpeerings" +// ServiceName is the name of this service. +const ServiceName = "vnetpeerings" // VnetPeeringScope defines the scope interface for a subnet service. type VnetPeeringScope interface { @@ -52,7 +53,7 @@ func New(scope VnetPeeringScope) *Service { // Name returns the service name. func (s *Service) Name() string { - return serviceName + return ServiceName } // Reconcile idempotently creates or updates a peering. @@ -73,14 +74,14 @@ func (s *Service) Reconcile(ctx context.Context) error { // 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 result error for _, peeringSpec := range specs { - if _, err := s.CreateOrUpdateResource(ctx, peeringSpec, serviceName); err != nil { + if _, err := s.CreateOrUpdateResource(ctx, peeringSpec, ServiceName); err != nil { if !azure.IsOperationNotDoneError(err) || result == nil { result = err } } } - s.Scope.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, result) + s.Scope.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, result) return result } @@ -102,13 +103,13 @@ func (s *Service) Delete(ctx context.Context) error { // 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) var result error for _, peeringSpec := range specs { - if err := s.DeleteResource(ctx, peeringSpec, serviceName); err != nil { + if err := s.DeleteResource(ctx, peeringSpec, ServiceName); err != nil { if !azure.IsOperationNotDoneError(err) || result == nil { result = err } } } - s.Scope.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, result) + 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 37949b08e72..206adedc153 100644 --- a/azure/services/vnetpeerings/vnetpeerings_test.go +++ b/azure/services/vnetpeerings/vnetpeerings_test.go @@ -90,8 +90,8 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs[:1]) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(&fakePeering1To2, nil) - p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(&fakePeering1To2, nil) + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, nil) }, }, { @@ -106,9 +106,9 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs[:2]) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(&fakePeering1To2, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(&fakePeering2To1, nil) - p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(&fakePeering1To2, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(&fakePeering2To1, nil) + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, nil) }, }, { @@ -116,10 +116,10 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringExtraSpecs) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(&fakePeering1To2, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(&fakePeering2To1, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeeringExtra, serviceName).Return(&fakePeeringExtra, nil) - p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(&fakePeering1To2, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(&fakePeering2To1, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeeringExtra, ServiceName).Return(&fakePeeringExtra, nil) + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, nil) }, }, { @@ -127,11 +127,11 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(&fakePeering1To2, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(&fakePeering2To1, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, serviceName).Return(&fakePeering1To3, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, serviceName).Return(&fakePeering3To1, nil) - p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(&fakePeering1To2, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(&fakePeering2To1, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(&fakePeering1To3, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(&fakePeering3To1, nil) + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, nil) }, }, { @@ -139,11 +139,11 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "#: Internal Server Error: StatusCode=500", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(&fakePeering1To2, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(&fakePeering2To1, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, serviceName).Return(nil, internalError) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, serviceName).Return(&fakePeering3To1, nil) - p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, internalError) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(&fakePeering1To2, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(&fakePeering2To1, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(nil, internalError) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(&fakePeering3To1, nil) + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError) }, }, { @@ -151,11 +151,11 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "#: Internal Server Error: StatusCode=500", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(&fakePeering1To2, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(&fakePeering2To1, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, serviceName).Return(nil, internalError) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, serviceName).Return(&fakePeering3To1, nil) - p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, internalError) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(&fakePeering1To2, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(&fakePeering2To1, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(nil, internalError) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(&fakePeering3To1, nil) + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError) }, }, { @@ -163,11 +163,11 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "#: Internal Server Error: StatusCode=500", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(&fakePeering1To2, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(nil, internalError) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, serviceName).Return(nil, notDoneError) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, serviceName).Return(&fakePeering3To1, nil) - p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, internalError) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(&fakePeering1To2, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(nil, internalError) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(nil, notDoneError) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(&fakePeering3To1, nil) + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError) }, }, { @@ -175,11 +175,11 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "#: Internal Server Error: StatusCode=500", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(&fakePeering1To2, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(&fakePeering2To1, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, serviceName).Return(nil, notDoneError) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, serviceName).Return(nil, internalError) - p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, internalError) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(&fakePeering1To2, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(&fakePeering2To1, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(nil, notDoneError) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(nil, internalError) + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError) }, }, { @@ -187,11 +187,11 @@ func TestReconcileVnetPeerings(t *testing.T) { expectedError: "operation type on Azure resource / is not done", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(&fakePeering1To2, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(&fakePeering2To1, nil) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, serviceName).Return(nil, notDoneError) - r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, serviceName).Return(&fakePeering3To1, nil) - p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, serviceName, notDoneError) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(&fakePeering1To2, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(&fakePeering2To1, nil) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(nil, notDoneError) + r.CreateOrUpdateResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(&fakePeering3To1, nil) + p.UpdatePutStatus(infrav1.VnetPeeringReadyCondition, ServiceName, notDoneError) }, }, } @@ -236,8 +236,8 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs[:1]) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(nil) - p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(nil) + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, nil) }, }, { @@ -252,9 +252,9 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs[:2]) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(nil) - p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(nil) + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, nil) }, }, { @@ -262,10 +262,10 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringExtraSpecs) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeeringExtra, serviceName).Return(nil) - p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeeringExtra, ServiceName).Return(nil) + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, nil) }, }, { @@ -273,11 +273,11 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, serviceName).Return(nil) - p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(nil) + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, nil) }, }, { @@ -285,11 +285,11 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "#: Internal Server Error: StatusCode=500", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, serviceName).Return(internalError) - r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, serviceName).Return(nil) - p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, internalError) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(internalError) + r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(nil) + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError) }, }, { @@ -297,11 +297,11 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "#: Internal Server Error: StatusCode=500", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(internalError) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, serviceName).Return(notDoneError) - r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, serviceName).Return(nil) - p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, internalError) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(internalError) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(notDoneError) + r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(nil) + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError) }, }, { @@ -309,11 +309,11 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "#: Internal Server Error: StatusCode=500", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, serviceName).Return(notDoneError) - r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, serviceName).Return(internalError) - p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, internalError) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(notDoneError) + r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(internalError) + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, internalError) }, }, { @@ -321,11 +321,11 @@ func TestDeleteVnetPeerings(t *testing.T) { expectedError: "operation type on Azure resource / is not done", expect: func(p *mock_vnetpeerings.MockVnetPeeringScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { p.VnetPeeringSpecs().Return(fakePeeringSpecs) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, serviceName).Return(notDoneError) - r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, serviceName).Return(nil) - p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, serviceName, notDoneError) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To2, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering2To1, ServiceName).Return(nil) + r.DeleteResource(gomockinternal.AContext(), &fakePeering1To3, ServiceName).Return(notDoneError) + r.DeleteResource(gomockinternal.AContext(), &fakePeering3To1, ServiceName).Return(nil) + p.UpdateDeleteStatus(infrav1.VnetPeeringReadyCondition, ServiceName, notDoneError) }, }, } diff --git a/controllers/azurecluster_reconciler.go b/controllers/azurecluster_reconciler.go index eb7be42cfaf..6501b585a8c 100644 --- a/controllers/azurecluster_reconciler.go +++ b/controllers/azurecluster_reconciler.go @@ -114,7 +114,16 @@ func (s *azureClusterService) Delete(ctx context.Context) error { return errors.Wrap(err, "failed to determine if the AzureCluster resource group is managed") } if managed { - // if the resource group is managed, we delete the entire resource group directly. + // If the resource group is managed, delete it. + // We need to explicitly delete vnet peerings, as it is not part of the resource group. + vnetPeeringsSvc, err := s.getService(vnetpeerings.ServiceName) + if err != nil { + return errors.Wrap(err, "failed to get vnet peerings service") + } + if err := vnetPeeringsSvc.Delete(ctx); err != nil { + return errors.Wrap(err, "failed to delete peerings") + } + // Delete the entire resource group directly. if err := groupSvc.Delete(ctx); err != nil { return errors.Wrap(err, "failed to delete resource group") } diff --git a/controllers/azurecluster_reconciler_test.go b/controllers/azurecluster_reconciler_test.go index 3028c7211ee..7cafec1e4ba 100644 --- a/controllers/azurecluster_reconciler_test.go +++ b/controllers/azurecluster_reconciler_test.go @@ -30,6 +30,7 @@ import ( "sigs.k8s.io/cluster-api-provider-azure/azure/scope" "sigs.k8s.io/cluster-api-provider-azure/azure/services/groups" "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/vnetpeerings" gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) @@ -100,20 +101,23 @@ func TestAzureClusterServiceReconcile(t *testing.T) { func TestAzureClusterServiceDelete(t *testing.T) { cases := map[string]struct { expectedError string - expect func(grp *mock_azure.MockServiceReconcilerMockRecorder, one *mock_azure.MockServiceReconcilerMockRecorder, two *mock_azure.MockServiceReconcilerMockRecorder, three *mock_azure.MockServiceReconcilerMockRecorder) + expect func(grp *mock_azure.MockServiceReconcilerMockRecorder, vpr *mock_azure.MockServiceReconcilerMockRecorder, one *mock_azure.MockServiceReconcilerMockRecorder, two *mock_azure.MockServiceReconcilerMockRecorder, three *mock_azure.MockServiceReconcilerMockRecorder) }{ "Resource Group is deleted successfully": { expectedError: "", - expect: func(grp *mock_azure.MockServiceReconcilerMockRecorder, one *mock_azure.MockServiceReconcilerMockRecorder, two *mock_azure.MockServiceReconcilerMockRecorder, three *mock_azure.MockServiceReconcilerMockRecorder) { + expect: func(grp *mock_azure.MockServiceReconcilerMockRecorder, vpr *mock_azure.MockServiceReconcilerMockRecorder, one *mock_azure.MockServiceReconcilerMockRecorder, two *mock_azure.MockServiceReconcilerMockRecorder, three *mock_azure.MockServiceReconcilerMockRecorder) { gomock.InOrder( grp.Name().Return(groups.ServiceName), grp.IsManaged(gomockinternal.AContext()).Return(true, nil), + grp.Name().Return(groups.ServiceName), + vpr.Name().Return(vnetpeerings.ServiceName), + vpr.Delete(gomockinternal.AContext()).Return(nil), grp.Delete(gomockinternal.AContext()).Return(nil)) }, }, "Error when checking if resource group is managed": { expectedError: "failed to determine if the AzureCluster resource group is managed: an error happened", - expect: func(grp *mock_azure.MockServiceReconcilerMockRecorder, one *mock_azure.MockServiceReconcilerMockRecorder, two *mock_azure.MockServiceReconcilerMockRecorder, three *mock_azure.MockServiceReconcilerMockRecorder) { + expect: func(grp *mock_azure.MockServiceReconcilerMockRecorder, vpr *mock_azure.MockServiceReconcilerMockRecorder, one *mock_azure.MockServiceReconcilerMockRecorder, two *mock_azure.MockServiceReconcilerMockRecorder, three *mock_azure.MockServiceReconcilerMockRecorder) { gomock.InOrder( grp.Name().Return(groups.ServiceName), grp.IsManaged(gomockinternal.AContext()).Return(false, errors.New("an error happened"))) @@ -121,28 +125,32 @@ func TestAzureClusterServiceDelete(t *testing.T) { }, "Resource Group delete fails": { expectedError: "failed to delete resource group: internal error", - expect: func(grp *mock_azure.MockServiceReconcilerMockRecorder, one *mock_azure.MockServiceReconcilerMockRecorder, two *mock_azure.MockServiceReconcilerMockRecorder, three *mock_azure.MockServiceReconcilerMockRecorder) { + expect: func(grp *mock_azure.MockServiceReconcilerMockRecorder, vpr *mock_azure.MockServiceReconcilerMockRecorder, one *mock_azure.MockServiceReconcilerMockRecorder, two *mock_azure.MockServiceReconcilerMockRecorder, three *mock_azure.MockServiceReconcilerMockRecorder) { gomock.InOrder( grp.Name().Return(groups.ServiceName), grp.IsManaged(gomockinternal.AContext()).Return(true, nil), + grp.Name().Return(groups.ServiceName), + vpr.Name().Return(vnetpeerings.ServiceName), + vpr.Delete(gomockinternal.AContext()).Return(nil), grp.Delete(gomockinternal.AContext()).Return(errors.New("internal error"))) }, }, "Resource Group not owned by cluster": { expectedError: "", - expect: func(grp *mock_azure.MockServiceReconcilerMockRecorder, one *mock_azure.MockServiceReconcilerMockRecorder, two *mock_azure.MockServiceReconcilerMockRecorder, three *mock_azure.MockServiceReconcilerMockRecorder) { + expect: func(grp *mock_azure.MockServiceReconcilerMockRecorder, vpr *mock_azure.MockServiceReconcilerMockRecorder, one *mock_azure.MockServiceReconcilerMockRecorder, two *mock_azure.MockServiceReconcilerMockRecorder, three *mock_azure.MockServiceReconcilerMockRecorder) { gomock.InOrder( grp.Name().Return(groups.ServiceName), grp.IsManaged(gomockinternal.AContext()).Return(false, nil), three.Delete(gomockinternal.AContext()).Return(nil), two.Delete(gomockinternal.AContext()).Return(nil), one.Delete(gomockinternal.AContext()).Return(nil), + vpr.Delete(gomockinternal.AContext()).Return(nil), grp.Delete(gomockinternal.AContext()).Return(nil)) }, }, "service delete fails": { expectedError: "failed to delete AzureCluster service two: some error happened", - expect: func(grp *mock_azure.MockServiceReconcilerMockRecorder, one *mock_azure.MockServiceReconcilerMockRecorder, two *mock_azure.MockServiceReconcilerMockRecorder, three *mock_azure.MockServiceReconcilerMockRecorder) { + expect: func(grp *mock_azure.MockServiceReconcilerMockRecorder, vpr *mock_azure.MockServiceReconcilerMockRecorder, one *mock_azure.MockServiceReconcilerMockRecorder, two *mock_azure.MockServiceReconcilerMockRecorder, three *mock_azure.MockServiceReconcilerMockRecorder) { gomock.InOrder( grp.Name().Return(groups.ServiceName), grp.IsManaged(gomockinternal.AContext()).Return(false, nil), @@ -162,11 +170,12 @@ func TestAzureClusterServiceDelete(t *testing.T) { mockCtrl := gomock.NewController(t) defer mockCtrl.Finish() groupsMock := mock_azure.NewMockServiceReconciler(mockCtrl) + vnetpeeringsMock := mock_azure.NewMockServiceReconciler(mockCtrl) svcOneMock := mock_azure.NewMockServiceReconciler(mockCtrl) svcTwoMock := mock_azure.NewMockServiceReconciler(mockCtrl) svcThreeMock := mock_azure.NewMockServiceReconciler(mockCtrl) - tc.expect(groupsMock.EXPECT(), svcOneMock.EXPECT(), svcTwoMock.EXPECT(), svcThreeMock.EXPECT()) + tc.expect(groupsMock.EXPECT(), vnetpeeringsMock.EXPECT(), svcOneMock.EXPECT(), svcTwoMock.EXPECT(), svcThreeMock.EXPECT()) s := &azureClusterService{ scope: &scope.ClusterScope{ @@ -174,6 +183,7 @@ func TestAzureClusterServiceDelete(t *testing.T) { }, services: []azure.ServiceReconciler{ groupsMock, + vnetpeeringsMock, svcOneMock, svcTwoMock, svcThreeMock,