feat: move cluster creation from frontend to backend by miguelsorianod · Pull Request #5120 · Azure/ARO-HCP

Miguel Soriano (miguelsorianod) · 2026-05-05T11:00:26Z

This commit moves cluster creation from frontend to backend. Frontend now just creates the corresponding ARM Operation, with an empty internalID, and it delegates all the actual cluster creation work to backend.

On backend now we check whether internalID is empty or not to assess whether the cluster create request to Clusters Service has been submitted. Once that occurs, we check the state.

The logic that performs cluster creation against Clusters Service has been moved to a backend operationcontroller named DispatchClusterCreate. The controller attempts to be resilient to failures.

This MR cannot be merged until cluster reads have been moved to backend (#4610)

This commit moves cluster creation from frontend to backend. Frontend now just creates the corresponding ARM Operation, with an empty internalID, and it delegates all the actual cluster creation work to backend. On backend now we check whether internalID is empty or not to assess whether the cluster create request to Clusters Service has been submitted. Once that occurs, we check the state. The logic that performs cluster creation against Clusters Service has been moved to a backend operationcontroller named DispatchClusterCreate. The controller attempts to be resilient to failures.

openshift-ci · 2026-05-05T11:00:32Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: miguelsorianod
Once this PR has been reviewed and has the lgtm label, please assign janboll for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2026-05-05T11:00:37Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Miguel Soriano (miguelsorianod) · 2026-05-05T11:02:20Z

-	}
-
-	initialClusterProperties := map[string]string{}
-	if len(f.clusterServiceProvisionShard) != 0 {


The functionality of the CSPropertyProvisionShardID, CSPropertyNoopProvision, CSPropertyNoopDeprovision was added in the beginning of the project. This PR removes the ability to set that completely. No permanent aro-hcp environments leverage it, the helm template of the frontend deployment does not even pass the corresponding frontend CLI flags. Technically some developer could be using it. If for some reason we want to keep that we would need to move it to backend.

Miguel Soriano (miguelsorianod) · 2026-05-05T11:03:34Z


 	rootCmd.Flags().StringVar(&opts.clustersServiceURL, "clusters-service-url", "https://api.openshift.com", "URL of the OCM API gateway.")
 	rootCmd.Flags().BoolVar(&opts.insecure, "insecure", false, "Skip validating TLS for clusters-service.")
-	rootCmd.Flags().StringVar(&opts.clusterServiceProvisionShard, "cluster-service-provision-shard", "", "Manually specify provision shard for all requests to cluster service")


Related to https://github.com/Azure/ARO-HCP/pull/5120/changes#r3187900975. If we decide we don't want to support that anymore the CLI flags can be removed safely from frontend because the helm deployment doesn't pass them.

Miguel Soriano (miguelsorianod) · 2026-05-05T11:06:43Z

+	if mrg == "" {
+		return utils.TrackError(fmt.Errorf("cluster %s has no managed resource group", cluster.Name))
+	}
+	existing, err := c.findAROHCPClusterByAzureInfo(ctx,


A challenge I found was that it could occur that inbetween the CS create request is received and we store the CS ID in the Cluster doc something fails. Then the challenge is: what do we do at that point? to interact with the CS API you leverage the CS Cluster ID.
What this PR does is to do a list in CS with a search string using the cluster's azure resource information: the azure subscription, tenant id, resource group name, and resource name. This assumes that no two clusters will be created with that information.

Although the PR uses that, CS currently does not support filtering by some of those fields, we would need to add it. Currently CS list only supports filtering by subscription id and resource group"

Miguel Soriano (miguelsorianod) · 2026-05-05T11:07:12Z

+		return utils.TrackError(err)
+	}
+
+	if cluster.ServiceProviderProperties.ActiveOperationID != "" &&


is this check needed and returning a non error the correct approach?

Miguel Soriano (miguelsorianod) · 2026-05-05T11:07:59Z

 		b.options.ClustersServiceClient,
 		activeOperationInformer,
 	)
+	dispatchClusterCreateController := operationcontrollers.NewDispatchClusterCreateController(


Although the PR implements create, I wonder whether we need to change something somewhere else. What happens if a delete occurs and the CS ID hasn't been stored for some reason?

Miguel Soriano (miguelsorianod) · 2026-05-05T11:08:19Z

+	if err != nil {
+		return nil, err
+	}
+	if len(matches) > 1 {


If we get more than one result we treat it as an error

Miguel Soriano (miguelsorianod) · 2026-05-05T11:09:13Z

-		// and we need to wire up a fail-safe where if we have no ID and we time out, we report the best failure we can.
-		return nil
-	}
+	// TODO we need to wire up a fail-safe where if we have no ID and we time out, we report the best failure we can.


See.

Additionally, what happens if for some reason the creation has failed in the CS side? and what if for some reason it's not filled in Cosmos in a timely manner?

Miguel Soriano (miguelsorianod) · 2026-05-05T11:09:46Z

 		database.OperationRequestCreate,
 		newInternalCluster.ID,
-		ptr.Deref(newInternalCluster.ServiceProviderProperties.ClusterServiceID, api.InternalID{}),
+		api.InternalID{},


In frontend we now pass an empty internal ID as now the responsibility is fully on backend

Miguel Soriano (miguelsorianod) · 2026-05-05T11:10:02Z

 		return fmt.Errorf("unexpected type %T", resultingUncastInternalCluster)
 	}

+	var resultingClusterServiceCluster *arohcpv1alpha1.Cluster // TODO remove this once we moved read from CS


temporary to silence the compiler. This depensd on the MR that moves reads out of frontend

Miguel Soriano (miguelsorianod) · 2026-05-05T11:11:26Z

+	if subscription.Properties == nil || subscription.Properties.TenantId == nil || *subscription.Properties.TenantId == "" {
+		return utils.TrackError(fmt.Errorf("subscription %s has no tenant id", operation.ExternalID.SubscriptionID))
+	}
+	tenantID := *subscription.Properties.TenantId


We get the tenant id from the cluster's subscription. The operation itself has a tenant but on cluster creation I observed the operation id tenant comes from a http header whereas the cluster tenant comes from the subscription. Because of this I retrieved the tenant id using the cluster's subscription (requires one db query more as we need to retrieve the susbcription)

Miguel Soriano (miguelsorianod) · 2026-05-05T11:16:13Z

+	clustersServiceClient ocm.ClusterServiceClientSpec
+}
+
+func NewDispatchClusterCreateController(


This dispatches CS cluster creation in a controller. At some point I am guessing we might need to do some resources precreation or calculation in RP itself before sending the call to CS. In that case, how would we coordinate this controller and the others?

Manyanda Chitimbo (machi1990) · 2026-05-05T11:16:47Z

Miguel Soriano (@miguelsorianod) , Jakob Gray (@JakobGray) had created #4821 does this PR supersedes it?

Miguel Soriano (miguelsorianod) · 2026-05-05T11:31:54Z

Miguel Soriano (@miguelsorianod) , Jakob Gray (@JakobGray) had created #4821 does this PR supersedes it?

I wasn't aware of that one. It's possible there's some overlap.

Miguel Soriano (miguelsorianod) · 2026-05-05T11:41:34Z

@@ -327,6 +327,11 @@ func (b *Backend) runBackendControllersUnderLeaderElection(ctx context.Context,
 		b.options.ClustersServiceClient,
 		activeOperationInformer,
 	)


The introduction of the changes here need to be actually split into two:

First introduce the backend controller. It should never execute because as soon as

Then introduce the frontend removal logic

That way the images can be deployed and potentially rolled back independently.

I think it should be safe to introduce 1 because it will only be executed when the operation exists and it doesn't have an internal id, and until 2 is introduced the cluster create operation should always have an internal id as the operation is created together with it set

openshift-ci · 2026-05-06T15:56:40Z

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Miguel Soriano (miguelsorianod) · 2026-07-21T16:46:33Z

Superseded by #6082 and #6121. Closing.

openshift-ci Bot added the do-not-merge/work-in-progress label May 5, 2026

Miguel Soriano (miguelsorianod) commented May 5, 2026

View reviewed changes

openshift-ci Bot added the needs-rebase label May 6, 2026

Miguel Soriano (miguelsorianod) closed this Jul 21, 2026

Uh oh!

Conversation

Miguel Soriano (miguelsorianod) commented May 5, 2026

Uh oh!

openshift-ci Bot commented May 5, 2026

Uh oh!

openshift-ci Bot commented May 5, 2026

Uh oh!

Miguel Soriano (miguelsorianod) May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Manyanda Chitimbo (machi1990) commented May 5, 2026

Uh oh!

Miguel Soriano (miguelsorianod) commented May 5, 2026

Uh oh!

Miguel Soriano (miguelsorianod) May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci Bot commented May 6, 2026

Uh oh!

Miguel Soriano (miguelsorianod) commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Miguel Soriano (miguelsorianod) May 5, 2026 •

edited

Loading

Miguel Soriano (miguelsorianod) May 5, 2026 •

edited

Loading