Feature Request - native Cluster Autoscaler support

[backjo]

Is this a BUG REPORT or FEATURE REQUEST?:
Feature Request
What happened:
It would be nice if cluster autoscaler was supported natively.
Today, registering instance groups with cluster autoscaler can be achieved through the manual addition of Tags in the CR, but this could be cumbersome for users to add to each and every CR - especially since most of the information is already present in the CR.

Proposal
Add a new field like "useClusterAutoscaler" to the InstanceGroup defintion which, if set, would automatically tag the ASG with the following tags.

k8s.io/cluster-autoscaler/<cluster-name>: owned
k8s.io/cluster-autoscaler/enabled: true
k8s.io/cluster-autoscaler/node-template/label/<my-label-key>: <my-label-value> (this would be repeated for each unique label) 
k8s.io/cluster-autoscaler/node-template/taint/my-taint: "true:NoSchedule" (this would be repeated for each unique taint)

[eytan-avisror]

Thanks @backjo, good idea.. though I think since this is more of a convenience feature, it might be more appropriate as an annotation vs. spec which refers to more concrete configuration.
For example:

annotations:
  instancemgr.keikoproj.io/cluster-autoscaler-enabled: "true"

Then, when we derive the tags to add to the ASG we can check for this annotation and add the relevant tags:

k8s.io/cluster-autoscaler/<cluster-name>: owned
k8s.io/cluster-autoscaler/enabled: true

Can you explain what is the purpose of the other two?

k8s.io/cluster-autoscaler/node-template/label/<my-label-key>: <my-label-value> (this would be repeated for each unique label) 
k8s.io/cluster-autoscaler/node-template/taint/my-taint: "true:NoSchedule" (this would be repeated for each unique taint)

[eytan-avisror]

BTW, you can also use the configmap default values to achieve this for all IGs in the cluster:
https://github.com/keikoproj/instance-manager/blob/master/docs/EKS.md#gitopsplatform-support-boundaries-and-default-values

[backjo]

@eytan-avisror totally agree on the annotation - good call.

k8s.io/cluster-autoscaler/node-template/label/<my-label-key>: <my-label-value> (this would be repeated for each unique label) 
k8s.io/cluster-autoscaler/node-template/taint/my-taint: "true:NoSchedule" (this would be repeated for each unique taint)

^ These tags are meant to inform cluster autoscaler about what taints and labels exist on the ASG. It will derive these automatically if the labels/taints are on a node in the ASG already, but if you want to be able to scale from 0 nodes, it needs these tags to know whether or not to scale the ASG.

[eytan-avisror]

@backjo
I believe only the first two are required, where the first is just in order to avoid conflict when there are multiple clusters in the same account:

k8s.io/cluster-autoscaler/<cluster-name>: owned
k8s.io/cluster-autoscaler/enabled: true

From docs:

It is recommended to use a second tag like 
k8s.io/cluster-autoscaler/<cluster-name> 
when k8s.io/cluster-autoscaler/enabled is used across many clusters to prevent ASGs from different clusters 
recognized as the node groups.

The other tags for label/taint are used by cluster-autoscaler as 'extra hints' - I am trying to understand the exact use-case but this is what the docs mention:

You may also provide additional hints to Cluster Autoscaler that the nodes will be labeled or tainted when they join the cluster, such as:

k8s.io/cluster-autoscaler/node-template/label/foo: bar
k8s.io/cluster-autoscaler/node-template/taint/dedicated: NoSchedule

NOTE: It is your responsibility to ensure such labels and/or taints are applied via the node's kubelet configuration at startup.

We can probably add relevant tags according to taints & labels provided in spec, but am still trying to understand why this is needed if you enable cluster-autoscaler per ASG - is this for use cases around mixedInstances / launch template?

[backjo]

@eytan-avisror

The hint tags are really only needed for instance groups that have 0 minimum nodes. See kubernetes/autoscaler#2418 for more context.

The use case I'm playing around with is adding these IGs for several namespaces and having them set to 0 minimum nodes - and only scaling up once workloads are deployed in those namespaces.

[eytan-avisror]

Got it.. makes sense.
So yeah, if in order to scale up from 0 we need those tags, we can definitely add these tags as well - let me know if you want to do this and we can discuss the implementation of this further.