Adding support for block volumes #380

avalluri · 2019-08-07T12:17:10Z

Signed-off-by: Amarnath Valluri amarnath.valluri@intel.com

okartau · 2019-08-09T06:19:34Z

should we also revert effect of #362 which added refusal all block volume requests

avalluri · 2019-08-13T12:06:43Z

should we also revert effect of #362 which added refusal all block volume requests

Thanks, @okartau. Good point, I reverted the change, let's see if it breaks andy tests

pkg/pmem-csi-driver/nodeserver.go

okartau · 2019-08-22T10:08:05Z

What about deployment examples, should we show provisioning of a block volume ?
Also, should we describe block volume support in README ?

avalluri · 2019-08-22T10:14:09Z

What about deployment examples, should we show provisioning of a block volume ?

I added an example app and PVC part of this change. Do you think need more?

Also, should we describe block volume support in README ?

Yes, I guess we should mention this in ReadMe, I will do that.

avalluri · 2019-08-22T12:18:56Z

Also, should we describe block volume support in README ?

Yes, I guess we should mention this in ReadMe, I will do that.

Updated ReadMe with Block volume usage, @okartau Can you check if it is clear enough.

okartau · 2019-08-22T16:42:36Z

README.md


+#### Note about raw block volumes
+
+Applications can use volumes provisioned by PMEM-CSI as [raw block devices](https://kubernetes.io/blog/2019/03/07/raw-block-volume-support-to-beta/). For provisioning a PMEM volume that supposed to use as raw block device, one has to create a `PersistentVolumeClaim` with `volumeMode: Block`. Check with provided example [PVC](deploy/common/pmem-pvc-block-volume.yaml) and [application](deploy/common/pmem-app-block-volume.yaml) for usage reference.


instead of:
For provisioning a PMEM volume that supposed to use as raw block device
how about simpler:
For provisioning a PMEM volume as a raw block device

similarly:
instead of:
Check with provided example
use:
See example

We can also wait for few more days when @pohly gets back, he can also take a look on these sentences.

I agree with @okartau's proposed language changes.

Those changes are al

Olev Kartau notifications@github.com writes:

Do we already have tests (e.g. from Kubernetes or csi-test) which cover block volumes? while working with #365 I noticed that if we dont respond NOT_SUPPORTED for block volume mode, and we enable VolumeMode suite, then some (just few) more tests are executed. I guess these may be about block volumes.
Can we pull that into this PR?

Now I pulled the commit from #365 that enables the block volume test suite.

avalluri · 2019-09-16T12:17:22Z

@pohly Can you give review comments.

pohly · 2019-09-25T12:51:17Z

Needs to be rebased...

pohly

Do we already have tests (e.g. from Kubernetes or csi-test) which cover block volumes?

okartau · 2019-09-25T13:25:24Z

Do we already have tests (e.g. from Kubernetes or csi-test) which cover block volumes?

while working with #365 I noticed that if we dont respond NOT_SUPPORTED for block volume mode, and we enable VolumeMode suite, then some (just few) more tests are executed.
I guess these may be about block volumes. Needs to be clarified more.

pohly · 2019-09-25T14:57:29Z

Olev Kartau <notifications@github.com> writes:

> Do we already have tests (e.g. from Kubernetes or csi-test) which cover block volumes? while working with #365 I noticed that if we dont respond NOT_SUPPORTED for block volume mode, and we enable VolumeMode suite, then some (just few) more tests are executed. I guess these may be about block volumes.

Can we pull that into this PR?

pohly · 2019-09-26T09:48:24Z

Looks like tests are failing?

https://cloudnative-k8sci.southcentralus.cloudapp.azure.com/blue/organizations/jenkins/pmem-csi/detail/PR-380/11/pipeline

[Fail] PMEM Volumes [Driver: pmem-csi] [Testpattern: Dynamic PV (block volmode)] volumeMode [It] should create sc, pod, pv, and pvc, read/write to the pv, and delete all created resources 

/go/src/github.com/intel/pmem-csi/vendor/k8s.io/kubernetes/test/e2e/storage/testsuites/volumemode.go:282

[Panic!] PMEM Volumes [Driver: pmem-csi] [Testpattern: Dynamic PV (filesystem volmode)] volumeMode [It] should create sc, pod, pv, and pvc, read/write to the pv, and delete all created resources 

/go/src/runtime/panic.go:82

The second one is a timeout.

okartau · 2019-09-26T09:56:29Z

log shows tester tries to run local kubectl, seems some commits from #365 were left out that work around local kubectl?
For some reason I dont see error "kubectl not found on PATH", have we started installing kubectl on tester host? If not, attempt to use local kubectl may be part of problem here.

avalluri · 2019-09-26T10:20:50Z

Looks like tests are failing?

https://cloudnative-k8sci.southcentralus.cloudapp.azure.com/blue/organizations/jenkins/pmem-csi/detail/PR-380/11/pipeline

[Fail] PMEM Volumes [Driver: pmem-csi] [Testpattern: Dynamic PV (block volmode)] volumeMode [It] should create sc, pod, pv, and pvc, read/write to the pv, and delete all created resources 

/go/src/github.com/intel/pmem-csi/vendor/k8s.io/kubernetes/test/e2e/storage/testsuites/volumemode.go:282

[Panic!] PMEM Volumes [Driver: pmem-csi] [Testpattern: Dynamic PV (filesystem volmode)] volumeMode [It] should create sc, pod, pv, and pvc, read/write to the pv, and delete all created resources 

/go/src/runtime/panic.go:82

The second one is a timeout.

The other failure was also timeout, while creating a Pod:

Sep 26 09:10:22.329 FAIL: Unexpected error:
    <*errors.errorString | 0xc0008aa0c0>: {
        s: "pod \"security-context-c5352c9b-e03c-11e9-99ce-0242ac110002\" is not Running: timed out waiting for the condition",
    }
    pod "security-context-c5352c9b-e03c-11e9-99ce-0242ac110002" is not Running: timed out waiting for the condition
occurred
github.com/intel/pmem-csi/vendor/k8s.io/kubernetes/test/e2e/storage/testsuites.(*volumeModeTestSuite).defineTests.func6()
	/go/src/github.com/intel/pmem-csi/vendor/k8s.io/kubernetes/test/e2e/storage/testsuites/volumemode.go:282 +0xbe8
github.com/intel/pmem-csi/test/e2e.RunE2ETests(0xc0003eb000)
	/go/src/github.com/intel/pmem-csi/test/e2e/e2e.go:150 +0xa4
github.com/intel/pmem-csi/test/e2e.TestE2E(0xc0003eb000)
	/go/src/github.com/intel/pmem-csi/test/e2e/e2e_test.go:53 +0x2b
testing.tRunner(0xc0003eb000, 0x1ad4dd8)
	/go/src/testing/testing.go:865 +0xc0
created by testing.(*T).Run
	/go/src/testing/testing.go:916 +0x35a

okartau · 2019-09-27T05:43:40Z

In driver logs we don't have good explanation why pod is not created.
From Volume POV, I see VolumeCreation with mode=block, but there is no PublishVolume for that VolumeID (and I think Stage should not happen for mode=block volumes?).
I suspect the way tester code creates this pod, is different from how other pods are created in other testing. VolumeMode test code uses CreateSecPodWithNodeName to create a pod: is this different from other testing we use? What is this "SELinuxLabel" security context involved?

okartau · 2019-09-27T06:55:33Z

I added debug trace in pv_util.go CreateSecPodWithNodeName() printing all args to MakeSecPod, and it appears this is called twice with almost same args (as per calling code in volumemode.go).
The only difference seems to be in pvclaims: in 1st call VolumeMode=*Filesystem, this succeeds.
2nd call will have VolumeMode=*Block, this fails after 5-minute timeout.

okartau · 2019-09-27T10:34:30Z

I think the error is the one from kubelet emitted close to pod creation failure point:

kubelet pmem-csi-clear-govm-worker2} FailedMapVolume: MapVolume.SetUp failed for volume "pvc-150511aa-e10b-11e9-b34d-0242ac110002" : volumeattachments.storage.k8s.io "csi-2bc0683aaf417707b935a60597d8945cce77abb97a08d8d816addd01b613e70b" is forbidden: User "system:node:pmem-csi-clear-govm-worker2" cannot get resource "volumeattachments" in API group "storage.k8s.io" at the cluster scope: no relationship found between node "pmem-csi-clear-govm-worker2" and this object.

This gives a hint it is a rights/permissions issue.
Search for other discussion by error text leads to
k3s-io/k3s#732

where a work-around was proposed via adding some rights like this:

rules:
- apiGroups:
  - storage.k8s.io
  resources:
  - volumeattachments
  verbs:
  - create
  - watch

Also, it is mentioned that this gets fixed by migration to k8s 1.15 (our CI still runs 1.14, also my trials on stand-alone cluster).

I will re-try in another stand-alone cluster running 1.15, to verify this "fixed in 1.15" claim.
Note that pointed discussion does not touch the aspect why this is related to Block mode

okartau · 2019-09-27T11:34:58Z

confirmed, Block-mode volume creation and publish works in 1.16 cluster (govm-ubuntu cluster with 4xVM).
There are other errors in test suite, but bock volume test does work.

pohly · 2019-10-15T09:53:03Z

@avalluri: what's the plan regarding Kubernetes 1.14? This isn't part of the pre-merge testing, but we will end up testing on it once it's merged.

pohly · 2019-10-16T14:00:36Z

test/e2e/storage/csi_volumes.go

 		// testsuites.InitSnapshottableTestSuite,
 		// testsuites.InitSubPathTestSuite,
 		// testsuites.InitVolumeIOTestSuite,
-		// testsuites.InitVolumeModeTestSuite,


Hmm, did the tests actually run for your in block mode? I only get:

PMEM Volumes /nvme/gopath/src/github.com/intel/pmem-csi/test/e2e/storage/csi_volumes.go:57 [Driver: pmem-csi] /nvme/gopath/src/github.com/intel/pmem-csi/test/e2e/storage/csi_volumes.go:102 [Testpattern: Dynamic PV (block volmode)] volumeMode /nvme/gopath/src/github.com/intel/pmem-csi/vendor/k8s.io/kubernetes/test/e2e/storage/testsuites/base.go:92 should not mount / map unused volumes in a pod [BeforeEach] /nvme/gopath/src/github.com/intel/pmem-csi/vendor/k8s.io/kubernetes/test/e2e/storage/testsuites/volumemode.go:324 Driver pmem-csi doesn't support -- skipping /nvme/gopath/src/github.com/intel/pmem-csi/vendor/k8s.io/kubernetes/test/e2e/storage/testsuites/base.go:151

That's because the default file system (i.e. the empty string) is no longer listed as supported:
1f61a3a

After adding that, it worked for me, also on Kubernetes 1.14, i.e. I couldn't reproduce the problem that @okartau had.

Hmm, did the tests actually run for your in block mode? I only get:

You are right, I realized this when working on ephemeral volumes branch and fixed here : c7f560c

Please cherry-pick c9bf3a4 to fix it in this PR.

pohly

@avalluri: please investigate why it failed on 1.13: #438 (comment)

Also, cherry-pick c9bf3a4 to ensure that that failing test actually runs.

pohly · 2019-10-18T09:47:11Z

README.md

-|--------------------+--------------------------------+ ------------------------
-| 1.13               | CSINodeInfo, CSIDriverRegistry | unsupported <sup>1</sup>
+|--------------------|--------------------------------|----------------
+| 1.13               | CSINodeInfo, CSIDriverRegistry,<br>CSIBlockVolume</br>| unsupported <sup>1</sup>


Why <br> for CSIBlockVolume but not the others?

The column is too wide after adding one more feature gate. So, I just made it multi-line column.

deploy/kubernetes-1.13/pmem-app-block-volume.yaml

avalluri · 2019-10-18T12:27:20Z

@pohly Is it good enough to merge.

pohly

Looks good to me. Let's merge once we have test results.

In commit 2e9fde4, we dropped support for v1.13, but tests are still pointing to storage class files in deploy/kubernetes-1.13. This change fixes this by pointing to deploy/common.

Code changes to support provisioning of raw block volumes. For block volumes driver need not to create a file-system in NodePublishVolume stage. With this change we only support fsdax raw block volumes. Should find a way to support 'dax' raw volume, which is a character device. Signed-off-by: Amarnath Valluri <amarnath.valluri@intel.com>

As now we support block volumes we no more return "unsupported" for block volumes. Hence reverting commit 3fcd755.

Several (all?) of the volumeMode tests run with the default filesystem (i.e. the empty string). If we don't list that as supported, raw block tests are not executed: S [SKIPPING] in Spec Setup (BeforeEach) [0.005 seconds] PMEM Volumes /nvme/gopath/src/github.com/intel/pmem-csi/test/e2e/storage/csi_volumes.go:57 [Driver: pmem-csi] /nvme/gopath/src/github.com/intel/pmem-csi/test/e2e/storage/csi_volumes.go:102 [Testpattern: Dynamic PV (block volmode)] volumes /nvme/gopath/src/github.com/intel/pmem-csi/vendor/k8s.io/kubernetes/test/e2e/storage/testsuites/base.go:92 should store data [BeforeEach] /nvme/gopath/src/github.com/intel/pmem-csi/vendor/k8s.io/kubernetes/test/e2e/storage/testsuites/volumes.go:146 Driver pmem-csi doesn't support -- skipping /nvme/gopath/src/github.com/intel/pmem-csi/vendor/k8s.io/kubernetes/test/e2e/storage/testsuites/base.go:151

avalluri changed the title ~~WIP: Adding support for block volumes~~ Adding support for block volumes Aug 20, 2019

avalluri force-pushed the blockvolume branch from 37ef005 to 82b2c8e Compare August 20, 2019 11:26

avalluri requested review from okartau and pohly and removed request for okartau August 22, 2019 08:01

okartau reviewed Aug 22, 2019

View reviewed changes

pkg/pmem-csi-driver/nodeserver.go Outdated Show resolved Hide resolved

okartau reviewed Aug 22, 2019

