fix cluster reconciler test

azure/services/groups/groups.go

@@ -97,7 +97,21 @@ func (s *Service) Delete(ctx context.Context) error {
 // IsManaged returns true if the ASO ResourceGroup was created by CAPZ,
 // meaning that the resource group's lifecycle is managed.
 func (s *Service) IsManaged(ctx context.Context) (bool, error) {
-	return aso.IsManaged(ctx, s.Scope.GetClient(), s.Scope.GroupSpec(), s.Scope.ClusterName())
+	managed, err := aso.IsManaged(ctx, s.Scope.GetClient(), s.Scope.GroupSpec(), s.Scope.ClusterName())
+	if err != nil {
+		return managed, err
+	}
+
+	// For ASO, "managed" only tells us that we're allowed to delete the ASO
+	// resource. We also need to check that deleting the ASO resource will really
+	// delete the underlying resource group by checking the ASO reconcile-policy.
+	spec := s.Scope.GroupSpec()
+	group := spec.ResourceRef()
+	err = s.Scope.GetClient().Get(ctx, client.ObjectKeyFromObject(group), group)
+	if err != nil {
+		return false, err
+	}
+	return group.GetAnnotations()[asoannotations.ReconcilePolicy] != string(asoannotations.ReconcilePolicyManage), nil
 }
 
 var _ azure.Pauser = (*Service)(nil)
@@ -110,23 +124,3 @@ func (s *Service) Pause(ctx context.Context) error {
 	}
 	return aso.PauseResource(ctx, s.Scope.GetClient(), groupSpec, s.Scope.ClusterName(), ServiceName)
 }
-
-// ShouldDeleteIndividualResources returns false if the resource group is
-// managed and reconciled by ASO, meaning that we can rely on a single resource
-// group delete operation as opposed to deleting every individual resource.
-func (s *Service) ShouldDeleteIndividualResources(ctx context.Context) bool {
-	// Since this is a best effort attempt to speed up delete, we don't fail the delete if we can't get the RG status.
-	// Instead, take the long way and delete all resources one by one.
-	managed, err := s.IsManaged(ctx)
-	if err != nil || !managed {
-		return true
-	}
-
-	// For ASO, "managed" only tells us that we're allowed to delete the ASO
-	// resource. We also need to check that deleting the ASO resource will really
-	// delete the underlying resource group by checking the ASO reconcile-policy.
-	spec := s.Scope.GroupSpec()
-	group := spec.ResourceRef()
-	err = s.Scope.GetClient().Get(ctx, client.ObjectKeyFromObject(group), group)
-	return err != nil || group.GetAnnotations()[asoannotations.ReconcilePolicy] != string(asoannotations.ReconcilePolicyManage)
-}

controllers/azurecluster_reconciler.go

@@ -163,15 +163,15 @@ func (s *azureClusterService) Delete(ctx context.Context) error {
 	ctx, _, done := tele.StartSpanWithLogger(ctx, "controllers.azureClusterService.Delete")
 	defer done()
 
-	groupSvc, err := GetService[*groups.Service](groups.ServiceName, s.services)
+	groupSvc, err := GetService(groups.ServiceName, s.services)
 	if err != nil {
 		return errors.Wrap(err, "failed to get group service")
 	}
 
 	if !ShouldDeleteIndividualResources(ctx, groupSvc, s.scope) {
 		// 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 := GetService[*vnetpeerings.Service](vnetpeerings.ServiceName, s.services)
+		vnetPeeringsSvc, err := GetService(vnetpeerings.ServiceName, s.services)
 		if err != nil {
 			return errors.Wrap(err, "failed to get vnet peerings service")
 		}

controllers/azurecluster_reconciler_test.go

@@ -213,10 +213,11 @@ func TestAzureClusterServiceDelete(t *testing.T) {
 			},
 			expect: func(grp *mock_azure.MockServiceReconcilerMockRecorder, vpr *mock_azure.MockServiceReconcilerMockRecorder, _ *mock_azure.MockServiceReconcilerMockRecorder, _ *mock_azure.MockServiceReconcilerMockRecorder, _ *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.Name().Return(groups.ServiceName),
 					grp.Delete(gomockinternal.AContext()).Return(nil))
 			},
 		},
@@ -248,10 +249,11 @@ func TestAzureClusterServiceDelete(t *testing.T) {
 			},
 			expect: func(grp *mock_azure.MockServiceReconcilerMockRecorder, vpr *mock_azure.MockServiceReconcilerMockRecorder, _ *mock_azure.MockServiceReconcilerMockRecorder, _ *mock_azure.MockServiceReconcilerMockRecorder, _ *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.Name().Return(groups.ServiceName),
 					grp.Delete(gomockinternal.AContext()).Return(errors.New("internal error")))
 			},
 		},
@@ -280,6 +282,8 @@ func TestAzureClusterServiceDelete(t *testing.T) {
 			},
 			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),
@@ -310,8 +314,10 @@ func TestAzureClusterServiceDelete(t *testing.T) {
 
 				return c
 			},
-			expect: func(_ *mock_azure.MockServiceReconcilerMockRecorder, _ *mock_azure.MockServiceReconcilerMockRecorder, _ *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(errors.New("some error happened")),
 					two.Name().Return("two"))

controllers/azuremachine_reconciler.go

@@ -156,7 +156,7 @@ func (s *azureMachineService) delete(ctx context.Context) error {
 	ctx, log, done := tele.StartSpanWithLogger(ctx, "controllers.azureMachineService.delete")
 	defer done()
 
-	groupSvc, err := GetService[*groups.Service](groups.ServiceName, s.services)
+	groupSvc, err := GetService(groups.ServiceName, s.services)
 	if err != nil {
 		return errors.Wrap(err, "failed to get group service")
 	}

controllers/helpers.go

@@ -37,7 +37,6 @@ 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/scope"
-	"sigs.k8s.io/cluster-api-provider-azure/azure/services/groups"
 	infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/feature"
 	"sigs.k8s.io/cluster-api-provider-azure/pkg/coalescing"
@@ -599,15 +598,18 @@ func GetAzureMachinePoolByName(ctx context.Context, c client.Client, namespace,
 
 // ShouldDeleteIndividualResources returns false if the resource group is managed and the whole cluster is being deleted
 // meaning that we can rely on a single resource group delete operation as opposed to deleting every individual VM resource.
-func ShouldDeleteIndividualResources(ctx context.Context, groupSvc *groups.Service, cluster azure.ClusterScoper) bool {
+func ShouldDeleteIndividualResources(ctx context.Context, grpSvc azure.ServiceReconciler, cluster azure.ClusterScoper) bool {
 	ctx, _, done := tele.StartSpanWithLogger(ctx, "controllers.ShouldDeleteIndividualResources")
 	defer done()
 
 	if cluster.GetDeletionTimestamp().IsZero() {
 		return true
 	}
 
-	return groupSvc.ShouldDeleteIndividualResources(ctx)
+	managed, err := grpSvc.IsManaged(ctx)
+	// Since this is a best effort attempt to speed up delete, we don't fail the delete if we can't get the RG status.
+	// Instead, take the long way and delete all resources one by one.
+	return err != nil || !managed
 }
 
 // GetClusterIdentityFromRef returns the AzureClusterIdentity referenced by the AzureCluster.
@@ -1062,16 +1064,11 @@ func ClusterPauseChangeAndInfrastructureReady(log logr.Logger) predicate.Funcs {
 }
 
 // GetService returns the service with the given name and type.
-func GetService[T azure.ServiceReconciler](name string, services []azure.ServiceReconciler) (T, error) {
-	var result T
+func GetService(name string, services []azure.ServiceReconciler) (azure.ServiceReconciler, error) {
 	for _, service := range services {
 		if service.Name() == name {
-			result, ok := service.(T)
-			if !ok {
-				return result, errors.Errorf("service %s is not of type %T", name, result)
-			}
-			return result, nil
+			return service, nil
 		}
 	}
-	return result, errors.Errorf("service %s not found", name)
+	return nil, errors.Errorf("service %s not found", name)
 }

exp/controllers/azuremachinepool_reconciler.go

@@ -106,7 +106,7 @@ func (s *azureMachinePoolService) Delete(ctx context.Context) error {
 	ctx, log, done := tele.StartSpanWithLogger(ctx, "controllers.azureMachinePoolService.Delete")
 	defer done()
 
-	groupSvc, err := infracontroller.GetService[*groups.Service](groups.ServiceName, s.services)
+	groupSvc, err := infracontroller.GetService(groups.ServiceName, s.services)
 	if err != nil {
 		return errors.Wrap(err, "failed to get group service")
 	}