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

Add an explanation of how the kubeadm kubelet dropin file works and a note about cAdvisor #4229

Merged
merged 1 commit into from
Jun 30, 2017
Merged

Add an explanation of how the kubeadm kubelet dropin file works and a note about cAdvisor #4229

merged 1 commit into from
Jun 30, 2017

Conversation

luxas
Copy link
Member

@luxas luxas commented Jun 29, 2017 •
edited by k8s-reviewable
Loading

/cc @roberthbailey @jbeda @chenopis @lukemarsden

FYI @dchen1107

This was discussed in kubernetes/release#356. I'm adding some context on how the dropin works and information about how the user might revert kubernetes/release#356 if they have to.

This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 29, 2017
This was referenced Jun 29, 2017
Merged
Closed
@chenopis chenopis added this to the 1.7 milestone Jun 29, 2017
chenopis
chenopis suggested changes Jun 29, 2017
View reviewed changes
Copy link
Contributor

@chenopis chenopis left a comment

Choose a reason for hiding this comment

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

Some minor grammar changes for consistency and markdown formatting issues.

docs/admin/kubeadm.md Outdated
@@ -525,6 +525,57 @@ Here `v1.7.x` means the "latest patch release of the v1.7 branch".

`${ARCH}` can be one of: `amd64`, `arm`, `arm64`, `ppc64le` or `s390x`.

## Managing the kubeadm dropin file for the kubelet
Copy link
Contributor

Choose a reason for hiding this comment

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

change all occurrences of "dropin" to "drop-in"

docs/admin/kubeadm.md Outdated
```

A breakdown of what/why:
- `--kubeconfig=/etc/kubernetes/kubelet.conf` point to the kubeconfig file that
Copy link
Contributor

Choose a reason for hiding this comment

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

"point" -> "points"

docs/admin/kubeadm.md Outdated
started until `kubeadm init` is run.
- `--pod-manifest-path=/etc/kubernetes/manifests` specifies from where to read
Static Pod manifests used for spinning up the control plane
- `--allow-privileged=true` allow this kubelet to run privileged Pods
Copy link
Contributor

Choose a reason for hiding this comment

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

"allow" -> "allows"

docs/admin/kubeadm.md Outdated
- `--pod-manifest-path=/etc/kubernetes/manifests` specifies from where to read
Static Pod manifests used for spinning up the control plane
- `--allow-privileged=true` allow this kubelet to run privileged Pods
- `--network-plugin=cni` use CNI networking
Copy link
Contributor

Choose a reason for hiding this comment

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

"use" -> "uses"

docs/admin/kubeadm.md Outdated
Static Pod manifests used for spinning up the control plane
- `--allow-privileged=true` allow this kubelet to run privileged Pods
- `--network-plugin=cni` use CNI networking
- `--cni-conf-dir=/etc/cni/net.d` where to look for the
Copy link
Contributor

Choose a reason for hiding this comment

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

add "specifies": "--cni-conf-dir=/etc/cni/net.d specifies where to look..."

docs/admin/kubeadm.md Outdated
`search` entries in Pods' `/etc/resolv.conf`
- `--client-ca-file=/etc/kubernetes/pki/ca.crt` authenticate requests to the Kubelet
API using this CA certificate.
- `--authorization-mode=Webhook` authorize requests to the Kubelet API by `POST`-ing
Copy link
Contributor

Choose a reason for hiding this comment

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

"authorize" -> "authorizes"

docs/admin/kubeadm.md Outdated
API using this CA certificate.
- `--authorization-mode=Webhook` authorize requests to the Kubelet API by `POST`-ing
a `SubjectAccessReview` to the API Server
- `--cadvisor-port=0` disable that cAdvisor listens to `0.0.0.0:4194` by default.
Copy link
Contributor

Choose a reason for hiding this comment

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

suggested edit: "--cadvisor-port=0 disables cAdvisor from listening to 0.0.0.0:4194 by default."

docs/admin/kubeadm.md Outdated
a `SubjectAccessReview` to the API Server
- `--cadvisor-port=0` disable that cAdvisor listens to `0.0.0.0:4194` by default.
cAdvisor will still be run inside of the kubelet and its API can be accessed at
`https://{node-ip}:10250/stats/`. If you want to enable that cAdvisor listens on
Copy link
Contributor

Choose a reason for hiding this comment

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

suggested edit: "If you want to enable cAdvisor to listen on a wide-open port, run:"

docs/admin/kubeadm.md
ExecStart=/usr/bin/kubelet $KUBELET_KUBECONFIG_ARGS $KUBELET_SYSTEM_PODS_ARGS $KUBELET_NETWORK_ARGS $KUBELET_DNS_ARGS $KUBELET_AUTHZ_ARGS $KUBELET_CADVISOR_ARGS $KUBELET_EXTRA_ARGS
```

A breakdown of what/why:
Copy link
Contributor

Choose a reason for hiding this comment

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

You should change these into an unordered (bulleted) list. Markdown is currently rendering them as a single paragraph, see preview: https://deploy-preview-4229--kubernetes-io-master-staging.netlify.com/docs/admin/kubeadm/

docs/admin/kubeadm.md Outdated
cAdvisor will still be run inside of the kubelet and its API can be accessed at
`https://{node-ip}:10250/stats/`. If you want to enable that cAdvisor listens on
a wide-open port, run:
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a newline above this line so that the shell snippet is rendered in a separate block on its own line, see preview: https://deploy-preview-4229--kubernetes-io-master-staging.netlify.com/docs/admin/kubeadm/

@chenopis chenopis changed the base branch from release-1.7 to master June 30, 2017 05:54
@chenopis chenopis changed the base branch from master to release-1.7 June 30, 2017 05:56
@chenopis chenopis changed the base branch from release-1.7 to master June 30, 2017 05:56
@chenopis chenopis changed the base branch from master to release-1.7 June 30, 2017 05:57
@chenopis
Copy link
Contributor

@luxas Can you rebase this onto master? Now that v1.7 is out, the release-v1.7 branch is closed. Thx

@luxas luxas force-pushed the add_kubeadm_cadvisor_note branch 2 times, most recently from 1ec1f02 to b49eb31 Compare June 30, 2017 06:53
@luxas luxas changed the base branch from release-1.7 to master June 30, 2017 06:55
@luxas
Copy link
Member Author

luxas commented Jun 30, 2017

@chenopis Done!

chenopis
chenopis suggested changes Jun 30, 2017
View reviewed changes
Copy link
Contributor

@chenopis chenopis left a comment

Choose a reason for hiding this comment

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

unordered list still not rendering correctly; might need a newline before it

docs/admin/kubeadm.md Outdated
```

A breakdown of what/why:
* `--kubeconfig=/etc/kubernetes/kubelet.conf` points to the kubeconfig file that
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need a newline before this line. It's currently not rendering the unordered list correctly. See the preview: https://deploy-preview-4229--kubernetes-io-master-staging.netlify.com/docs/admin/kubeadm/#managing-the-kubeadm-drop-in-file-for-the-kubelet

Copy link
Member Author

Choose a reason for hiding this comment

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

Argh. Let's try again.

@luxas luxas force-pushed the add_kubeadm_cadvisor_note branch from b49eb31 to df25909 Compare June 30, 2017 07:34
@chenopis
Copy link
Contributor

@luxas I'm going to merge this. Any additional changes can be done in a follow-up PR.

@chenopis chenopis merged commit b9f0a5d into kubernetes:master Jun 30, 2017
@chenopis chenopis self-assigned this Aug 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chenopis chenopis chenopis approved these changes

@jbeda jbeda Awaiting requested review from jbeda

@lukemarsden lukemarsden Awaiting requested review from lukemarsden

@roberthbailey roberthbailey Awaiting requested review from roberthbailey

Assignees

@jbeda jbeda

@errordeveloper errordeveloper

@chenopis chenopis

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Milestone
1.7
Development

Successfully merging this pull request may close these issues.
5 participants
@luxas @chenopis @jbeda @errordeveloper @k8s-ci-robot