Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

Cluster autoscaler: add handlign for nill mig configs (in case not all l node pools are autoscaled) #1301

Conversation

mwielgus
Copy link
Contributor

With the previous fix #1282 a possibility for null pointer exceptions/panics was opened as GceManager started to return nills in case of nodes not belonging to an autoscaled mig. This PR fixes the NPE possibilities there and should be included in the release version ASAP.

cc: @piosz @jszczepkowski @fgrzadkowski

@piosz
Copy link
Contributor

piosz commented Jul 1, 2016

LGTM

@piosz piosz added the lgtm Indicates that a PR is ready to be merged. label Jul 1, 2016
@mwielgus mwielgus merged commit e2a8815 into kubernetes-retired:master Jul 1, 2016
@fgrzadkowski
Copy link
Contributor

@mwielgus Why this didn't come up during tests for #1282? AFAIU it should always panic if we have a non-autoscaled mig, right?

@mwielgus
Copy link
Contributor Author

mwielgus commented Jul 1, 2016

Honestly - I don't know - i updated the cluster autoscaler on multi-node-pool cluster on @olekzabl cluster and it correctly scaled it down. @olekzabl do you still have this cluster running?

I'm recreating the cluster so we can do the test again.

@olekzabl
Copy link

olekzabl commented Jul 1, 2016

The resources we've talked about are here:

  • https://test-container.sandbox.googleapis.com/
  • project: cloud-kubernetes-dev
  • olekz-cluster49 (1.3.0-beta.1):
    • default-pool of size 1 (autoscaling off),
    • pool-1 of requested size 1-2 (autoscaling on), originally had size 3 for >2h but now has size 1
  • olekz-cluster52 (1.3.0-beta.2):
    • default-pool of requested size 1-2 (autoscaling on), still has size 3 after >1d of running
  • olekz-cluster53 (1.3.0-beta.2.191 - this was the version you advised me to request):
    • like ...-52; in error state since creation, and gcloud told me this version is somehow bad. (?)

mwielgus added a commit to kubernetes/autoscaler that referenced this pull request Apr 18, 2017
…null-handling-for-migconfigs

Cluster autoscaler: add handlign for nill mig configs (in case not all l node pools are autoscaled)
mwielgus added a commit to kubernetes/autoscaler that referenced this pull request Apr 18, 2017
…null-handling-for-migconfigs

Cluster autoscaler: add handlign for nill mig configs (in case not all l node pools are autoscaled)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/autoscaler lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants