Skip to content

Add support for pausing cluster reconcilation #1769

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

Karthik-K-N
Copy link
Contributor

@Karthik-K-N Karthik-K-N commented May 10, 2024

What this PR does / why we need it:

This PR adds support for pausing a cluster reconcilation, to facilitate that it adds event filters and predicates while setting up the controller.

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

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:

Add support for pausing IBMPowerVSCluster and IBMPowerVSMachine reconcilation

@k8s-ci-robot 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 May 10, 2024
@Karthik-K-N
Copy link
Contributor Author

/hold
Need some more testing

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 10, 2024
Copy link

netlify bot commented May 10, 2024

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

Name Link
🔨 Latest commit cec2ee6
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-cluster-api-ibmcloud/deploys/672870ae706b9c0008ad291d
😎 Deploy Preview https://deploy-preview-1769--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 Karthik-K-N force-pushed the use-pause branch 3 times, most recently from 51d7ea1 to 7fd342e Compare May 14, 2024 08:03
@Karthik-K-N
Copy link
Contributor Author

Karthik-K-N commented May 14, 2024

Not able to completely test the end to end flow with create-infra workflow (I could see the machines are going into emergency mode, this is not related to this PR.
),but I could see the reconcilations being paused after adding cluster.x-k8s.io/paused: "true".
I will be trying with old workflow.

@Karthik-K-N
Copy link
Contributor Author

Tested on cluster with infra create workflow and works as expected. on setting cluster.x-k8s.io/paused: "true" on IBMPowerVSCluster resource I could see , reconcilation pausing.

I0516 08:32:51.374797       1 generic_predicates.go:191] "Resource is paused, will not attempt to map resource" predicate="ResourceNotPaused" eventType="update" namespace="default" ="capi-karthik-160511"
I0516 08:32:51.374850       1 generic_predicates.go:191] "Resource is paused, will not attempt to map resource" predicate="ResourceNotPaused" eventType="update" namespace="default" ="capi-karthik-160511"

@Karthik-K-N
Copy link
Contributor Author

Ready for review
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 16, 2024
@mkumatag mkumatag added this to the Next milestone May 17, 2024
@Karthik-K-N Karthik-K-N force-pushed the use-pause branch 2 times, most recently from 72c5cb1 to da823c1 Compare June 28, 2024 09:15
@Karthik-K-N
Copy link
Contributor Author

Even this PR is ready for review

Trying to delete a paused machine


"Resource is paused, will not attempt to map resource" predicate="ResourceNotPaused" eventType="update" namespace="default" ="karthikkn-capi-powervs-control-plane-pqkp5"

@mkumatag mkumatag modified the milestones: Next, v0.9.0 Jul 23, 2024
@mkumatag
Copy link
Member

mkumatag commented Aug 6, 2024

/cc @dharaneeshvrd

Copy link
Contributor

@dharaneeshvrd dharaneeshvrd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes lgtm.
One suggestion is that, if you can document how to use this feature that would be great IMO.

@Karthik-K-N
Copy link
Contributor Author

Re-based and updated the PR

@Karthik-K-N
Copy link
Contributor Author

Karthik-K-N commented Nov 4, 2024

Changes lgtm. One suggestion is that, if you can document how to use this feature that would be great IMO.

Thanks, This change is more towards developer oriented, Use case is to pause the reconciliation when external controller is handling it.
More info about the cluster-api provided tools for pausing is available in api doc, For reference

  1. https://github.com/kubernetes-sigs/cluster-api/blob/main/api/v1beta1/cluster_types.go#L47-L49
  2. https://github.com/kubernetes-sigs/cluster-api/blob/main/api/v1beta1/common_types.go#L98-L103
  3. clusterctl command: https://cluster-api.sigs.k8s.io/clusterctl/commands/alpha-rollout#pauseresume

Thought no need of having it under capibm book as there is no mention about it in capi book as well. Please let me know your opinion.

@dharaneeshvrd
Copy link
Contributor

Ok fine in that case.

Copy link
Contributor

@Prajyot-Parab Prajyot-Parab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dharaneeshvrd, Karthik-K-N, 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:

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 k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 4, 2024
@k8s-ci-robot k8s-ci-robot merged commit 2a3745e into kubernetes-sigs:main Nov 4, 2024
12 of 13 checks passed
@Karthik-K-N Karthik-K-N deleted the use-pause branch November 4, 2024 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/provider/ibmcloud Issues or PRs related to ibmcloud provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support pausing the cluster
5 participants