kubernetes-sigs / cluster-api-provider-ibmcloud

Cluster API Provider for IBM Cloud
https://cluster-api-ibmcloud.sigs.k8s.io
Apache License 2.0
60 stars 76 forks source link

Refactor PowerVSClusterscope function to faciliate UT integration #1827

Closed Karthik-K-N closed 3 days ago

Karthik-K-N commented 1 month ago

What this PR does / why we need it:

  1. This PR refactors the code to make use of factory functions which can be used during unit test cases for easy overriding of the function.
  2. Changes are made to PowerVSClient to reuse the session or authenticator if already present in the options passed
  3. Added some sample UT to demonstrate the usage of factory functions

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:

/area provider/ibmcloud

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

Release note:

Refactor PowerVSClusterscope function to faciliate UT integration
netlify[bot] commented 1 month ago

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

Name Link
Latest commit e8a3e483431c2ebc071fb9c26992afd3814762be
Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-cluster-api-ibmcloud/deploys/6683e7a139e2830009a36ad7
Deploy Preview https://deploy-preview-1827--kubernetes-sigs-cluster-api-ibmcloud.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Karthik-K-N commented 1 month ago

/hold

testing pending.

Karthik-K-N commented 1 month ago

@mkumatag @Amulyam24 @dharaneeshvrd Please take a look at the approach and let me know your opinion. If you think of any better approach dont forget to comment here.

Karthik-K-N commented 1 week ago

I have verified the workflow with cluster creation and able to see cluster getting created successfully.

karthikkn@Karthiks-MacBook-Pro templates % oc get ibmpowervsmachines
NAME                                         CLUSTER                  AGE     READY   STATE    HEALTH
karthikkn-capi-powervs-control-plane-8mt9p   karthikkn-capi-powervs   30m     true    ACTIVE   OK
karthikkn-capi-powervs-control-plane-hjpzj   karthikkn-capi-powervs   16m     true    ACTIVE   OK
karthikkn-capi-powervs-control-plane-k55d2   karthikkn-capi-powervs   8m36s   true    ACTIVE   WARNING
karthikkn-capi-powervs-md-0-tmx5r-84rnw      karthikkn-capi-powervs   50m     true    ACTIVE   WARNING

karthikkn@Karthiks-MacBook-Pro templates % oc get machines
NAME                                         CLUSTER                  NODENAME                                     PROVIDERID                                                                                         PHASE     AGE     VERSION
karthikkn-capi-powervs-control-plane-8mt9p   karthikkn-capi-powervs   karthikkn-capi-powervs-control-plane-8mt9p   ibmpowervs://osa/osa21/b15f67d5-2f86-4b96-980e-2015dc2bd0de/48a630f7-3eea-4115-ad7d-a1b4491063cb   Running   31m     v1.29.3
karthikkn-capi-powervs-control-plane-hjpzj   karthikkn-capi-powervs   karthikkn-capi-powervs-control-plane-hjpzj   ibmpowervs://osa/osa21/b15f67d5-2f86-4b96-980e-2015dc2bd0de/cb927986-7164-4920-a1cf-9ad04be46ca9   Running   17m     v1.29.3
karthikkn-capi-powervs-control-plane-k55d2   karthikkn-capi-powervs   karthikkn-capi-powervs-control-plane-k55d2   ibmpowervs://osa/osa21/b15f67d5-2f86-4b96-980e-2015dc2bd0de/ae20b96f-6bce-4b4f-bd07-a023f75a095c   Running   9m42s   v1.29.3
karthikkn-capi-powervs-md-0-tmx5r-84rnw      karthikkn-capi-powervs   karthikkn-capi-powervs-md-0-tmx5r-84rnw      ibmpowervs://osa/osa21/b15f67d5-2f86-4b96-980e-2015dc2bd0de/a2402d38-6984-46c4-a3ef-5eede7d2cccb   Running   52m     v1.29.3
Karthik-K-N commented 1 week ago

@dharaneeshvrd @mkumatag Please review this PR when you get some time, This will unblock in adding unit test cases.

dharaneeshvrd commented 1 week ago

Changes looks good to me as well!

Karthik-K-N commented 3 days ago

/hold cancel

k8s-ci-robot commented 3 days ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Karthik-K-N, mkumatag

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](https://github.com/kubernetes-sigs/cluster-api-provider-ibmcloud/blob/main/OWNERS)~~ [mkumatag] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment