Rework multi-volume test to use StatefulSet by msau42 · Pull Request #66925 · kubernetes/kubernetes

msau42 · 2018-08-02T18:09:17Z

What this PR does / why we need it:
The e2e test that got added as part of #66832 fails in a multi-zone environment because the volumes get provisioned in random zones. This PR reworks the test to use StatefulSet instead, which handles provisioning multiple PVCs in the same zone.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Release note:

NONE

msau42 · 2018-08-02T18:09:38Z

/assign @davidz627

msau42 · 2018-08-02T20:56:00Z

/retest

davidz627

mostly looks good. just some questions and readability things.

davidz627 · 2018-08-02T22:54:18Z

test/e2e/storage/persistent_volumes.go

why are we sleeping for so long?

I just want to simulate a long running process

davidz627 · 2018-08-02T22:55:31Z

test/e2e/storage/persistent_volumes.go

do we have to scale down before deleting? If so, can you add a comment for why

mainly because i wanted to use the statefulset testing library which doesn't have a method to wait for deletion, and this is what they do.

davidz627 · 2018-08-02T23:00:23Z

test/e2e/storage/persistent_volumes.go

Is it necessary to double validate here? We're already deleting the statefulset and bringing a new one up.

not strictly. I can remove it

davidz627 · 2018-08-02T23:02:25Z

test/e2e/storage/persistent_volumes.go

just use i.

davidz627 · 2018-08-02T23:02:36Z

test/e2e/storage/persistent_volumes.go

just use i

davidz627 · 2018-08-02T23:04:14Z

test/e2e/storage/persistent_volumes.go

do we need this? there is no corresponding service being created

davidz627 · 2018-08-02T23:07:31Z

test/e2e/storage/persistent_volumes.go

from the top level of this test it is not clear where the volume mount names are coming from, it would be better if you could somehow extract the volume mount paths out to this top level and pass it into this function so that the test is slightly more readable.
#testingOnTheToiletEp529

msau42 · 2018-08-03T00:40:21Z

Updated

davidz627

small comment then lgtm

davidz627 · 2018-08-03T00:44:26Z

test/e2e/storage/persistent_volumes.go

clean up the statefulset after test?

davidz627 · 2018-08-03T00:46:16Z

test/e2e/storage/persistent_volumes.go

do we need to setting this explicitly to ""

nope i've removed

neolit123 · 2018-08-03T00:49:33Z

/sig storage

msau42 · 2018-08-03T01:37:14Z

updated

msau42 · 2018-08-03T02:20:49Z

/test pull-kubernetes-e2e-gce-device-plugin-gpu

davidz627 · 2018-08-03T17:16:42Z

/lgtm

k8s-ci-robot · 2018-08-03T17:17:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: davidz627, msau42

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~test/e2e/storage/OWNERS~~ [msau42]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-github-robot · 2018-08-03T21:36:01Z

Automatic merge from submit-queue (batch tested with PRs 66933, 66925). If you want to cherry-pick this change to another branch, please follow the instructions here.

…-upstream-release-1.10 Automatic merge from submit-queue. Automated cherry pick of #66925: Rework multi-volume test to use StatefulSet Cherry pick of #66925 on release-1.10. #66925: Rework multi-volume test to use StatefulSet

…-upstream-release-1.11 Automatic merge from submit-queue. Automated cherry pick of #66925: Rework multi-volume test to use StatefulSet Cherry pick of #66925 on release-1.11. #66925: Rework multi-volume test to use StatefulSet

…-upstream-release-1.9 Automatic merge from submit-queue. Automated cherry pick of #66832: Detect if GCE PD udev link is wrong and try to correct it Cherry pick of #66832 on release-1.9. #66832: Detect if GCE PD udev link is wrong and try to correct it #66925: Rework multi-volume test to use StatefulSet

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. 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 Aug 2, 2018

k8s-ci-robot requested review from jeffvance and rootfs August 2, 2018 18:09

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 2, 2018

k8s-ci-robot assigned davidz627 Aug 2, 2018

davidz627 reviewed Aug 2, 2018

View reviewed changes

msau42 force-pushed the udev branch from ba4751f to eb8f218 Compare August 3, 2018 00:39

davidz627 reviewed Aug 3, 2018

View reviewed changes

test/e2e/storage/persistent_volumes.go Outdated

Copy link
Copy Markdown

Contributor

davidz627 Aug 3, 2018

Choose a reason for hiding this comment

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

clean up the statefulset after test?

davidz627 reviewed Aug 3, 2018

View reviewed changes

k8s-ci-robot added the sig/storage Categorizes an issue or PR as relevant to SIG Storage. label Aug 3, 2018

Rework multi-volume test to use StatefulSet

4fbf25f

msau42 force-pushed the udev branch from eb8f218 to 4fbf25f Compare August 3, 2018 01:36

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 3, 2018

k8s-github-robot merged commit babdbff into kubernetes:master Aug 3, 2018

This was referenced Aug 3, 2018

Automated cherry pick of #66925: Rework multi-volume test to use StatefulSet #66976

Merged

Automated cherry pick of #66925: Rework multi-volume test to use StatefulSet #66977

Merged

dims mentioned this pull request Aug 3, 2018

Update the nginx image with docker.io/nginx image #66055

Merged

msau42 mentioned this pull request Aug 4, 2018

Automated cherry pick of #66832: Detect if GCE PD udev link is wrong and try to correct it #66880

Merged

msau42 deleted the udev branch November 17, 2018 21:06

Conversation

msau42 commented Aug 2, 2018

Uh oh!

msau42 commented Aug 2, 2018

Uh oh!

msau42 commented Aug 2, 2018

Uh oh!

davidz627 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msau42 commented Aug 3, 2018

Uh oh!

davidz627 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

neolit123 commented Aug 3, 2018

Uh oh!

msau42 commented Aug 3, 2018

Uh oh!

msau42 commented Aug 3, 2018

Uh oh!

davidz627 commented Aug 3, 2018

Uh oh!

k8s-ci-robot commented Aug 3, 2018

Uh oh!

k8s-github-robot commented Aug 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants