Add IsManaged to service reconciler interface

azure/errors.go

@@ -26,9 +26,6 @@ import (
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 )
 
-// ErrNotOwned is returned when a resource can't be deleted because it isn't owned.
-var ErrNotOwned = errors.New("resource is not managed and cannot be deleted")
-
 const codeResourceGroupNotFound = "ResourceGroupNotFound"
 
 // ResourceGroupNotFound parses the error to check if it's a resource group not found error.

azure/interfaces.go

@@ -33,6 +33,7 @@ type Reconciler interface {
 // ServiceReconciler is an Azure service reconciler which can reconcile an Azure service.
 type ServiceReconciler interface {
 	Name() string
+	IsManaged(ctx context.Context) (bool, error)
 	Reconciler
 }
 

azure/scope/cluster.go

@@ -860,8 +860,6 @@ func (s *ClusterScope) UpdateDeleteStatus(condition clusterv1.ConditionType, ser
 	switch {
 	case err == nil:
 		conditions.MarkFalse(s.AzureCluster, condition, infrav1.DeletedReason, clusterv1.ConditionSeverityInfo, "%s successfully deleted", service)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(s.AzureCluster, condition, infrav1.DeletingReason, clusterv1.ConditionSeverityInfo, "%s deleting", service)
 	default:
@@ -874,8 +872,6 @@ func (s *ClusterScope) UpdatePutStatus(condition clusterv1.ConditionType, servic
 	switch {
 	case err == nil:
 		conditions.MarkTrue(s.AzureCluster, condition)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(s.AzureCluster, condition, infrav1.CreatingReason, clusterv1.ConditionSeverityInfo, "%s creating or updating", service)
 	default:
@@ -888,8 +884,6 @@ func (s *ClusterScope) UpdatePatchStatus(condition clusterv1.ConditionType, serv
 	switch {
 	case err == nil:
 		conditions.MarkTrue(s.AzureCluster, condition)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(s.AzureCluster, condition, infrav1.UpdatingReason, clusterv1.ConditionSeverityInfo, "%s updating", service)
 	default:

azure/scope/machine.go

@@ -665,8 +665,6 @@ func (m *MachineScope) UpdateDeleteStatus(condition clusterv1.ConditionType, ser
 	switch {
 	case err == nil:
 		conditions.MarkFalse(m.AzureMachine, condition, infrav1.DeletedReason, clusterv1.ConditionSeverityInfo, "%s successfully deleted", service)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(m.AzureMachine, condition, infrav1.DeletingReason, clusterv1.ConditionSeverityInfo, "%s deleting", service)
 	default:
@@ -679,8 +677,6 @@ func (m *MachineScope) UpdatePutStatus(condition clusterv1.ConditionType, servic
 	switch {
 	case err == nil:
 		conditions.MarkTrue(m.AzureMachine, condition)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(m.AzureMachine, condition, infrav1.CreatingReason, clusterv1.ConditionSeverityInfo, "%s creating or updating", service)
 	default:
@@ -693,8 +689,6 @@ func (m *MachineScope) UpdatePatchStatus(condition clusterv1.ConditionType, serv
 	switch {
 	case err == nil:
 		conditions.MarkTrue(m.AzureMachine, condition)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(m.AzureMachine, condition, infrav1.UpdatingReason, clusterv1.ConditionSeverityInfo, "%s updating", service)
 	default:

azure/scope/machinepool.go

@@ -626,8 +626,6 @@ func (m *MachinePoolScope) UpdateDeleteStatus(condition clusterv1.ConditionType,
 	switch {
 	case err == nil:
 		conditions.MarkFalse(m.AzureMachinePool, condition, infrav1.DeletedReason, clusterv1.ConditionSeverityInfo, "%s successfully deleted", service)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(m.AzureMachinePool, condition, infrav1.DeletingReason, clusterv1.ConditionSeverityInfo, "%s deleting", service)
 	default:
@@ -640,8 +638,6 @@ func (m *MachinePoolScope) UpdatePutStatus(condition clusterv1.ConditionType, se
 	switch {
 	case err == nil:
 		conditions.MarkTrue(m.AzureMachinePool, condition)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(m.AzureMachinePool, condition, infrav1.CreatingReason, clusterv1.ConditionSeverityInfo, "%s creating or updating", service)
 	default:
@@ -654,8 +650,6 @@ func (m *MachinePoolScope) UpdatePatchStatus(condition clusterv1.ConditionType,
 	switch {
 	case err == nil:
 		conditions.MarkTrue(m.AzureMachinePool, condition)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(m.AzureMachinePool, condition, infrav1.UpdatingReason, clusterv1.ConditionSeverityInfo, "%s updating", service)
 	default:

azure/scope/machinepoolmachine.go

@@ -184,8 +184,6 @@ func (s *MachinePoolMachineScope) UpdateDeleteStatus(condition clusterv1.Conditi
 	switch {
 	case err == nil:
 		conditions.MarkFalse(s.AzureMachinePoolMachine, condition, infrav1.DeletedReason, clusterv1.ConditionSeverityInfo, "%s successfully deleted", service)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(s.AzureMachinePoolMachine, condition, infrav1.DeletingReason, clusterv1.ConditionSeverityInfo, "%s deleting", service)
 	default:
@@ -198,8 +196,6 @@ func (s *MachinePoolMachineScope) UpdatePutStatus(condition clusterv1.ConditionT
 	switch {
 	case err == nil:
 		conditions.MarkTrue(s.AzureMachinePoolMachine, condition)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(s.AzureMachinePoolMachine, condition, infrav1.CreatingReason, clusterv1.ConditionSeverityInfo, "%s creating or updating", service)
 	default:
@@ -212,8 +208,6 @@ func (s *MachinePoolMachineScope) UpdatePatchStatus(condition clusterv1.Conditio
 	switch {
 	case err == nil:
 		conditions.MarkTrue(s.AzureMachinePoolMachine, condition)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(s.AzureMachinePoolMachine, condition, infrav1.UpdatingReason, clusterv1.ConditionSeverityInfo, "%s updating", service)
 	default:

azure/scope/managedcontrolplane.go

@@ -695,8 +695,6 @@ func (s *ManagedControlPlaneScope) UpdateDeleteStatus(condition clusterv1.Condit
 	switch {
 	case err == nil:
 		conditions.MarkFalse(s.PatchTarget, condition, infrav1.DeletedReason, clusterv1.ConditionSeverityInfo, "%s successfully deleted", service)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(s.PatchTarget, condition, infrav1.DeletingReason, clusterv1.ConditionSeverityInfo, "%s deleting", service)
 	default:
@@ -709,8 +707,6 @@ func (s *ManagedControlPlaneScope) UpdatePutStatus(condition clusterv1.Condition
 	switch {
 	case err == nil:
 		conditions.MarkTrue(s.PatchTarget, condition)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(s.PatchTarget, condition, infrav1.CreatingReason, clusterv1.ConditionSeverityInfo, "%s creating or updating", service)
 	default:
@@ -723,8 +719,6 @@ func (s *ManagedControlPlaneScope) UpdatePatchStatus(condition clusterv1.Conditi
 	switch {
 	case err == nil:
 		conditions.MarkTrue(s.PatchTarget, condition)
-	case errors.Is(err, azure.ErrNotOwned):
-		// do nothing
 	case azure.IsOperationNotDoneError(err):
 		conditions.MarkFalse(s.PatchTarget, condition, infrav1.UpdatingReason, clusterv1.ConditionSeverityInfo, "%s updating", service)
 	default:

azure/services/availabilitysets/availabilitysets.go

@@ -119,3 +119,8 @@ func (s *Service) Delete(ctx context.Context) error {
 	s.Scope.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, resultingErr)
 	return resultingErr
 }
+
+// IsManaged returns always returns true as CAPZ does not support BYO availability set.
+func (s *Service) IsManaged(ctx context.Context) (bool, error) {
+	return true, nil
+}

azure/services/bastionhosts/bastionhosts.go

@@ -92,3 +92,8 @@ func (s *Service) Delete(ctx context.Context) error {
 	s.Scope.UpdateDeleteStatus(infrav1.BastionHostReadyCondition, serviceName, resultingErr)
 	return resultingErr
 }
+
+// IsManaged returns always returns true as CAPZ does not support BYO bastion.
+func (s *Service) IsManaged(ctx context.Context) (bool, error) {
+	return true, nil
+}

azure/services/disks/disks.go

@@ -91,3 +91,8 @@ func (s *Service) Delete(ctx context.Context) error {
 	s.Scope.UpdateDeleteStatus(infrav1.DisksReadyCondition, serviceName, result)
 	return result
 }
+
+// IsManaged returns always returns true as CAPZ does not support BYO disk.
+func (s *Service) IsManaged(ctx context.Context) (bool, error) {
+	return true, nil
+}

azure/services/groups/groups.go

@@ -93,7 +93,7 @@ func (s *Service) Delete(ctx context.Context) error {
 	}
 
 	// check that the resource group is not BYO.
-	managed, err := s.IsGroupManaged(ctx)
+	managed, err := s.IsManaged(ctx)
 	if err != nil {
 		if azure.ResourceNotFound(err) {
 			// already deleted or doesn't exist, cleanup status and return.
@@ -104,19 +104,19 @@ func (s *Service) Delete(ctx context.Context) error {
 		return errors.Wrap(err, "could not get resource group management state")
 	}
 	if !managed {
-		log.V(2).Info("Should not delete resource group in unmanaged mode")
-		return azure.ErrNotOwned
+		log.V(2).Info("Skipping resource group deletion in unmanaged mode")
+		return nil
 	}
 
 	err = s.DeleteResource(ctx, groupSpec, ServiceName)
 	s.Scope.UpdateDeleteStatus(infrav1.ResourceGroupReadyCondition, ServiceName, err)
 	return err
 }
 
-// IsGroupManaged returns true if the resource group has an owned tag with the cluster name as value,
+// IsManaged returns true if the resource group has an owned tag with the cluster name as value,
 // meaning that the resource group's lifecycle is managed.
-func (s *Service) IsGroupManaged(ctx context.Context) (bool, error) {
-	ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.Service.IsGroupManaged")
+func (s *Service) IsManaged(ctx context.Context) (bool, error) {
+	ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.Service.IsManaged")
 	defer done()
 
 	groupSpec := s.Scope.GroupSpec()

azure/services/groups/groups_test.go

@@ -27,7 +27,6 @@ import (
 	"github.com/golang/mock/gomock"
 	. "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/groups/mock_groups"
 	gomockinternal "sigs.k8s.io/cluster-api-provider-azure/internal/test/matchers/gomock"
@@ -146,7 +145,7 @@ func TestDeleteGroups(t *testing.T) {
 		},
 		{
 			name:          "resource group is not managed by capz",
-			expectedError: azure.ErrNotOwned.Error(),
+			expectedError: "",
 			expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
 				s.GroupSpec().AnyTimes().Return(&fakeGroupSpec)
 				m.Get(gomockinternal.AContext(), &fakeGroupSpec).Return(sampleBYOGroup, nil)

azure/services/inboundnatrules/inboundnatrules.go

@@ -138,3 +138,8 @@ func (s *Service) Delete(ctx context.Context) error {
 	s.Scope.UpdateDeleteStatus(infrav1.InboundNATRulesReadyCondition, serviceName, result)
 	return result
 }
+
+// IsManaged returns always returns true as CAPZ does not support BYO inbound NAT rules.
+func (s *Service) IsManaged(ctx context.Context) (bool, error) {
+	return true, nil
+}

azure/services/loadbalancers/loadbalancers.go

@@ -117,3 +117,8 @@ func (s *Service) Delete(ctx context.Context) error {
 	s.Scope.UpdateDeleteStatus(infrav1.LoadBalancersReadyCondition, serviceName, result)
 	return result
 }
+
+// IsManaged returns always returns true as CAPZ does not support BYO load balancers.
+func (s *Service) IsManaged(ctx context.Context) (bool, error) {
+	return true, nil
+}

azure/services/managedclusters/managedclusters.go

@@ -377,3 +377,8 @@ func (s *Service) Delete(ctx context.Context) error {
 	klog.V(2).Infof("successfully deleted managed cluster %s ", s.Scope.ClusterName())
 	return nil
 }
+
+// IsManaged returns always returns true as CAPZ does not support BYO managed cluster.
+func (s *Service) IsManaged(ctx context.Context) (bool, error) {
+	return true, nil
+}

azure/services/natgateways/natgateways.go

@@ -67,11 +67,11 @@ func (s *Service) Reconcile(ctx context.Context) error {
 	ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout)
 	defer cancel()
 
-	if !s.Scope.Vnet().IsManaged(s.Scope.ClusterName()) {
+	if managed, err := s.IsManaged(ctx); err == nil && !managed {
 		log.V(4).Info("Skipping nat gateways reconcile in custom vnet mode")
-
-		s.Scope.UpdatePutStatus(infrav1.NATGatewaysReadyCondition, serviceName, nil)
 		return nil
+	} else if err != nil {
+		return errors.Wrap(err, "failed to check if NAT gateways are managed")
 	}
 
 	// We go through the list of NatGatewaySpecs to reconcile each one, independently of the resultingErr of the previous one.
@@ -115,11 +115,11 @@ func (s *Service) Delete(ctx context.Context) error {
 	ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout)
 	defer cancel()
 
-	if !s.Scope.Vnet().IsManaged(s.Scope.ClusterName()) {
+	if managed, err := s.IsManaged(ctx); err == nil && !managed {
 		log.V(4).Info("Skipping nat gateway deletion in custom vnet mode")
-
-		s.Scope.UpdateDeleteStatus(infrav1.NATGatewaysReadyCondition, serviceName, nil)
 		return nil
+	} else if err != nil {
+		return errors.Wrap(err, "failed to check if NAT gateways are managed")
 	}
 
 	specs := s.Scope.NatGatewaySpecs()
@@ -141,3 +141,11 @@ func (s *Service) Delete(ctx context.Context) error {
 	s.Scope.UpdateDeleteStatus(infrav1.NATGatewaysReadyCondition, serviceName, resultingErr)
 	return resultingErr
 }
+
+// IsManaged returns true if the NAT gateways' lifecycles are managed.
+func (s *Service) IsManaged(ctx context.Context) (bool, error) {
+	_, _, done := tele.StartSpanWithLogger(ctx, "natgateways.Service.IsManaged")
+	defer done()
+
+	return s.Scope.IsVnetManaged(), nil
+}