Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

autoscaler: update cloud-config-controller #1268

Merged

Conversation

cmcconnell1
Copy link
Contributor

modifies the behavior of the autoscaler to terminate worker and
controller nodes no longer needed when minSize is set manually.

Note that I had to delete the initial autoscaler pod after
deploying a new cluster with this modification in place.
After that initial autoscaler pod was deleted the second
pod functioned without errors and as expected.
I will update the issue comments with more details.

Fixes #1253

modifies the behavior of the autoscaler to terminate worker and
controller nodes no longer needed when minSize is set manually.

Note that I had to delete the initial autoscaler pod after
deploying a new cluster with this modification in place.
After that initial autoscaler pod was deleted the second
pod functioned without errors and as expected.
I will update the issue comments with more details.

Fixes kubernetes-retired#1253
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Apr 30, 2018
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Apr 30, 2018
@codecov-io
Copy link

codecov-io commented May 1, 2018

Codecov Report

Merging #1268 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1268   +/-   ##
=======================================
  Coverage   36.63%   36.63%           
=======================================
  Files          63       63           
  Lines        3882     3882           
=======================================
  Hits         1422     1422           
  Misses       2242     2242           
  Partials      218      218

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5bb948d...89e76ac. Read the comment docs.

@mumoshu
Copy link
Contributor

mumoshu commented May 1, 2018

@cmcconnell1 Hey. Thank you very much for your effort!
I wanted to merge this soon but you seem to miss CLA. Would you mind signing it?

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels May 1, 2018
@mumoshu mumoshu added this to the v0.9.10 milestone May 1, 2018
@cmcconnell1
Copy link
Contributor Author

Hello @mumoshu thanks for the quick response. I have completed the requisite signing of the CLA. Please let me know if there's anything else needed.

@mumoshu
Copy link
Contributor

mumoshu commented May 1, 2018

@cmcconnell1 Thanks for the quick response, too!

My only concern is this:

Note that I had to delete the initial autoscaler pod after
deploying a new cluster with this modification in place.

Do you mean that, you created a brand new cluster WITH this change, and then you had to remove the autoscaler pod by hand to make it actually work?

@cmcconnell1
Copy link
Contributor Author

Yes @mumoshu that is what I saw. I updated the logs from the initial crashing AS pod here
1253#

@cmcconnell1
Copy link
Contributor Author

I am deploying a new fresh cluster now to validate if that happens again.
kube-aws version v0.9.10-rc.3

Copy link
Contributor

@mumoshu mumoshu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you very much for your contribution 🎉

@mumoshu mumoshu merged commit 15458e8 into kubernetes-retired:master May 3, 2018
davidmccormick pushed a commit to HotelsDotCom/kube-aws that referenced this pull request Jul 18, 2018
…-v0.10.1-release to hcom-flavour

* commit '910b3d7e27a9cebef937a8b83b02efa6632bb7a4': (21 commits)
  Clean up removal of service-account-signing key
  Removal of service-signing-key loading needs bind to all interfaces
  Merge Kiich dashboard resource limits setting from upstream
  Merge in enabling aggregator + certs
  0.10.x Add kubernetes manifests from customfiles (kubernetes-retired#1400)
  Remove dependency on alpine:lastest docker image - use already present hyperkube image instead.
  0.10.x Extend CustomFiles to be able to render go templates  (kubernetes-retired#1398)
  0.10.x migration preparation release (kubernetes-retired#1379)
  Remove etcd ENI/EIP outputs from the control plane stack and etcd-environment metadata section on the nodepools if Kubernetes.Networking.SelfHosting is Enabled. This is to break the dependency that exists on the nodestacks on the etcd resources in the controlplane stack.
  Fix worker kubelet.service not to fail on the first run
  Fix controller kubelet.service not to fail on the first run
  Fix etcd breakage due to strange systemd behavior
  autoscaler: update cloud-config-controller (kubernetes-retired#1268)
  remove potential circular dependency
  Actually depend on var-lib-etcd2.mount service to be active
  Disable coreOS auto-updates
  removed unused s3 flags from commands
  update kubernetes-dashboard to 1.8.3
  Latest versions of Calico node/typha Removes work-around for Typha issue https://github.com/projectcalico/typha/issues/118
  Fix typo of Getting Started.
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants