Add v1beta2 changes to IBMPowerVSImage #2412

arshadd-b · 2025-07-01T16:54:50Z

What this PR does / why we need it:

This PR consists of following changes

Adopt CAPI v1beta2 status changes
Standardize the controller flow by utilising CAPI provided helper functions
Removed usage of deprecated github.com/pkg/errors package.

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 #2389

Special notes for your reviewer:

/area provider/ibmcloud

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

Add v1beta2 changes to IBMPowerVSImage

netlify · 2025-07-01T16:54:56Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-ibmcloud ready!

Name	Link
🔨 Latest commit	`e6945d2`
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-cluster-api-ibmcloud/deploys/68a84bcf8980d300089e800e
😎 Deploy Preview	https://deploy-preview-2412.cluster-api-ibmcloud.sigs.k8s.io
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

k8s-ci-robot · 2025-07-01T16:55:00Z

Hi @arshadd-b. 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.

Karthik-K-N

Thanks for the PR, I will take a another look once you addressed the comments.

Overall lets revisit the log statement and make them better.

api/v1beta1/ibmpowervs_conversion.go

api/v1beta2/conditions_consts.go

cloud/scope/powervs_image.go

controllers/ibmpowervsimage_controller.go

Amulyam24 · 2025-07-11T07:20:21Z

/ok-to-test

arshadd-b · 2025-07-21T05:14:03Z

Image queued

Image Importing

Image Ready

Karthik-K-N

One last thing, otherwise LGTM

Karthik-K-N · 2025-07-30T06:12:51Z

controllers/ibmpowervsimage_controller.go

+
+func patchIBMPowerVSImage(ctx context.Context, patchHelper *v1beta1patch.Helper, ibmPowerVSImage *infrav1.IBMPowerVSImage) error {
+	// always update the readyCondition.
+	v1beta1conditions.SetSummary(ibmPowerVSImage,


Hey, Seems like you need to add back that previous change set of making sure conidtion is always set.

Also lets add a UT to make sure, the condition is always set in the first reconcilation

@arshadd-b please address the review comments.

Added the condition back and added UT to check the condition is getting set.

arshadd-b · 2025-08-12T05:07:19Z

/retest

Karthik-K-N

Couple of small things otherwise, LGTM, Thanks for the PR.

Since we don't have CI to test the flow, At the end of addressing all the changes, Do you mind testing the cluster creation with infra creation flow and share the output in the PR.

controllers/ibmpowervsimage_controller.go

controllers/ibmpowervsimage_controller_test.go

Karthik-K-N · 2025-08-12T10:12:00Z

controllers/ibmpowervsimage_controller_test.go

+		name           string
+		powervsCluster *infrav1.IBMPowerVSCluster
+		powervsImage   *infrav1.IBMPowerVSImage
+		expectError    bool


Is it possible to just add another case, where the worksapceNotReady state is set?

sure will check and add

We have to mock the GetServiceInstance and it is not straight forward. Will create follow up for this
cc: @Karthik-K-N

Prajyot-Parab · 2025-08-19T18:36:33Z

@arshadd-b We have a major release in the coming weeks. Please prioritize this task accordingly.

arshadd-b · 2025-08-20T04:13:32Z

@arshadd-b We have a major release in the coming weeks. Please prioritize this task accordingly.

sure

Prajyot-Parab · 2025-08-22T10:02:31Z

@arshadd-b Please squash the commits.

api/v1beta2/conditions_consts.go

cloud/scope/powervs_image.go

Karthik-K-N · 2025-08-22T10:15:16Z

cloud/scope/powervs_image.go

 		record.Warnf(i.IBMPowerVSImage, "FailedCreateImageImportJob", "Failed image import job creation - %v", err)
 		return nil, nil, err
 	}
-	i.Info("New import job request created")
+	log.Info("New import job request created", "jobID", *jobRef.ID)


Is it safe to deref the jobRef, I see there is no nil check for jobRef?

I have removed this jobID

Karthik-K-N

/lgtm

Thank you.

k8s-ci-robot · 2025-08-22T12:07:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Amulyam24, arshadd-b, Prajyot-Parab

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:

~~OWNERS~~ [Prajyot-Parab]

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 area/provider/ibmcloud Issues or PRs related to ibmcloud provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 1, 2025

k8s-ci-robot requested review from Karthik-K-N and Prajyot-Parab July 1, 2025 16:54

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 1, 2025

arshadd-b changed the title ~~Add v1beta2 changes to IBMPowerVSImage~~ [WIP] Add v1beta2 changes to IBMPowerVSImage Jul 1, 2025

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jul 1, 2025

arshadd-b force-pushed the update-powervs-image branch from 9632fae to a3756f6 Compare July 2, 2025 15:56

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 2, 2025

arshadd-b force-pushed the update-powervs-image branch 4 times, most recently from eaf2b42 to 1be332d Compare July 7, 2025 06:00

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 7, 2025

arshadd-b force-pushed the update-powervs-image branch from 0956e60 to 322a3dc Compare July 8, 2025 05:00

Karthik-K-N reviewed Jul 9, 2025

View reviewed changes

controllers/ibmpowervsimage_controller.go Show resolved Hide resolved

controllers/ibmpowervsimage_controller.go Show resolved Hide resolved

controllers/ibmpowervsimage_controller.go Outdated Show resolved Hide resolved

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 Jul 11, 2025

arshadd-b force-pushed the update-powervs-image branch from 75af036 to 7d51cd4 Compare July 15, 2025 04:27

arshadd-b requested a review from Karthik-K-N July 18, 2025 07:25

arshadd-b changed the title ~~[WIP] Add v1beta2 changes to IBMPowerVSImage~~ Add v1beta2 changes to IBMPowerVSImage Jul 18, 2025

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 18, 2025

Karthik-K-N reviewed Jul 30, 2025

View reviewed changes

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 12, 2025

arshadd-b force-pushed the update-powervs-image branch 2 times, most recently from da50462 to f64ee6a Compare August 12, 2025 06:34

arshadd-b requested review from Karthik-K-N and Amulyam24 August 12, 2025 06:59

Karthik-K-N reviewed Aug 12, 2025

View reviewed changes

arshadd-b force-pushed the update-powervs-image branch 7 times, most recently from 885c76e to 71e2e39 Compare August 22, 2025 09:20

Karthik-K-N reviewed Aug 22, 2025

View reviewed changes

arshadd-b force-pushed the update-powervs-image branch from a9bedbc to 728a270 Compare August 22, 2025 10:43

Add v1beta2 changes to IBMPowerVSImage

e6945d2

arshadd-b force-pushed the update-powervs-image branch from 728a270 to e6945d2 Compare August 22, 2025 10:51

Karthik-K-N reviewed Aug 22, 2025

View reviewed changes

k8s-ci-robot assigned Karthik-K-N Aug 22, 2025

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

Prajyot-Parab approved these changes Aug 22, 2025

View reviewed changes

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

k8s-ci-robot merged commit 4a0fa23 into kubernetes-sigs:main Aug 22, 2025
14 checks passed

Karthik-K-N mentioned this pull request Aug 22, 2025

Standerdise the controller workflow #2129

Closed

5 tasks

Add v1beta2 changes to IBMPowerVSImage #2412

Add v1beta2 changes to IBMPowerVSImage #2412

Conversation

arshadd-b commented Jul 1, 2025

Uh oh!

netlify bot commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for kubernetes-sigs-cluster-api-ibmcloud ready!

Uh oh!

k8s-ci-robot commented Jul 1, 2025

Uh oh!

Karthik-K-N left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Amulyam24 commented Jul 11, 2025

Uh oh!

arshadd-b commented Jul 21, 2025

Uh oh!

Karthik-K-N left a comment

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!

arshadd-b commented Aug 12, 2025

Uh oh!

Karthik-K-N left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

Prajyot-Parab commented Aug 19, 2025

Uh oh!

arshadd-b commented Aug 20, 2025

Uh oh!

Prajyot-Parab commented Aug 22, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karthik-K-N left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Aug 22, 2025

Uh oh!

Uh oh!

Uh oh!

netlify bot commented Jul 1, 2025 •

edited

Loading

Karthik-K-N left a comment •

edited

Loading