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.

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

kubeadm cri installation instructions #10186

Closed
wants to merge 1 commit into from

Conversation

vincepri
Copy link
Member

@vincepri vincepri commented Sep 4, 2018

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 4, 2018
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 4, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: zacharysarah

If they are not already assigned, you can assign the PR to them by writing /assign @zacharysarah in a comment when ready.

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

@k8sio-netlify-preview-bot
Copy link
Collaborator

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 7876861

https://deploy-preview-10186--kubernetes-io-master-staging.netlify.com

@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Sep 4, 2018

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 22ca1d1

https://deploy-preview-10186--kubernetes-io-master-staging.netlify.com

@timothysc
Copy link
Member

/cc @kubernetes/sig-cluster-lifecycle-pr-reviews @kubernetes/sig-node-pr-reviews

@k8s-ci-robot k8s-ci-robot requested a review from a team September 4, 2018 20:13
@k8s-ci-robot k8s-ci-robot added sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/node Categorizes an issue or PR as relevant to SIG Node. labels Sep 4, 2018
echo 1 > /proc/sys/net/bridge/bridge-nf-call-iptables
echo 1 > /proc/sys/net/bridge/bridge-nf-call-ip6tables
echo 1 > /proc/sys/net/ipv4/ip_forward
sysctl -p
Copy link
Member

Choose a reason for hiding this comment

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

Should we be telling users to modify /etc/sysctl.conf instead of using echo? Otherwise sysctl -p doesn't really have any effect here, and the changes will not persist through a reboot.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Maybe we can add a file to /etc/sysctl.d/? Something like 99-kubernetes-cri.conf should do the trick.

systemctl start containerd

# Setup kubelet to use containerd runtime endpoint and reload systemd.
cat > /etc/systemd/system/kubelet.service.d/20-cri.conf <<EOF
Copy link
Member

Choose a reason for hiding this comment

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

We really shouldn't be telling users to override the unit in this way, instead they should be passing this info in to the kubeadm config.

Copy link
Member Author

@vincepri vincepri Sep 4, 2018

Choose a reason for hiding this comment

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

@detiber Something like

nodeRegistration:
  kubeletExtraArgs:
    container-runtime-endpoint: unix:///var/run/containerd/containerd.sock

would work?

Copy link
Member

@neolit123 neolit123 left a comment

Choose a reason for hiding this comment

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

@vincepri
thanks a lot for the PR!
added some minor comments.

@@ -0,0 +1,144 @@
---
Copy link
Member

Choose a reason for hiding this comment

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

we should move this doc to somewhere else.
reference/setup-tools/kubeadm/ is the folder about the kubeadm CLI flags docs.
and move the kubeadm init content too.

my initial suggestion would be:
-> docs/setup/independent
if this guide is kubeadm related.

if not it has to go somewhere else.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. The goal is to have this as a general CRI installation document and reference it as needed. Where would you suggest to have it reside?

Copy link
Member

Choose a reason for hiding this comment

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

the structure of the k8s.io documentation is bit confusing.
we have both tasks and setup and the answer to the question is difficult.

i would add this under a new folder under docs/setup/ called cri (title: Kubernetes CRI):

the folder needs an _index.md as this one to set the title:
https://github.com/kubernetes/website/tree/master/content/en/docs/setup/independent

the name cri-installation.md as a file under that folder SGTM.

---
reviewers:
- vincepri
title: CRI Installation
Copy link
Member

Choose a reason for hiding this comment

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

please lowercase Installation i think we should have this consistent for other pages too.

@neolit123
Copy link
Member

thank you for the update @vincepri
let's see if others have more comments.

@vincepri
Copy link
Member Author

vincepri commented Sep 4, 2018

@neolit123 @detiber Thank you for the feedback! I addressed all the comments. Let me know if there is anything else.

Signed-off-by: Vince Prignano <vince@vincepri.com>
@neolit123
Copy link
Member

@vincepri
thanks for the update removing the docker instructions from the general kubeadm setup guide.
LGTM

@vincepri vincepri changed the title WIP: kubeadm cri installation instructions kubeadm cri installation instructions Sep 5, 2018
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 5, 2018

Refer to the [official Docker installation guides](https://docs.docker.com/engine/installation/)
for more information.
Refer to the [CRI installation](/docs/setup/cri/cri-installation/) guide for more information.
Copy link
Member

Choose a reason for hiding this comment

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

We will need to outline that Docker is still the default and if another CRI and outline the command line overrides in the kubeadm init / join below.

Copy link
Member

Choose a reason for hiding this comment

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

You might say something like:

By default, kubernetes is configured to work with docker (18.06). In order to enable other CRIs please consult https://kubernetes.io/docs/reference/setup-tools/kubeadm/kubeadm-init/ and https://kubernetes.io/docs/reference/setup-tools/kubeadm/kubeadm-join/ instructions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is the docker version relevant? In the CRI docs instructions we're still suggesting to install 17.03, should we update it? This is especially relevant for Ubuntu 18.04.

Copy link
Member

@neolit123 neolit123 Sep 6, 2018

Choose a reason for hiding this comment

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

so that's a good question.
17.03 is the last verified version by SIG node and as Tim mentioned yesterday they are not going to verify a newer version.

these docs here seem out of date:
https://docs.docker.com/release-notes/docker-ce/

i think this shows the truth:
https://github.com/docker/docker-ce/releases
18.06.1-ce - latest edge
18.03.1-ce - latest stable

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should include the information you just shared in the CRI document, under the Docker section. The install-kubeadm one can be left version-less?

Copy link
Member

Choose a reason for hiding this comment

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

it really depends of what version is tested with k8s, because we cannot recommend even a stable docker version if it's broken with k8s latest.
i will defer to others for more comments.

Copy link
Member Author

Choose a reason for hiding this comment

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

@timothysc thoughts/preferences on the above?

Copy link
Member

Choose a reason for hiding this comment

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

@vincepri
PSA, we are soon going to merge a PR that updates the max validated version of docker for kubeadm to 18.06:
kubernetes/kubernetes#68495

we need to reflect this in the docs as recommended version.
at least attempt to, given all the distro flavors that we have to support.

@timothysc
Copy link
Member

/assign @Bradamant3

Since v1.6.0, Kubernetes has enabled the use of CRI, Container Runtime Interface, by default.
The container runtime used by default is Docker, which is enabled through the built-in
`dockershim` CRI implementation inside of the `kubelet`.
From v1.12.0 the suggested kubeadm CRI is containerd. For further information refer to [CRI Installation](/docs/setup/cri/cri-installation/) instructions.
Copy link

Choose a reason for hiding this comment

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

Can you elaborate on this a bit? Why containerd is suggested kubeadm CRI? Why not CRI-O or both?

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe this was due to the lack of tests with docker, not totally sure about CRI-O. I'll defer to @timothysc.

Copy link
Member

Choose a reason for hiding this comment

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

@neolit123 once we have CI-Signal back we need to double check the versions.

Copy link
Member

Choose a reason for hiding this comment

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

my vote would be to not go with this statement for 1.12.0.
docker is currently the CRI for most of kubeadm users and also all of the kubeadm e2e here are docker too:
https://k8s-testgrid.appspot.com/sig-cluster-lifecycle-all

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @neolit123 here. Let's not promote switching to other runtimes just yet. Let's provide more information on how to set up CRI runtimes and how to use them. That should be enough for 1.12 I believe.

Copy link
Member

Choose a reason for hiding this comment

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

we are pending on decision for this.
i don't have strong opinions here.

the state of tests from sig-node for containerd are yellow-ish:
https://k8s-testgrid.appspot.com/sig-node-containerd

we do not have any tests for kubeadm and containerd yet, but our docker tests are passing at least.

Copy link
Member

Choose a reason for hiding this comment

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

[Service]
Environment="KUBELET_EXTRA_ARGS=--container-runtime=remote --container-runtime-endpoint=$RUNTIME_ENDPOINT"
EOF
systemctl daemon-reload
Copy link
Contributor

Choose a reason for hiding this comment

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

Instructions to update systemd config was quite useful. I'd suggest not to remove it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Given that this is now covered by kubeadm directly, I think it's safe to remove. The kubelet systemd example should live under kubelet docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Current documentation uses both ways of configuring kubelet - using configuration file and systemd drop-ins. In my opinion removing this can create confusion among users who prefer to use latter approach.

@@ -249,7 +249,7 @@ networking:
podSubnet: ""
serviceSubnet: 10.96.0.0/12
nodeRegistration:
criSocket: /var/run/dockershim.sock
criSocket: /var/run/containerd/containerd.sock
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd leave this as is as docker is still a default runtime.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, thanks!

@@ -368,41 +368,22 @@ Here's a breakdown of what/why:
certificates from the `kube-apiserver` when the certificate expiration approaches.
* `--cert-dir`the directory where the TLS certs are located.

### Use kubeadm with other CRI runtimes
### Use kubeadm with containerd
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd propose to leave this as it is and add setup instructions for at least containerd and CRI-O.

Copy link
Contributor

Choose a reason for hiding this comment

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

looked at rktlet and frakti. Not sure I understand why to remove them from this document.

Copy link
Member Author

Choose a reason for hiding this comment

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

We could add instructions for other runtimes with kubeadm to this page later on, does that sound good? We should definitely track these.

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 it would be better to leave links to other runtimes documentation even if we don't have our instructions for those. People at least would know that those runtimes exist and they can be also potentially used.

@timothysc
Copy link
Member

Let's get the CI signal back, double check versions and loop back to this once the mains are back online.

After installing containerd, you should set `--cri-socket` in kubeadm init and kubeadm reset. Or, in alternative to command line flags, supply the containerd socket in your kubeadm configuration as shown in the example below:
```yaml
nodeRegistration:
criSocket: /var/run/containerd/containerd.sock
Copy link
Contributor

Choose a reason for hiding this comment

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

I did a fresh pull of containerd and the socket path I get is /run/containerd/containerd.sock

@bart0sh
Copy link
Contributor

bart0sh commented Sep 14, 2018

@vincepri Here is an example of what I meant: #10299

The idea is to provide instructions for the CRI runtimes that we can create without changing current documentation too much.

@ALL Does this make sense?

@vincepri
Copy link
Member Author

Closing in favor of #10299

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/node Categorizes an issue or PR as relevant to SIG Node. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet