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

Adding containerd support to kubeadm related scripts/specs/etc #112

Merged
merged 1 commit into from Nov 10, 2020

Conversation

marosset
Copy link
Contributor

@marosset marosset commented Oct 26, 2020

Adding support for configuring a cluster that uses containerd to the existing kubeadm implementionstations.

Verify the following:

  • Add docker node to a cluster using overlay networking
  • Add containerd node to a cluster using overlay networking
  • Add docker node to a cluster using bridge/host-gw networking
  • Add containerd node to a cluster using bridge/host-gw networking

Notes:
Install-containerd.ps1 needs to be downloaded and run before running PrepareNode.ps1.
This assumes a completely vanilla Windows installation and may need to enable Windows features which will require a reboot.

After running Install-Containerd.ps1 to completion run PrepaseNode.ps1 -ContainerRuntime containerd.

Kube-proxy/CNI deployment should be the same as with docker.

All of these will be captured in updates to kubernetes/website updates to https://kubernetes.io/docs/tasks/administer-cluster/kubeadm/adding-windows-nodes/

@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 Oct 26, 2020
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 26, 2020
Copy link
Contributor

@michmike michmike left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: marosset, michmike

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 lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 27, 2020
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 28, 2020
@jsturtevant jsturtevant self-assigned this Oct 28, 2020
@marosset
Copy link
Contributor Author

marosset commented Nov 2, 2020

/assign @benmoss @jsturtevant

I think this is ready for review.

@marosset marosset changed the title WIP - Adding containerd support to kubeadm related scripts/specs/etc Adding containerd support to kubeadm related scripts/specs/etc Nov 2, 2020
@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 Nov 2, 2020
@marosset
Copy link
Contributor Author

marosset commented Nov 3, 2020

I've started some doc updates for using these changes kubernetes/website#24862

Related to kubernetes/enhancements#1001

Copy link
Contributor

@jsturtevant jsturtevant left a comment

Choose a reason for hiding this comment

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

Left a few comments.

kubeadm/scripts/PrepareNode.ps1 Outdated Show resolved Hide resolved
kubeadm/scripts/PrepareNode.ps1 Show resolved Hide resolved
kubeadm/scripts/Install-Containerd.ps1 Outdated Show resolved Hide resolved
kubeadm/scripts/PrepareNode.ps1 Show resolved Hide resolved
kubeadm/scripts/Install-Containerd.ps1 Show resolved Hide resolved
kubeadm/flannel/flannel-overlay.yml Show resolved Hide resolved
kubeadm/flannel/flannel-host-gw.yml Show resolved Hide resolved
kubeadm/flannel/flannel-overlay.yml Show resolved Hide resolved
Copy link

@daschott daschott left a comment

Choose a reason for hiding this comment

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

I left a few notes/open questions on the CNI configs

kubeadm/flannel/flannel-overlay.yml Show resolved Hide resolved
kubeadm/flannel/flannel-host-gw.yml Show resolved Hide resolved
kubeadm/flannel/flannel-host-gw.yml Show resolved Hide resolved
@jsturtevant
Copy link
Contributor

Over all looks good. Had one question on the PA address section

detect docker/containerd via named pipes

PR feedback from David

PR feedback
@jsturtevant
Copy link
Contributor

I was able to join a node:

kubectl get nodes -o wide
NAME           STATUS   ROLES    AGE   VERSION   INTERNAL-IP   EXTERNAL-IP   OS-IMAGE                         KERNEL-VERSION     CONTAINER-RUNTIME
controller-0   Ready    master   19h   v1.19.3   10.240.0.10   <none>        Ubuntu 18.04.4 LTS               5.3.0-1028-azure   docker://19.3.13
worker-win-2   Ready    <none>   17h   v1.19.3   10.240.0.22   <none>        Windows Server 2019 Datacenter   10.0.17763.1397    containerd://1.4.1

@jsturtevant
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 10, 2020
@k8s-ci-robot k8s-ci-robot merged commit afc5781 into kubernetes-sigs:master Nov 10, 2020
@marosset marosset deleted the kubeadm-containerd branch May 19, 2023 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. 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

8 participants