Fix Portworx CSI storage capabilities #2789

arnongilboa · 2023-07-05T16:11:11Z

What this PR does / why we need it:
Add preferred ReadWriteMany, Filesystem capability to the pxd.portworx.com provisioner.

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

Special notes for your reviewer:

Release note:

Add preferred ReadWriteMany, Filesystem capability to the pxd.portworx.com provisioner

akalenyu · 2023-07-18T15:00:45Z

pkg/storagecapabilities/storagecapabilities.go

@@ -89,7 +89,7 @@ var CapabilitiesByProvisionerKey = map[string][]StorageCapabilities{
 	"pxd.openstorage.org/shared": createOpenStorageSharedVolumeCapabilities(),
 	"pxd.openstorage.org":        createRWOBlockAndFilesystemCapabilities(),
 	"pxd.portworx.com/shared":    createOpenStorageSharedVolumeCapabilities(),
-	"pxd.portworx.com":           createRWOBlockAndFilesystemCapabilities(),
+	"pxd.portworx.com":           {{rwx, file}, {rwo, block}, {rwo, file}},


Are we sure this can be static? no dependency on shared=true?
https://docs.portworx.com/operations/operate-kubernetes/storage-operations/kubernetes-storage-101/volumes/#readwritemany-and-readwriteonce

no need for shared: true in the storage class parameters for it to be ReadWriteMany, as far as I saw.
I used a PVC of Filesystem ReadWriteMany with px-csi-db SC.

aglitke · 2023-08-08T15:19:11Z

pkg/storagecapabilities/storagecapabilities.go

@@ -89,7 +89,7 @@ var CapabilitiesByProvisionerKey = map[string][]StorageCapabilities{
 	"pxd.openstorage.org/shared": createOpenStorageSharedVolumeCapabilities(),
 	"pxd.openstorage.org":        createRWOBlockAndFilesystemCapabilities(),
 	"pxd.portworx.com/shared":    createOpenStorageSharedVolumeCapabilities(),


Noticed that we are saying RWX/Block is supported but https://github.com/libopenstorage/openstorage/blob/master/csi/controller.go#L408-L412 begs to differ?

Seems we may need to revamp and correct the portworx caps and backport to previous releases too.

Ok, so the code impliciltly says RWX/File,RWO/Block,RWO/File are supported in the shared provisioners?

awels · 2023-08-25T18:25:37Z

/test pull-containerized-data-importer-e2e-upg

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

jpeimer · 2023-09-04T10:17:40Z

pkg/storagecapabilities/storagecapabilities.go

 	"pxd.portworx.com/shared":    createOpenStorageSharedVolumeCapabilities(),
-	"pxd.portworx.com":           createRWOBlockAndFilesystemCapabilities(),
+	"pxd.portworx.com":           createOpenStorageSharedVolumeCapabilities(),


Created DVs on pxd.portworx.com - all these Succeeded: {RWX, FS}, {RWO, Block}, {RWO, FS}

kubevirt-bot · 2023-09-04T10:20:11Z

@jpeimer: changing LGTM is restricted to collaborators

In response to this:

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.

akalenyu · 2023-09-04T10:30:26Z

/cc @pureneelesh @luayalem

Hey! We are trying to provide some defaults for Portworx storage classes
Does this look right?

containerized-data-importer/pkg/storagecapabilities/storagecapabilities.go

Lines 298 to 304 in e5ba88e

    
           func createOpenStorageSharedVolumeCapabilities() []StorageCapabilities { 
        
           	return []StorageCapabilities{ 
        
           		{rwx, file}, 
        
           		{rwo, block}, 
        
           		{rwo, file}, 
        
           	} 
        
           }

Also IIUC "shared" is deprecated, and our code still filters based on that

containerized-data-importer/pkg/storagecapabilities/storagecapabilities.go

Lines 234 to 240 in e5ba88e

    
           "pxd.portworx.com": func(sc *storagev1.StorageClass) string { 
        
           	//https://docs.portworx.com/portworx-install-with-kubernetes/storage-operations/csi/volumelifecycle/#create-shared-csi-enabled-volumes 
        
           	val := sc.Parameters["shared"] 
        
           	if val == "true" { 
        
           		return "pxd.portworx.com/shared" 
        
           	} 
        
           	return "pxd.portworx.com"

kubevirt-bot · 2023-09-04T10:30:31Z

@akalenyu: GitHub didn't allow me to request PR reviews from the following users: luayalem, pureneelesh.

Note that only kubevirt members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @pureneelesh @luayalem

Hey! We are trying to provide some defaults for Portworx storage classes
Does this look right?

containerized-data-importer/pkg/storagecapabilities/storagecapabilities.go

Lines 298 to 304 in e5ba88e

func createOpenStorageSharedVolumeCapabilities() []StorageCapabilities {

return []StorageCapabilities{

{rwx, file},

{rwo, block},

{rwo, file},

}

}

Also IIUC "shared" is deprecated, and our code still filters based on that

containerized-data-importer/pkg/storagecapabilities/storagecapabilities.go

Lines 234 to 240 in e5ba88e

"pxd.portworx.com": func(sc *storagev1.StorageClass) string {

//https://docs.portworx.com/portworx-install-with-kubernetes/storage-operations/csi/volumelifecycle/#create-shared-csi-enabled-volumes

val := sc.Parameters["shared"]

if val == "true" {

return "pxd.portworx.com/shared"

}

return "pxd.portworx.com"

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.

akalenyu · 2023-09-04T13:58:44Z

Following offline discussion, we will pursue pinpointed portworx variation filtering outside of this PR.
For now, this is fixing an error (rwx, block) and will benefit users.

/approve

kubevirt-bot · 2023-09-04T14:05:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akalenyu

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

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

akalenyu · 2023-09-05T09:06:59Z

/lgtm

akalenyu · 2023-09-05T09:37:33Z

/test pull-containerized-data-importer-e2e-upg

akalenyu · 2023-09-05T16:15:42Z

/cherrypick release-v1.57

kubevirt-bot · 2023-09-05T16:16:43Z

@akalenyu: new pull request created: #2890

In response to this:

/cherrypick release-v1.57

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.

arnongilboa · 2023-09-14T09:43:35Z

/cherrypick release-v1.56

kubevirt-bot · 2023-09-14T09:44:14Z

@arnongilboa: #2789 failed to apply on top of branch "release-v1.56":

Applying: Fix Portworx CSI storage capabilities
Using index info to reconstruct a base tree...
M	pkg/storagecapabilities/storagecapabilities.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/storagecapabilities/storagecapabilities.go
CONFLICT (content): Merge conflict in pkg/storagecapabilities/storagecapabilities.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Fix Portworx CSI storage capabilities
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-v1.56

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.

Manual backport of kubevirt#2789 Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

Manual backport of kubevirt#2789, plus minor alignments with main branch. Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

Manual backport of #2789, plus minor alignments with main branch. Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

Manual backport of kubevirt#2789, plus minor alignments with main branch. Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

Manual backport of #2789, plus minor alignments with main branch. Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

kubevirt-bot requested review from aglitke and maya-r July 5, 2023 16:11

arnongilboa changed the title ~~[WIP] Add rwx,file capability to pxd.portworx.com provisioner~~ Add rwx,file capability to pxd.portworx.com provisioner Jul 11, 2023

kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 11, 2023

akalenyu reviewed Jul 18, 2023

View reviewed changes

aglitke reviewed Aug 8, 2023

View reviewed changes

Fix Portworx CSI storage capabilities

e5ba88e

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

arnongilboa changed the title ~~Add rwx,file capability to pxd.portworx.com provisioner~~ Fix Portworx CSI storage capabilities Sep 3, 2023

arnongilboa force-pushed the portworx_rwx_file branch from ddc55f5 to e5ba88e Compare September 3, 2023 07:44

jpeimer approved these changes Sep 4, 2023

View reviewed changes

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 4, 2023

kubevirt-bot assigned akalenyu Sep 5, 2023

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 5, 2023

kubevirt-bot merged commit 2577211 into kubevirt:main Sep 5, 2023
18 checks passed

kubevirt-bot mentioned this pull request Sep 5, 2023

[release-v1.57] Fix Portworx CSI storage capabilities #2890

Merged

arnongilboa added a commit to arnongilboa/containerized-data-importer that referenced this pull request Sep 14, 2023

[release-v1.56] Fix Portworx CSI storage capabilities

25880c5

Manual backport of kubevirt#2789 Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

arnongilboa mentioned this pull request Sep 14, 2023

[release-v1.56] Fix Portworx CSI storage capabilities #2897

Merged

kubevirt-bot pushed a commit that referenced this pull request Sep 14, 2023

[release-v1.56] Fix Portworx CSI storage capabilities (#2897)

5ff3c92

Manual backport of #2789, plus minor alignments with main branch. Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

arnongilboa mentioned this pull request Sep 19, 2023

[release-v1.55] Fix Portworx CSI storage capabilities #2902

Merged

kubevirt-bot pushed a commit that referenced this pull request Sep 20, 2023

[release-v1.55] Fix Portworx CSI storage capabilities (#2902)

7b31b51

Manual backport of #2789, plus minor alignments with main branch. Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Portworx CSI storage capabilities #2789

Fix Portworx CSI storage capabilities #2789

arnongilboa commented Jul 5, 2023 •

edited

akalenyu Jul 18, 2023

arnongilboa Jul 18, 2023

orenc1 Jul 18, 2023

aglitke Aug 8, 2023

aglitke Aug 8, 2023

arnongilboa Aug 14, 2023

awels commented Aug 25, 2023

jpeimer Sep 4, 2023

kubevirt-bot commented Sep 4, 2023

akalenyu commented Sep 4, 2023

kubevirt-bot commented Sep 4, 2023

akalenyu commented Sep 4, 2023

kubevirt-bot commented Sep 4, 2023

akalenyu commented Sep 5, 2023

akalenyu commented Sep 5, 2023

akalenyu commented Sep 5, 2023

kubevirt-bot commented Sep 5, 2023

arnongilboa commented Sep 14, 2023

kubevirt-bot commented Sep 14, 2023

Fix Portworx CSI storage capabilities #2789

Fix Portworx CSI storage capabilities #2789

Conversation

arnongilboa commented Jul 5, 2023 • edited

akalenyu Jul 18, 2023

Choose a reason for hiding this comment

arnongilboa Jul 18, 2023

Choose a reason for hiding this comment

orenc1 Jul 18, 2023

Choose a reason for hiding this comment

aglitke Aug 8, 2023

Choose a reason for hiding this comment

aglitke Aug 8, 2023

Choose a reason for hiding this comment

arnongilboa Aug 14, 2023

Choose a reason for hiding this comment

awels commented Aug 25, 2023

jpeimer Sep 4, 2023

Choose a reason for hiding this comment

kubevirt-bot commented Sep 4, 2023

akalenyu commented Sep 4, 2023

kubevirt-bot commented Sep 4, 2023

akalenyu commented Sep 4, 2023

kubevirt-bot commented Sep 4, 2023

akalenyu commented Sep 5, 2023

akalenyu commented Sep 5, 2023

akalenyu commented Sep 5, 2023

kubevirt-bot commented Sep 5, 2023

arnongilboa commented Sep 14, 2023

kubevirt-bot commented Sep 14, 2023

arnongilboa commented Jul 5, 2023 •

edited