feat: assign cluster version on control plane creation by JakobGray · Pull Request #4477 · Azure/ARO-HCP

Jakob Gray (JakobGray) · 2026-03-16T05:33:47Z

What

feat: assign cluster version on control plane creation using Cincinnati client

Why

The control plane will use the latest appropriate version and not need to immediately upgrade after install.

Special notes for your reviewer

For now, version resolution is done in the frontend. This will migrate to the backend in the future (ARO-24824) following changes to move cluster service create calls to the backend (ARO-24384)

Copilot

Pull request overview

Adds frontend-time OpenShift version resolution so newly created control planes start on the latest appropriate z-stream (preferably a “gateway” to the next minor), avoiding an immediate post-install upgrade.

Changes:

Introduces internal/version.ResolveInitialVersion which queries Cincinnati to pick an initial X.Y.Z (preferring gateway releases).
Updates frontend cluster creation to resolve and pass a concrete CS version ID into ocm.BuildCSCluster during create.
Removes hardcoded stable patch defaults in NewOpenShiftVersionXYZ (X.Y now defaults to X.Y.0) and adjusts conversion tests accordingly.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test-integration/utils/integrationutils/utils.go	Updates `NewFrontend` call signature; currently passes nil Cincinnati client.
internal/version/resolve.go	New Cincinnati-backed resolver for initial desired version selection.
internal/version/resolve_test.go	Unit tests for initial version resolution behavior and error cases.
internal/ocm/convert.go	Adds `resolvedVersionID` parameter to `BuildCSCluster`/`withImmutableAttributes` for create-time version override.
internal/ocm/client.go	Removes hardcoded patch constants; X.Y defaults to X.Y.0 unless caller supplies X.Y.Z.
internal/ocm/convert_test.go	Updates expected version IDs and adds coverage for `resolvedVersionID` override.
frontend/pkg/frontend/frontend.go	Extends `Frontend` to hold a Cincinnati client.
frontend/pkg/frontend/cluster.go	Resolves initial version on cluster create and passes it into CS cluster build.
frontend/pkg/frontend/testhelpers.go	Updates `NewFrontend` call signature in test helper.
frontend/pkg/frontend/frontend_test.go	Updates `NewFrontend` call signature in unit tests.
frontend/cmd/cmd.go	Wires up a real Cincinnati client and passes it into `NewFrontend`.
frontend/go.mod / frontend/go.sum	Adds dependency entries needed for Cincinnati client usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+	resolvedVersion, err := internalversion.ResolveInitialVersion(ctx, f.cincinnatiClient,
+		newInternalCluster.CustomerProperties.Version.ChannelGroup,
+		newInternalCluster.CustomerProperties.Version.ID)
+	if err != nil {
+		return utils.TrackError(err)
+	}
+	resolvedVersionID := ocm.NewOpenShiftVersionXYZ(resolvedVersion.String(),
+		newInternalCluster.CustomerProperties.Version.ChannelGroup)
+	logger.Info("Resolved initial cluster version", "customerVersion", newInternalCluster.CustomerProperties.Version.ID, "resolvedVersion", resolvedVersionID)


 	fakeAuditClient := &FakeOTELClient{}
 	metricsRegistry := prometheus.NewRegistry()
-	aroHCPFrontend := frontend.NewFrontend(logger, frontendListener, frontendMetricsListener, metricsRegistry, storageIntegrationTestInfo.CosmosClient(), clusterServiceMockInfo.MockClusterServiceClient, fakeAuditClient, "fake-location", "", false, false, true)
+	aroHCPFrontend := frontend.NewFrontend(logger, frontendListener, frontendMetricsListener, metricsRegistry, storageIntegrationTestInfo.CosmosClient(), clusterServiceMockInfo.MockClusterServiceClient, nil, fakeAuditClient, "fake-location", "", false, false, true)


+// resolvedVersionID, when non-empty, is used directly as the CS version ID (in "openshift-vX.Y.Z"
+// format) instead of deriving it from the cluster's Version.ID via NewOpenShiftVersionXYZ.


Jakob Gray (JakobGray) · 2026-03-16T16:12:39Z

Jakob Gray (@JakobGray) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
@microsoft-github-policy-service agree [company="{your company}"]
Options:

(default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
(when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"
Contributor License Agreement

@microsoft-github-policy-service agree [company="{Red Hat}"]

Jakob Gray (JakobGray) · 2026-03-16T16:36:18Z

@microsoft-github-policy-service agree company="Red Hat"

Jameel Briones (JameelB) · 2026-03-17T17:07:54Z

@@ -767,19 +761,11 @@ func NewOpenShiftVersionXYZ(v, cg string) string {
 			parts = append(parts, "0")
 		}

-		// If no patch version provided (X.Y format), append default patch version
-		// Otherwise preserve the provided patch version (X.Y.Z format)
+		// If no patch version provided (X.Y format), default to .0.
+		// Callers that need a specific Z-stream should resolve via Cincinnati
+		// before calling this function and pass X.Y.Z directly.
 		if len(parts) == 2 {
-			// TODO: Will change once we support allowing users to select a cluster installation version.
-			// hardcode patch versions for now
-			switch v {
-			case "4.19":
-				parts = append(parts, OpenShift419Patch)
-			case "4.20":
-				parts = append(parts, OpenShift420Patch)
-			default:
-				parts = append(parts, "0")
-			}
+			parts = append(parts, "0")
 		}


why not make it required to have v contain a valid x.y.z format here? Since a version resolver is being added, the defaulting should probably be handled by it as well to separate concerns. As far as i recall, we also aim to ensure that an x.y.z version is to be provided on np creation/update. Once that change is in, the caller (for cluster/np) should be able to provide a valid version and this NewOpenShiftVersionXYZ can focus on doing just the conversion? wdyt?

Copilot

Pull request overview

Adds initial OpenShift version resolution at control plane creation time so newly created clusters start on an appropriate Z-stream version (and don’t immediately upgrade), and introduces verification/tests around the resolved install version.

Changes:

Introduce internal/version Cincinnati-based resolver for initial install version selection (gateway-aware).
Wire version resolution into frontend cluster creation (and refactor backend controller to reuse shared resolver).
Add unit tests and e2e verifier to validate installed version/minor expectations.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
test/util/verifiers/installed_version.go	New e2e verifier that checks the installed version stream and compares against Cincinnati resolution.
test/e2e/complete_cluster_create.go	Hooks new installed-version verifier into the “cluster is viable” e2e flow.
test-integration/utils/integrationutils/utils.go	Updates `NewFrontend` call to pass the new Cincinnati client parameter.
internal/version/resolve_test.go	Adds unit tests for the initial version resolution logic.
internal/version/resolve.go	New shared resolver implementing Cincinnati-based version selection and gateway checks.
internal/ocm/convert_test.go	Updates tests for new version defaulting and new `BuildCSCluster` signature.
internal/ocm/convert.go	Extends `BuildCSCluster`/immutables to accept a resolved CS version ID override.
internal/ocm/client.go	Simplifies `NewOpenShiftVersionXYZ` to use semver parsing (drops hardcoded patch constants).
frontend/pkg/frontend/testhelpers.go	Updates frontend test helper for new `NewFrontend` parameter.
frontend/pkg/frontend/frontend_test.go	Updates frontend tests for new `NewFrontend` parameter.
frontend/pkg/frontend/frontend.go	Stores a Cincinnati client on `Frontend` and threads it through constructor.
frontend/pkg/frontend/cluster.go	Resolves initial version during create flow and passes it into CS cluster build.
frontend/go.sum	Adds checksums for new dependencies (CVO Cincinnati client and transitives).
frontend/go.mod	Adds dependency on `openshift/cluster-version-operator` and related indirects.
frontend/cmd/cmd.go	Constructs a Cincinnati client and injects it into the frontend.
backend/pkg/controllers/upgradecontrollers/utils.go	Removes duplicated gateway-check helper (logic moved to `internal/version`).
backend/pkg/controllers/upgradecontrollers/control_plane_desired_version_controller.go	Refactors to use shared `internal/version` resolver functions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+func ResolveInitialVersion(ctx context.Context, cincinnatiClient cincinatti.Client, channelGroup string, customerDesiredMinor string) (semver.Version, error) {
+	logger := utils.LoggerFromContext(ctx)
+	logger.Info("Resolving initial desired version", "customerDesiredMinor", customerDesiredMinor, "channelGroup", channelGroup)
+
+	// ParseTolerant handles both "4.19" and "4.19.0" formats
+	customerDotZeroRelease := api.Must(semver.ParseTolerant(customerDesiredMinor))
+
+	if cincinnatiClient == nil {
+		logger.Info("No Cincinnati client available, falling back to X.Y.0", "version", customerDotZeroRelease.String())
+		return customerDotZeroRelease, nil
+	}


+		for _, candidate := range candidateReleases {
+			candidateTargetVersion := semver.MustParse(candidate.Version)
+
+			if candidateTargetVersion.Major != targetMinorVersion.Major || candidateTargetVersion.Minor != targetMinorVersion.Minor {
+				continue
+			}


+	initialEntry := clusterVersion.Status.History[len(clusterVersion.Status.History)-1]
+	installedVersion, err := semver.ParseTolerant(initialEntry.Version)
+	if err != nil {
+		return fmt.Errorf("failed to parse installed version %q from history: %w", initialEntry.Version, err)
+	}
+
+	desiredMinor := api.Must(semver.ParseTolerant(v.customerDesiredMinor))



+	// TODO(ARO-24384): This Cincinnati client will be removed once cluster creation
+	// in Cluster Service is done asynchronously from the backend.
+	cincinnatiClient := cincinnati.NewClient(uuid.New(), http.DefaultTransport.(*http.Transport), "ARO-HCP", cincinatti.NewAlwaysConditionRegistry())


-func BuildCSCluster(resourceID *azcorearm.ResourceID, requestHeader http.Header, hcpCluster *api.HCPOpenShiftCluster, requiredProperties map[string]string, oldClusterServiceCluster *arohcpv1alpha1.Cluster) (*arohcpv1alpha1.ClusterBuilder, *arohcpv1alpha1.ClusterAutoscalerBuilder, error) {
+// resolvedVersionID, when non-empty, is used directly as the CS version ID (in "openshift-vX.Y.Z"
+// format) instead of deriving it from the cluster's Version.ID via NewOpenShiftVersionXYZ.
+func BuildCSCluster(resourceID *azcorearm.ResourceID, requestHeader http.Header, hcpCluster *api.HCPOpenShiftCluster, requiredProperties map[string]string, oldClusterServiceCluster *arohcpv1alpha1.Cluster, resolvedVersionID string) (*arohcpv1alpha1.ClusterBuilder, *arohcpv1alpha1.ClusterAutoscalerBuilder, error) {


Manyanda Chitimbo (machi1990) · 2026-03-18T14:50:47Z

+	if err != nil {
+		ginkgo.GinkgoLogr.Info("WARNING: failed to resolve expected version from Cincinnati (non-fatal)", "error", err)
+		return nil
+	}


We can return the error

Manyanda Chitimbo (machi1990) · 2026-03-18T15:16:51Z

-
-		initialDesiredVersion, err := c.findLatestVersionInMinor(ctx, cincinnatiClient, channelGroup, customerDotZeroRelease, []semver.Version{customerDotZeroRelease})
+		initialVersion, err := versionpkg.ResolveInitialVersion(ctx, cincinnatiClient, channelGroup, customerDesiredMinor)
 		if err != nil {


np: We can aim to retry up to 900ms if we get 5xx

All of this will go away once Azure#4477 is implemented. This was observed in the test that was skipped with ``` skip [github.com/Azure/ARO-HCP/test/e2e/nodepool_minor_upgrade.go:153]: skipping: no Cincinnati upgrade path from nodepool version 4.20.16 to any version <= 4.21.0 (lowest control plane version); cannot exercise minor nodepool upgrade ``` because the nodepool version picked was 4.20.16 and lowest control plane minor used in our validation was still at 4.20 which was what was installed by default.

All of this will go away once #4477 is implemented. This was observed in the test that was skipped with ``` skip [github.com/Azure/ARO-HCP/test/e2e/nodepool_minor_upgrade.go:153]: skipping: no Cincinnati upgrade path from nodepool version 4.20.16 to any version <= 4.21.0 (lowest control plane version); cannot exercise minor nodepool upgrade ``` because the nodepool version picked was 4.20.16 and lowest control plane minor used in our validation was still at 4.20 which was what was installed by default.

Copilot

Pull request overview

Copilot reviewed 20 out of 22 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-20T19:03:41Z

 	newInternalCluster.Identity.UserAssignedIdentities = nil

+	// For now, version resolution is done here in the frontend.  This will be moved to the backend in the future (ARO-24824)
+	// following the changes to move cluster service create calls to the backend (ARO-24384).


createHCPCluster now calls internalversion.ResolveInitialVersion with f.cincinnatiClient without guarding against it being nil. Several unit tests construct a Frontend with a nil Cincinnati client (e.g., frontend/pkg/frontend/frontend_test.go), so this will panic if any test or future code path exercises cluster creation. Consider validating cincinnatiClient is non-nil in NewFrontend (and failing fast) or returning a tracked error here when it is nil.

Suggested change

// following the changes to move cluster service create calls to the backend (ARO-24384).

// following the changes to move cluster service create calls to the backend (ARO-24384).

if f.cincinnatiClient == nil {

return utils.TrackError(fmt.Errorf("cincinnati client is not configured"))

}

Copilot · 2026-03-20T19:03:41Z

-		if len(cg) > 0 && cg != "stable" {
-			csVersion = csVersion + "-" + cg
-		}
+	if cg != "stable" {


NewOpenShiftVersionXYZ now appends -<channelGroup> for any cg != "stable", including the empty string. If cg is empty (possible in older/migrated records), this produces an invalid CS version like openshift-v4.19.0-. Consider restoring the previous guard (cg != "stable" && cg != "") so empty channel groups don't generate a trailing dash.

Suggested change

if cg != "stable" {

if cg != "stable" && cg != "" {

Manyanda Chitimbo (machi1990) · 2026-03-20T19:13:35Z

/lgtm
/hold
The unit test needs fixing + a green e2e.
Once fixed and green let's get one of David Eads (@deads2k) Ben Vesel (@bennerv) to appprove

openshift-ci · 2026-03-20T19:13:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JakobGray, machi1990
Once this PR has been reviewed and has the lgtm label, please assign roivaz 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-03-20T19:41:37Z

New changes are detected. LGTM label has been removed.

Jakob Gray (JakobGray) · 2026-03-20T19:43:03Z

/lgtm /hold The unit test needs fixing + a green e2e. Once fixed and green let's get one of David Eads (@deads2k) Ben Vesel (@bennerv) to appprove

Unit tests needed a rebase. E2E test has issue from RoleAssignmentLimitExceeded

Enhance the frontend to utilize the Cincinnati client for version resolution during cluster creation. Use the latest appropriate z-stream version to align with automated z-stream upgrades.

Copilot

Pull request overview

Copilot reviewed 20 out of 22 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

openshift-ci · 2026-03-24T16:15:26Z

Jakob Gray (@JakobGray): The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-parallel	`0b221bd`	link	true	`/test e2e-parallel`

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

David Eads (deads2k)

Do not assign this version in the frontend. Move it to the backend. If there is precursor work, do that.

Manyanda Chitimbo (machi1990) · 2026-03-24T20:57:45Z

Do not assign this version in the frontend. Move it to the backend. If there is precursor work, do that.

David Eads (@deads2k) the idea was to approach it iteratively as Matthew Barnes (@mbarnes) has https://redhat.atlassian.net/browse/ARO-24384 (to move cluster creation call in CS to the backend) in progress

We wanted to assign latest cluster install version first and hence the move to doing it in the frontend and followup with the backend change. Hence the overall work was divided as:

get the frontend to pick the latest by calling cincinatti
and use that to call cs
in a followup add a controller that will do the above point.
eventually when cs is no longer than call maestro

I see the need about combining the first three steps together especially of the possible latency introduced by the cincinatti call or how we could fail cluster creation instantly if we get an intermittent 5xx and just wire in an additional controller to handle the creation to CS async and prepare us for when we'll be creating the HostedCluster directly via maestro.
I've been discussing with Jakob Gray (@JakobGray) via chat and the way I proposed to approach it is:

Take out the piece that calls CS creation from the frontend
The cp version controller will take care of computing the initial desired version for us
- We'll need to ensure that this piece to read the cluster uuid is done conditionally only when the cluster service id is set. Otherwise, we default to use a generated one (initialised once and to be used for all new clusters )
Add another controller that is responsible of triggering the cluster creation call in CS
- For a given cluster, it checks if the cluster service id is set, if set do nothing
- If not set then the controller does the following:
  - The controller operates on the serviceProviderCluster.Spec.ControlPlaneVersion.DesiredVersion
  - if desired isn't set then do nothing
  - If desired is set
    - Search in CS for a record that matches the cluster's properties (subscription, resourcegroup and resource name)
    - If there is a hit then this cluster was created but we never gotten to save its CS id, pick it and store it in cosmo
    - If there is no hit, then create the cluster in CS using the desired version and the rest of clusters properties and store the CS' id in cosmos.

How does that sound as possible logic? Is there anything you'll add or take out?

Worth noting that:

Operation controllers and other controllers might need to be modified to take onto account the emptness of the CS' id as that's now set async.
We'll also have to see what Matthew Barnes (@mbarnes) has done as he had https://redhat.atlassian.net/browse/ARO-24384 (to move the creation to backend in progress) so that this work doesn't step onto the feet of his work.

David Eads (deads2k) · 2026-03-24T21:21:43Z

How does that sound as possible logic? Is there anything you'll add or take out?

Sounds like a good plan. If we have a way to specify the cluster-service ID on creation, that would be preferred to

Search in CS for a record that matches the cluster's properties (subscription, resourcegroup and resource name)

If not, then this works.

Manyanda Chitimbo (machi1990) · 2026-03-24T21:27:15Z

How does that sound as possible logic? Is there anything you'll add or take out?

Sounds like a good plan. If we have a way to specify the cluster-service ID on creation, that would be preferred to

Awasom, thank you David Eads (@deads2k) for the feedback. cc Jakob Gray (@JakobGray) once you've the reply from Matthew Barnes (@mbarnes) regarding where https://redhat.atlassian.net/browse/ARO-24384 is, we can proceed forward.

Search in CS for a record that matches the cluster's properties (subscription, resourcegroup and resource name)

If not, then this works.

No, CS generates the id and there is no way to specify it without changing CS' API as it is readonly field

Matthew Barnes (mbarnes) · 2026-03-26T14:44:27Z

cc Jakob Gray (@JakobGray) once you've the reply from Matthew Barnes (@mbarnes) regarding where https://redhat.atlassian.net/browse/ARO-24384 is, we can proceed forward.

Jakob contacted me about that. I'm working on it as time permits. To do it safely it's going to have to be a multi-deployment transition. I posted my plan in ARO-24384.

openshift-ci · 2026-04-02T09:46:56Z

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.

Manyanda Chitimbo (machi1990) · 2026-04-13T08:05:11Z

Superseded by #4821

Copilot AI review requested due to automatic review settings March 16, 2026 05:33

openshift-ci Bot requested review from David Eads (deads2k) and Matthew Barnes (mbarnes) March 16, 2026 05:33

Copilot started reviewing on behalf of Jakob Gray (JakobGray) March 16, 2026 05:34 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes