Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cluster-autoscaler FAQ: clarify what "deleting nodes" means in this context #2217

Merged
merged 2 commits into from
Jul 30, 2019
Merged

cluster-autoscaler FAQ: clarify what "deleting nodes" means in this context #2217

merged 2 commits into from
Jul 30, 2019

Conversation

matthiasr
Copy link

I was trying to debug why, in our (custom) clusters, nodes continue to exist (with status NotReady) when the cluster-autoscaler deletes them.

"delete node" has a specific meaning in the context of the Kubernetes
API: deleting the node object.

In the end, I found that it is not actually expected to do that: kube-controller-manager (or cloud-controller-manager) is. The repeated use of "delete(ing/s) node" in the FAQ obscured this.

I am proposing to

  • change the FAQ to say "terminate"
    • it may make sense to change the flags later as well?
  • add an FAQ entry that makes this relationship clear

Matthias Rampke added 2 commits July 26, 2019 13:18
cluster-autoscaler: Replace ambiguous "delete node" wording in FAQ
8695a83 
"delete node" has a specific meaning in the context of the Kubernetes
API: deleting the node object. However, the cluster-autoscaler never
does that; it terminates the underlying instance and expects the
[cloud-node-controller](https://kubernetes.io/docs/concepts/architecture/cloud-controller/#node-controller)
to remove the corresponding node from Kubernetes.

Replace all mentions of "deleting" a node with "terminating" to
disambiguate this.

Signed-off-by: Matthias Rampke <[email protected]>
cluster-autoscaler: add FAQ entry for the mechanics of "delete node"
d0eaa47 
This had me stumped for the better part of a day. While the
cluster-autoscaler "deletes" nodes, it does not actually delete the Node
object from the Kubernetes API.

In normal operations, with a well-configured cluster, this is a minor
point; however, when debugging why nodes do not get deleted, the
inconsistent terminology can be a major headache. This FAQ entry should
clarify the difference for anyone who needs to know.

Signed-off-by: Matthias Rampke <[email protected]>
@k8s-ci-robot
Copy link
Contributor

Welcome @matthiasr!

It looks like this is your first PR to kubernetes/autoscaler 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/autoscaler has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 26, 2019
@k8s-ci-robot k8s-ci-robot requested review from Jeffwan and piosz July 26, 2019 13:39
@bskiba
Copy link
Member

bskiba commented Jul 26, 2019

/assign @MaciekPytel

@bskiba bskiba added area/cluster-autoscaler documentation kind/documentation Categorizes issue or PR as related to documentation. labels Jul 26, 2019
@MaciekPytel
Copy link
Contributor

/lgtm
/approve

Thanks for contributing this!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 30, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MaciekPytel

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 30, 2019
@k8s-ci-robot k8s-ci-robot merged commit 55082d6 into kubernetes:master Jul 30, 2019
@danielmellado danielmellado mentioned this pull request Oct 24, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jeffwan Jeffwan Awaiting requested review from Jeffwan

@piosz piosz Awaiting requested review from piosz

Assignees

@MaciekPytel MaciekPytel

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/cluster-autoscaler cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. documentation kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@matthiasr @k8s-ci-robot @bskiba @MaciekPytel