More e2e tests #173

dkoshkin · 2019-01-07T18:50:44Z

Is this a bug fix or adding new feature?
/Feature
Fixes: #118

What is this PR about? / Why do we need it?
Introduces many of the tests outlined in #118, the ones that are left are the Failure Scenarios

In this PR a few of the tests require EBS volumes to already exist, I've created a new cloud package in the e2e test folder that will Create/Delete volumes, unfortunately much of the code is duplicated from the pkg/ directory but it didn't feel right trying to reuse that code in the test.

What testing is done?

Running Suite: AWS EBS CSI Driver End-to-End Tests
==================================================
Random Seed: 1548253558 - Will randomize all specs
Will run 30 of 30 specs

Running in parallel across 32 nodes

[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "io1" and fsType "ext4"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:54
•
------------------------------
[ebs-csi-e2e] [single-az] Pre-Provisioned
  [env] should write and read to a pre-provisioned volume
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/pre_provsioning.go:125
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should delete PV with reclaimPolicy "Delete"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:269
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "st1" and encryption
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:82
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "gp2" and encryption
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:82
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "io1" and encryption
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:82
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "gp2" and fsType "ext2"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:54
•
------------------------------
[ebs-csi-e2e] [multi-az] Dynamic Provisioning
  [env] should allow for topology aware volume with specified zone in allowedTopologies
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:358
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "sc1" and fsType "ext2"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:54
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "sc1" and fsType "ext3"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:54
•
------------------------------
[ebs-csi-e2e] [single-az] Pre-Provisioned
  [env] should use a pre-provisioned volume and mount it as readOnly in a pod
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/pre_provsioning.go:149
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create multiple PV objects, bind to PVCs and attach all to different pods
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:166
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create multiple PV objects, bind to PVCs and attach all to different pods on the same node
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:204
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  [env] should retain PV with reclaimPolicy "Retain"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:286
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "io1" and fsType "ext2"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:54
•
------------------------------
[ebs-csi-e2e] [multi-az] Dynamic Provisioning
  should allow for topology aware volume scheduling
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:331
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "gp2" and fsType "ext3"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:54
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create multiple PV objects, bind to PVCs and attach all to a single pod
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:133
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "st1" and fsType "ext4"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:54
•
------------------------------
[ebs-csi-e2e] [single-az] Pre-Provisioned
  [env] should use a pre-provisioned volume and delete PV with reclaimPolicy "Delete"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/pre_provsioning.go:191
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "st1" and fsType "ext3"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:54
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "st1" and fsType "ext2"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:54
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with provided mountOptions
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:108
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "sc1" and fsType "ext4"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:54
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "gp2" and fsType "ext4"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:54
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand and mount it as readOnly in a pod
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:244
•
------------------------------
[ebs-csi-e2e] [single-az] Pre-Provisioned
  [env] should use a pre-provisioned volume and retain PV with reclaimPolicy "Retain"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/pre_provsioning.go:174
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "io1" and fsType "ext3"
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:54
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a volume on demand with volumeType "sc1" and encryption
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:82
•
------------------------------
[ebs-csi-e2e] [single-az] Dynamic Provisioning
  should create a deployment object, write and read to it, delete the pod and write and read to it again
  /Users/dkoshkin/src/github.com/kubernetes-sigs/aws-ebs-csi-driver/tests/e2e/dynamic_provisioning.go:315
•
Ran 30 of 30 Specs in 535.110 seconds
SUCCESS! -- 30 Passed | 0 Failed | 0 Pending | 0 Skipped

k8s-ci-robot · 2019-01-07T18:50:57Z

Hi @dkoshkin. Thanks for your PR.

I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

coveralls · 2019-01-07T18:54:51Z

Pull Request Test Coverage Report for Build 307

3 of 4 (75.0%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 50.352%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/driver/node.go	0	1	0.0%

Totals
Change from base Build 292:	0.0%
Covered Lines:	572
Relevant Lines:	1136

💛 - Coveralls

coveralls · 2019-01-07T18:54:52Z

Pull Request Test Coverage Report for Build 373

36 of 55 (65.45%) changed or added relevant lines in 4 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage decreased (-0.7%) to 63.022%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/cloud/fakes.go	0	3	0.0%
pkg/cloud/cloud.go	32	48	66.67%

Files with Coverage Reduction	New Missed Lines	%
pkg/cloud/cloud.go	1	69.58%

Totals
Change from base Build 370:	-0.7%
Covered Lines:	755
Relevant Lines:	1198

💛 - Coveralls

leakingtapan · 2019-01-07T23:11:48Z

/ok-to-test

leakingtapan

I've created a new cloud package in the e2e test folder that will Create/Delete volumes, unfortunately much of the code is duplicated from the pkg/ directory but it didn't feel right trying to reuse that code in the test.

What's the reason you think it is worth to duplicate the code?

tests/e2e/cloud/aws.go

tests/e2e/cloud/cloud.go

tests/e2e/pre_provsioning.go

docs/README.md

tests/e2e/dynamic_provisioning.go

tests/e2e/testsuites/concurrent_writer_volume_tester.go

leakingtapan · 2019-01-15T04:09:37Z

tests/e2e/testsuites/pre_provisioned_read_only_volume_tester.go

+	for _, pod := range t.Pods {
+		tpod, cleanup := pod.SetupWithPreProvisionedVolumes(client, namespace, t.CSIDriver)
+		// defer must be called here for resources not get removed before using them
+		for i := range cleanup {


NIT: Feels cleaner to implement the cleanup returned from SetupWithPreProvisionedVolumes to clean all the volumes inside the cleanup itself.

tests/e2e/testsuites/read_only_volume_tester.go

tests/e2e/testsuites/reclaim_policy_tester.go

leakingtapan · 2019-01-23T06:03:40Z

@dkoshkin could you update the test done section, the result is outdated with latest changes

docs/README.md

tests/e2e/testsuites/testsuites.go

leakingtapan · 2019-01-24T20:00:40Z

tests/e2e/testsuites/dynamically_provisioned_delete_pod_tester.go

+// DynamicallyProvisionedDeletePodTest will provision required StorageClass and Deployment
+// Testing if the Pod can write and read to mounted volumes
+// Deleting a pod, and again testing if the Pod can write and read to mounted volumes
+type DynamicallyProvisionedDeletePodTest struct {


I didn't find where do we run this test case, and im not sure why do we need to test it through deployment. For the sake of limiting the scope of this PR, can we hold off on this test case?

The file was probably collapsed, here is the test: https://github.com/kubernetes-sigs/aws-ebs-csi-driver/pull/173/files#diff-4b7e52cf33a96cc1c8a4d9974c34027aR315

I would prefer to keep this test as its actually pretty important, this is the only one thats tests the scenario of what happens when a pod restarts and the volume gets remounted, it tests the detach/attach flow and validates that the volume is not being formatted after the second attach.

leakingtapan · 2019-01-25T05:23:35Z

@dkoshkin let's squash the commits into one and run ./hack/update-gofmt to format the files before final merge after all comments are addressed

dkoshkin · 2019-01-25T15:58:44Z

@leakingtapan after your LGTM, I'll squash. gofmt is all good, I've been doing that before pushing.

pkg/driver/driver.go

leakingtapan · 2019-01-25T18:04:50Z

/lgtm

Have one minor comment about nil passing.

* Update README with more detail to run E2E * Cleanup cloud code * Delete pod E2e test * Properly delete EBS volumes with Retain reclaimPolicy * Properly cleanup PV for pre-provisioned tests * Prefix tester structs with DynamicallyProvisioned * Automatically create/delete volumes for pre-provisioned tests * Modify E2E tests Ginkgo structure * Colocated pods E2E test * Topology aware E2E test * ReclaimPolicy E2E test * Pre-provisioned volume E2E test * Add more E2E tests * should create a volume on demand and mount it as readOnly in a pod * should create a volume on demand with provided mountOptions * should create a volume with encryption (+2 squashed commits)

dkoshkin · 2019-01-25T19:58:02Z

Squashed and rebased onto master and fixed the failing unit tests.

leakingtapan · 2019-01-25T21:07:06Z

/lgtm
/approve

k8s-ci-robot · 2019-01-25T21:07:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dkoshkin, leakingtapan

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [leakingtapan]

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

leakingtapan · 2019-01-25T21:25:46Z

The coverage drop is caused by new constructor introduced for e2e testing.

…openshift-4.7-ose-aws-ebs-csi-driver Updating ose-aws-ebs-csi-driver builder & base images to be consistent with ART

k8s-ci-robot requested review from jsafrane and leakingtapan January 7, 2019 18:50

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 7, 2019

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jan 7, 2019

dkoshkin force-pushed the e2e-tests branch from bd4168e to ef64f01 Compare January 7, 2019 19:32

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 7, 2019

dkoshkin force-pushed the e2e-tests branch 7 times, most recently from 7aacc38 to 4d44efc Compare January 11, 2019 13:35

leakingtapan reviewed Jan 15, 2019

View reviewed changes

dkoshkin force-pushed the e2e-tests branch from 4d44efc to 8ece701 Compare January 23, 2019 00:17

dkoshkin force-pushed the e2e-tests branch 2 times, most recently from 1387209 to f2ab799 Compare January 24, 2019 00:45

leakingtapan reviewed Jan 24, 2019

View reviewed changes

dkoshkin force-pushed the e2e-tests branch from f2ab799 to 127d772 Compare January 25, 2019 14:04

leakingtapan reviewed Jan 25, 2019

View reviewed changes

pkg/driver/driver.go Outdated Show resolved Hide resolved

k8s-ci-robot assigned leakingtapan Jan 25, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 25, 2019

leakingtapan mentioned this pull request Jan 25, 2019

Refactor metadata handling logic #188

Closed

dkoshkin force-pushed the e2e-tests branch from 127d772 to 664ff9a Compare January 25, 2019 19:43

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 25, 2019

dkoshkin force-pushed the e2e-tests branch from 664ff9a to 520b672 Compare January 25, 2019 19:45

dkoshkin force-pushed the e2e-tests branch from 520b672 to 423eb7a Compare January 25, 2019 19:52

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 25, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 25, 2019

leakingtapan merged commit 523ed3d into kubernetes-sigs:master Jan 25, 2019

dkoshkin deleted the e2e-tests branch January 28, 2019 15:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More e2e tests #173

More e2e tests #173

dkoshkin commented Jan 7, 2019 •

edited by leakingtapan

Loading

k8s-ci-robot commented Jan 7, 2019

coveralls commented Jan 7, 2019

coveralls commented Jan 7, 2019 •

edited

Loading

leakingtapan commented Jan 7, 2019

leakingtapan left a comment

leakingtapan Jan 15, 2019

leakingtapan commented Jan 23, 2019

leakingtapan Jan 24, 2019

dkoshkin Jan 25, 2019

leakingtapan commented Jan 25, 2019

dkoshkin commented Jan 25, 2019

leakingtapan commented Jan 25, 2019

dkoshkin commented Jan 25, 2019 •

edited

Loading

leakingtapan commented Jan 25, 2019

k8s-ci-robot commented Jan 25, 2019

leakingtapan commented Jan 25, 2019

More e2e tests #173

More e2e tests #173

Conversation

dkoshkin commented Jan 7, 2019 • edited by leakingtapan Loading

k8s-ci-robot commented Jan 7, 2019

coveralls commented Jan 7, 2019

Pull Request Test Coverage Report for Build 307

💛 - Coveralls

coveralls commented Jan 7, 2019 • edited Loading

Pull Request Test Coverage Report for Build 373

💛 - Coveralls

leakingtapan commented Jan 7, 2019

leakingtapan left a comment

Choose a reason for hiding this comment

leakingtapan Jan 15, 2019

Choose a reason for hiding this comment

leakingtapan commented Jan 23, 2019

leakingtapan Jan 24, 2019

Choose a reason for hiding this comment

dkoshkin Jan 25, 2019

Choose a reason for hiding this comment

leakingtapan commented Jan 25, 2019

dkoshkin commented Jan 25, 2019

leakingtapan commented Jan 25, 2019

dkoshkin commented Jan 25, 2019 • edited Loading

leakingtapan commented Jan 25, 2019

k8s-ci-robot commented Jan 25, 2019

leakingtapan commented Jan 25, 2019

dkoshkin commented Jan 7, 2019 •

edited by leakingtapan

Loading

coveralls commented Jan 7, 2019 •

edited

Loading

dkoshkin commented Jan 25, 2019 •

edited

Loading