Eksctl: Allow modifying the maxSize value of a NodeGroup w/o changing desired count

[nemo83]

Before creating a feature request, please search existing feature requests to see if you find a similar one. If there is a similar feature request please up-vote it and/or add your comments to it instead

Why do you want this feature?
I want to be able to modify the only maxSize value of a NodeGroup w/o having to manually scale the cluster. Currently I can only set the desired size and if desired is > current max gets set to desired. I want let the autoscaler to the scaling and only set the new maxSize.

What feature/behavior/change do you want?
Read above.

Do not hesitate, when appropriate, to share the exact commands or API you would like, and/or to share a diagram (e.g.: asciiflow.com): "a picture is worth a thousand words".

eksctl scale nodegroup -f spec
OR
eksctl scale nodegroup --clustername name --maxSize 20

[sayboras]

Seems like if --nodes is bigger than max size, it will update max size as well as per https://github.com/weaveworks/eksctl/blob/8184b8d11b1e983dc60966ec9d35c5b45663ebcf/pkg/cfn/manager/nodegroup.go#L215-L216

Anyway, config file option seems to be planned before https://github.com/weaveworks/eksctl/blob/master/pkg/ctl/scale/nodegroup.go#L46-L48.

[nemo83]

Seems like if --nodes is bigger than max size, it will update max size as well as per

https://github.com/weaveworks/eksctl/blob/8184b8d11b1e983dc60966ec9d35c5b45663ebcf/pkg/cfn/manager/nodegroup.go#L215-L216

Anyway, config file option seems to be planned before https://github.com/weaveworks/eksctl/blob/master/pkg/ctl/scale/nodegroup.go#L46-L48.

That's true, but 2 things:

1. I don't want to do the autoscaler job.
2. I want to possibly go from 10 nodes to 20 max. and I won't want to go directly to 20, it's autoscaler role to tune accordingly. I just want to set the max.

[sayboras]

That's true, but 2 things:

1. I don't want to do the autoscaler job.
2. I want to possibly go from 10 nodes to 20 max. and I won't want to go directly to 20, it's autoscaler role to tune accordingly. I just want to set the max.

Good points. Min and Max size should be considered as boundary.

@martina-if @marccarre @cPu1 for your inputs.

[sayboras]

Before implementing the changes, I would like to discuss the approach as below:

1. Update CLI option flag --node behaviour, so that value must be within the range of [min, max]. Return error if not. This is BREAKING change.
2. Add support for CLI flags --min and --max
3. Add support for declarative configuration file with -f flag.

Please let me know if the above if fine. @martina-if @cPu1 @marccarre

[martina-if]

Hi @sayboras thank you for taking the lead on this :) Here are my thoughts:

1a. If --nodes is specified and nothing else then keep the current behavior and extend the max value to match it.
1b. if --nodes is specified and --max too but it's lower then return an error

1. sounds good
2. sounds good

Does this make sense?

[sayboras]

I am still thinking that we should make the same behavior as ASG, that desired number of nodes must must be within the range :). Changing the max value seems implicitly might confuse experienced users, who are already familiar with exsiting ASG :)

Please find below the quick screen shot, in which I only changed desired nodes in AWS Consule UI.

[martina-if]

@sayboras Thank you for the example from the console.

I think in this case I prefer to keep the existing behavior for the same reason: avoiding confusion and not break the API.

In a GUI, I can understand why the choice is to display an error instead of subtly change the value of another input box. But in our case that has been the previous behavior.

I am not completely against it, since it also makes sense. @marccarre @cPu1 what do you guys think?

[marccarre]

Do we know the rationale for the current behaviour? (@cPu1, @martina-if, or anyone else?)
Not knowing it myself, I'd think that it would make more sense to enforce min <= nodes <= max and behave as suggested by @sayboras. It is true that it would be a breaking change, but on the other hand, it makes eksctl less permissive, so at least it is less dangerous than the other way around?

[cPu1]

Update CLI option flag --node behaviour, so that value must be within the range of [min, max]. Return error if not. This is BREAKING change.

I agree with making this breaking change. The maxSize value serves as an upper bound (for cost reasons?) and it's safer to have the user explicitly specify it when scaling to a value > maxSize to avoid unintentional changes. Similarly, the minSize value hints that at least minSize nodes must be running in the cluster to accommodate all pods and meet the availability requirements, so it's best to make it explicit when scaling to a value < minSize.

To make it easier for users running into the new validation error, we should also print the suggested command that'd fix the error.

[sayboras]

Cool, thanks all for your input @martina-if @marccarre @cPu1 :+1: :100:

If it's not urgent, I can work on this sometime this week :100: