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

Support using docker containerd in COS and Ubuntu on GCE. #77889

Merged
merged 1 commit into from
May 18, 2019
Merged

Support using docker containerd in COS and Ubuntu on GCE. #77889

merged 1 commit into from
May 18, 2019

Conversation

Random-Liu
Copy link
Member

@Random-Liu Random-Liu commented May 15, 2019 •
edited

This PR:

  • Update COS to cos-73-11647-163-0 (a stable version);
  • Support using the containerd shipped as part of Docker in COS and Ubuntu on GCE.

To use it, just run:

export KUBE_CONTAINER_RUNTIME=containerd
cluster/kube-up.sh

/cc @dchen1107 @yujuhong @yguo0905
Signed-off-by: Lantao Liu lantaol@google.com

Support starting Kubernetes on GCE using containerd in COS and Ubuntu with `KUBE_CONTAINER_RUNTIME=containerd`.

@Random-Liu Random-Liu added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/node Categorizes an issue or PR as relevant to SIG Node. kind/feature Categorizes issue or PR as related to a new feature. labels May 15, 2019
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 15, 2019
@k8s-ci-robot k8s-ci-robot added the sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. label May 15, 2019
@Random-Liu
Copy link
Member Author

/retest

@Random-Liu Random-Liu force-pushed the support-using-containerd-in-cos branch from 48ca59c to 328560d Compare May 15, 2019 00:28
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 15, 2019
yujuhong
yujuhong reviewed May 15, 2019
View reviewed changes
cluster/gce/config-default.sh Outdated
CONTAINER_RUNTIME_NAME=${KUBE_CONTAINER_RUNTIME_NAME:-containerd}
CONTAINER_RUNTIME_ENDPOINT=${KUBE_CONTAINER_RUNTIME_ENDPOINT:-unix:///run/containerd/containerd.sock}
LOAD_IMAGE_COMMAND=${KUBE_LOAD_IMAGE_COMMAND:-ctr -n=k8s.io images import}
KUBELET_TEST_ARGS="${KUBE_KUBELET_EXTRA_ARGS:-} --runtime-cgroups=/system.slice/containerd.service"
Copy link
Contributor

Choose a reason for hiding this comment

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

How about appending to KUBELET_TEST_ARGS directly? That way if line 97 changes, this wouldn't break

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

cluster/gce/config-default.sh
@@ -95,6 +95,12 @@ MASTER_EXTRA_METADATA=${KUBE_MASTER_EXTRA_METADATA:-${KUBE_EXTRA_METADATA:-}}
NODE_EXTRA_METADATA=${KUBE_NODE_EXTRA_METADATA:-${KUBE_EXTRA_METADATA:-}}
# KUBELET_TEST_ARGS are extra arguments passed to kubelet.
KUBELET_TEST_ARGS=${KUBE_KUBELET_EXTRA_ARGS:-}
if [[ "${CONTAINER_RUNTIME}" == "containerd" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm torn between leaving this as it is, or grouping the block with the docker settings above.

Copy link
Member Author

Choose a reason for hiding this comment

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

Like this?

cluster/gce/util.sh Outdated
@@ -782,8 +782,8 @@ function construct-linux-kubelet-flags {
if [[ -n "${NODE_TAINTS:-}" ]]; then
flags+=" --register-with-taints=${NODE_TAINTS}"
fi
if [[ -n "${CONTAINER_RUNTIME:-}" ]]; then
flags+=" --container-runtime=${CONTAINER_RUNTIME}"
if [[ "${CONTAINER_RUNTIME:-docker}" != "docker" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we default CONTAINER_RUNTIME to docker anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

cluster/gce/gci/configure-helper.sh Outdated
assemble-docker-flags
elif [[ "${container_runtime}" == "containerd" ]]; then
if [[ -e "${KUBE_HOME}/bin/gke-internal-configure-helper.sh" ]]; then
gke-setup-containerd
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm....so we still keep an internal copy of setup function.....is that necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed

@Random-Liu
Support using docker containerd in COS and Ubuntu on GCE.
bc1a78d 
Signed-off-by: Lantao Liu <lantaol@google.com>
@Random-Liu Random-Liu force-pushed the support-using-containerd-in-cos branch from 328560d to bc1a78d Compare May 16, 2019 20:54
@Random-Liu
Copy link
Member Author

/test pull-kubernetes-e2e-gce-device-plugin-gpu

@yujuhong
Copy link
Contributor

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Random-Liu, yujuhong

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 May 17, 2019
@Random-Liu
Copy link
Member Author

/test pull-kubernetes-e2e-gce-100-performance

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

@k8s-ci-robot k8s-ci-robot merged commit 72f6954 into kubernetes:master May 18, 2019
This was referenced May 19, 2019
Closed
Merged
@Random-Liu Random-Liu deleted the support-using-containerd-in-cos branch May 20, 2019 22:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yujuhong yujuhong yujuhong left review comments

@jszczepkowski jszczepkowski Awaiting requested review from jszczepkowski

@MaciekPytel MaciekPytel Awaiting requested review from MaciekPytel

Assignees

@yujuhong yujuhong

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. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Random-Liu @yujuhong @k8s-ci-robot @fejta-bot