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.

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

bugfix: mount /sys/fs/cgroup into container #1314

Merged
merged 1 commit into from
May 14, 2018
Merged

bugfix: mount /sys/fs/cgroup into container #1314

merged 1 commit into from
May 14, 2018

Conversation

fuweid
Copy link
Contributor

@fuweid fuweid commented May 14, 2018

Signed-off-by: Wei Fu fhfuwei@163.com

Ⅰ. Describe what this PR did

Mount /sys/fs/cgroup into the container.

Ⅱ. Does this pull request fix one issue?

#1247

Ⅲ. Describe how you did it

Use oci.Spec and containerd will help us to do mount.

Ⅳ. Describe how to verify it

➜  pouch git:(bugfix_add_mount_spec_in_oci) pouch run -it registry.hub.docker.com/library/busybox:1.28 sh
/ # ls -al /sys/fs/cgroup/
total 0
dr-xr-xr-x   13 root     root           340 May 14 09:16 .
drwxr-xr-x    9 root     root             0 May 14 08:20 ..
drwxr-xr-x    2 root     root             0 May 14 09:16 blkio
lrwxrwxrwx    1 root     root            11 May 14 09:16 cpu -> cpu,cpuacct
drwxr-xr-x    2 root     root             0 May 14 09:16 cpu,cpuacct
lrwxrwxrwx    1 root     root            11 May 14 09:16 cpuacct -> cpu,cpuacct
drwxr-xr-x    2 root     root             0 May 14 09:16 cpuset
drwxr-xr-x    2 root     root             0 May 14 09:16 devices
drwxr-xr-x    2 root     root             0 May 14 09:16 freezer
drwxr-xr-x    2 root     root             0 May 14 09:16 hugetlb
drwxr-xr-x    2 root     root             0 May 14 09:16 memory
lrwxrwxrwx    1 root     root            16 May 14 09:16 net_cls -> net_cls,net_prio
drwxr-xr-x    2 root     root             0 May 14 09:16 net_cls,net_prio
lrwxrwxrwx    1 root     root            16 May 14 09:16 net_prio -> net_cls,net_prio
drwxr-xr-x    2 root     root             0 May 14 09:16 perf_event
drwxr-xr-x    2 root     root             0 May 14 09:16 pids
drwxr-xr-x    2 root     root             0 May 14 09:16 systemd

Ⅴ. Special notes for reviews

@fuweid fuweid requested review from allencloud and Ace-Tang May 14, 2018 09:17
@pouchrobot pouchrobot added areas/test kind/bug This is bug report for project size/M labels May 14, 2018
//
// TODO: The github.com/containerd/containerd/oci provides oci.WithMounts
// in v1.1.0. Before we upgrade it, we use assign way to handle the case.
container.Spec.Mounts = newOCILinuxMounts()
Copy link
Contributor

Choose a reason for hiding this comment

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

@fuweid , can you put these logic into file like daemon/mgr/spec_mount.go, since this is oci spec related ?

@fuweid fuweid changed the title bugfix: mount host's dir into container by oci.Specs [WIP] bugfix: mount host's dir into container by oci.Specs May 14, 2018
Signed-off-by: Wei Fu <fhfuwei@163.com>
@pouchrobot pouchrobot added size/S and removed size/M labels May 14, 2018
@fuweid fuweid changed the title [WIP] bugfix: mount host's dir into container by oci.Specs [WIP] bugfix: mount /sys/fs/cgroup into container May 14, 2018
@fuweid fuweid changed the title [WIP] bugfix: mount /sys/fs/cgroup into container bugfix: mount /sys/fs/cgroup into container May 14, 2018
@fuweid
Copy link
Contributor Author

fuweid commented May 14, 2018

@Ace-Tang I have updated and Please take a look. Thanks

@codecov-io
Copy link

Codecov Report

Merging #1314 into master will decrease coverage by 0.99%.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #1314    +/-   ##
========================================
- Coverage    17.4%   16.41%    -1%     
========================================
  Files         189      183     -6     
  Lines       11767    11314   -453     
========================================
- Hits         2048     1857   -191     
+ Misses       9572     9321   -251     
+ Partials      147      136    -11
Impacted Files Coverage Δ
ctrd/container.go 0% <ø> (ø) ⬆️
daemon/mgr/spec_mount.go 0% <0%> (ø) ⬆️
pkg/utils/utils.go 69.79% <0%> (-2.6%) ⬇️
apis/server/container_bridge.go 0% <0%> (ø) ⬆️
daemon/mgr/container.go 0% <0%> (ø) ⬆️
apis/server/router.go 0% <0%> (ø) ⬆️
daemon/mgr/container_logs.go
daemon/logger/jsonfile/utils.go
daemon/containerio/jsonfile.go
daemon/logger/jsonfile/jsonfile_read.go
... and 4 more

@Ace-Tang
Copy link
Contributor

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label May 14, 2018
@Ace-Tang Ace-Tang merged commit de46ffa into AliyunContainerService:master May 14, 2018
@allencloud
Copy link
Collaborator

I think we could add an integration test case for this feature, right? @fuweid

@fuweid fuweid deleted the bugfix_add_mount_spec_in_oci branch May 14, 2018 13:52
@fuweid
Copy link
Contributor Author

fuweid commented May 14, 2018

@allencloud , yes. I will do that in the following PR.But Reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
areas/test kind/bug This is bug report for project LGTM one maintainer or community participant agrees to merge the pull reuqest. size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants