Skip to content

Conversation

@Ace-Tang
Copy link
Contributor

runc get cgroup parent through /proc/self/cgroup, so can not get cgroup path just
join string like "/sys/fs/cgroup"+"subsystem"+"cgroup-parent"+"containerid", get corrent
cgroup path from /proc/self/cgroup in test.

fixes: #2685

Signed-off-by: Ace-Tang [email protected]

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

fixes: #2685

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

runc get cgroup parent through `/proc/self/cgroup`, so can not get cgroup path just
join string like "/sys/fs/cgroup"+"subsystem"+"cgroup-parent"+"containerid", get corrent
cgroup path from `/proc/self/cgroup` in test.

fixes: #2685

Signed-off-by: Ace-Tang <[email protected]>
@codecov
Copy link

codecov bot commented Jan 23, 2019

Codecov Report

Merging #2688 into master will increase coverage by 4.36%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2688      +/-   ##
=========================================
+ Coverage   63.03%   67.4%   +4.36%     
=========================================
  Files         276     278       +2     
  Lines       17356   17387      +31     
=========================================
+ Hits        10941   11720     +779     
+ Misses       5206    4386     -820     
- Partials     1209    1281      +72
Flag Coverage Δ
#criv1alpha2_test 39.29% <ø> (?)
#integration_test_0 36.45% <ø> (ø) ⬆️
#integration_test_2 36.62% <ø> (ø) ⬆️
#integration_test_3 35.12% <ø> (?)
#node_e2e_test 35.12% <ø> (+0.02%) ⬆️
#unittest 27.37% <ø> (ø) ⬆️
Impacted Files Coverage Δ
cri/ocicni/cni_manager.go 58.82% <0%> (-11.77%) ⬇️
daemon/mgr/spec_apparmor_linux.go 20% <0%> (ø)
daemon/mgr/spec_seccomp_linux.go 71.42% <0%> (ø)
daemon/mgr/container_stats.go 71.83% <0%> (+1.14%) ⬆️
apis/server/router.go 90.74% <0%> (+1.23%) ⬆️
daemon/mgr/system.go 74.8% <0%> (+1.52%) ⬆️
pkg/meta/store.go 68.99% <0%> (+1.55%) ⬆️
ctrd/client.go 65.73% <0%> (+1.68%) ⬆️
daemon/mgr/container_utils.go 85.11% <0%> (+1.78%) ⬆️
daemon/containerio/io.go 72.81% <0%> (+1.94%) ⬆️
... and 41 more

@pouchrobot pouchrobot added areas/test kind/bug This is bug report for project size/M labels Jan 23, 2019
@Ace-Tang
Copy link
Contributor Author

FAIL: z_cli_daemon_test.go:478: PouchDaemonSuite.TestRestartStoppedContainerAfterDaemonRestart
z_cli_daemon_test.go:516:
    res.Assert(c, icmd.Success)
/home/travis/gopath/src/github.com/alibaba/pouch/vendor/github.com/gotestyourself/gotestyourself/icmd/command.go:61:
    t.Fatalf("at %s:%d - %s\n", filepath.Base(file), line, err.Error())
... Error: at z_cli_daemon_test.go:516 - 
Command:  /usr/local/bin/pouch --host unix:///tmp/test/pouch/pouchd.sock start -a PouchDaemonSuite.TestRestartStoppedContainerAfterDaemonRestart
ExitCode: 1
Error:    exit status 1
Stdout:   
Stderr:   Error: failed to start container PouchDaemonSuite.TestRestartStoppedContainerAfterDaemonRestart: {"message":"failed to create container(7d4d5fc514ae18d734d19713e6afb7add1c40f9b6faaeaed381f244ba0a0152b) on containerd: failed to create container 7d4d5fc514ae18d734d19713e6afb7add1c40f9b6faaeaed381f244ba0a0152b: container \"7d4d5fc514ae18d734d19713e6afb7add1c40f9b6faaeaed381f244ba0a0152b\": already exists: already existed"}

Seems not related with my modify

@Ace-Tang
Copy link
Contributor Author

hack/install/install_nsenter.sh
>>>> install nsenter-2.24.1 <<<<
--2019-01-23 13:20:39--  https://pouch-oss.oss-cn-beijing.aliyuncs.com/pouch-test/ubuntu/nsenter-2.24.1
Resolving pouch-oss.oss-cn-beijing.aliyuncs.com (pouch-oss.oss-cn-beijing.aliyuncs.com)... 59.110.190.36, 59.110.190.32
Connecting to pouch-oss.oss-cn-beijing.aliyuncs.com (pouch-oss.oss-cn-beijing.aliyuncs.com)|59.110.190.36|:443... connected.

oss network connect down

@HusterWan
Copy link
Contributor

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Jan 24, 2019
@HusterWan
Copy link
Contributor

FAIL: z_cli_daemon_test.go:478: PouchDaemonSuite.TestRestartStoppedContainerAfterDaemonRestart
z_cli_daemon_test.go:516:
    res.Assert(c, icmd.Success)
/home/travis/gopath/src/github.com/alibaba/pouch/vendor/github.com/gotestyourself/gotestyourself/icmd/command.go:61:
    t.Fatalf("at %s:%d - %s\n", filepath.Base(file), line, err.Error())
... Error: at z_cli_daemon_test.go:516 - 
Command:  /usr/local/bin/pouch --host unix:///tmp/test/pouch/pouchd.sock start -a PouchDaemonSuite.TestRestartStoppedContainerAfterDaemonRestart
ExitCode: 1
Error:    exit status 1
Stdout:   
Stderr:   Error: failed to start container PouchDaemonSuite.TestRestartStoppedContainerAfterDaemonRestart: {"message":"failed to create container(7d4d5fc514ae18d734d19713e6afb7add1c40f9b6faaeaed381f244ba0a0152b) on containerd: failed to create container 7d4d5fc514ae18d734d19713e6afb7add1c40f9b6faaeaed381f244ba0a0152b: container \"7d4d5fc514ae18d734d19713e6afb7add1c40f9b6faaeaed381f244ba0a0152b\": already exists: already existed"}

Seems not related with my modify

Since this case is nothing to do with this PR, so I think we can merge this PR first.

Copy link
Contributor

@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.

LGTM

@fuweid fuweid merged commit b3e63d1 into AliyunContainerService:master Jan 24, 2019
rudyfly pushed a commit that referenced this pull request Jan 25, 2019
in ##2688, I make a mistake to find container cgroup path, since when
containeruse cgroup namespace, we can not get cgroup path from container
with`exec cat proc/self/cgroups`. Skip some test if container use cgroup
namespace.

fixes: #2685

Signed-off-by: Ace-Tang <[email protected]>
Ace-Tang added a commit that referenced this pull request Mar 12, 2019
in ##2688, I make a mistake to find container cgroup path, since when
containeruse cgroup namespace, we can not get cgroup path from container
with`exec cat proc/self/cgroups`. Skip some test if container use cgroup
namespace.

fixes: #2685

Signed-off-by: Ace-Tang <[email protected]>
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/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] memory controller missing on ubuntu

4 participants