View reviewed changes

pkg/pmem-csi-driver/nodeserver.go Outdated Show resolved Hide resolved

avalluri force-pushed the blockvolume branch from 82b2c8e to 78f7fef Compare August 22, 2019 12:15

avalluri force-pushed the blockvolume branch from 78f7fef to a0bb52d Compare August 22, 2019 12:24

okartau reviewed Aug 22, 2019

View reviewed changes

avalluri force-pushed the blockvolume branch from a0bb52d to 0ece180 Compare August 23, 2019 08:49

avalluri force-pushed the blockvolume branch 2 times, most recently from 50a59da to 43cedd4 Compare September 18, 2019 06:02

pohly reviewed Sep 25, 2019

View reviewed changes

avalluri force-pushed the blockvolume branch from 43cedd4 to 4ea6ae7 Compare September 26, 2019 08:24

okartau mentioned this pull request Sep 26, 2019

TRY: blockvolume without local kubectl support, combining #365 and #380 #421

Closed

avalluri force-pushed the blockvolume branch 2 times, most recently from efc6409 to 34a2515 Compare October 7, 2019 06:47

avalluri mentioned this pull request Oct 8, 2019

WIP: Kubernetes v1.16 update #424

Closed

avalluri force-pushed the blockvolume branch 2 times, most recently from 8bea046 to 5ca43e4 Compare October 15, 2019 07:52

pohly reviewed Oct 16, 2019

View reviewed changes

pohly mentioned this pull request Oct 16, 2019

Adding support for block volumes #438

Closed

pohly suggested changes Oct 16, 2019

View reviewed changes

avalluri force-pushed the blockvolume branch from 5ca43e4 to 8cddad9 Compare October 18, 2019 00:44

pohly mentioned this pull request Oct 18, 2019

drop support for Kubernetes 1.13 #442

Merged

avalluri force-pushed the blockvolume branch from 8cddad9 to b905e73 Compare October 18, 2019 09:10

pohly suggested changes Oct 18, 2019

View reviewed changes

avalluri force-pushed the blockvolume branch from b905e73 to 13cd30c Compare October 18, 2019 10:03

pohly approved these changes Oct 18, 2019

View reviewed changes

avalluri and others added 5 commits October 18, 2019 16:40

test: Remove leftover traces of Kubernetes 1.13

02fbde7

In commit 2e9fde4, we dropped support for v1.13, but tests are still pointing to storage class files in deploy/kubernetes-1.13. This change fixes this by pointing to deploy/common.

Revert "Check for Volume Capability.access_type"

01ef372

As now we support block volumes we no more return "unsupported" for block volumes. Hence reverting commit 3fcd755.

test/e2e/storage/csi_volumes.go: enable VolumeMode test suite

4b24659

avalluri force-pushed the blockvolume branch from 13cd30c to 064bdf5 Compare October 18, 2019 14:32

pohly approved these changes Oct 19, 2019

View reviewed changes

pohly merged commit 484ee99 into intel:devel Oct 19, 2019


		#### Note about raw block volumes

		Applications can use volumes provisioned by PMEM-CSI as [raw block devices](https://kubernetes.io/blog/2019/03/07/raw-block-volume-support-to-beta/). For provisioning a PMEM volume that supposed to use as raw block device, one has to create a `PersistentVolumeClaim` with `volumeMode: Block`. Check with provided example [PVC](deploy/common/pmem-pvc-block-volume.yaml) and [application](deploy/common/pmem-app-block-volume.yaml) for usage reference.

Adding support for block volumes #380

Adding support for block volumes #380

Uh oh!

Conversation

avalluri commented Aug 7, 2019

Uh oh!

okartau commented Aug 9, 2019

Uh oh!

avalluri commented Aug 13, 2019

Uh oh!

Uh oh!

Uh oh!

okartau commented Aug 22, 2019

Uh oh!

avalluri commented Aug 22, 2019

Uh oh!

avalluri commented Aug 22, 2019

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!

avalluri commented Sep 16, 2019

Uh oh!

pohly commented Sep 25, 2019

Uh oh!

pohly left a comment

Choose a reason for hiding this comment

Uh oh!

okartau commented Sep 25, 2019

Uh oh!

pohly commented Sep 25, 2019 via email

Uh oh!

pohly commented Sep 26, 2019

Uh oh!

okartau commented Sep 26, 2019

Uh oh!

avalluri commented Sep 26, 2019

Uh oh!

okartau commented Sep 27, 2019

Uh oh!

okartau commented Sep 27, 2019

Uh oh!

okartau commented Sep 27, 2019

Uh oh!

okartau commented Sep 27, 2019

Uh oh!

pohly commented Oct 15, 2019

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!

pohly 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!

Uh oh!

avalluri commented Oct 18, 2019

Uh oh!

pohly left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development