🐛 Add Version to ROSA Control Plane Status #5443

PanSpagetka · 2025-03-25T09:23:28Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

Based on the doc for CAPI control plane contract here the RosaControlPlane status must have version field present the current version.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Checklist:

Release note:

Add Version to ROSA Control Plane Status

k8s-ci-robot · 2025-03-25T09:23:38Z

Hi @PanSpagetka. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

serngawy · 2025-03-25T18:07:45Z

/ok-to-test

serngawy

sounds good to me, just to confirm did you run an upgrade and see the version changed during upgrade ?

PanSpagetka · 2025-03-26T12:33:28Z

sounds good to me, just to confirm did you run an upgrade and see the version changed during upgrade ?

@serngawy Yep, version changed after update was completed.

serngawy · 2025-03-26T13:25:13Z

looks good, Not sure why the apidiff-main job fail , may it need rebase

serngawy · 2025-03-26T17:23:47Z

/test pull-cluster-api-provider-aws-test

serngawy · 2025-03-26T17:45:01Z

TestRosaMachinePoolReconcile rosamachinepool_controller_test.go:373 is failing due to not having version in rosacontrolplane->status

serngawy · 2025-03-27T18:32:02Z

/lgtm

serngawy · 2025-04-03T14:12:49Z

controlplane/rosa/controllers/rosacontrolplane_controller_test.go

+		Status: rosacontrolplanev1.RosaControlPlaneStatus{
+			Ready: true,
+			ID:    "rosa-control-plane-1",
+			// Version: "4.15.20",


remove commented code

serngawy · 2025-04-03T14:25:05Z

controlplane/rosa/controllers/rosacontrolplane_controller_test.go

+	// Add conditions, can't do this duirng creation
+	cpPh, err := patch.NewHelper(rosaControlPlane, testEnv)
+	g.Expect(err).ShouldNot(HaveOccurred())
+	rosaControlPlane.Status = rosacontrolplanev1.RosaControlPlaneStatus{


Not sure, why patching the status ? we should check the status after Reconcile.
You may just patch the ID other fields should be expected after Reconcile

We need to set conditions for Reconcile to end without an error.

okay, but version and ready should set during Reconcile

serngawy · 2025-04-03T14:35:13Z

controlplane/rosa/controllers/rosacontrolplane_controller_test.go

+
+	errGet2 := testEnv.Get(ctx, key, cp)
+	g.Expect(errGet2).NotTo(HaveOccurred())
+	g.Expect(cp.Status.Version).To(Equal("4.15.20"))


this should be compared with the cp.spec.version

PanSpagetka · 2025-04-08T10:20:42Z

/test pull-cluster-api-provider-aws-apidiff-main

serngawy · 2025-04-08T13:43:59Z

controlplane/rosa/controllers/rosacontrolplane_controller_test.go

+
+	errGet2 := testEnv.Get(ctx, key, cp)
+	g.Expect(errGet2).NotTo(HaveOccurred())
+	g.Expect(cp.Status.Version).To(Equal("4.15.20"))


Suggested change

g.Expect(cp.Status.Version).To(Equal("4.15.20"))

g.Expect(cp.Status.Version).To(Equal(cp.Spec.Version))

serngawy · 2025-04-08T13:47:58Z

controlplane/rosa/controllers/rosacontrolplane_controller.go

@@ -94,6 +94,7 @@ type ROSAControlPlaneReconciler struct {
 	Endpoints        []scope.ServiceEndpoint
 	NewStsClient     func(cloud.ScopeUsage, cloud.Session, logger.Wrapper, runtime.Object) stsiface.STSAPI
 	NewOCMClient     func(ctx context.Context, rosaScope *scope.ROSAControlPlaneScope) (rosa.OCMClient, error)
+	restClientConfig *restclient.Config


Suggested change

restClientConfig *restclient.Config

// Exposing the restClientConfig for integration test, No need to initialize

restClientConfig *restclient.Config

serngawy · 2025-04-09T14:09:37Z

/lgtm

serngawy · 2025-04-09T14:10:35Z

@damdo would you give this PR an approve

richardcase · 2025-04-13T15:57:14Z

/approve

k8s-ci-robot · 2025-04-13T15:57:20Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: richardcase

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~config/crd/bases/OWNERS~~ [richardcase]
~~controlplane/rosa/OWNERS~~ [richardcase]
~~exp/controllers/OWNERS~~ [richardcase]

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

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/bug Categorizes issue or PR as related to a bug. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 25, 2025

k8s-ci-robot requested review from damdo and serngawy March 25, 2025 09:23

k8s-ci-robot added needs-priority needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 25, 2025

PanSpagetka force-pushed the add-status-version-rosa-cp branch from 9152b52 to d2f0071 Compare March 25, 2025 09:24

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 25, 2025

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 25, 2025

serngawy reviewed Mar 25, 2025

View reviewed changes

serngawy mentioned this pull request Mar 25, 2025

🌱 Add Rosa Control Plane integration test #5444

Closed

5 tasks

serngawy approved these changes Mar 26, 2025

View reviewed changes

PanSpagetka force-pushed the add-status-version-rosa-cp branch from d2f0071 to 25e09e9 Compare March 26, 2025 14:22

PanSpagetka force-pushed the add-status-version-rosa-cp branch from 25e09e9 to 686fb12 Compare March 27, 2025 09:30

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 27, 2025

k8s-ci-robot assigned serngawy Mar 27, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 27, 2025

PanSpagetka force-pushed the add-status-version-rosa-cp branch from 686fb12 to 2ca7744 Compare April 3, 2025 07:40

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Apr 3, 2025

PanSpagetka force-pushed the add-status-version-rosa-cp branch 2 times, most recently from 42a6d52 to f8fb3dc Compare April 3, 2025 08:11

serngawy reviewed Apr 3, 2025

View reviewed changes

PanSpagetka force-pushed the add-status-version-rosa-cp branch 2 times, most recently from 4989d61 to 383d22a Compare April 8, 2025 10:09

PanSpagetka force-pushed the add-status-version-rosa-cp branch from 383d22a to bb7fd96 Compare April 8, 2025 10:21

serngawy reviewed Apr 8, 2025

View reviewed changes

serngawy approved these changes Apr 8, 2025

View reviewed changes

Add Version to ROSA Control Plane Status

2927d42

PanSpagetka force-pushed the add-status-version-rosa-cp branch from bb7fd96 to 2927d42 Compare April 9, 2025 07:43

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 9, 2025

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 13, 2025

k8s-ci-robot merged commit e9f2823 into kubernetes-sigs:main Apr 13, 2025
17 checks passed

	g.Expect(cp.Status.Version).To(Equal("4.15.20"))
	g.Expect(cp.Status.Version).To(Equal(cp.Spec.Version))

	restClientConfig *restclient.Config
	// Exposing the restClientConfig for integration test, No need to initialize
	restClientConfig *restclient.Config

🐛 Add Version to ROSA Control Plane Status #5443

🐛 Add Version to ROSA Control Plane Status #5443

Uh oh!

Conversation

PanSpagetka commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Mar 25, 2025

Uh oh!

serngawy commented Mar 25, 2025

Uh oh!

serngawy left a comment

Choose a reason for hiding this comment

Uh oh!

PanSpagetka commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serngawy commented Mar 26, 2025

Uh oh!

serngawy commented Mar 26, 2025

Uh oh!

serngawy commented Mar 26, 2025

Uh oh!

serngawy commented Mar 27, 2025

Uh oh!

serngawy Apr 3, 2025

Choose a reason for hiding this comment

Uh oh!

serngawy Apr 3, 2025

Choose a reason for hiding this comment

Uh oh!

PanSpagetka Apr 3, 2025

Choose a reason for hiding this comment

Uh oh!

serngawy Apr 3, 2025

Choose a reason for hiding this comment

Uh oh!

serngawy Apr 3, 2025

Choose a reason for hiding this comment

Uh oh!

PanSpagetka commented Apr 8, 2025

Uh oh!

serngawy Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

serngawy Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

serngawy commented Apr 9, 2025

Uh oh!

serngawy commented Apr 9, 2025

Uh oh!

richardcase commented Apr 13, 2025

Uh oh!

k8s-ci-robot commented Apr 13, 2025

Uh oh!

Uh oh!

Uh oh!

PanSpagetka commented Mar 25, 2025 •

edited

Loading

PanSpagetka commented Mar 26, 2025 •

edited

Loading