Skip to content

Completely remove in-tree dockershim from kubelet#97252

Merged
k8s-ci-robot merged 3 commits intokubernetes:masterfrom
dims:drop-dockershim
Dec 8, 2021
Merged

Completely remove in-tree dockershim from kubelet#97252
k8s-ci-robot merged 3 commits intokubernetes:masterfrom
dims:drop-dockershim

Conversation

@dims
Copy link
Member

@dims dims commented Dec 11, 2020 •
edited
Loading

Related to https://git.k8s.io/enhancements/keps/sig-node/2221-remove-dockershim

This has been pending for a really long time, dockershim has been a drag on sig-node and a huge technical debt. Please see KEP above for relevant details. There are activities in progress to alleviate adoption issues in terms of documentation / surveys / alternate implementations etc. For this PR, we just remove the code that has been deprecated a long time ago in #94624

Signed-off-by: Davanum Srinivas davanum@gmail.com

What type of PR is this?
/kind cleanup
/kind deprecation

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Action Required: Docker runtime support using dockshim in the kubelet is now completely removed in 1.24. The kubelet used to have a a module called "dockershim" which implements CRI support for Docker and it has seen maintenance issues in the Kubernetes community. From 1.24 onwards, please move to a container runtime that is a full-fledged implementation of CRI (v1alpha1 or v1 compliant) as they become available.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@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. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-kind Indicates a PR lacks a `kind/foo` label and requires one. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Dec 11, 2020
@k8s-ci-robot k8s-ci-robot added area/dependency Issues or PRs related to dependency changes area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework area/kubelet area/test sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Dec 11, 2020
@dims
Copy link
Member Author

dims commented Dec 11, 2020

/test pull-kubernetes-node-e2e-containerd

@danwinship
Copy link
Contributor

danwinship commented Dec 12, 2020

/uncc

@k8s-ci-robot k8s-ci-robot removed the request for review from danwinship December 12, 2020 16:08
@dims dims force-pushed the drop-dockershim branch 2 times, most recently from c8dd372 to 2fd754e Compare December 13, 2020 19:02
@dims dims removed the request for review from fabriziopandini December 13, 2020 19:04
@dims
Copy link
Member Author

dims commented Dec 13, 2020

/test pull-kubernetes-node-e2e-containerd

1 similar comment
@dims
Copy link
Member Author

dims commented Dec 13, 2020

/test pull-kubernetes-node-e2e-containerd

@dims dims mentioned this pull request Dec 15, 2020
Closed
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 8, 2021
ehashman
ehashman reviewed Dec 8, 2021
View reviewed changes
Copy link
Member

@ehashman ehashman left a comment

Choose a reason for hiding this comment

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

good night, sweet prince 🐳

@ehashman
Copy link
Member

ehashman commented Dec 8, 2021

/lgtm
/hold

/assign @SergeyKanzhelev

@ehashman
Copy link
Member

ehashman commented Dec 8, 2021

/triage accepted

@SergeyKanzhelev
Copy link
Member

SergeyKanzhelev commented Dec 8, 2021

/hold cancel
/lgtm

@aojea
Copy link
Member

aojea commented Dec 8, 2021

/honk

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Dec 8, 2021

@aojea:
goose image

Details

In response to this:

/honk

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@neolit123 neolit123 mentioned this pull request Dec 8, 2021
Closed
@endocrimes
Copy link
Member

endocrimes commented Dec 8, 2021

/lgtm

:shipit: 🎉

@sftim sftim mentioned this pull request Dec 8, 2021
Closed
neolit123
neolit123 reviewed Dec 8, 2021
View reviewed changes
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.

should these flags be removed?

https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/config/flags.go#L92-L112

but note that pod-infra-container-image is definitely not a dockershim only flag.
currently it is used for preventing the kubelet from GC-ing the pause image.

i cannot comment on the other ones, but all must be verified if "dockershim only".

@endocrimes
Copy link
Member

endocrimes commented Dec 8, 2021

@neolit123 Filled an issue for cleanup, we should definitely drop the unused ones (and update the other for now) #106893

@jsturtevant jsturtevant mentioned this pull request Dec 8, 2021
Merged
@dims
Copy link
Member Author

dims commented Dec 8, 2021

image

@mrunalp mrunalp mentioned this pull request Dec 8, 2021
Merged
@BenTheElder BenTheElder mentioned this pull request Dec 9, 2021
Closed
@jsturtevant jsturtevant mentioned this pull request Dec 9, 2021
Merged
@aojea aojea mentioned this pull request Dec 9, 2021
Closed
@saschagrunert saschagrunert mentioned this pull request Dec 10, 2021
Merged
@Rajalakshmi-Girish Rajalakshmi-Girish mentioned this pull request Dec 13, 2021
Merged
@ionutbalutoiu ionutbalutoiu mentioned this pull request Dec 13, 2021
Merged
@reylejano reylejano mentioned this pull request Dec 16, 2021
Merged
@aojea aojea mentioned this pull request Dec 23, 2021
Merged
This was referenced Jan 4, 2022
Closed
Merged
Merged
@yanghesong yanghesong mentioned this pull request Jan 13, 2022
Closed
@gracenng gracenng mentioned this pull request Mar 21, 2022
Closed
12 tasks
@dims dims mentioned this pull request Nov 17, 2022
Closed
@zouyee zouyee mentioned this pull request Oct 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@liggitt liggitt liggitt left review comments

+2 more reviewers

@neolit123 neolit123 neolit123 left review comments

@ehashman ehashman ehashman left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@mrunalp mrunalp

@endocrimes endocrimes

@ehashman ehashman

@SergeyKanzhelev SergeyKanzhelev

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/cloudprovider area/dependency Issues or PRs related to dependency changes area/e2e-test-framework Issues or PRs related to refactoring the kubernetes e2e test framework area/kubelet area/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. 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-action-required Denotes a PR that introduces potentially breaking changes that require user action. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Projects

SIG Node CI/Test Board
Archived in project
SIG Node: code and documentation PRs
Archived in project

Milestone

v1.24

Development

Successfully merging this pull request may close these issues.

10 participants

@dims @danwinship @aojea @k8s-ci-robot @mrunalp @liggitt @ehashman @SergeyKanzhelev @endocrimes @neolit123