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

test: add missing integration test for PouchContainer #2612

Closed

Conversation

allencloud
Copy link
Collaborator

@allencloud allencloud commented Dec 25, 2018

Signed-off-by: Allen Sun allensun.shl@alibaba-inc.com

Ⅰ. Describe what this PR did

This PR fixed the following issues:

  • start an exec in a non-running container should return a status code of 409.

In 内部PouchContainer NightlyBuild 测试看板 20181224-041536:


TestCase | 测试通过率 | Alios310 | Alios310 | Alios310 | Alios49 | Alios49 | Alios49 | Owner
-- | -- | -- | -- | -- | -- | -- | -- | --
PouchLogsSuite.TestLogsWithDetails | 83.3% | PASS | PASS | PASS | FAIL | PASS | PASS | zhangyue zy675793960@yeah.net
APIContainerAttachSuite.TestContainerAttachNotFound | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | letty letty.ll@alibaba-inc.com
APIContainerAttachSuite.TestContainerAttachStdin | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | letty letty.ll@alibaba-inc.com
APIContainerCheckpointSuite.TestCheckpointCreateAPI | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Ace-Tang aceapril@126.com
APIContainerCheckpointSuite.TestCheckpointDelAPI | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Ace-Tang aceapril@126.com
APIContainerCheckpointSuite.TestCheckpointListAPI | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Ace-Tang aceapril@126.com
APIContainerCreateSuite.TestBadParam | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | letty letty.ll@alibaba-inc.com
APIContainerExecInspectSuite.TestContainerExecInspectOk | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Allen Sun allensun.shl@alibaba-inc.com
APIContainerExecStartSuite.TestContainerExecStartDup | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | letty.ll letty.ll@alibaba-inc.com
APIContainerExecStartSuite.TestContainerExecStartPaused | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | letty.ll letty.ll@alibaba-inc.com
APIContainerExecStartSuite.TestContainerExecStartStopped | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | letty.ll letty.ll@alibaba-inc.com
APIContainerStartSuite.TestInvalidParam | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | letty letty.ll@alibaba-inc.com
PouchCheckpointSuite.TestCheckpointCreate | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Ace-Tang aceapril@126.com
PouchCheckpointSuite.TestCheckpointDelete | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Ace-Tang aceapril@126.com
PouchCheckpointSuite.TestCheckpointList | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Ace-Tang aceapril@126.com
PouchCommitSuite.TestCommitBindMount | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Ace-Tang aceapril@126.com
PouchCommitSuite.TestCommitContainer | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Ace-Tang aceapril@126.com
PouchCommitSuite.TestCommitHardLink | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Ace-Tang aceapril@126.com
PouchCommitSuite.TestCommitNewFile | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Ace-Tang aceapril@126.com
PouchNetworkSuite.TestNetworkConnectWithRestart | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Eric Li lcy041536@gmail.com
PouchRichContainerSuite.TestRichContainerSystemdWorks | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | letty letty.ll@alibaba-inc.com
PouchRunBlkioSuite.TestRunBlockIOWeightDevice | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | letty letty.ll@alibaba-inc.com
PouchRunDeviceSuite.TestRunDeviceDirectory | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | HusterWan zirenwan@gmail.com
PouchSnapshotterSuite.TestNotSetSnapshotter | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | NA
PouchSnapshotterSuite.TestOldSnapshotterNotClean | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | NA
PouchSnapshotterSuite.TestSetDefaultSnapshotter | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | NA
PouchStartSuite.TestStartFromCheckpoint | 100.0% | SKIP | SKIP | SKIP | SKIP | SKIP | SKIP | Ace-Tang aceapril@126.com

We skipped lots of test cases.

This PR tries to add some. And could you help to try to make the rest ones to move on(Whether to continue developing or skipping).

Ⅱ. Does this pull request fix one issue?

no

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

added

Ⅳ. Describe how to verify it

none

Ⅴ. Special notes for reviews

@sunyuan3 @chuanchang

@codecov
Copy link

codecov bot commented Dec 25, 2018

Codecov Report

Merging #2612 into master will decrease coverage by 0.74%.
The diff coverage is 81.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2612      +/-   ##
==========================================
- Coverage   69.45%   68.71%   -0.75%     
==========================================
  Files         277      277              
  Lines       17429    17437       +8     
==========================================
- Hits        12106    11982     -124     
- Misses       3999     4103     +104     
- Partials     1324     1352      +28
Flag Coverage Δ
#criv1alpha2_test 39.37% <18.18%> (-0.06%) ⬇️
#integration_test_0 36.57% <18.18%> (+0.04%) ⬆️
#integration_test_1 35.72% <27.27%> (-0.07%) ⬇️
#integration_test_2 36.57% <63.63%> (+0.09%) ⬆️
#integration_test_3 ?
#node_e2e_test 35.39% <9.09%> (+0.09%) ⬆️
#unittest 28.5% <0%> (-0.02%) ⬇️
Impacted Files Coverage Δ
pkg/errtypes/errors.go 92% <100%> (+0.69%) ⬆️
daemon/mgr/container.go 57.91% <100%> (-2.92%) ⬇️
apis/server/router.go 91.35% <100%> (ø) ⬆️
daemon/mgr/container_exec.go 81.59% <100%> (+1.92%) ⬆️
apis/server/exec_bridge.go 63.33% <50%> (-0.63%) ⬇️
daemon/logger/syslog/fmt.go 0% <0%> (-33.34%) ⬇️
daemon/logger/syslog/syslog.go 59.34% <0%> (-16.49%) ⬇️
daemon/mgr/container_checkpoint.go 59.03% <0%> (-9.64%) ⬇️
apis/server/container_bridge.go 83.42% <0%> (-7.03%) ⬇️
cri/stream/portforward/httpstream.go 70.94% <0%> (-6.84%) ⬇️
... and 21 more

@chuanchang
Copy link
Contributor

@allencloud it's failed to run make check, the details as follows.

#git rev-parse HEAD
bc9cddc3b8aadf82022d444966a835f39e4a4818

#make check
gometalinter
gometalinter --config .gometalinter.json ./...
test/api_container_exec_start_test.go:162:39:error: undefined: content (vet)
test/api_container_exec_start_test.go:164:67:error: undefined: body (vet)
test/api_container_exec_start_test.go:178:39:error: undefined: content (vet)
test/api_container_exec_start_test.go:186:12:error: no new variables on left side of := (vet)
test/api_container_exec_start_test.go:201:39:error: undefined: content (vet)
test/api_container_exec_start_test.go:202:67:error: undefined: body (vet)
test/api_container_exec_start_test.go:206:66:error: undefined: body (vet)
make: *** [gometalinter] Error 1

@allencloud allencloud force-pushed the add-test branch 2 times, most recently from 9c2621d to 2c48bc7 Compare December 28, 2018 02:47
@pouchrobot pouchrobot added size/L and removed size/M labels Dec 28, 2018
@allencloud allencloud force-pushed the add-test branch 2 times, most recently from a1ca710 to 6f38812 Compare December 28, 2018 02:59
@chuanchang
Copy link
Contributor

chuanchang commented Dec 28, 2018

@allencloud there are 4 cases are failure in CI.

----------------------------------------------------------------------
FAIL: /home/travis/gopath/src/github.com/alibaba/pouch/test/api_container_exec_inspect_test.go:28: APIContainerExecInspectSuite.TestContainerExecInspectOk
/home/travis/gopath/src/github.com/alibaba/pouch/test/api_container_exec_inspect_test.go:80:
    c.Assert(execInspect02.Running, check.Equals, true)
... obtained bool = false
... expected bool = true
----------------------------------------------------------------------
FAIL: /home/travis/gopath/src/github.com/alibaba/pouch/test/api_container_exec_start_test.go:195: APIContainerExecStartSuite.TestContainerExecStartDup
/home/travis/gopath/src/github.com/alibaba/pouch/test/api_container_exec_start_test.go:213:
    CheckRespStatus(c, resp, 500)
/home/travis/gopath/src/github.com/alibaba/pouch/test/util_api.go:27:
    c.Assert(resp.StatusCode, check.Equals, status, check.Commentf("Response Body: %v", string(body)))
... obtained int = 200
... expected int = 500
... Response Body: �(failed to create containerIO: conflict
----------------------------------------------------------------------
FAIL: /home/travis/gopath/src/github.com/alibaba/pouch/test/api_container_exec_start_test.go:173: APIContainerExecStartSuite.TestContainerExecStartPaused
/home/travis/gopath/src/github.com/alibaba/pouch/test/api_container_exec_start_test.go:186:
    CheckRespStatus(c, resp, 200)
/home/travis/gopath/src/github.com/alibaba/pouch/test/util_api.go:27:
    c.Assert(resp.StatusCode, check.Equals, status, check.Commentf("Response Body: %v", string(body)))
... obtained int = 204
... expected int = 200
... Response Body: 
----------------------------------------------------------------------
FAIL: /home/travis/gopath/src/github.com/alibaba/pouch/test/api_container_exec_start_test.go:156: APIContainerExecStartSuite.TestContainerExecStartStopped
/home/travis/gopath/src/github.com/alibaba/pouch/test/api_container_exec_start_test.go:162:
    execid := CreateExecCmdOk(c, cname, "echo", "test")
/home/travis/gopath/src/github.com/alibaba/pouch/test/util_api.go:27:
    c.Assert(resp.StatusCode, check.Equals, status, check.Commentf("Response Body: %v", string(body)))
... obtained int = 500
... expected int = 201
... Response Body: {"message":"container af955a13ea8d00c9137c73e391b9d60413f00609e9324e6e76b0810542f190b8 is not running"}

@allencloud
Copy link
Collaborator Author

Unfortunately, the image pulling part fails with:

----------------------------------------------------------------------
FAIL: /home/travis/gopath/src/github.com/alibaba/pouch/test/z_cli_daemon_test.go:219: PouchDaemonSuite.TestDaemonRestart
/home/travis/gopath/src/github.com/alibaba/pouch/test/z_cli_daemon_test.go:232:
    c.Fatalf("pull image failed, err:%v", result)
... Error: pull image failed, err:
Command:  /usr/local/bin/pouch --host unix:///tmp/test/pouch/pouchd.sock pull registry.hub.docker.com/library/busybox:1.28
ExitCode: 1
Error:    exit status 1
Stdout:   registry.hub.docker.com/library/busybox:1.28: resolving
Stderr:   Error: failed to display progress: failed to pull image: registry.hub.docker.com/library/busybox:1.28 not found
----------------------------------------------------------------------

I think there is much possibility that this is a flaky case due to network issue. While I think we should try to add some retry logistics in it to reduce the failure possibility of outer issues.

@allencloud allencloud added this to the v1.2.0 milestone Jan 16, 2019

c.Assert(execInspect01.Running, check.Equals, false)
c.Assert(execInspect01.ExitCode, check.Equals, int64(0))
execid := CreateExecCmdOk(c, cname, "sleep", "9")
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure that the sleep is good call. I think we can hold connection for cat command. The cat process will be running until we close the connection. It can help to make the case stable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am wondering if we could keep it there currently. Still keep the original test case, and the test case could be done in the following releases.

Signed-off-by: Allen Sun <allensun.shl@alibaba-inc.com>
@pouchrobot
Copy link
Collaborator

ping @allencloud
Conflict happens after merging a previous commit.
Please rebase the branch against master and push it back again. Thanks a lot.

@fuweid
Copy link
Contributor

fuweid commented Oct 15, 2019

closing it because it is not active right now. please feel free to reopen it.

@fuweid fuweid closed this Oct 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants