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

Improve image test and make test run in parallel. #252

Merged
merged 1 commit into from
Feb 21, 2018

Conversation

Random-Liu
Copy link
Contributor

@Random-Liu Random-Liu commented Feb 17, 2018

Fixes #241.

This PR:

  1. Added images/ directory which contains test images used in CRI validation test. @runcom We can also move your test image to images/. I'll push it to gcr.io/cri-tools for you.
  2. Improve image test to make it be able to run in parallel with other tests.

By running test in parallel, it only takes <20s now.

•
Ran 56 of 57 Specs in 18.565 seconds
SUCCESS! -- 56 Passed | 0 Failed | 0 Pending | 1 Skipped 

Signed-off-by: Lantao Liu [email protected]

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 17, 2018
@runcom
Copy link
Contributor

runcom commented Feb 17, 2018

LGTM but Travis doesn't like the changes

@runcom
Copy link
Contributor

runcom commented Feb 19, 2018

@Random-Liu Travis' failing

@runcom
Copy link
Contributor

runcom commented Feb 20, 2018

ping @Random-Liu @feiskyer

@Random-Liu
Copy link
Contributor Author

Random-Liu commented Feb 20, 2018

@runcom @feiskyer Fixed the test.

The previous test image I was using is a scratch image with an ENV set.
However, docker returns 0 size for that image, and it fails the size check https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/remote/remote_image.go#L88.

I'm not sure whether we should do the check in kubelet. But here I changed the test image to busybox + a file to make the size non-zero.

@Random-Liu Random-Liu added this to the v1.0.0-beta.0 milestone Feb 21, 2018
@Random-Liu
Copy link
Contributor Author

I think @feiskyer is on vacation. Apply LGTM based on #252 (comment)

@Random-Liu Random-Liu added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 21, 2018
@Random-Liu Random-Liu merged commit 8ec2cbd into kubernetes-sigs:master Feb 21, 2018
@Random-Liu Random-Liu deleted the run-test-in-parallel branch February 21, 2018 22:23
@feiskyer
Copy link
Member

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants