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

Add cluster e2e for cri-containerd. #5370

Merged
merged 1 commit into from
Nov 7, 2017

Conversation

Random-Liu
Copy link
Member

@Random-Liu Random-Liu commented Nov 7, 2017

Add e2e test for cri-containerd.

This depends on kubernetes/kubernetes#54962.
We can get this merged first, but the test will keep failing until kubernetes/kubernetes#54962 is fixed.

For kubernetes/enhancements#286.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 7, 2017
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 7, 2017
ENABLE_POD_SECURITY_POLICY=true

# Envs for cri-containerd.
KUBE_MASTER_EXTRA_METADATA="user-data=${GOPATH}/src/github.com/kubernetes-incubator/cri-containerd/test/e2e/master.yaml,cri-containerd-configure-sh=${GOPATH}/src/github.com/kubernetes-incubator/cri-containerd/test/configure.sh"
Copy link
Member

Choose a reason for hiding this comment

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

we don't allow $Var in env file, it's not gonna expand. Maybe just use /go/src/... here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Changed to reply on cwd.

# For now explicitly test etcd v2 mode in this suite.
STORAGE_BACKEND=etcd2
TEST_ETCD_IMAGE=2.2.1
TEST_ETCD_VERSION=2.2.1
Copy link
Member

Choose a reason for hiding this comment

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

(do you still wanna test against old etcd?, up to you)

Copy link
Member Author

Choose a reason for hiding this comment

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

I see that all tests are still using this.

Copy link
Member

Choose a reason for hiding this comment

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

yeah the e2e-gce uses this which I'm not sure why, anyway

Copy link
Member Author

Choose a reason for hiding this comment

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

At least ci-kubernetes-e2e-gci-gce.env is still using it.

"--check-leaked-resources",
"--env-file=jobs/platform/gce.env",
"--env-file=jobs/env/ci-cri-containerd-e2e-gce.env",
"--extract=ci/latest",
Copy link
Member

Choose a reason for hiding this comment

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

I think after --extract cwd will become /workspace/kubernetes, before that it's inside the repo. Is this still correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

What?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

if you wanna merge and watch the actual behavior I'm also fine

Copy link
Member Author

Choose a reason for hiding this comment

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

What about now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is extract=ci/latest required for e2e test?

Copy link
Member

Choose a reason for hiding this comment

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

ok :-)

@krzyzacy
Copy link
Member

krzyzacy commented Nov 7, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 7, 2017
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: krzyzacy, Random-Liu

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 7, 2017
@k8s-ci-robot k8s-ci-robot merged commit bf34075 into kubernetes:master Nov 7, 2017
@k8s-ci-robot
Copy link
Contributor

@Random-Liu: I updated Prow config for you!

In response to this:

Add e2e test for cri-containerd.

This depends on kubernetes/kubernetes#54962.
We can get this merged first, but the test will keep failing until kubernetes/kubernetes#54962 is fixed.

For kubernetes/enhancements#286.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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.

3 participants