wbuchwalter / Kubernetes-acs-engine-autoscaler

[Deprecated] Node-level autoscaler for Kubernetes clusters created with acs-engine.
Other
71 stars 22 forks source link

upgraded azure-cli to 2.0.24 #77

Closed yaron-idan closed 6 years ago

yaron-idan commented 6 years ago

Also removed the prompting package that wasn't used and no longer exists, which seems to make tests pass again. I've tested this change on one of our clusters and checked that cluster successfully scales in and out, if there are more ways you can think of for making sure I didn't introduce breaking changes I would love to hear about it.

wbuchwalter commented 6 years ago

@yaron-idan How many scale-in/out have you tried? I have also updated the dependencies on my side for another unrelated issue, but I'm having the issue you described intermittently (about 50% of my scale out fail).

yaron-idan commented 6 years ago

I've tried 2 in a row (scale out, back in, back out and back in again) since the problem seems to effect the second scale out, as the scale in doesn't delete the resources properly. Did you update to the same version as me? It seems that updating to the latest version introduces a change in the params get_mgmt_service_client expects to get, so I've chosen the version just before this change was introduced.

wbuchwalter commented 6 years ago

I've updated to the very last version (which I assumed would also fix this bug). Do you mind trying a few more scale outs and let me know if the issue seem to be indeed fixed?

yaron-idan commented 6 years ago

Not at all, I'll do that on sunday though since the weekend is starting here right now and I don't wanna cause trouble off working hours. Will update when I have the results.

yaron-idan commented 6 years ago

Hey @wbuchwalter , I've been checking the autoscaler while it was scaling in and out today and it seems to do it's job well. None of the errors are happening. Same error just came back after the autoscaler behaved well all day... However, from looking at the conflicts with the master branch I see you've implemented the same fix, and your code actually looks better (as it's works with the current versions of the azure sdk).

Should I close this PR and try the current autoscaler master instead? On the same note, is there any plan to release a new version of the autoscaler any time soon?

wbuchwalter commented 6 years ago

Yep, if this didn't fix your issue then let's close this one. You can use the latest version of the autoscaler by using the helm chart from this repo (in helm-chart) and chaging the image tag to latest in the values.yaml file. I will release an official 3.0.0 once I figure out what is causing this bug,

Thanks for your help.