-
Notifications
You must be signed in to change notification settings - Fork 611
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[cinder-csi-plugin] Validate volume capabilities in RPC call CreateVolume
#2729
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Niclas Schad <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Welcome @nschad! |
Hi @nschad. 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 Once the patch is verified, the new status will be reflected by the 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-sigs/prow repository. |
@nschad thanks for the PR. can you share a detailed step-by-step instructions on how to reproduce a problem? |
Sure
apiVersion: apps/v1
kind: Deployment
metadata:
name: simple-deployment
spec:
replicas: 2
selector:
matchLabels:
app: simple-app
template:
metadata:
labels:
app: simple-app
spec:
containers:
- name: simple-container
image: nginx
volumeMounts:
- mountPath: /mnt/data
name: simple-volume
volumes:
- name: simple-volume
persistentVolumeClaim:
claimName: simple-pvc
---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
name: simple-pvc
spec:
accessModes:
- ReadWriteMany # RWX access mode
resources:
requests:
storage: 5Gi
In my opinion the volume shouldn't be provisioned in the first place with AccessMode |
I see, but I wonder whether this can be considered as a default behavior for someone else. Do you know what's the behavior of other CSI drivers, e.g. GCE or AWS? UPD: basically I expect that csi-provisioner handles this and doesn't allow to create a |
Yes, for example:
Yeah maybe but it's just not the reality currently. There is the RPC call |
What this PR does / why we need it:
Cinder currently only supports
VolumeCapability_AccessMode_SINGLE_NODE_WRITER
, therefore we should not allow it to provision volumes with AccessModeRWX
. This will only lead to "errors" when replica > 1.Additionally, the spec states (as I understand it) that the
CreateVolume
RPC call must support all specified volume capabilities, which was not the case previously. So this is why I added the check here.Example of Error Message in K8s
Which issue this PR fixes(if applicable):
fixes #
Special notes for reviewers:
Release note: