Skip to content

Shim v1: Check task list to avoid unnecessary cleanup.#3209

Merged
estesp merged 1 commit intocontainerd:masterfrom
Random-Liu:fix-v1-shim-cleanup
Apr 16, 2019
Merged

Shim v1: Check task list to avoid unnecessary cleanup.#3209
estesp merged 1 commit intocontainerd:masterfrom
Random-Liu:fix-v1-shim-cleanup

Conversation

@Random-Liu
Copy link
Copy Markdown
Member

@Random-Liu Random-Liu commented Apr 11, 2019 •
edited
Loading

When using v1 shim, if we:

  1. Create a container;
  2. Restart containerd;
  3. Stop the container.

There will always be 2 TaskExit events, and an annoying failed to delete bundle cleanup error:

INFO[2019-04-11T16:24:48.724104626-07:00] StopPodSandbox for "a"                       
DEBU[2019-04-11T16:24:48.866783282-07:00] event published                               ns=k8s.io topic=/tasks/exit type=containerd.events.TaskExit
DEBU[2019-04-11T16:24:48.866904411-07:00] Received containerd event timestamp - 2019-04-11 23:24:48.866764485 +0000 UTC, namespace - "k8s.io", topic - "/tasks/exit" 
INFO[2019-04-11T16:24:48.867071445-07:00] TaskExit event &TaskExit{ContainerID:ac959381b7c00ea872d3e09030e4157cbd81c5205a8d3060069b5e21a40f57d9,ID:ac959381b7c00ea872d3e09030e4157cbd81c5205a8d3060069b5e21a40f57d9,Pid:38654,ExitStatus:137,ExitedAt:2019-04-11 23:24:48.84406586 +0000 UTC,XXX_unrecognized:[],} 
DEBU[2019-04-11T16:24:48.946495221-07:00] event published                               ns=k8s.io topic=/tasks/exit type=containerd.events.TaskExit
DEBU[2019-04-11T16:24:48.947075480-07:00] event published                               ns=k8s.io topic=/tasks/delete type=containerd.events.TaskDelete
ERRO[2019-04-11T16:24:48.948457505-07:00] failed to delete bundle                       error="Failed to remove both bundle and workdir locations: rename /var/lib/containerd/io.containerd.runtime.v1.linux/k8s.io/ac959381b7c00ea872d3e09030e4157cbd81c5205a8d3060069b5e21a40f57d9 /var/lib/containerd/io.containerd.runtime.v1.linux/k8s.io/.ac959381b7c00ea872d3e09030e4157cbd81c5205a8d3060069b5e21a40f57d9: no such file or directory: rename /run/containerd/io.containerd.runtime.v1.linux/k8s.io/ac959381b7c00ea872d3e09030e4157cbd81c5205a8d3060069b5e21a40f57d9 /run/containerd/io.containerd.runtime.v1.linux/k8s.io/.ac959381b7c00ea872d3e09030e4157cbd81c5205a8d3060069b5e21a40f57d9: no such file or directory"
DEBU[2019-04-11T16:24:48.948534957-07:00] event published                               ns=k8s.io topic=/tasks/delete type=containerd.events.TaskDelete
DEBU[2019-04-11T16:24:48.948569968-07:00] Received containerd event timestamp - 2019-04-11 23:24:48.946474445 +0000 UTC, namespace - "k8s.io", topic - "/tasks/exit" 
INFO[2019-04-11T16:24:48.948609563-07:00] TaskExit event &TaskExit{ContainerID:ac959381b7c00ea872d3e09030e4157cbd81c5205a8d3060069b5e21a40f57d9,ID:ac959381b7c00ea872d3e09030e4157cbd81c5205a8d3060069b5e21a40f57d9,Pid:38654,ExitStatus:137,ExitedAt:2019-04-11 23:24:48.946442088 +0000 UTC,XXX_unrecognized:[],} 
INFO[2019-04-11T16:24:49.083226672-07:00] TearDown network for sandbox "ac959381b7c00ea872d3e09030e4157cbd81c5205a8d3060069b5e21a40f57d9" successfully 
INFO[2019-04-11T16:24:49.083301140-07:00] StopPodSandbox for "a" returns successfully

This is because the OnClose function we installed after containerd restarts always do cleanup when the container dies. But the cleanup is unnecessary when the container is stopped by client.

Signed-off-by: Lantao Liu [email protected]

@codecov-io
Copy link
Copy Markdown

codecov-io commented Apr 11, 2019

Codecov Report

Merging #3209 into master will decrease coverage by 4.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3209      +/-   ##
==========================================
- Coverage   48.81%   44.76%   -4.05%     
==========================================
  Files         102      113      +11     
  Lines        9538    12125    +2587     
==========================================
+ Hits         4656     5428     +772     
- Misses       4056     5862    +1806     
- Partials      826      835       +9
Flag Coverage Δ
#linux 48.81% <ø> (ø) ⬆️
#windows 40.01% <ø> (?)
Impacted Files Coverage Δ
snapshots/native/native.go 43.04% <0%> (-9.99%) ⬇️
metadata/snapshot.go 45.8% <0%> (-8.96%) ⬇️
archive/tar.go 43.79% <0%> (-7.07%) ⬇️
metadata/containers.go 47.97% <0%> (-6.62%) ⬇️
content/local/writer.go 57.84% <0%> (-6.36%) ⬇️
content/local/store.go 49.51% <0%> (-5.15%) ⬇️
metadata/images.go 57.57% <0%> (-4.99%) ⬇️
archive/tar_opts.go 28.57% <0%> (-4.77%) ⬇️
archive/compression/compression.go 58.69% <0%> (-4.7%) ⬇️
metadata/buckets.go 56.33% <0%> (-4.6%) ⬇️
... and 63 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5f4c977...9cc5878. Read the comment docs.

@crosbymichael
Copy link
Copy Markdown
Member

crosbymichael commented Apr 16, 2019

LGTM

estesp
estesp approved these changes Apr 16, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 4c16017 into containerd:master Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@estesp estesp estesp approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Random-Liu @codecov-io @crosbymichael @estesp