Skip to content
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

Add flag to force remove long unregistered nodes #7493

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

BigDarkClown
Copy link
Contributor

What type of PR is this?

/kind feature

What this PR does / why we need it:

Problem:

The Cluster Autoscaler sometimes cannot remove long unregistered nodes. This happens when removing them would leave their node group with fewer than the minimum size required. This wastes resources, as the nodes cannot be used and might be as well deleted.

Solution:
  • We've added a new option that lets you forcefully remove these unused machines.
  • This option only works if your cloud provider allows deleting machines without checking if it violates the minimum size rule.
Background:

Does this PR introduce a user-facing change?

Change adds a flag --force-delete-long-unregistered-nodes, which if used enables the Cluster Autoscaler to remove the long unregistered nodes even if it would break the min size constraints of their node group.

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 13, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BigDarkClown

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 cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/cluster-autoscaler area/provider/alicloud Issues or PRs related to the AliCloud cloud provider implementation approved Indicates a PR has been approved by an approver from all required OWNERS files. area/provider/aws Issues or PRs related to aws provider labels Nov 13, 2024
@k8s-ci-robot k8s-ci-robot added area/provider/azure Issues or PRs related to azure provider area/provider/cluster-api Issues or PRs related to Cluster API provider area/provider/digitalocean Issues or PRs related to digitalocean provider area/provider/equinixmetal Issues or PRs related to the Equinix Metal cloud provider for Cluster Autoscaler area/provider/externalgrpc Issues or PRs related to the External gRPC provider area/provider/gce area/provider/hetzner Issues or PRs related to Hetzner provider area/provider/ionoscloud area/provider/kwok Issues or PRs related to the kwok cloud provider for Cluster Autoscaler area/provider/linode Issues or PRs related to linode provider area/provider/magnum Issues or PRs related to the Magnum cloud provider for Cluster Autoscaler area/provider/oci Issues or PRs related to oci provider size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/provider/rancher labels Nov 13, 2024
@Shubham82
Copy link
Contributor

Hi @BigDarkClown, the mandatory test case Tests / test-and-verify (pull_request) is failed, Please address it.

Thanks!

@Shubham82
Copy link
Contributor

There are the gofmt and spelling errors, due to which this test case failed.

Run hack/update-gofmt.sh and hack/verify-spelling.sh to fix this issue.

@k8s-ci-robot 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 Nov 14, 2024
@BigDarkClown
Copy link
Contributor Author

There are the gofmt and spelling errors, due to which this test case failed.

Run hack/update-gofmt.sh and hack/verify-spelling.sh to fix this issue.

Thanks @Shubham82, I've updated now. Turns out that gofmt picked up a lot of problems in cloudproviders I've modified.

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/cluster-autoscaler area/provider/alicloud Issues or PRs related to the AliCloud cloud provider implementation area/provider/aws Issues or PRs related to aws provider area/provider/azure Issues or PRs related to azure provider area/provider/cluster-api Issues or PRs related to Cluster API provider area/provider/digitalocean Issues or PRs related to digitalocean provider area/provider/equinixmetal Issues or PRs related to the Equinix Metal cloud provider for Cluster Autoscaler area/provider/externalgrpc Issues or PRs related to the External gRPC provider area/provider/gce area/provider/hetzner Issues or PRs related to Hetzner provider area/provider/ionoscloud area/provider/kwok Issues or PRs related to the kwok cloud provider for Cluster Autoscaler area/provider/linode Issues or PRs related to linode provider area/provider/magnum Issues or PRs related to the Magnum cloud provider for Cluster Autoscaler area/provider/oci Issues or PRs related to oci provider area/provider/rancher cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants