-
Notifications
You must be signed in to change notification settings - Fork 1.4k
📖 Document providers that support deletion single MachinePool Machines #12872
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
📖 Document providers that support deletion single MachinePool Machines #12872
Conversation
fabriziopandini
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
LGTM label has been added. Git tree hash: 531bcbbb06b10af71fb50b14c08dccfa1cbb43cd
|
fed69a5 to
8b335bb
Compare
fabriziopandini
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to merge after rebase
8b335bb to
7d14a43
Compare
|
@fabriziopandini I rebased |
| | [OCI](https://oracle.github.io/cluster-api-provider-oci/managed/managedcluster.html) | `OCIManagedMachinePool`<br> `OCIMachinePool` | Implemented | Yes; doesn't have support for deletion of single machine | | ||
| | [Scaleway](https://github.com/scaleway/cluster-api-provider-scaleway/blob/main/docs/scalewaymanagedmachinepool.md) | `ScalewayManagedMachinePool` | Implemented | No | | ||
|
|
||
| Providers may support the deletion of single machine pool `Machine` objects. That allows, for example, using `MachineHealthCheck` to remediate machines that became unhealthy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this true today? I thought MHC is not supported yet for MachinePools?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm living in the world where #11392 is already merged (due to our fork) :), so the feature is live and working in my company. If you don't mind, I'd already document it that way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just think it's confusing for most of our users if we document something here that only works on your fork. Can we add a small note like "once PR ... is merged"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I added a note.
7d14a43 to
8991d17
Compare
|
Thx. I believe this one is still open: #12872 (comment) |
8991d17 to
18dd19e
Compare
|
Thx! /lgtm |
|
LGTM label has been added. Git tree hash: 9aba9093f920c0aebcc466b28bc499486010cc77
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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 |
What this PR does / why we need it:
Follow-up for #12797, as suggested by @richardcase
/area machinepool