Cluster-autoscaler: fix for multi-mig autoscaling #1282
Conversation
|
@mwielgus Please explain what is the bug and how do we fix this. |
| return mig, nil | ||
|
|
||
| for _, mig := range m.migs { | ||
|
|
|
Fixes: #1283 Previously we were rising an error if we didn't have mig config for all nodes in the cluster. This error was equivalent to cluster being not in stable state and preventing CA from any scale up or scale downs. Now we raise this error only if the node has the prefix corresponding to some mig base name but the mig doesn't have this node on the list. Otherwise we assume that the node belongs to non-autoscaled mig and continue with all activities. |
|
LGTM |
| if mig.config.Project == instance.Project && | ||
| mig.config.Zone == instance.Zone && | ||
| strings.HasPrefix(instance.Name, mig.basename) { | ||
| if err := m.regenerateCache(); err != nil { |
There was a problem hiding this comment.
I think it'd be readable if you restructure the code such that:
- in a for loop check if we have any corresponding loop
- if we have a corresponding mig regenerate cache and return mig
- if don't have it return nils
|
@fgrzadkowski sorry, too late, i will do it in a follow-up pr. |
|
@mwielgus At the very least please answer to my question regarding base name in the first run. |
…node Cluster-autoscaler: fix for multi-mig autoscaling
…node Cluster-autoscaler: fix for multi-mig autoscaling
cc: @fgrzadkowski @piosz @jszczepkowski
Fixes #1283