Skip to content
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

Support for devAttribs #75

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

pavels
Copy link

@pavels pavels commented Dec 19, 2023

Allow to set DevAttribs for LUN creation.

This allows user to set for example emulate_tpu and enable space reclamation.

Example storage class would look like this:

apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
  name: nas-iscsi
provisioner: csi.san.synology.com
parameters:
  dsm: 'my-nas'
  location: '/volume1'
  fsType: 'xfs'
  formatOptions: '-K'
  devAttribs: 'emulate_tpu'
reclaimPolicy: Retain
allowVolumeExpansion: true

I also added support to explicitly set flag to 0 (the strange syntax with - suffix is actually similar to how labels and annotations are set using kubectl)

@gjrtimmer
Copy link

@chihyuwu Could you please look at this, I hope it gets merged soon.

@gjrtimmer
Copy link

@pavels Would it be possible to also set the directory chmod with this? So, containers like PostgreSQL use a separate user. I would like the ability for the mounted folder to be set to 777 so that containers like PostgreSQL do not need an initializing task that sets the permissions.

I was thinking, can it be related to the func createTargetMountPath which sets 0750 on the mounted folder. Unfortunately I do not know yet how to turn it into a parameter. Can you give some advice?

@pavels
Copy link
Author

pavels commented Feb 22, 2024

@pavels Would it be possible to also set the directory chmod with this? So, containers like PostgreSQL use a separate user. I would like the ability for the mounted folder to be set to 777 so that containers like PostgreSQL do not need an initializing task that sets the permissions.

I was thinking, can it be related to the func createTargetMountPath which sets 0750 on the mounted folder. Unfortunately I do not know yet how to turn it into a parameter. Can you give some advice?

This PR is specifically about creation flags, which are specific to the Synology CSI driver and therefore must be configured on driver level (StorageClass config gets written to PersistentVolume object on creation and from there, it is picked up by the specific CSI driver)

Permissions on the other hand is handled by kubernetes itself - see https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#configure-volume-permission-and-ownership-change-policy-for-pods

There is an option to delegate ownership change to CSI driver if the driver supports it - if you take a look at

// csi.NodeServiceCapability_RPC_VOLUME_MOUNT_GROUP,
- NodeServiceCapability_RPC_VOLUME_MOUNT_GROUP is commented out, therefore kubernetes will use internal mechanism to set permissions (which is probably fine, this as i understand is primarily intended for drivers, that doesn't support changing permission using chown / chmod and where permissions must be set as mount parameters or something similar - or some kind of optimizations on CSI driver level)

the 0750 in createTargetMountPath is probably just a conservative default and should be overwritten om pod startup when securityContext is set

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants