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

allow to set a volume as readOnly (apply only in csi driver) #62

Merged
merged 1 commit into from
Jul 8, 2024

Conversation

jagedn
Copy link
Collaborator

@jagedn jagedn commented Jul 6, 2024

Add a readOnly method in the VolumeSpec

apply only in csi drivers

Tested against of our nfazure, setting secondary volume as readOnly:

"Volumes": {
        "vol_1": {
          "Name": "",
          "Type": "csi",
          "Source": "nextflow-fs-volume",
          "ReadOnly": true,
          "AccessMode": "multi-node-reader-only",
          "AttachmentMode": "file-system",
          "MountOptions": null,
          "PerAlloc": false
        },
        "vol_0": {
          "Name": "",
          "Type": "csi",
          "Source": "nextflow-fs-volume",
          "ReadOnly": false,
          "AccessMode": "multi-node-multi-writer",
          "AttachmentMode": "file-system",
          "MountOptions": null,
          "PerAlloc": false
        }

closes #60

Signed-off-by: Jorge Aguilera <jorge@edn.es>
@jagedn jagedn requested review from abhi18av and matthdsm July 6, 2024 22:45
@jagedn jagedn added the enhancement New feature or request label Jul 6, 2024
Copy link
Member

@abhi18av abhi18av left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to merge - thanks @jagedn !

As this PR addresses #60 (by @jhaezebr ) what's the best way to test this? Are you able to build this locally and test or are you relying upon the latest release cycles?

@jagedn
Copy link
Collaborator Author

jagedn commented Jul 7, 2024

The snippet attached was generated in the nfazure cluster running our validation/run-all --nfazure script

@abhi18av
Copy link
Member

abhi18av commented Jul 8, 2024

Thanks for highlighting this @jagedn , I think that moving forward @tomiles @matthdsm and @jhaezebr should be able to test the plugin with with --local or --nfazure setup and we can merge this already ✅

@abhi18av abhi18av merged commit b7d5ae0 into master Jul 8, 2024
2 checks passed
@abhi18av abhi18av deleted the readonly-volumes branch July 8, 2024 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to mount read only csi volumes
2 participants