Refactor reconcilation of resources #1937

Amulyam24 · 2024-09-02T10:37:53Z

What this PR does / why we need it:

This PR refactors the reconciliation of resources to follow a unit format as below

Fetch ID from IBMPowerVSCluster status object. If ID exists, check the status of the resource in cloud and return
If ID is not present in the status, try to fetch it from the IBMPowerVSCluster spec and check if the resource exists in cloud. Here we can check either via name or ID passed in the spec. If resource with given ID doesn't exist, return err, or else check the status of the resource in cloud and set IBMPowerVSCluster.status accordingly
If resource with given name doesn't exist, proceed with creating the resource. If it exists, check the status of the resource in cloud. Set IBMPowerVSCluster.status accordingly in both cases

The changes in this PR has been tested with the following scenarios

Resource names are set in spec and they do not exist
Resource names are set in spec and they exist
Resource IDs are set in spec and they do not exist
Resource IDs are set in spec and they exist
Spec with only required fields

Note:

Adjustments were made to support if ID or Name are set in spec
The changes in this PR were also tested with TG changes from existing PR Attach connections to existing transit gateway when its not there #1901
Caveats noticed where in terms of handling existing DHCP server ID set in spec and it has been documented in Validate DHCP server status before reusing existing DHCP network #1933

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 #1907 #1726

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:

Refactor reconcilation of resources

netlify · 2024-09-02T10:38:16Z

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

Name	Link
🔨 Latest commit	`ec8b9db`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-ibmcloud/deploys/6704fdbfca8fba0008a735d2
😎 Deploy Preview	https://deploy-preview-1937--kubernetes-sigs-cluster-api-ibmcloud.netlify.app
📱 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 site configuration.

Amulyam24 · 2024-09-02T10:50:53Z

/cc @Karthik-K-N

cloud/scope/powervs_cluster.go

Amulyam24 · 2024-09-03T05:33:15Z

/cc @dharaneeshvrd

cloud/scope/powervs_cluster.go

controllers/ibmpowervscluster_controller.go

Karthik-K-N

/lgtm

Amulyam24 · 2024-09-13T10:09:04Z

/hold

Testing the changes after rebase.

api/v1beta2/ibmpowervscluster_webhook.go

cloud/scope/powervs_cluster.go

Amulyam24 · 2024-09-24T10:02:55Z

/unhold

PR is ready for review, PTAL

dharaneeshvrd

Overall LGTM

cloud/scope/powervs_cluster.go

controllers/ibmpowervscluster_controller.go

cloud/scope/powervs_cluster.go

cloud/scope/powervs_cluster_test.go

Karthik-K-N

One suggestion , Otherwise LGTM

api/v1beta2/ibmpowervscluster_webhook.go

Amulyam24 · 2024-10-08T09:46:09Z

@Karthik-K-N, added a couple of minor changes. PTAL!

Karthik-K-N

Thank you.
/lgtm

Amulyam24 · 2024-10-15T06:51:14Z

@mkumatag, PTAL!
The following/open UT PRs will require changes as per refactoring being added in this PR.

mkumatag

relying on @Karthik-K-N 's LGTM, thanks.

k8s-ci-robot · 2024-10-15T12:18:04Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Amulyam24, 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~~ [mkumatag]

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 the area/provider/ibmcloud Issues or PRs related to ibmcloud provider label Sep 2, 2024

k8s-ci-robot requested review from mkumatag and Prajyot-Parab September 2, 2024 10:38

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 2, 2024

k8s-ci-robot requested a review from Karthik-K-N September 2, 2024 10:50

Amulyam24 force-pushed the reconcile_refactor branch from af41ebc to e44da66 Compare September 2, 2024 10:54

Amulyam24 commented Sep 2, 2024

View reviewed changes

cloud/scope/powervs_cluster.go Show resolved Hide resolved

Amulyam24 force-pushed the reconcile_refactor branch from e44da66 to 052e37a Compare September 3, 2024 04:41

k8s-ci-robot requested a review from dharaneeshvrd September 3, 2024 05:33

Karthik-K-N reviewed Sep 3, 2024

View reviewed changes

Amulyam24 force-pushed the reconcile_refactor branch 2 times, most recently from f91b98c to 77355f6 Compare September 3, 2024 07:47

Karthik-K-N reviewed Sep 4, 2024

View reviewed changes

Amulyam24 force-pushed the reconcile_refactor branch from 77355f6 to 2034107 Compare September 4, 2024 10:32

Karthik-K-N reviewed Sep 10, 2024

View reviewed changes

k8s-ci-robot assigned Karthik-K-N Sep 10, 2024

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 10, 2024

Amulyam24 force-pushed the reconcile_refactor branch from 2034107 to 22d4a38 Compare September 13, 2024 09:56

k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 13, 2024

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 13, 2024

mkumatag requested changes Sep 16, 2024

View reviewed changes

api/v1beta2/ibmpowervscluster_webhook.go Outdated Show resolved Hide resolved

api/v1beta2/ibmpowervscluster_webhook.go Outdated Show resolved Hide resolved

cloud/scope/powervs_cluster.go Outdated Show resolved Hide resolved

Karthik-K-N mentioned this pull request Sep 17, 2024

EPIC - Introduce new API Specs and infra creation flow #1611

Open

9 tasks

Amulyam24 force-pushed the reconcile_refactor branch from 22d4a38 to 43a11d9 Compare September 23, 2024 10:09

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 23, 2024

Amulyam24 force-pushed the reconcile_refactor branch 5 times, most recently from 0efc9f9 to 6b84b4d Compare September 24, 2024 10:01

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 24, 2024

dharaneeshvrd reviewed Sep 24, 2024

View reviewed changes

cloud/scope/powervs_cluster.go Outdated Show resolved Hide resolved

cloud/scope/powervs_cluster.go Show resolved Hide resolved

Amulyam24 force-pushed the reconcile_refactor branch 2 times, most recently from 199af10 to 50f61e3 Compare September 25, 2024 05:27

Karthik-K-N reviewed Oct 1, 2024

View reviewed changes

Amulyam24 force-pushed the reconcile_refactor branch 2 times, most recently from 523596d to d469456 Compare October 3, 2024 13:16

carmal891 mentioned this pull request Oct 4, 2024

Adds UT for ReconcileLoadBalancers function #1979

Merged

Karthik-K-N reviewed Oct 8, 2024

View reviewed changes

api/v1beta2/ibmpowervscluster_webhook.go Outdated Show resolved Hide resolved

api/v1beta2/ibmpowervscluster_webhook.go Show resolved Hide resolved

Amulyam24 force-pushed the reconcile_refactor branch from d469456 to 56b874e Compare October 8, 2024 09:32

Refactor reconcilation of resources

ec8b9db

Amulyam24 force-pushed the reconcile_refactor branch from 56b874e to ec8b9db Compare October 8, 2024 09:39

Karthik-K-N reviewed Oct 8, 2024

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 8, 2024

carmal891 mentioned this pull request Oct 14, 2024

Configure Loadbalancer back-end pool only for controlplane machines #2004

Merged

Amulyam24 mentioned this pull request Oct 15, 2024

Add UT for ReconcileNetwork and its subfunctions #1928

Merged

mkumatag approved these changes Oct 15, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 15, 2024

k8s-ci-robot merged commit 3edcccb into kubernetes-sigs:main Oct 15, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor reconcilation of resources #1937

Refactor reconcilation of resources #1937

Amulyam24 commented Sep 2, 2024 •

edited

Loading

netlify bot commented Sep 2, 2024 •

edited

Loading

Amulyam24 commented Sep 2, 2024

Amulyam24 commented Sep 3, 2024

Karthik-K-N left a comment

Amulyam24 commented Sep 13, 2024

Amulyam24 commented Sep 24, 2024

dharaneeshvrd left a comment

Karthik-K-N left a comment •

edited

Loading

Amulyam24 commented Oct 8, 2024

Karthik-K-N left a comment

Amulyam24 commented Oct 15, 2024

mkumatag left a comment

k8s-ci-robot commented Oct 15, 2024

Refactor reconcilation of resources #1937

Refactor reconcilation of resources #1937

Conversation

Amulyam24 commented Sep 2, 2024 • edited Loading

netlify bot commented Sep 2, 2024 • edited Loading

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

Amulyam24 commented Sep 2, 2024

Amulyam24 commented Sep 3, 2024

Karthik-K-N left a comment

Choose a reason for hiding this comment

Amulyam24 commented Sep 13, 2024

Amulyam24 commented Sep 24, 2024

dharaneeshvrd left a comment

Choose a reason for hiding this comment

Karthik-K-N left a comment • edited Loading

Choose a reason for hiding this comment

Amulyam24 commented Oct 8, 2024

Karthik-K-N left a comment

Choose a reason for hiding this comment

Amulyam24 commented Oct 15, 2024

mkumatag left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 15, 2024

Amulyam24 commented Sep 2, 2024 •

edited

Loading

netlify bot commented Sep 2, 2024 •

edited

Loading

Karthik-K-N left a comment •

edited

Loading