Gate SPC deletion + delete-operation on removal of all remaining ApplyDesires

backend/pkg/controllers/cluster/deletion/cluster_child_resources_cleanup_controller.go

@@ -271,6 +271,27 @@ func (c *clusterChildResourcesCleanupController) extraDeleteGateShouldDeleteServ
 		return false, nil
 	}
 
+	// Wait until every ApplyDesire for this cluster has been removed. Each authoring
+	// controller is responsible for deleting the ApplyDesires it owns during cluster
+	// deletion; we only remove the ServiceProviderCluster document (which carries the
+	// ManagementClusterResourceID needed to reach them) once they are all gone. Report
+	// the per-controller breakdown so it is clear which controller is lagging.
+	remaining, applyDesireBreakdown, err := c.remainingApplyDesires(
+		ctx,
+		spc,
+		serviceProviderClusterResourceID.SubscriptionID,
+		serviceProviderClusterResourceID.ResourceGroupName,
+		clusterName,
+	)
+	if err != nil {
+		return false, utils.TrackError(fmt.Errorf("failed to check remaining ApplyDesires: %w", err))
+	}
+	if remaining > 0 {
+		logger.Info(fmt.Sprintf("waiting for %d ApplyDesire(s) to be deleted before removing ServiceProviderCluster: %s", remaining, applyDesireBreakdown),
+			"serviceProviderClusterResourceID", spc.ResourceID.String())
+		return false, nil
+	}
+
 	// Check if there are any cluster-scoped kube-applier *Desire documents remaining.
 	if spc.Status.ManagementClusterResourceID != nil {
 		kaClient := c.kubeApplierDBClients.For(ctx, spc.Status.ManagementClusterResourceID)
@@ -307,6 +328,41 @@ func (c *clusterChildResourcesCleanupController) extraDeleteGateShouldDeleteServ
 	return true, nil
 }
 
+// remainingApplyDesires reports how many ApplyDesires still exist for the cluster
+// owning the given ServiceProviderCluster, along with a human-readable breakdown
+// grouped by the authoring controller. Each authoring controller is responsible for
+// deleting the ApplyDesires it owns during cluster deletion; this controller only
+// verifies they are gone before removing the ServiceProviderCluster document (which
+// carries the ManagementClusterResourceID needed to reach them). A nil management
+// cluster reference or unavailable kube-applier client is treated as "no remaining
+// ApplyDesires", consistent with the best-effort behavior elsewhere in this file.
+func (c *clusterChildResourcesCleanupController) remainingApplyDesires(ctx context.Context, spc *coreapi.ServiceProviderCluster, subscriptionID, resourceGroupName, clusterName string) (int, string, error) {
+	logger := utils.LoggerFromContext(ctx)
+
+	if spc == nil || spc.Status.ManagementClusterResourceID == nil {
+		return 0, "", nil
+	}
+
+	managementClusterID := spc.Status.ManagementClusterResourceID
+	kubeApplierDBClient := c.kubeApplierDBClients.For(ctx, managementClusterID)
+	if kubeApplierDBClient == nil {
+		logger.Info("no kube-applier client for management cluster; treating ApplyDesires as gone",
+			"managementClusterResourceID", managementClusterID.String())
+		return 0, "", nil
+	}
+
+	applyDesireCRUD, err := kubeApplierDBClient.ApplyDesiresForCluster(subscriptionID, resourceGroupName, clusterName)
+	if err != nil {
+		return 0, "", utils.TrackError(fmt.Errorf("failed to get kube-applier CRUD for ApplyDesire precondition: %w", err))
+	}
+
+	total, breakdown, err := kubeappliercosmosstorage.SummarizeApplyDesiresByController(ctx, applyDesireCRUD)
+	if err != nil {
+		return 0, "", utils.TrackError(err)
+	}
+	return total, breakdown, nil
+}
+
 // ensureClusterScopedKubeApplierResourcesDeleted ensures that the cluster-scoped *Desire documents are deleted
 // from the database. *Desire documents on non-cluster scoped resources are deleted by their corresponding deletion controllers.
 func (c *clusterChildResourcesCleanupController) ensureClusterScopedKubeApplierResourcesDeleted(ctx context.Context, clusterResourceID *azcorearm.ResourceID) error {

backend/pkg/controllers/cluster/deletion/cluster_child_resources_cleanup_controller_test.go

@@ -729,3 +729,207 @@ func TestIsUnderSkippedSubtree(t *testing.T) {
 		})
 	}
 }
+
+func TestClusterChildResourcesCleanupController_remainingApplyDesires(t *testing.T) {
+	managementClusterResourceID := metadataapi.Must(azcorearm.ParseResourceID(
+		"/providers/microsoft.redhatopenshift/stamps/1/managementclusters/default"))
+	unregisteredManagementClusterResourceID := metadataapi.Must(azcorearm.ParseResourceID(
+		"/providers/microsoft.redhatopenshift/stamps/1/managementclusters/unregistered"))
+
+	newSPC := func(mc *azcorearm.ResourceID) *coreapi.ServiceProviderCluster {
+		spcResourceID := metadataapi.Must(azcorearm.ParseResourceID(
+			"/subscriptions/" + testSubscriptionID +
+				"/resourceGroups/" + testResourceGroupName +
+				"/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/" + testClusterName +
+				"/serviceProviderClusters/default"))
+		return &coreapi.ServiceProviderCluster{
+			CosmosMetadata: coreapi.CosmosMetadata{
+				ResourceID:   spcResourceID,
+				PartitionKey: strings.ToLower(spcResourceID.SubscriptionID),
+			},
+			Status: coreapi.ServiceProviderClusterStatus{
+				ManagementClusterResourceID: mc,
+			},
+		}
+	}
+	newApplyDesire := func(name string, tags map[string]string) *kubeapplierapi.ApplyDesire {
+		resourceID := metadataapi.Must(azcorearm.ParseResourceID(
+			kubeapplierapi.ToClusterScopedApplyDesireResourceIDString(
+				testSubscriptionID, testResourceGroupName, testClusterName, name)))
+		return &kubeapplierapi.ApplyDesire{
+			CosmosMetadata: coreapi.CosmosMetadata{
+				ResourceID:   resourceID,
+				PartitionKey: strings.ToLower(managementClusterResourceID.String()),
+			},
+			Spec: kubeapplierapi.ApplyDesireSpec{
+				ManagementCluster: managementClusterResourceID,
+			},
+			Tags: tags,
+		}
+	}
+	taggedDesire := func(name, controllerName string) *kubeapplierapi.ApplyDesire {
+		return newApplyDesire(name, map[string]string{kubeapplierapi.TagControllerName: controllerName})
+	}
+	untaggedDesire := func(name string) *kubeapplierapi.ApplyDesire {
+		return newApplyDesire(name, nil)
+	}
+
+	testCases := []struct {
+		name               string
+		spc                *coreapi.ServiceProviderCluster
+		kubeApplierDesires []any
+		wantTotal          int
+		wantBreakdown      string
+	}{
+		{
+			name:               "tagged ApplyDesire present -> counted by controller",
+			spc:                newSPC(managementClusterResourceID),
+			kubeApplierDesires: []any{taggedDesire("desire-a", "test-controller")},
+			wantTotal:          1,
+			wantBreakdown:      "1 for controller test-controller",
+		},
+		{
+			name:               "untagged ApplyDesire present -> counted as unknown",
+			spc:                newSPC(managementClusterResourceID),
+			kubeApplierDesires: []any{untaggedDesire("desire-a")},
+			wantTotal:          1,
+			wantBreakdown:      "1 for controller unknown",
+		},
+		{
+			// Breakdown is sorted by controller NAME, not by count: "aaa-controller"
+			// has more desires than the "unknown" bucket yet still sorts first. This
+			// would fail if the formatted "%d for controller %s" strings were sorted
+			// (that orders by leading count).
+			name: "multiple controllers -> breakdown sorted by controller name, not count",
+			spc:  newSPC(managementClusterResourceID),
+			kubeApplierDesires: []any{
+				taggedDesire("desire-a", "aaa-controller"),
+				taggedDesire("desire-b", "aaa-controller"),
+				untaggedDesire("desire-c"),
+			},
+			wantTotal:     3,
+			wantBreakdown: "2 for controller aaa-controller, 1 for controller unknown",
+		},
+		{
+			name:          "no ApplyDesires -> none remaining",
+			spc:           newSPC(managementClusterResourceID),
+			wantTotal:     0,
+			wantBreakdown: "",
+		},
+		{
+			name:          "nil management cluster resource ID -> none remaining",
+			spc:           newSPC(nil),
+			wantTotal:     0,
+			wantBreakdown: "",
+		},
+		{
+			name:               "unregistered management cluster (nil client) -> none remaining",
+			spc:                newSPC(unregisteredManagementClusterResourceID),
+			kubeApplierDesires: []any{taggedDesire("desire-a", "test-controller")},
+			wantTotal:          0,
+			wantBreakdown:      "",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := utils.ContextWithLogger(context.Background(), testr.New(t))
+
+			mockKubeApplierDBClients := kubeappliercosmosstoragetesting.NewMockKubeApplierDBClients()
+			mockKubeApplierClient, err := kubeappliercosmosstoragetesting.NewMockKubeApplierDBClientWithResources(ctx, tc.kubeApplierDesires)
+			require.NoError(t, err)
+			mockKubeApplierDBClients.Register(managementClusterResourceID, mockKubeApplierClient)
+
+			syncer := &clusterChildResourcesCleanupController{
+				kubeApplierDBClients: mockKubeApplierDBClients,
+			}
+
+			total, breakdown, err := syncer.remainingApplyDesires(ctx, tc.spc, testSubscriptionID, testResourceGroupName, testClusterName)
+			require.NoError(t, err)
+			assert.Equal(t, tc.wantTotal, total)
+			assert.Equal(t, tc.wantBreakdown, breakdown)
+		})
+	}
+}
+
+func TestClusterChildResourcesCleanupController_extraDeleteGate_ApplyDesires(t *testing.T) {
+	managementClusterResourceID := metadataapi.Must(azcorearm.ParseResourceID(
+		"/providers/microsoft.redhatopenshift/stamps/1/managementclusters/default"))
+
+	spcResourceID := metadataapi.Must(azcorearm.ParseResourceID(
+		"/subscriptions/" + testSubscriptionID +
+			"/resourceGroups/" + testResourceGroupName +
+			"/providers/Microsoft.RedHatOpenShift/hcpOpenShiftClusters/" + testClusterName +
+			"/serviceProviderClusters/default"))
+
+	newSPC := func() *coreapi.ServiceProviderCluster {
+		return &coreapi.ServiceProviderCluster{
+			CosmosMetadata: coreapi.CosmosMetadata{
+				ResourceID:   spcResourceID,
+				PartitionKey: strings.ToLower(spcResourceID.SubscriptionID),
+			},
+			Status: coreapi.ServiceProviderClusterStatus{
+				ManagementClusterResourceID: managementClusterResourceID,
+			},
+		}
+	}
+	newApplyDesire := func(name string, tags map[string]string) *kubeapplierapi.ApplyDesire {
+		resourceID := metadataapi.Must(azcorearm.ParseResourceID(
+			kubeapplierapi.ToClusterScopedApplyDesireResourceIDString(
+				testSubscriptionID, testResourceGroupName, testClusterName, name)))
+		return &kubeapplierapi.ApplyDesire{
+			CosmosMetadata: coreapi.CosmosMetadata{
+				ResourceID:   resourceID,
+				PartitionKey: strings.ToLower(managementClusterResourceID.String()),
+			},
+			Spec: kubeapplierapi.ApplyDesireSpec{
+				ManagementCluster: managementClusterResourceID,
+			},
+			Tags: tags,
+		}
+	}
+
+	testCases := []struct {
+		name               string
+		kubeApplierDesires []any
+		wantShouldDelete   bool
+	}{
+		{
+			name:               "tagged ApplyDesire present -> SPC deletion blocked",
+			kubeApplierDesires: []any{newApplyDesire("desire-a", map[string]string{kubeapplierapi.TagControllerName: "test-controller"})},
+			wantShouldDelete:   false,
+		},
+		{
+			name:               "untagged ApplyDesire present -> SPC deletion blocked",
+			kubeApplierDesires: []any{newApplyDesire("desire-a", nil)},
+			wantShouldDelete:   false,
+		},
+		{
+			name:             "no kube-applier desires -> SPC deletion allowed",
+			wantShouldDelete: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx := utils.ContextWithLogger(context.Background(), testr.New(t))
+
+			mockResourcesDBClient, err := corecosmosstoragetesting.NewMockResourcesDBClientWithResources(ctx, []any{newSPC()})
+			require.NoError(t, err)
+
+			mockKubeApplierDBClients := kubeappliercosmosstoragetesting.NewMockKubeApplierDBClients()
+			mockKubeApplierClient, err := kubeappliercosmosstoragetesting.NewMockKubeApplierDBClientWithResources(ctx, tc.kubeApplierDesires)
+			require.NoError(t, err)
+			mockKubeApplierDBClients.Register(managementClusterResourceID, mockKubeApplierClient)
+
+			syncer := &clusterChildResourcesCleanupController{
+				resourcesDBClient:    mockResourcesDBClient,
+				kubeApplierDBClients: mockKubeApplierDBClients,
+			}
+
+			shouldDelete, err := syncer.extraDeleteGateShouldDeleteServiceProviderCluster(ctx, spcResourceID)
+			require.NoError(t, err)
+			assert.Equal(t, tc.wantShouldDelete, shouldDelete)
+		})
+	}
+}

backend/pkg/controllers/cluster/operations/operation_cluster_delete.go

@@ -179,6 +179,19 @@ func (c *operationClusterDelete) SynchronizeOperation(ctx context.Context, key c
 		return nil
 	}
 
+	// Hold the delete operation non-terminal until every ApplyDesire for the cluster
+	// has been removed. Placed after the deadline check above so the timeout-failure
+	// path still fires if this cleanup stalls; that path (buildDeletionTimeoutMessage)
+	// surfaces the same per-controller breakdown on the operation.
+	remainingApplyDesires, applyDesireBreakdown, err := c.countRemainingApplyDesires(ctx, cluster)
+	if err != nil {
+		return utils.TrackError(fmt.Errorf("failed to check remaining ApplyDesires: %w", err))
+	}
+	if remainingApplyDesires > 0 {
+		logger.Info("waiting for ApplyDesires to be deleted before completing delete operation", "remaining", remainingApplyDesires, "breakdown", applyDesireBreakdown)
+		return nil
+	}
+
 	if !c.shouldReconcileOperationAndResourceStatus(cluster) {
 		return nil
 	}
@@ -196,6 +209,39 @@ func (c *operationClusterDelete) shouldReconcileOperationAndResourceStatus(clust
 		cluster.ServiceProviderProperties.ClusterServiceID != nil
 }
 
+// countRemainingApplyDesires returns how many ApplyDesires still exist for the
+// cluster, along with a stable, human-readable per-controller breakdown (see
+// kubeappliercosmosstorage.SummarizeApplyDesiresByController). It is the single source
+// used both to hold the delete operation non-terminal (SynchronizeOperation) and to
+// describe the remaining ApplyDesires in the delete-timeout message
+// (buildDeletionTimeoutMessage). A missing ServiceProviderCluster, a nil
+// ManagementClusterResourceID, or an unavailable kube-applier client is treated as
+// "no remaining ApplyDesires".
+func (c *operationClusterDelete) countRemainingApplyDesires(ctx context.Context, cluster *coreapi.HCPOpenShiftCluster) (int, string, error) {
+	spc, err := c.resourcesDBClient.ServiceProviderClusters(cluster.ID.SubscriptionID, cluster.ID.ResourceGroupName, cluster.ID.Name).Get(ctx, coreapi.ServiceProviderClusterResourceName)
+	if cosmosstorageutils.IsNotFoundError(err) {
+		return 0, "", nil
+	}
+	if err != nil {
+		return 0, "", fmt.Errorf("failed to get ServiceProviderCluster: %w", err)
+	}
+	if spc.Status.ManagementClusterResourceID == nil {
+		return 0, "", nil
+	}
+
+	kubeApplierDBClient := c.kubeApplierDBClients.For(ctx, spc.Status.ManagementClusterResourceID)
+	if kubeApplierDBClient == nil {
+		return 0, "", nil
+	}
+
+	applyDesireCRUD, err := kubeApplierDBClient.ApplyDesiresForCluster(cluster.ID.SubscriptionID, cluster.ID.ResourceGroupName, cluster.ID.Name)
+	if err != nil {
+		return 0, "", fmt.Errorf("failed to get kube-applier CRUD for ApplyDesires: %w", err)
+	}
+
+	return kubeappliercosmosstorage.SummarizeApplyDesiresByController(ctx, applyDesireCRUD)
+}
+
 func (c *operationClusterDelete) reconcileOperationAndResourceStatus(ctx context.Context, operation *coreapi.Operation, cluster *coreapi.HCPOpenShiftCluster) error {
 	logger := utils.LoggerFromContext(ctx)
 
@@ -262,6 +308,15 @@ func (c *operationClusterDelete) buildDeletionTimeoutMessage(ctx context.Context
 		states = append(states, currState.WithSource("hostedCluster"))
 	}
 
+	if remaining, breakdown, err := c.countRemainingApplyDesires(ctx, cluster); err != nil {
+		errs = append(errs, err)
+	} else if remaining > 0 {
+		states = append(states, operationbase.NewOperationState(coreapi.ProvisioningStateDeleting,
+			fmt.Sprintf("%d ApplyDesire(s) still exist: %s", remaining, breakdown)).WithSource("applyDesires"))
+	} else {
+		states = append(states, operationbase.NewOperationState(coreapi.ProvisioningStateSucceeded, "").WithSource("applyDesires"))
+	}
+
 	if err := errors.Join(errs...); err != nil {
 		logger.Error(err, "errors building deletion timeout message")
 	}