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,26 @@ func (c *clusterChildResourcesCleanupController) extraDeleteGateShouldDeleteServ
 		return false, nil
 	}
 
+	// Check that the ClusterResourcesController has removed all the ApplyDesires it
+	// owns. That controller is responsible for deleting its own desires during
+	// cluster deletion; we only remove the ServiceProviderCluster document (which
+	// carries the ManagementClusterResourceID needed to reach them) once they are gone.
+	applyDesiresGone, err := c.clusterResourceApplyDesiresGone(
+		ctx,
+		spc,
+		serviceProviderClusterResourceID.SubscriptionID,
+		serviceProviderClusterResourceID.ResourceGroupName,
+		clusterName,
+	)
+	if err != nil {
+		return false, utils.TrackError(fmt.Errorf("failed to check ClusterResourcesController ApplyDesire precondition: %w", err))
+	}
+	if !applyDesiresGone {
+		logger.Info("waiting for ClusterResourcesController to delete its ApplyDesires before removing ServiceProviderCluster",
+			"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 +327,59 @@ func (c *clusterChildResourcesCleanupController) extraDeleteGateShouldDeleteServ
 	return true, nil
 }
 
+// clusterResourceApplyDesiresGone reports whether all ApplyDesires tagged by the
+// ClusterResourcesController have been removed for the cluster owning the given
+// ServiceProviderCluster. The ClusterResourcesController is responsible for
+// deleting its own desires 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 gone, consistent with
+// the best-effort behavior elsewhere in this file.
+func (c *clusterChildResourcesCleanupController) clusterResourceApplyDesiresGone(ctx context.Context, spc *coreapi.ServiceProviderCluster, subscriptionID, resourceGroupName, clusterName string) (bool, error) {
+	logger := utils.LoggerFromContext(ctx)
+
+	if spc == nil || spc.Status.ManagementClusterResourceID == nil {
+		return true, nil
+	}
+
+	managementClusterID := spc.Status.ManagementClusterResourceID
+	kubeApplierDBClient := c.kubeApplierDBClients.For(ctx, managementClusterID)
+	if kubeApplierDBClient == nil {
+		logger.Info("no kube-applier client for management cluster; treating ClusterResourcesController ApplyDesires as gone",
+			"managementClusterResourceID", managementClusterID.String())
+		return true, nil
+	}
+
+	applyDesireCRUD, err := kubeApplierDBClient.ApplyDesiresForCluster(subscriptionID, resourceGroupName, clusterName)
+	if err != nil {
+		return false, utils.TrackError(fmt.Errorf("failed to get kube-applier CRUD for ApplyDesire precondition: %w", err))
+	}
+
+	applyDesireIterator, err := applyDesireCRUD.List(ctx, &cosmosstorageutils.DBClientListResourceDocsOptions{})
+	if err != nil {
+		return false, utils.TrackError(fmt.Errorf("failed to list ApplyDesire documents for precondition check: %w", err))
+	}
+
+	for _, desire := range applyDesireIterator.Items(ctx) {
+		if desire.Tags == nil {
+			continue
+		}
+		if desire.Tags[kubeapplierapi.TagControllerName] == kubeapplierapi.ClusterResourcesControllerName {
+			// A tagged ApplyDesire still exists; the gate is not satisfied. Surface
+			// any iteration error observed so far before short-circuiting.
+			if err := applyDesireIterator.GetError(); err != nil {
+				return false, utils.TrackError(fmt.Errorf("error iterating ApplyDesires for precondition check: %w", err))
+			}
+			return false, nil
+		}
+	}
+	if err := applyDesireIterator.GetError(); err != nil {
+		return false, utils.TrackError(fmt.Errorf("error iterating ApplyDesires for precondition check: %w", err))
+	}
+
+	return true, 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,180 @@ func TestIsUnderSkippedSubtree(t *testing.T) {
 		})
 	}
 }
+
+func TestClusterChildResourcesCleanupController_clusterResourceApplyDesiresGone(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 string) *kubeapplierapi.ApplyDesire {
+		return newApplyDesire(name, map[string]string{kubeapplierapi.TagControllerName: kubeapplierapi.ClusterResourcesControllerName})
+	}
+	untaggedDesire := func(name string) *kubeapplierapi.ApplyDesire {
+		return newApplyDesire(name, nil)
+	}
+
+	testCases := []struct {
+		name               string
+		spc                *coreapi.ServiceProviderCluster
+		kubeApplierDesires []any
+		wantGone           bool
+	}{
+		{
+			name:               "tagged ClusterResourcesController ApplyDesire present -> not gone",
+			spc:                newSPC(managementClusterResourceID),
+			kubeApplierDesires: []any{taggedDesire("cluster-resource-desire")},
+			wantGone:           false,
+		},
+		{
+			name:               "only untagged ApplyDesire present -> gone",
+			spc:                newSPC(managementClusterResourceID),
+			kubeApplierDesires: []any{untaggedDesire("other-controller-desire")},
+			wantGone:           true,
+		},
+		{
+			name:     "no ApplyDesires -> gone",
+			spc:      newSPC(managementClusterResourceID),
+			wantGone: true,
+		},
+		{
+			name:     "nil management cluster resource ID -> gone",
+			spc:      newSPC(nil),
+			wantGone: true,
+		},
+		{
+			name:               "unregistered management cluster (nil client) -> gone",
+			spc:                newSPC(unregisteredManagementClusterResourceID),
+			kubeApplierDesires: []any{taggedDesire("cluster-resource-desire")},
+			wantGone:           true,
+		},
+	}
+
+	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,
+			}
+
+			gone, err := syncer.clusterResourceApplyDesiresGone(ctx, tc.spc, testSubscriptionID, testResourceGroupName, testClusterName)
+			require.NoError(t, err)
+			assert.Equal(t, tc.wantGone, gone)
+		})
+	}
+}
+
+func TestClusterChildResourcesCleanupController_extraDeleteGate_ClusterResourceApplyDesires(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,
+			},
+		}
+	}
+	taggedDesire := func(name 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: map[string]string{kubeapplierapi.TagControllerName: kubeapplierapi.ClusterResourcesControllerName},
+		}
+	}
+
+	testCases := []struct {
+		name               string
+		kubeApplierDesires []any
+		wantShouldDelete   bool
+	}{
+		{
+			name:               "tagged ClusterResourcesController ApplyDesire present -> SPC deletion blocked",
+			kubeApplierDesires: []any{taggedDesire("cluster-resource-desire")},
+			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,18 @@ func (c *operationClusterDelete) SynchronizeOperation(ctx context.Context, key c
 		return nil
 	}
 
+	// Hold the delete operation non-terminal until the ClusterResourcesController has
+	// removed all the ApplyDesires it owns. Placed after the deadline check above so
+	// the timeout-failure path still fires if this cleanup stalls.
+	applyDesiresGone, err := c.clusterResourceApplyDesiresGone(ctx, cluster)
+	if err != nil {
+		return utils.TrackError(fmt.Errorf("failed to check ClusterResourcesController ApplyDesire precondition: %w", err))
+	}
+	if !applyDesiresGone {
+		logger.Info("waiting for ClusterResourcesController to delete its ApplyDesires before completing delete operation")
+		return nil
+	}
+
 	if !c.shouldReconcileOperationAndResourceStatus(cluster) {
 		return nil
 	}
@@ -196,6 +208,59 @@ func (c *operationClusterDelete) shouldReconcileOperationAndResourceStatus(clust
 		cluster.ServiceProviderProperties.ClusterServiceID != nil
 }
 
+// clusterResourceApplyDesiresGone reports whether all ApplyDesires tagged by the
+// ClusterResourcesController have been removed for the given cluster. The
+// ClusterResourcesController deletes its own desires during cluster deletion; this
+// controller holds the delete operation non-terminal until they are gone. A missing
+// ServiceProviderCluster, a nil ManagementClusterResourceID, or an unavailable
+// kube-applier client is treated as gone.
+func (c *operationClusterDelete) clusterResourceApplyDesiresGone(ctx context.Context, cluster *coreapi.HCPOpenShiftCluster) (bool, error) {
+	spc, err := c.resourcesDBClient.ServiceProviderClusters(cluster.ID.SubscriptionID, cluster.ID.ResourceGroupName, cluster.ID.Name).Get(ctx, coreapi.ServiceProviderClusterResourceName)
+	if cosmosstorageutils.IsNotFoundError(err) {
+		return true, nil
+	}
+	if err != nil {
+		return false, utils.TrackError(fmt.Errorf("failed to get ServiceProviderCluster: %w", err))
+	}
+	if spc.Status.ManagementClusterResourceID == nil {
+		return true, nil
+	}
+
+	kubeApplierDBClient := c.kubeApplierDBClients.For(ctx, spc.Status.ManagementClusterResourceID)
+	if kubeApplierDBClient == nil {
+		return true, nil
+	}
+
+	applyDesireCRUD, err := kubeApplierDBClient.ApplyDesiresForCluster(cluster.ID.SubscriptionID, cluster.ID.ResourceGroupName, cluster.ID.Name)
+	if err != nil {
+		return false, utils.TrackError(fmt.Errorf("failed to get kube-applier CRUD for ApplyDesire precondition: %w", err))
+	}
+
+	applyDesireIterator, err := applyDesireCRUD.List(ctx, &cosmosstorageutils.DBClientListResourceDocsOptions{})
+	if err != nil {
+		return false, utils.TrackError(fmt.Errorf("failed to list ApplyDesire documents for precondition check: %w", err))
+	}
+
+	for _, desire := range applyDesireIterator.Items(ctx) {
+		if desire.Tags == nil {
+			continue
+		}
+		if desire.Tags[kubeapplierapi.TagControllerName] == kubeapplierapi.ClusterResourcesControllerName {
+			// A tagged ApplyDesire still exists; the gate is not satisfied. Surface
+			// any iteration error observed so far before short-circuiting.
+			if err := applyDesireIterator.GetError(); err != nil {
+				return false, utils.TrackError(fmt.Errorf("error iterating ApplyDesires for precondition check: %w", err))
+			}
+			return false, nil
+		}
+	}
+	if err := applyDesireIterator.GetError(); err != nil {
+		return false, utils.TrackError(fmt.Errorf("error iterating ApplyDesires for precondition check: %w", err))
+	}
+
+	return true, nil
+}
+
 func (c *operationClusterDelete) reconcileOperationAndResourceStatus(ctx context.Context, operation *coreapi.Operation, cluster *coreapi.HCPOpenShiftCluster) error {
 	logger := utils.LoggerFromContext(ctx)