-
Notifications
You must be signed in to change notification settings - Fork 637
✨ feat: Implement autoscaling from zero by auto-populating AWSMachineTemplate capacity #5711
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
base: main
Are you sure you want to change the base?
✨ feat: Implement autoscaling from zero by auto-populating AWSMachineTemplate capacity #5711
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
Hi @LiangquanLi930. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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-sigs/prow repository. |
|
/ok-to-test |
f1ee365 to
3be8f4d
Compare
01be987 to
915f55b
Compare
915f55b to
b3850d1
Compare
2a9823c to
63118be
Compare
|
@richardcase @nrb @chrischdi Thanks for your review again! When you have time, could you help to review again? Thanks! |
10f3258 to
00485e3
Compare
|
/label tide/merge-method-squash |
| return ctrl.NewControllerManagedBy(mgr). | ||
| For(&infrav1.AWSMachineTemplate{}). | ||
| WithOptions(options). | ||
| WithEventFilter(predicates.ResourceNotPausedAndHasFilterLabel(mgr.GetScheme(), log.GetLogger(), r.WatchFilterValue)). |
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.
@chrischdi is this predicates.ResourceNotPausedAndHasFilterLabel the new equivalent to paused.EnsurePausedCondition later on in the reconcile loop ? I guess it is similar but behaves differently as the paused.EnsurePausedCondition check allows for doing extra steps before deciding to stop reconciling the resource right?
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.
Yes, these two (the predicate and the EnsurePausedCondition function) are supposed to work together.
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.
So are we saying we need @LiangquanLi930 to also add paused.EnsurePausedCondition ?
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
One nit.
I won't add an approve because I'm not a maintainer.
Note: I did not fully review if the unit tests cover all reasonable cases or if they should be expanded.
|
LGTM label has been added. Git tree hash: 71a6824f8c228b2fff65322ecb245a02a075d5ca
|
00485e3 to
b398ffc
Compare
|
New changes are detected. LGTM label has been removed. |
…deInfo Add AWSMachineTemplateReconciler to automatically populate capacity and node info fields by querying AWS EC2 API. This completes the autoscaling from zero implementation by ensuring the required metadata is available without manual configuration. Changes include: - Add NodeInfo struct with Architecture and OperatingSystem fields to AWSMachineTemplate status - Implement controller that queries EC2 API for instance type specifications - Auto-populate CPU, memory, pods, and ephemeral storage capacity - Auto-detect architecture (amd64/arm64) and OS (linux/windows) from AMI - Add conversion logic for backward compatibility with v1beta1 - Enable status subresource on AWSMachineTemplate CRD - Add comprehensive unit tests (351 lines) covering various scenarios - Add RBAC permissions for controller operations The controller automatically populates these fields when an AWSMachineTemplate is created or updated, eliminating the need for manual configuration and enabling Cluster Autoscaler to make informed scaling decisions from zero nodes. Related: https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20210310-opt-in-autoscaling-from-zero.md Squashed from 5 commits: - 9a92a43 Implement autoscaling from zero by auto-populating AWSMachineTemplate capacity - 86fe072 add AWSMachineTemplate NodeInfo - ddaf62c Fix review comments - 4ea52c8 Fix review comments 2 - b398ffc Fix review comments 3
b398ffc to
c0657c7
Compare
|
squash commits |
df0338b to
351ee0d
Compare
351ee0d to
641a0ca
Compare
|
@LiangquanLi930: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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-sigs/prow repository. I understand the commands that are listed here. |
✨ Implement autoscaling from zero by auto-populating AWSMachineTemplate capacity and NodeInfo
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR implements the Cluster API autoscaling from zero proposal for CAPA by adding a controller that automatically populates
AWSMachineTemplate.Status.Capacitywith instance type information.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 #
Special notes for your reviewer:
Checklist:
Release note: