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: replace k8s-sigs/cri-tools with alibaba/cri-tools #2226

Merged
merged 1 commit into from
Sep 17, 2018

Conversation

starnop
Copy link
Contributor

@starnop starnop commented Sep 11, 2018

Signed-off-by: Starnop [email protected]

Ⅰ. Describe what this PR did

Currently, PouchContainer has lots of advantages over other container runtimes. We have deliver those capabilities to Kubernetes by the extension of CRI API.

Meanwhile, in order to ensure the robustness of the project, we should code more tests to cover the extend features.

Now, we have fork the repo from kubernetes-sigs/cri-tools to alibaba/cri-tools , and the tests has been added, the tests have been covered so far as follows:

  • test has been done

    • create container with :
      • diskquota
      • the Image which has volumes
      • envs
    • start container above.
    • get container status above and expect GetQuotaId, GetResources, GetMounts and GetEnvs not nil.
    • get image status above.
  • test will been done

    • make runtime choosing supported.
    • make lxcfs configurable supported.

so it's time to integrate into the CI of alibaba/pouch.

Ⅱ. Does this pull request fix one issue?

None.

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

Just a shell.

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

The package of cri-tools has been moved from kubernetes-incubator to kubernetes-sigs.
linker : kubernetes-sigs/cri-tools#376

@allencloud
Copy link
Collaborator

professional delivery 👍

@starnop starnop force-pushed the ci-ali-cri-tools branch 2 times, most recently from 1d3e789 to fddad42 Compare September 11, 2018 08:27
@Ace-Tang
Copy link
Contributor

Hi, @starnop @fuweid , how about this pr going? and how about add exec fail test to cri tools going, @starnop ?

@starnop
Copy link
Contributor Author

starnop commented Sep 11, 2018

@Ace-Tang the test of cri-tools :alibaba-archive/cri-tools#2 @fuweid @allencloud @YaoZengzeng PTAL. :)

@Ace-Tang
Copy link
Contributor

Please merge #2224 first, or the cri test will fail since exec fail test can not pass.

@allencloud
Copy link
Collaborator

the commits are so complicated, please change them. @starnop

@codecov-io
Copy link

codecov-io commented Sep 12, 2018

Codecov Report

Merging #2226 into master will decrease coverage by 0.99%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #2226    +/-   ##
========================================
- Coverage    66.5%   65.51%    -1%     
========================================
  Files         208      208            
  Lines       16755    16841    +86     
========================================
- Hits        11143    11033   -110     
- Misses       4282     4483   +201     
+ Partials     1330     1325     -5
Flag Coverage Δ
#criv1alpha1test 32.91% <ø> (+0.19%) ⬆️
#criv1alpha2test 21.99% <ø> (-11.59%) ⬇️
#integrationtest 39.9% <ø> (-0.05%) ⬇️
#nodee2etest 33.66% <ø> (-0.02%) ⬇️
#unittest 23.85% <ø> (ø) ⬆️
Impacted Files Coverage Δ
cri/v1alpha2/server.go 44.18% <0%> (-35.66%) ⬇️
apis/server/container_bridge.go 60.11% <0%> (-19.15%) ⬇️
cri/ocicni/cni_manager.go 65% <0%> (-15%) ⬇️
cri/v1alpha2/cri_wrapper.go 56.48% <0%> (-7.95%) ⬇️
apis/server/utils.go 61.9% <0%> (-4.77%) ⬇️
cri/v1alpha2/cri_utils.go 87.83% <0%> (-3.16%) ⬇️
cri/stream/remotecommand/httpstream.go 43.52% <0%> (-3.11%) ⬇️
cri/v1alpha2/cri.go 69.8% <0%> (-2.08%) ⬇️
daemon/logger/jsonfile/utils.go 71.54% <0%> (-1.63%) ⬇️
daemon/mgr/container.go 57.39% <0%> (-0.21%) ⬇️
... and 3 more

@allencloud allencloud changed the title test: replace kubernetes-sigs/cri-tools with alibaba/cri-tools test: replace k8s-sigs/cri-tools with alibaba/cri-tools Sep 12, 2018
@allencloud
Copy link
Collaborator

LGTM

@pouchrobot pouchrobot added the LGTM one maintainer or community participant agrees to merge the pull reuqest. label Sep 17, 2018
@allencloud allencloud merged commit b926849 into AliyunContainerService:master Sep 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
areas/orchestration areas/test 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