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.

Sign up for GitHub

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

Jump to bottom

feat(clusterapi): per nodeGroup autoscaling options #6743

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(clusterapi): per nodeGroup autoscaling options #6743

wants to merge 1 commit into from

Conversation

mweibel
Copy link
Contributor

@mweibel mweibel commented Apr 22, 2024

What type of PR is this?

/kind feature

What this PR does / why we need it:

Implements overrding of autoscaling options per NodeGroup (MachineDeployment/Set/Pool). This allows us to improve scale up/down behavior depending on the node groups configuration.

Which issue(s) this PR fixes:

Fixes #6742

Special notes for your reviewer:

The annotation prefix is intentionally does not contain cluster-api-autoscaler-node-group- after the slash because the annotation key must be less than 63 chars long.

Does this PR introduce a user-facing change?

Added configurable autoscaling options to clusterapi provider allowing users to configure e.g. `--scale-down-unneeded-time` on a per node group level.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 22, 2024
@k8s-ci-robot
Copy link
Contributor

Hi @mweibel. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 22, 2024
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mweibel
Once this PR has been reviewed and has the lgtm label, please assign detiber for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found 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 the area/provider/cluster-api Issues or PRs related to Cluster API provider label Apr 22, 2024
@mweibel
Copy link
Contributor Author

mweibel commented May 13, 2024 •
edited

ping @elmiko - maybe you have some time to review this other PR of mine. Thanks :)

or give /ok-to-test would also be nice 👍

@elmiko
Copy link
Contributor

elmiko commented May 13, 2024

apologies @mweibel

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 13, 2024
@elmiko
Copy link
Contributor

elmiko commented May 13, 2024

this seems like a cool feature, i need some time to study the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enxebre enxebre Awaiting requested review from enxebre

@jackfrancis jackfrancis Awaiting requested review from jackfrancis

Assignees
No one assigned
Labels
area/cluster-autoscaler area/provider/cluster-api Issues or PRs related to Cluster API provider 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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CAPI autoscaler does not support per NodeGroup autoscaling options
3 participants
@mweibel @k8s-ci-robot @elmiko