Skip to content

cri: close fifos when container is deleted#6842

Merged
estesp merged 1 commit intocontainerd:mainfrom
henry118:fifo
Apr 22, 2022
Merged

cri: close fifos when container is deleted#6842
estesp merged 1 commit intocontainerd:mainfrom
henry118:fifo

Conversation

@henry118
Copy link
Copy Markdown
Member

@henry118 henry118 commented Apr 21, 2022

cri: close fifos when container is deleted

Fixes: #6834

Testing 

$ sudo critest -ginkgo.focus 'runtime should support removing created container'
critest version: 1.20.0-24-g53ad8bb7
Running Suite: CRI validation
=============================
Random Seed: 1650576768 - Will randomize all specs
Will run 1 of 97 specs

SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
------------------------------
[k8s.io] Container runtime should support basic operations on container
  runtime should support removing created container [Conformance]
  /tmp/tmp.KuBJ4DNF8R/cri-tools/pkg/validate/container.go:111
[BeforeEach] [k8s.io] Container
  /tmp/tmp.KuBJ4DNF8R/cri-tools/pkg/framework/framework.go:50
[BeforeEach] [k8s.io] Container
  /tmp/tmp.KuBJ4DNF8R/cri-tools/pkg/validate/container.go:63
[BeforeEach] runtime should support basic operations on container
  /tmp/tmp.KuBJ4DNF8R/cri-tools/pkg/validate/container.go:72
[It] runtime should support removing created container [Conformance]
  /tmp/tmp.KuBJ4DNF8R/cri-tools/pkg/validate/container.go:111
STEP: create container
STEP: Get image status for image: busybox:1.28
STEP: Create container.
Apr 21 21:32:50.839: INFO: Created container "40b4cb3869d7b83140ce357fb9a7abd9985c070ee90dc2f7797aaac380131264"

STEP: test remove container
STEP: Remove container for containerID: 40b4cb3869d7b83140ce357fb9a7abd9985c070ee90dc2f7797aaac380131264
Apr 21 21:32:50.842: INFO: Removed container "40b4cb3869d7b83140ce357fb9a7abd9985c070ee90dc2f7797aaac380131264"

STEP: List containers for containerID: 40b4cb3869d7b83140ce357fb9a7abd9985c070ee90dc2f7797aaac380131264
[AfterEach] runtime should support basic operations on container
  /tmp/tmp.KuBJ4DNF8R/cri-tools/pkg/validate/container.go:76
STEP: stop PodSandbox
STEP: delete PodSandbox
[AfterEach] [k8s.io] Container
  /tmp/tmp.KuBJ4DNF8R/cri-tools/pkg/framework/framework.go:51
•SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
Ran 1 of 97 Specs in 2.075 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 96 Skipped
PASS

$ sudo lsof -p $(pidof containerd)
COMMAND     PID USER   FD      TYPE             DEVICE SIZE/OFF      NODE NAME
container 10814 root  cwd       DIR              259,1     4096 247464070 /home/ec2-user/go/src/github.com/henry118/containerd
container 10814 root  rtd       DIR              259,1      237        96 /
container 10814 root  txt       REG              259,1 59427040   8411081 /usr/bin/containerd
container 10814 root  mem-W     REG              259,1   262144 163620864 /var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/metadata.db
container 10814 root  mem-W     REG              259,1   753664 172023349 /var/lib/containerd/io.containerd.metadata.v1.bolt/meta.db
container 10814 root  mem       REG              259,1  2021864  12584057 /usr/lib64/libc-2.26.so
container 10814 root  mem       REG              259,1   149416  12584075 /usr/lib64/libpthread-2.26.so
container 10814 root  mem       REG              259,1    19208  12584061 /usr/lib64/libdl-2.26.so
container 10814 root  mem       REG              259,1   174280  12584050 /usr/lib64/ld-2.26.so
container 10814 root    0u      CHR              136,2      0t0         5 /dev/pts/2
container 10814 root    1u      CHR              136,2      0t0         5 /dev/pts/2
container 10814 root    2u      CHR              136,2      0t0         5 /dev/pts/2
container 10814 root    3uW     REG              259,1   753664 172023349 /var/lib/containerd/io.containerd.metadata.v1.bolt/meta.db
container 10814 root    4u  a_inode               0,12        0     13869 [eventpoll]
container 10814 root    5r     FIFO               0,11      0t0    380290 pipe
container 10814 root    6w     FIFO               0,11      0t0    380290 pipe
container 10814 root    7u  a_inode               0,12        0     13869 [eventpoll]
container 10814 root    8r  a_inode               0,12        0     13869 inotify
container 10814 root    9u  a_inode               0,12        0     13869 [eventpoll]
container 10814 root   10r     FIFO               0,11      0t0    378687 pipe
container 10814 root   11w     FIFO               0,11      0t0    378687 pipe
container 10814 root   12u     unix 0xffff9fcc9c00dc00      0t0    378688 /run/containerd/containerd.sock.ttrpc
container 10814 root   13u     unix 0xffff9fcc9c00e400      0t0    378689 /run/containerd/containerd.sock
container 10814 root   14u     IPv4             378690      0t0       TCP localhost:41933 (LISTEN)
container 10814 root   22uW     REG              259,1   262144 163620864 /var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/metadata.db

Signed-off-by: Henry Wang [email protected]

@k8s-ci-robot
Copy link
Copy Markdown

k8s-ci-robot commented Apr 21, 2022

Hi @henry118. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci Bot commented Apr 21, 2022

Build succeeded.

fuweid
fuweid approved these changes Apr 22, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

Thanks!

estesp
estesp approved these changes Apr 22, 2022
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 6111b5e into containerd:main Apr 22, 2022
@henry118 henry118 deleted the fifo branch April 22, 2022 16:09
This was referenced Apr 26, 2022
Merged
Merged
@fuweid fuweid added cherry-picked/1.5.x PR commits are cherry-picked into release/1.5 branch cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch and removed cherry-pick/1.5.x labels Apr 27, 2022
@thaJeztah thaJeztah mentioned this pull request Jul 12, 2022
Merged
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

@fuweid fuweid fuweid approved these changes

Assignees

No one assigned

Labels

cherry-picked/1.5.x PR commits are cherry-picked into release/1.5 branch cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch needs-ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[CRI] container IO will be leaky if removing a created container

4 participants

@henry118 @k8s-ci-robot @estesp @fuweid