Skip to content

✨ Support RKE2ControlPlane mhc remediation #627

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 4 commits into from
May 8, 2025

Conversation

MaxFedotov
Copy link
Contributor

@MaxFedotov MaxFedotov commented Apr 18, 2025

kind/feature

What this PR does / why we need it:

This PR adds support for RKE2ControlPlane remediation. The code is taken from KCP (https://github.com/kubernetes-sigs/cluster-api/blob/main/controlplane/kubeadm/internal/controllers/remediation.go) and adopted for RKE2. It also adds a new rcp_remediation_test e2e test, based on CAPI e2e KCPRemediationSpec.

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

Special notes for your reviewer:

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

@MaxFedotov MaxFedotov requested a review from a team as a code owner April 18, 2025 11:34
@furkatgofurov7 furkatgofurov7 added kind/feature area/controller Indicates an issue or PR related to the controllers labels Apr 22, 2025
Copy link
Contributor

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

Hey, thanks for the PR 👍🏼
I have not looked into the logic yet, but can you please first rename the KCP mentions with RKE2ControlPlane everywhere (comments, file names, vars etc)?

@furkatgofurov7 furkatgofurov7 changed the title support rke2ControlPlane mhc remediation ✨ SupportRKE2ControlPlane mhc remediation Apr 22, 2025
@furkatgofurov7 furkatgofurov7 changed the title ✨ SupportRKE2ControlPlane mhc remediation ✨ Support RKE2ControlPlane mhc remediation Apr 22, 2025
@MaxFedotov
Copy link
Contributor Author

Hey @furkatgofurov7, done. The only place where i left kcp is in tests, because i've used the standard capi KCPRemediationSpec test

@furkatgofurov7
Copy link
Contributor

@MaxFedotov you can run make lint locally to make sure lint CI passes here

@MaxFedotov
Copy link
Contributor Author

@furkatgofurov7, done

@furkatgofurov7 furkatgofurov7 moved this to PR to be reviewed in CAPI / Turtles Apr 29, 2025
@MaxFedotov
Copy link
Contributor Author

hey @furkatgofurov7! Is there anything else i can do to proceed with this PR? Thanks!

anmazzotti
anmazzotti previously approved these changes May 1, 2025
@MaxFedotov MaxFedotov dismissed stale reviews from alexander-demicev and anmazzotti via 6c7b9d7 May 2, 2025 11:42
@MaxFedotov MaxFedotov force-pushed the mhc_remediation branch 2 times, most recently from aa8fd1f to ba4e051 Compare May 2, 2025 11:46
@MaxFedotov
Copy link
Contributor Author

MaxFedotov commented May 2, 2025

Sorry for the force push, had to rebase the branch to the latest main to fix the conflicts

@MaxFedotov MaxFedotov force-pushed the mhc_remediation branch 3 times, most recently from aaa45ce to 600be9a Compare May 7, 2025 09:21
@MaxFedotov
Copy link
Contributor Author

@furkatgofurov7 done, changes in makefile and dockerfile rollbacked

Copy link
Contributor

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

@MaxFedotov thanks for patience and taking a time to address the review comments. This change LGTM, can you please squash the commits as much as needed to avoid duplicate commit messages to have a logically coherent commit flow? Thanks!

MaxFedotov added 2 commits May 7, 2025 13:12
…ve cluster-template-rcp-remediation to a separate InfrastructureProvider config
@MaxFedotov
Copy link
Contributor Author

@furkatgofurov7 done!

furkatgofurov7
furkatgofurov7 previously approved these changes May 7, 2025
@MaxFedotov
Copy link
Contributor Author

MaxFedotov commented May 7, 2025

@furkatgofurov7 looking on the test error and the cluster-api code, template name should be cluster-template-kcp-remediation.yaml because it also depends on the hardcoded specName variable - https://github.com/kubernetes-sigs/cluster-api/blob/main/test/e2e/kcp_remediations.go#L93

I will make a commit with rename, can we try to run the tests using the cluster-template-kcp-remediation.yaml?

@MaxFedotov
Copy link
Contributor Author

All tests green, great :)

Copy link
Contributor

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

Great work, thanks for the PR!

@furkatgofurov7 furkatgofurov7 added this pull request to the merge queue May 8, 2025
Merged via the queue into rancher:main with commit d0a0ed1 May 8, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from PR to be reviewed to Done in CAPI / Turtles May 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/controller Indicates an issue or PR related to the controllers kind/feature
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Support RKE2ControlPlane remediation by MHC
5 participants