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 cri test for pouch #524

Merged
merged 1 commit into from
Jan 11, 2018

Conversation

czm4514
Copy link
Contributor

@czm4514 czm4514 commented Jan 9, 2018

Signed-off-by: czm4514 [email protected]

1.Describe what this PR did
this PR is to add cri test for pouch.
2.Does this pull request fix one issue?

3.Describe how you did it
i use the cri-tools project to add cri test for pouch. there are lots of test cases in the project.
4.Describe how to verify it
when you run the test-cri.sh ,
./test-cri.sh
you could get the information like this(the variable focus is set to "Runtime"):

/root/gocodes/bin/critest
1 attempt "ctr version"! Trying again in 1 seconds...
2 attempt "ctr version"! Trying again in 2 seconds...
3 attempt "ctr version"! Trying again in 3 seconds...
4 attempt "ctr version"! Trying again in 4 seconds...
Running Suite: E2ECRI Suite
===========================
Random Seed: 1515481734 - Will randomize all specs
Will run 2 of 55 specs

SSSSSSSSSSSSSSSS
------------------------------
[k8s.io] Runtime info runtime should support returning runtime info
  runtime should return runtime conditions [Conformance]
  /root/gocodes/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/runtime_info.go:48
[BeforeEach] [k8s.io] Runtime info
  /root/gocodes/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:50
[BeforeEach] [k8s.io] Runtime info
  /root/gocodes/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/runtime_info.go:39
[It] runtime should return runtime conditions [Conformance]
  /root/gocodes/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/runtime_info.go:48
STEP: test runtime status
[AfterEach] [k8s.io] Runtime info
  /root/gocodes/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:51
•SSSSSSSSSSSSSSSSSSSSSSS
------------------------------
[k8s.io] Runtime info runtime should support returning runtime info
  runtime should return version info [Conformance]
  /root/gocodes/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/runtime_info.go:43
[BeforeEach] [k8s.io] Runtime info
  /root/gocodes/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:50
[BeforeEach] [k8s.io] Runtime info
  /root/gocodes/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/runtime_info.go:39
[It] runtime should return version info [Conformance]
  /root/gocodes/src/github.com/kubernetes-incubator/cri-tools/pkg/validate/runtime_info.go:43
Jan  9 15:08:54.098: INFO: Get version info succeed
[AfterEach] [k8s.io] Runtime info
  /root/gocodes/src/github.com/kubernetes-incubator/cri-tools/pkg/framework/framework.go:51
•SSSSSSSSSSSSSS
Ran 2 of 55 Specs in 0.004 seconds
SUCCESS! -- 2 Passed | 0 Failed | 0 Pending | 53 Skipped PASS

Ginkgo ran 1 suite in 85.546375ms
Test Suite Passed
./test-utils.sh: line 54: 13560 Terminated              keepalive "containerd" ${RESTART_WAIT_PERIOD} &>${report_dir}/containerd.log
./test-utils.sh: line 54: 13694 Terminated              keepalive "pouchd ${POUCH_FLAGS}" ${RESTART_WAIT_PERIOD} &>${report_dir}/pouch.log

5.Special notes for reviews
when you want to run Specified test case ,you can through the param "--focus" to filter. you can get mor informations from https://github.com/kubernetes-incubator/cri-tools. you can use the cri-tools through the command: critest -f "image status|public image" validation

@pouchrobot
Copy link
Collaborator

We found this is your first time to contribute to Pouch, @czm4514
👏 We really appreciate it.
Just remind that you have read the contribution guide: https://github.com/alibaba/pouch/blob/master/CONTRIBUTING.md
If you didn't, you should do that first. If done, welcome again and please enjoy hacking! 🍻

@czm4514 czm4514 changed the title add cri test for pouch feature: add cri test for pouch Jan 9, 2018
@czm4514
Copy link
Contributor Author

czm4514 commented Jan 9, 2018

do you have any idea about this feature?
@Letty5411

@allencloud
Copy link
Collaborator

Please finish details about the PR description. And sign off your commits.

@codecov-io
Copy link

codecov-io commented Jan 9, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #524      +/-   ##
==========================================
+ Coverage   14.49%   20.47%   +5.97%     
==========================================
  Files          64       39      -25     
  Lines        3173     2056    -1117     
==========================================
- Hits          460      421      -39     
+ Misses       2666     1590    -1076     
+ Partials       47       45       -2
Impacted Files Coverage Δ
daemon/mgr/container_types.go 0% <0%> (-18.19%) ⬇️
cli/volume.go
cli/cli.go
cli/rename.go
cli/unpause.go
cli/main.go
cli/exec.go
cli/rmi.go
cli/command.go
cli/inspect.go
... and 16 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 580accd...e5ac03a. Read the comment docs.

@allencloud
Copy link
Collaborator

Could you help this to integration test system in hack/make.sh? @Letty5411

@@ -0,0 +1,3 @@
RUNC_VERSION=74a17296470088de3805e138d3d87c62e613dfc4
CONTAINERD_VERSION=6c7abf7c76c1973d4fb4b0bad51691de84869a51
Copy link
Contributor

Choose a reason for hiding this comment

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

RUNC_VERSION and CONTAINERD_VERSION are not used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes,It's really not used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed unused variables

fi
sudo pkill containerd
keepalive "containerd" ${RESTART_WAIT_PERIOD} &> ${report_dir}/containerd.log &
containerd_pid=$!
Copy link
Contributor

Choose a reason for hiding this comment

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

line 39-41, please check the existence of containerd first, kill it only if containerd doesn't exist.

keepalive "pouchd ${POUCH_FLAGS}" \
${RESTART_WAIT_PERIOD} &> ${report_dir}/pouch.log &
pouch_pid=$!
readiness_check "pouch version"
Copy link
Contributor

Choose a reason for hiding this comment

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

line 47-50, please also check pouchd process first

@Letty5411
Copy link
Contributor

@czm4514 Please also use 'git rebase -i HEAD~N' to rebase this PR.

FOCUS=${FOCUS:-""}

# SKIP skips the test to skip.
SKIP=${SKIP:-""}
Copy link
Contributor

@Letty5411 Letty5411 Jan 9, 2018

Choose a reason for hiding this comment

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

Change the ENV FOCUS SKIP to CRI_FOCUS,CRI_SKIP

@allencloud
Copy link
Collaborator

After

Please also use 'git rebase -i HEAD~N' to rebase this PR.

Please git commit -s --amend to sign off you commit.

Maybe you need to git config --user and git config --email to add you personnel info at first. @czm4514

@CLAassistant
Copy link

CLAassistant commented Jan 9, 2018

CLA assistant check
All committers have signed the CLA.

@Letty5411
Copy link
Contributor

Also change the title from "feature:" to "test:", the same to the commit message. As this is a test PR.

@allencloud allencloud added this to the v0.1-Milestone milestone Jan 9, 2018
@czm4514 czm4514 changed the title feature: add cri test for pouch test: add cri test for pouch Jan 10, 2018
@Letty5411
Copy link
Contributor

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Jan 11, 2018
@Letty5411 Letty5411 merged commit e46d85b into AliyunContainerService:master Jan 11, 2018
@allencloud
Copy link
Collaborator

Feel so good to see the merging. Thanks very much for your work. @czm4514

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

Successfully merging this pull request may close these issues.

6 participants