Raw volumes coverage improvements #269

taaraora · 2020-05-31T22:54:39Z

Raw volumes were added to the next cases:

ExpandVolume [Controller Server]
CreateSnapshot [Controller Server]
DeleteSnapshot [Controller Server]
ListSnapshots [Controller Server]
ControllerPublishVolume
CreateVolume
- from snapshot
- from source volume

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test

/kind feature

/kind flake

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

--csi.testvolumeaccesstype=block is now also used by the following tests: ExpandVolume [Controller Server], CreateSnapshot [Controller Server], DeleteSnapshot [Controller Server], ListSnapshots [Controller Server], ControllerPublishVolume, Create Volume from snapshot, Clone Volume

k8s-ci-robot · 2020-05-31T22:54:47Z

Hi @taaraora. Thanks for your PR.

I'm waiting for a kubernetes-csi 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.

pohly · 2020-06-02T08:52:28Z

pkg/sanity/sanity.go

@@ -190,7 +190,7 @@ func NewTestConfig() TestConfig {
 		StagingPath:          os.TempDir() + "/csi-staging",
 		CreatePathCmdTimeout: 10 * time.Second,
 		RemovePathCmdTimeout: 10 * time.Second,
-		TestVolumeSize:       10 * 1024 * 1024 * 1024, // 10 GiB
+		TestVolumeSize:       1 * 1024 * 1024 * 1024, // 10 GiB


Comment also needs to be updated.

I had not enough space on my k8s cluster and didn't find flag to set test volume size at that time, that's why I changed it. But this change is not intended to be in final patch and I finally found --csi.testvolumesize flag.
It is rolled back now.

pohly · 2020-06-02T08:53:23Z

/ok-to-test

Please add some short release note text to the PR description.

- ExpandVolume [Controller Server] - CreateSnapshot [Controller Server] - DeleteSnapshot [Controller Server] - ListSnapshots [Controller Server] - ControllerPublishVolume - CreateVolume - from snapshot - from source volume Signed-off-by: Dmytro Tsapko <dmytro.tsapko@gmail.com>

taaraora · 2020-06-03T11:54:29Z

/retest

taaraora · 2020-06-03T12:10:02Z

@pohly release note text was added.

msau42 · 2020-06-04T02:10:33Z

/assign @jsafrane

pohly · 2020-06-04T07:27:51Z

pkg/sanity/controller.go

@@ -416,7 +416,8 @@ var _ = DescribeSanity("Controller Service [Controller Server]", func(sc *TestCo
 			Expect(serverError.Code()).To(Equal(codes.InvalidArgument))
 		})

-		It("should return appropriate values SingleNodeWriter NoCapacity Type:Mount", func() {
+		// TODO: whether CreateVolume request with no capacity should fail or not depends on driver implementation


The commit message mentions added new tests, but all I find in the diff are changes to existing tests?

In this case here, the test doesn't even get changed, just its name.

I'm confused, sorry.

The idea of this PR is to make possible running all the above-mentioned test cases for raw volumes.
Currently when sanity tests are executed with the flag --csi.testvolumeaccesstype=block above mentioned test cases are still executed for mount volumes.
My patch is based on the point, that all those cases use MakeCreateVolumeReq function for volume creation and I've just made it to produce raw volume when tests are executed for raw volumes.

@pohly In commit message I wanted to say that raw volumes support was added to all those tests.
I've checked that list of tests cases and sure that when tests are executed with --csi.testvolumeaccesstype=block flag, all calls which bear volume access type are constructed with proper volume access type.

pohly

I still find the release notes confusing:

Raw volumes tests were introduced for the next cases: ExpandVolume [Controller Server], CreateSnapshot [Controller Server], DeleteSnapshot [Controller Server], ListSnapshots [Controller Server], ControllerPublishVolume, Create Volume from snapshot, Clone Volume.

Is the following description correct? If so, please update the release notes:

--csi.testvolumeaccesstype=block is now also used by the following tests: ExpandVolume [Controller Server], CreateSnapshot [Controller Server], DeleteSnapshot [Controller Server], ListSnapshots [Controller Server], ControllerPublishVolume, Create Volume from snapshot, Clone Volume

taaraora · 2020-06-08T17:14:44Z

I agree I've updated release note.

pohly

/lgtm

pohly · 2020-06-08T17:24:47Z

@jsafrane do you want to approve?

pohly · 2020-06-09T08:14:32Z

/approve

k8s-ci-robot · 2020-06-09T08:14:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pohly, taaraora

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~~ [pohly]

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

k8s-ci-robot requested review from msau42 and saad-ali May 31, 2020 22:54

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 May 31, 2020

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 31, 2020

taaraora force-pushed the raw-volumes-update branch 2 times, most recently from af9d6fb to e3f6925 Compare June 1, 2020 23:57

pohly reviewed Jun 2, 2020

View reviewed changes

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 Jun 2, 2020

taaraora force-pushed the raw-volumes-update branch from e3f6925 to 6f6334d Compare June 3, 2020 11:49

taaraora changed the title ~~[WIP] Raw volumes coverage improvements~~ Raw volumes coverage improvements Jun 3, 2020

taaraora requested a review from pohly June 3, 2020 17:29

k8s-ci-robot assigned jsafrane Jun 4, 2020

pohly reviewed Jun 4, 2020

View reviewed changes

pohly reviewed Jun 8, 2020

View reviewed changes

taaraora requested a review from pohly June 8, 2020 17:13

pohly reviewed Jun 8, 2020

View reviewed changes

k8s-ci-robot assigned pohly Jun 8, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 8, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 9, 2020

k8s-ci-robot merged commit 2302fbd into kubernetes-csi:master Jun 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Raw volumes coverage improvements #269

Raw volumes coverage improvements #269

taaraora commented May 31, 2020 •

edited

Loading

k8s-ci-robot commented May 31, 2020

pohly Jun 2, 2020

taaraora Jun 3, 2020

pohly commented Jun 2, 2020

taaraora commented Jun 3, 2020

taaraora commented Jun 3, 2020

msau42 commented Jun 4, 2020

pohly Jun 4, 2020

taaraora Jun 4, 2020

taaraora Jun 4, 2020

pohly left a comment

taaraora commented Jun 8, 2020

pohly left a comment

pohly commented Jun 8, 2020

pohly commented Jun 9, 2020

k8s-ci-robot commented Jun 9, 2020

Raw volumes coverage improvements #269

Raw volumes coverage improvements #269

Conversation

taaraora commented May 31, 2020 • edited Loading

k8s-ci-robot commented May 31, 2020

pohly Jun 2, 2020

Choose a reason for hiding this comment

taaraora Jun 3, 2020

Choose a reason for hiding this comment

pohly commented Jun 2, 2020

taaraora commented Jun 3, 2020

taaraora commented Jun 3, 2020

msau42 commented Jun 4, 2020

pohly Jun 4, 2020

Choose a reason for hiding this comment

taaraora Jun 4, 2020

Choose a reason for hiding this comment

taaraora Jun 4, 2020

Choose a reason for hiding this comment

pohly left a comment

Choose a reason for hiding this comment

taaraora commented Jun 8, 2020

pohly left a comment

Choose a reason for hiding this comment

pohly commented Jun 8, 2020

pohly commented Jun 9, 2020

k8s-ci-robot commented Jun 9, 2020

taaraora commented May 31, 2020 •

edited

Loading