fix: make frontend cluster create idempotent to prevent CS duplicate collisions

backend/pkg/controllers/clustercreation/cluster_cluster_service_create_controller.go

@@ -17,7 +17,6 @@ package clustercreation
 import (
 	"context"
 	"fmt"
-	"strings"
 	"time"
 
 	arohcpv1alpha1 "github.com/openshift-online/ocm-sdk-go/arohcp/v1alpha1"
@@ -126,7 +125,7 @@ func (c *clusterClusterServiceCreateSyncer) SyncOnce(ctx context.Context, key co
 	tenantID := *subscription.Properties.TenantId
 	mrg := cluster.CustomerProperties.Platform.ManagedResourceGroup
 
-	csCluster, err := c.findAROHCPClusterByAzureInfo(ctx, key.SubscriptionID, key.ResourceGroupName, key.HCPClusterName, tenantID, mrg)
+	csCluster, err := ocm.FindClusterByAzureInfo(ctx, c.clustersServiceClient, key.SubscriptionID, key.ResourceGroupName, key.HCPClusterName, tenantID, mrg)
 	if err != nil {
 		return utils.TrackError(err)
 	}
@@ -169,69 +168,6 @@ func (c *clusterClusterServiceCreateSyncer) createPreconditionDesiredVersionReso
 	return false, nil
 }
 
-// findAROHCPClusterByAzureInfo returns the Cluster Service cluster whose Azure
-// metadata matches the given subscription, resource group, ARM resource name,
-// tenant ID, and managed resource group name (MRG).
-// It returns (nil, nil) when no such cluster exists.
-// An error is returned if more than one cluster is returned matching the Azure metadata, as it should be unique.
-func (c *clusterClusterServiceCreateSyncer) findAROHCPClusterByAzureInfo(ctx context.Context, subscriptionID, resourceGroupName, resourceName, tenantID, managedResourceGroupName string) (*arohcpv1alpha1.Cluster, error) {
-	// Subscription ID, resource group, and cluster name are lowercased when building the Cluster Service
-	// cluster (see withImmutableAttributes in convert.go).
-	wantSub := strings.ToLower(subscriptionID)
-	wantRG := strings.ToLower(resourceGroupName)
-	wantName := strings.ToLower(resourceName)
-	// Tenant ID and managed resource group are not lowercased in the OCM CS
-	// builder (see withImmutableAttributes in convert.go), we keep the casing as it is.
-	wantTenant := tenantID
-	wantMRG := managedResourceGroupName
-	search := c.clustersServiceClusterByAzureInfoSearchString(wantSub, wantRG, wantName, wantTenant, wantMRG)
-	matches, err := c.csClustersMatchingClusterByAzureInfo(ctx, c.clustersServiceClient.ListClusters(search), wantSub, wantRG, wantName, wantTenant, wantMRG)
-	if err != nil {
-		return nil, err
-	}
-	if len(matches) > 1 {
-		return nil, fmt.Errorf(
-			"cluster service returned %d clusters for one Azure resource (expected exactly 1): "+
-				"subscription_id=%q resource_group=%q resource_name=%q tenant_id=%q managed_resource_group=%q",
-			len(matches), wantSub, wantRG, wantName, wantTenant, wantMRG,
-		)
-	}
-	if len(matches) == 1 {
-		return matches[0], nil
-	}
-	return nil, nil
-}
-
-func (c *clusterClusterServiceCreateSyncer) clustersServiceClusterByAzureInfoSearchString(wantSub, wantRG, wantName, wantTenant, wantMRG string) string {
-	return fmt.Sprintf(
-		"azure.subscription_id = '%s' and azure.resource_group_name = '%s' and azure.resource_name = '%s' and "+
-			"azure.tenant_id = '%s' and azure.managed_resource_group_name = '%s'",
-		wantSub, wantRG, wantName, wantTenant, wantMRG,
-	)
-}
-
-func (c *clusterClusterServiceCreateSyncer) csClustersMatchingClusterByAzureInfo(ctx context.Context, it ocm.ClusterListIterator, wantSub, wantRG, wantName, wantTenant, wantMRG string) ([]*arohcpv1alpha1.Cluster, error) {
-	var res []*arohcpv1alpha1.Cluster
-	for csCluster := range it.Items(ctx) {
-		az := csCluster.Azure()
-		if az == nil {
-			continue
-		}
-		if az.SubscriptionID() != wantSub ||
-			az.ResourceGroupName() != wantRG ||
-			az.ResourceName() != wantName ||
-			az.TenantID() != wantTenant ||
-			az.ManagedResourceGroupName() != wantMRG {
-			continue
-		}
-		res = append(res, csCluster)
-	}
-	if err := it.GetError(); err != nil {
-		return nil, err
-	}
-	return res, nil
-}
-
 func (c *clusterClusterServiceCreateSyncer) createClusterServiceCluster(ctx context.Context, cluster *api.HCPOpenShiftCluster, serviceProviderCluster *api.ServiceProviderCluster, tenantID string) (*arohcpv1alpha1.Cluster, error) {
 	logger := utils.LoggerFromContext(ctx)
 

backend/pkg/controllers/clustercreation/cluster_cluster_service_create_controller_test.go

@@ -201,7 +201,7 @@ func TestClusterClusterServiceCreate_SyncOnce(t *testing.T) {
 			setupMockCS: func(ctrl *gomock.Controller) ocm.ClusterServiceClientSpec {
 				mockCS := ocm.NewMockClusterServiceClientSpec(ctrl)
 				// Build the CS cluster with Azure fields matching the test cluster so it
-				// passes the csClustersMatchingClusterByAzureInfo filter.
+				// passes the FindClusterByAzureInfo filter.
 				csCluster, err := arohcpv1alpha1.NewCluster().
 					HREF(testClusterServiceIDStr).
 					Azure(arohcpv1alpha1.NewAzure().
@@ -276,7 +276,7 @@ func TestClusterClusterServiceCreate_SyncOnce(t *testing.T) {
 	}
 }
 
-func TestClusterClusterServiceCreate_findAROHCPClusterByAzureInfo(t *testing.T) {
+func TestFindClusterByAzureInfo(t *testing.T) {
 	azureTestCluster := func(t *testing.T, sub, rg, name, tenant, mrg string) *arohcpv1alpha1.Cluster {
 		t.Helper()
 		c, err := arohcpv1alpha1.NewCluster().
@@ -300,7 +300,9 @@ func TestClusterClusterServiceCreate_findAROHCPClusterByAzureInfo(t *testing.T)
 	defaultMRG := "arohcp-mycluster-uuid"
 
 	searchString := func(sub, rg, name, tenant, mrg string) string {
-		return (&clusterClusterServiceCreateSyncer{}).clustersServiceClusterByAzureInfoSearchString(
+		return fmt.Sprintf(
+			"azure.subscription_id = '%s' and azure.resource_group_name = '%s' and azure.resource_name = '%s' and "+
+				"azure.tenant_id = '%s' and azure.managed_resource_group_name = '%s'",
 			strings.ToLower(sub), strings.ToLower(rg), strings.ToLower(name), tenant, mrg,
 		)
 	}
@@ -496,8 +498,7 @@ func TestClusterClusterServiceCreate_findAROHCPClusterByAzureInfo(t *testing.T)
 			ctrl := gomock.NewController(t)
 			mockCS, want := tt.setupMockCS(t, ctrl, wantSearch)
 
-			s := &clusterClusterServiceCreateSyncer{clustersServiceClient: mockCS}
-			got, err := s.findAROHCPClusterByAzureInfo(ctx, sub, rg, resName, tenant, mrg)
+			got, err := ocm.FindClusterByAzureInfo(ctx, mockCS, sub, rg, resName, tenant, mrg)
 
 			if tt.wantErr {
 				require.Error(t, err)

frontend/pkg/frontend/cluster.go

@@ -369,26 +369,48 @@ func (f *Frontend) createHCPCluster(writer http.ResponseWriter, request *http.Re
 		tenantID = *subscription.Properties.TenantId
 	}
 
-	initialClusterProperties := map[string]string{}
-	if len(f.clusterServiceProvisionShard) != 0 {
-		initialClusterProperties[ocm.CSPropertyProvisionShardID] = f.clusterServiceProvisionShard
-	}
-	if f.clusterServiceNoopProvision {
-		initialClusterProperties[ocm.CSPropertyNoopProvision] = ocm.CSPropertyEnabled
-	}
-	if f.clusterServiceNoopDeprovision {
-		initialClusterProperties[ocm.CSPropertyNoopDeprovision] = ocm.CSPropertyEnabled
-	}
-	newClusterServiceClusterBuilder, newClusterServiceAutoscalerBuilder, err := ocm.BuildCSCluster(newInternalCluster.ID, tenantID, newInternalCluster, initialClusterProperties, nil, nil)
-	if err != nil {
-		return utils.TrackError(err)
-	}
 	logger.Info(fmt.Sprintf("creating resource %s", newInternalCluster.ID))
-	resultingClusterServiceCluster, err := f.clusterServiceClient.PostCluster(ctx, newClusterServiceClusterBuilder, newClusterServiceAutoscalerBuilder)
+
+	// Check if CS already has a cluster with matching Azure metadata. This
+	// handles the retry-after-partial-failure scenario: a prior attempt may
+	// have created the CS cluster but failed to write the Cosmos documents,
+	// leaving an orphan in CS that would cause PostCluster to reject with
+	// "Duplicate ARO-HCP cluster name" (see ARO-27951).
+	resultingClusterServiceCluster, err := ocm.FindClusterByAzureInfo(
+		ctx, f.clusterServiceClient,
+		newInternalCluster.ID.SubscriptionID,
+		newInternalCluster.ID.ResourceGroupName,
+		newInternalCluster.Name,
+		tenantID,
+		newInternalCluster.CustomerProperties.Platform.ManagedResourceGroup,
+	)
 	if err != nil {
 		return utils.TrackError(err)
 	}
 
+	if resultingClusterServiceCluster == nil {
+		initialClusterProperties := map[string]string{}
+		if len(f.clusterServiceProvisionShard) != 0 {
+			initialClusterProperties[ocm.CSPropertyProvisionShardID] = f.clusterServiceProvisionShard
+		}
+		if f.clusterServiceNoopProvision {
+			initialClusterProperties[ocm.CSPropertyNoopProvision] = ocm.CSPropertyEnabled
+		}
+		if f.clusterServiceNoopDeprovision {
+			initialClusterProperties[ocm.CSPropertyNoopDeprovision] = ocm.CSPropertyEnabled
+		}
+		newClusterServiceClusterBuilder, newClusterServiceAutoscalerBuilder, err := ocm.BuildCSCluster(newInternalCluster.ID, tenantID, newInternalCluster, initialClusterProperties, nil, nil)
+		if err != nil {
+			return utils.TrackError(err)
+		}
+		resultingClusterServiceCluster, err = f.clusterServiceClient.PostCluster(ctx, newClusterServiceClusterBuilder, newClusterServiceAutoscalerBuilder)
+		if err != nil {
+			return utils.TrackError(err)
+		}
+	} else {
+		logger.Info("Reusing existing Cluster Service cluster found by Azure metadata", "csClusterHREF", resultingClusterServiceCluster.HREF())
+	}
+
 	csID, err := api.NewInternalID(resultingClusterServiceCluster.HREF())
 	if err != nil {
 		return utils.TrackError(err)

internal/ocm/client.go

@@ -845,3 +845,53 @@ func ConvertOpenShiftVersionAddPrefix(v string) string {
 	}
 	return v
 }
+
+// FindClusterByAzureInfo searches Cluster Service for a cluster matching the
+// given Azure metadata (subscription, resource group, resource name, tenant,
+// managed resource group). Returns (nil, nil) when no match exists. Returns an
+// error if more than one matching cluster is found.
+func FindClusterByAzureInfo(ctx context.Context, client ClusterServiceClientSpec, subscriptionID, resourceGroupName, resourceName, tenantID, managedResourceGroupName string) (*arohcpv1alpha1.Cluster, error) {
+	wantSub := strings.ToLower(subscriptionID)
+	wantRG := strings.ToLower(resourceGroupName)
+	wantName := strings.ToLower(resourceName)
+	wantTenant := tenantID
+	wantMRG := managedResourceGroupName
+
+	search := fmt.Sprintf(
+		"azure.subscription_id = '%s' and azure.resource_group_name = '%s' and azure.resource_name = '%s' and "+
+			"azure.tenant_id = '%s' and azure.managed_resource_group_name = '%s'",
+		wantSub, wantRG, wantName, wantTenant, wantMRG,
+	)
+
+	it := client.ListClusters(search)
+	var matches []*arohcpv1alpha1.Cluster
+	for csCluster := range it.Items(ctx) {
+		az := csCluster.Azure()
+		if az == nil {
+			continue
+		}
+		if az.SubscriptionID() != wantSub ||
+			az.ResourceGroupName() != wantRG ||
+			az.ResourceName() != wantName ||
+			az.TenantID() != wantTenant ||
+			az.ManagedResourceGroupName() != wantMRG {
+			continue
+		}
+		matches = append(matches, csCluster)
+	}
+	if err := it.GetError(); err != nil {
+		return nil, err
+	}
+
+	if len(matches) > 1 {
+		return nil, fmt.Errorf(
+			"cluster service returned %d clusters for one Azure resource (expected exactly 1): "+
+				"subscription_id=%q resource_group=%q resource_name=%q tenant_id=%q managed_resource_group=%q",
+			len(matches), wantSub, wantRG, wantName, wantTenant, wantMRG,
+		)
+	}
+	if len(matches) == 1 {
+		return matches[0], nil
+	}
+	return nil, nil
+}