[cinder-csi-plugin] support "multiattach" volume type #1073
Conversation
k8s-ci-robot
added
release-note-none
k8s-ci-robot
added
the
size/M
|
Build succeeded.
|
|
Build failed.
|
|
Build failed.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
Build failed.
|
|
Build failed.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
k8s-ci-robot
added
size/L
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
Build failed.
|
|
/test cloud-provider-openstack-multinode-csi-migration-test |
|
@ramineni: The specified target(s) for
Use 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. |
|
@wangxiyuan: The specified target(s) for
Use 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. |
|
Build succeeded.
|
There was a problem hiding this comment.
Do we need to change here https://github.com/kubernetes/cloud-provider-openstack/blob/master/pkg/csi/cinder/driver.go#L84? If yes, how we could give the csi driver deployer the ability to define if the MULTI_NODE_MULTI_WRITER access mode is supported?
docs/using-cinder-csi-plugin.md
Outdated
| name: csi-sc-cinderplugin | ||
| provisioner: cinder.csi.openstack.org | ||
| parameters: | ||
| type: multiattach |
There was a problem hiding this comment.
I'm wondering why it's related to volume type? I thought it's related to accessModes when defining PersistentVolumeClaim.
There was a problem hiding this comment.
@lingxiankong right , there are 2 ways of doing it (either way creating volume type is required if we need to attach to more than one node)
- Support automatically create volume type of multiattach , depending on the access modes that are specified in pvc (ReadWriteMany, ReadOnlyMany..).
- Use the existing field of volume type we already support and create volume of that type.
So, in the initial support we provide in k8s, I thought it would be better to leave it for operator if they really would like to create the volume of type multiattach,(understanding their limitations ) instead of creating it automatically for them which might cause issues later on .
There was a problem hiding this comment.
So is accessModes actually supported in cinder-csi? Is there any difference when user specifies ReadWriteOnce and ReadWriteMany?
There was a problem hiding this comment.
I think leave to operator to decide is a better option here..
if customer has requirement on option 1) above, we can consider it later on
ramineni
changed the title
| @@ -151,7 +155,7 @@ func (cs *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol | |||
| return nil, status.Error(codes.Internal, fmt.Sprintf("DeleteVolume failed with error %v", err)) | |||
There was a problem hiding this comment.
as you change the log from InfoF to error at line 130, maybe consider to update 154 as well
| } | ||
| return "", fmt.Errorf("disk %q is attached to a different compute: %q, should be detached before proceeding", volumeID, volume.Attachments[0].ServerID) | ||
| return "", fmt.Errorf("disk %q is not attached to compute: %q", volumeID, instanceID) |
There was a problem hiding this comment.
not sure this compute is valid ? seems this is virtual machine ? or guest ?
at least compute make me think about compute node...
There was a problem hiding this comment.
right, its compute node
docs/using-cinder-csi-plugin.md
Outdated
| @@ -434,6 +434,24 @@ Following prerequisites needed for volume cloning to work : | |||
|
|
|||
| Sample yamls can be found [here](https://github.com/kubernetes/cloud-provider-openstack/tree/master/examples/cinder-csi-plugin/clone) | |||
|
|
|||
| ### Multi-Attach Volumes | |||
|
|
|||
| To avail the multiattach feature of cinder, specify the 'type' of the volume as 'multiattach' in storageclass definition. | |||
There was a problem hiding this comment.
This sounds like a volume type named multiattach must exist in Cinder, but actually what we need is a volume type which enables multiattach in its extra-spec capability. We should make it clear for end users.
There was a problem hiding this comment.
this is a good suggestion,
Starting from the Queens release the ability to attach a volume to multiple hosts/servers requires that the volume is of a special type that includes an extra-spec capability setting of multiattach=<is> True. You can create the volume type the following way: reflect your suggestion
docs/using-cinder-csi-plugin.md
Outdated
| name: csi-sc-cinderplugin | ||
| provisioner: cinder.csi.openstack.org | ||
| parameters: | ||
| type: multiattach |
There was a problem hiding this comment.
So is accessModes actually supported in cinder-csi? Is there any difference when user specifies ReadWriteOnce and ReadWriteMany?
ramineni
force-pushed
the
multiattach
branch
from
0489de7
to
9534503
Compare
k8s-ci-robot
added
size/L
Correct
accessModes specify the capabilities that should be supported by volume , right now, unless he specify explicit volume type of multiattach , there is no affect of that |
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
Build succeeded.
|
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
@jichenjc PTAL |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jichenjc 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
@ramineni Thanks for implementing this. As you did this, I assume that you also use this in production in some capacity. Are you able to share some insights how you manage a cluster filesystem ontop of this? |
|
Did anyone managed to run this in production with k8s creating the cluster filesystem on the attached volume? |
What this PR does / why we need it:
When the type of the volume is of "multiattach", it can be attached to multiple instances.
Currently type of the volume can be specified as part of parameters['type'] of storageclass , https://github.com/kubernetes/cloud-provider-openstack/blob/master/pkg/csi/cinder/controllerserver.go#L63
This PR enables the same for multiattach type as well .
Which issue this PR fixes(if applicable):
fixes #729
Special notes for reviewers:
Specify the 'type' as 'multiattach' in storageclass as specified in example doc.
type should exist in backend.
Release note: