Created unit tests for GCE cloud provider storage interface. #44884

verult · 2017-04-24T23:22:46Z

Currently covers CreateDisk and DeleteDisk, GetAutoLabelsForPD
Created ServiceManager interface in gce.go to facilitate mocking in tests.

What this PR does / why we need it:
Increasing test coverage for GCE Persistent Disk.

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

Release note:

NONE

k8s-reviewable · 2017-04-24T23:22:53Z

This change is

k8s-ci-robot · 2017-04-24T23:22:53Z

Hi @verult. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot 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.

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. I understand the commands that are listed here.

verult · 2017-04-25T00:30:28Z

/approve

verult · 2017-04-25T00:30:56Z

/approve

verult · 2017-04-25T00:37:51Z

/assign @msau42 @jingxu97

cblecker · 2017-04-25T00:44:44Z

@k8s-bot ok to test

msau42 · 2017-04-27T02:33:29Z

pkg/cloudprovider/providers/gce/gce.go

+}
+
+type ServiceManager interface {
+	CreateDisk(


Can you add comments to each functions in the interface?

msau42 · 2017-04-27T02:42:53Z

pkg/cloudprovider/providers/gce/gce_disks_test.go

+		t.Error("Ops used in WaitForZoneOp does not match what's returned by CreateDisk.")
+	}
+	// Partial check of equality between disk description sent to GCE and parameters of method.
+	if fakeManager.diskToCreate.Name != diskName || fakeManager.diskToCreate.SizeGb != sizeGb {


I would also verify the tags, and diskTypeUri fields. And make each field a separate check.

msau42 · 2017-04-27T02:46:00Z

pkg/cloudprovider/providers/gce/gce_disks_test.go

+			t.Errorf("Error creating disk in zone '%v'; error: \"%v\"", zone, err)
+		}
+	}
+}


Another test case for CreateDisk is the disk was already created.

msau42 · 2017-04-27T20:06:53Z

Also instead of calling it GCE storage class, I would call it GCE cloud provider storage interface. We have a Kubernetes concept called storage classes, which is totally different than this.

msau42 · 2017-05-19T17:44:54Z

pkg/cloudprovider/providers/gce/gce.go

+
+type ServiceManager interface {
+	// Creates a new persistent disk on GCE with the given disk spec.
+	CreateDisk(


To conform with the rest of the files, can you put all the arguments on the same line?

verult · 2017-05-24T22:49:10Z

/release-note-none

- Currently covers CreateDisk and DeleteDisk, GetAutoLabelsForPD - Created ServiceManager interface in gce.go to facilitate mocking in tests.

k8s-ci-robot · 2017-05-24T23:33:02Z

@verult: The following test(s) failed:

Test name	Commit	Details	Rerun command
pull-kubernetes-federation-e2e-gce	`2141b0f`	link	`@k8s-bot pull-kubernetes-federation-e2e-gce test this`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

msau42 · 2017-05-24T23:47:41Z

@k8s-bot pull-kubernetes-unit test this

msau42 · 2017-05-25T02:08:01Z

/lgtm

jingxu97 · 2017-05-26T18:10:01Z

/approve

k8s-github-robot · 2017-05-26T18:10:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jingxu97, msau42, verult

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

~~pkg/cloudprovider/providers/gce/OWNERS~~ [jingxu97]

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

k8s-github-robot · 2017-05-26T19:58:03Z

Automatic merge from submit-queue (batch tested with PRs 46383, 45645, 45923, 44884, 46294)

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 24, 2017

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 Apr 24, 2017

k8s-github-robot assigned luxas and caesarxuchao Apr 24, 2017

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Apr 24, 2017

k8s-ci-robot assigned jingxu97 and msau42 Apr 25, 2017

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Apr 25, 2017

caesarxuchao removed their assignment Apr 25, 2017

luxas assigned roberthbailey and unassigned luxas Apr 25, 2017

verult force-pushed the master branch 3 times, most recently from 0dfb427 to e818754 Compare April 26, 2017 20:01

msau42 reviewed Apr 27, 2017

View reviewed changes

roberthbailey assigned saad-ali and unassigned roberthbailey Apr 27, 2017

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 28, 2017

verult force-pushed the master branch from e818754 to ec580de Compare April 28, 2017 20:53

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 28, 2017

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 6, 2017

verult force-pushed the master branch from ec580de to b077d24 Compare May 16, 2017 23:46

verult changed the title ~~Created unit tests for GCE storage class.~~ Created unit tests for GCE cloud provider storage interface. May 16, 2017

k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 16, 2017

verult force-pushed the master branch from b077d24 to 5995fd8 Compare May 17, 2017 01:01

msau42 reviewed May 19, 2017

View reviewed changes

verult force-pushed the master branch from 5995fd8 to d75e2e8 Compare May 24, 2017 17:41

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels May 24, 2017

Created unit tests for GCE cloud provider storage interface.

2141b0f

- Currently covers CreateDisk and DeleteDisk, GetAutoLabelsForPD - Created ServiceManager interface in gce.go to facilitate mocking in tests.

verult force-pushed the master branch from d75e2e8 to 2141b0f Compare May 24, 2017 22:50

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

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 26, 2017

k8s-github-robot merged commit f8cfeef into kubernetes:master May 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Created unit tests for GCE cloud provider storage interface. #44884

Created unit tests for GCE cloud provider storage interface. #44884

verult commented Apr 24, 2017 •

edited

k8s-reviewable commented Apr 24, 2017

k8s-ci-robot commented Apr 24, 2017

verult commented Apr 25, 2017

verult commented Apr 25, 2017

verult commented Apr 25, 2017

cblecker commented Apr 25, 2017

msau42 Apr 27, 2017

msau42 Apr 27, 2017

msau42 Apr 27, 2017

msau42 commented Apr 27, 2017

msau42 May 19, 2017

verult commented May 24, 2017

k8s-ci-robot commented May 24, 2017 •

edited

msau42 commented May 24, 2017

msau42 commented May 25, 2017

jingxu97 commented May 26, 2017

k8s-github-robot commented May 26, 2017

k8s-github-robot commented May 26, 2017

Created unit tests for GCE cloud provider storage interface. #44884

Created unit tests for GCE cloud provider storage interface. #44884

Conversation

verult commented Apr 24, 2017 • edited

k8s-reviewable commented Apr 24, 2017

k8s-ci-robot commented Apr 24, 2017

verult commented Apr 25, 2017

verult commented Apr 25, 2017

verult commented Apr 25, 2017

cblecker commented Apr 25, 2017

msau42 Apr 27, 2017

Choose a reason for hiding this comment

msau42 Apr 27, 2017

Choose a reason for hiding this comment

msau42 Apr 27, 2017

Choose a reason for hiding this comment

msau42 commented Apr 27, 2017

msau42 May 19, 2017

Choose a reason for hiding this comment

verult commented May 24, 2017

k8s-ci-robot commented May 24, 2017 • edited

msau42 commented May 24, 2017

msau42 commented May 25, 2017

jingxu97 commented May 26, 2017

k8s-github-robot commented May 26, 2017

k8s-github-robot commented May 26, 2017

verult commented Apr 24, 2017 •

edited

k8s-ci-robot commented May 24, 2017 •

edited