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

Removing dockershim from kubelet #1985

Merged
merged 1 commit into from
Oct 6, 2020
Merged

Removing dockershim from kubelet #1985

merged 1 commit into from
Oct 6, 2020

Conversation

dims
Copy link
Member

@dims dims commented Sep 14, 2020

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

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 14, 2020
@k8s-ci-robot k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/node Categorizes an issue or PR as relevant to SIG Node. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 14, 2020
@dims dims force-pushed the remove-dockershim-from-kubelet branch from 1299e13 to f37c573 Compare September 14, 2020 22:43
@dims dims changed the title Removing dockershim from kubelet [WIP] Removing dockershim from kubelet Sep 14, 2020
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 14, 2020
@dims
Copy link
Member Author

dims commented Sep 14, 2020

/hold for review

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 14, 2020
@dims dims force-pushed the remove-dockershim-from-kubelet branch from f37c573 to 5c76548 Compare September 14, 2020 22:51
@dims dims mentioned this pull request Sep 14, 2020
Merged
odinuge
odinuge reviewed Sep 15, 2020
View reviewed changes
Copy link
Member

@odinuge odinuge left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @dims (and @mattjmcnaughton)! I agree on the overall idea as discussed previously, and as a sig-node contributor it would also make "my life" a lot easier. It would also help making the codebase cleaner and easier to work with. 😄

Still need to read through and think about the details, but overall this looks good!

keps/sig-node/1985-remove-dockershim/README.md Outdated Show resolved Hide resolved
liggitt
liggitt reviewed Sep 15, 2020
View reviewed changes
keps/sig-node/1985-remove-dockershim/README.md Outdated
- flags that are used to configure dockershim.
- support to get container logs when docker uses journald as the driver.
- logic of moving docker processes to a given cgroup
- TBD anything else?
Copy link
Member

Choose a reason for hiding this comment

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

note: complete before merge

Copy link
Member Author

Choose a reason for hiding this comment

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

Dropping this.

@mattjmcnaughton
Copy link
Contributor

Thanks for your thought and leadership on this project @dims and @derekwaynecarr :)

@karan
Copy link

karan commented Sep 21, 2020

/cc @SergeyKanzhelev

@saschagrunert saschagrunert mentioned this pull request Sep 25, 2020
Closed
10 tasks
@dims dims changed the title [WIP] Removing dockershim from kubelet Removing dockershim from kubelet Oct 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 Oct 2, 2020
@dims dims force-pushed the remove-dockershim-from-kubelet branch from 5c76548 to 1ab0605 Compare October 2, 2020 14:50
@dims
Removing dockershim from kubelet
2b5ccee 
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
@dims dims force-pushed the remove-dockershim-from-kubelet branch from 1ab0605 to 2b5ccee Compare October 6, 2020 16:58
@dchen1107
Copy link
Member

/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 Oct 6, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dchen1107, dims

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

@dims
Copy link
Member Author

dims commented Oct 6, 2020

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 6, 2020
@k8s-ci-robot k8s-ci-robot merged commit 7ed98cd into kubernetes:master Oct 6, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.20 milestone Oct 6, 2020
@rata rata mentioned this pull request Oct 8, 2020
Open
@mikebrow mikebrow mentioned this pull request Oct 19, 2020
Closed
@neolit123 neolit123 mentioned this pull request Dec 2, 2020
Closed
@jeremyrickard jeremyrickard mentioned this pull request Dec 3, 2020
Open
krmayankk
krmayankk reviewed Dec 10, 2020
View reviewed changes
keps/sig-node/1985-remove-dockershim/README.md
Target releases: 1.22

Actions:
- Document and announce migration guide.
Copy link

Choose a reason for hiding this comment

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

is there a tracking issue for this migration guide, i think it would help to include in it what steps do existing users take for moving from docker to containerd running on there hosts ?

Copy link
Member

Choose a reason for hiding this comment

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

I started this: kubernetes/website#25787

This was referenced Dec 29, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krmayankk krmayankk krmayankk left review comments

@mauriciovasquezbernal mauriciovasquezbernal mauriciovasquezbernal left review comments

@dchen1107 dchen1107 dchen1107 left review comments

@odinuge odinuge odinuge left review comments

@liggitt liggitt liggitt left review comments

@BenTheElder BenTheElder BenTheElder left review comments

@neolit123 neolit123 neolit123 left review comments

@danwinship danwinship danwinship left review comments

@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr

@SergeyKanzhelev SergeyKanzhelev Awaiting requested review from SergeyKanzhelev

Assignees

@dchen1107 dchen1107

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/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. 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
v1.20
Development

Successfully merging this pull request may close these issues.