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

Support autoscaling from zero #3679

Closed
com6056 opened this issue Aug 19, 2022 · 8 comments · Fixed by #3684
Closed

Support autoscaling from zero #3679

com6056 opened this issue Aug 19, 2022 · 8 comments · Fixed by #3684
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@com6056
Copy link

com6056 commented Aug 19, 2022

/kind feature

Describe the solution you'd like
[A clear and concise description of what you want to happen.]

Add support for autoscaling from zero defined in https://github.com/kubernetes-sigs/cluster-api/blob/main/docs/proposals/20210310-opt-in-autoscaling-from-zero.md

Just implemented upstream in kubernetes/autoscaler#4840

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

Environment:

  • Cluster-api-provider-aws version:
  • Kubernetes version: (use kubectl version):
  • OS (e.g. from /etc/os-release):
@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 19, 2022
@Skarlso
Copy link
Contributor

Skarlso commented Aug 20, 2022

/triage accepted

I saw this and wanted to do it! :)

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 20, 2022
@Skarlso
Copy link
Contributor

Skarlso commented Aug 20, 2022

/assign Skarlso

@Skarlso
Copy link
Contributor

Skarlso commented Aug 20, 2022

Ping @richardcase, @sedefsavas and @Ankitasw for a second opinion. :)

@Skarlso
Copy link
Contributor

Skarlso commented Aug 20, 2022

Also, now I'm wondering, when and how do we usually do triage-es?

@Skarlso
Copy link
Contributor

Skarlso commented Aug 22, 2022

My understanding from reading this is as follows:

  • Create a Status field in AWSMachineTemplate and reconcile it
  • Based on those templates, create annotations on the machine deployment following the names outlined within
  • ?
  • Profit.

I'm not sure what else is required. Do we just have to add the annotations based on our spec fields and reconcile them back into the status field? Is that it? :) ( ignoring validations and the likes for now ).

Maybe reconciling it back into the spec if the annotations exist, might disrupt some flow. Like, it would cause a diff since local resources don't contain those settings.

Found in the Code:

	// The annotations should override any values from the status block of the machine template.
	// We loop through the status block capacity first, then overwrite any values with the
	// annotation capacities.

So status IS something the autoscaler will pick up.

@Skarlso
Copy link
Contributor

Skarlso commented Aug 22, 2022

And preventing CAPA itself from creating a machine instance where it expects one to be if these fields are defined.

@Skarlso
Copy link
Contributor

Skarlso commented Aug 22, 2022

Aha

	infraref, found, err := unstructured.NestedStringMap(r.unstructured.Object, "spec", "template", "spec", "infrastructureRef")

@Skarlso
Copy link
Contributor

Skarlso commented Aug 22, 2022

Answer from cluster-api channel:

Yes, it's enough to define the status.
Reference implementation can be found here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-priority triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants