-
Notifications
You must be signed in to change notification settings - Fork 63
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
feat: Add support to specify file permissions for pvc hostpaths #157
base: develop
Are you sure you want to change the base?
Conversation
Sorry for last PR with a bad rebase when signing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @sushiMix. There is already one file-mode/file-mode API in place over at the openebs/dynamic-nfs-provisioner repo.
PR link: openebs-archive/dynamic-nfs-provisioner#125
While I don't have any issues with this implementation, it'd be useful to have a stable cas-config API across openebs storage engines. Also, the above implemetation supports file-permissions as well, which may be useful to your use-case.
ef56055
to
584d3e1
Compare
Hello @niladrih I did a test with storage class and then pvc annotation configuration. After I did rebase and squashed the commits. I only support the mode changed. The user and group are left to kubelet behavior (FsGroup is supported for local volume in kubernetes). Need a bit more code change to update the command sent. |
Great! Could you also add a documentation .md file in the docs directory with instructions on using this feature? |
Codecov Report
@@ Coverage Diff @@
## develop #157 +/- ##
===========================================
- Coverage 38.00% 37.61% -0.40%
===========================================
Files 36 36
Lines 3365 3400 +35
===========================================
Hits 1279 1279
- Misses 2004 2039 +35
Partials 82 82
|
I updated the QuickStart, do you need a more detailed documentation? @niladrih I added a specific page. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the changes @sushiMix! I have left some comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, the enabled key is not required here. Mentioning the details in 'data' is good enough.
86246dc
to
815a205
Compare
@niladrih Happy New Year, I had time to perform code modification. |
Signed-off-by: sushiMix <[email protected]>
Co-authored-by: Niladri Halder <[email protected]> Signed-off-by: sushiMix <[email protected]>
Co-authored-by: Niladri Halder <[email protected]> Signed-off-by: sushiMix <[email protected]>
Co-authored-by: Niladri Halder <[email protected]> Signed-off-by: sushiMix <[email protected]>
Co-authored-by: Niladri Halder <[email protected]> Signed-off-by: sushiMix <[email protected]>
Signed-off-by: sushiMix <[email protected]>
Signed-off-by: sushiMix <[email protected]>
Hi @sushiMix. Could you add ginkgo tests to this PR to test your feature? You could use https://github.com/openebs-archive/dynamic-nfs-provisioner/blob/develop/tests/nfs_server_file_permissions_test.go as an example in this case. Let me know if you need help with this. Also, I am sorry I haven't been able to review this! |
Pull Request template
Why is this PR required? What issue does it fix?:
Fixes #95.
What this PR does?:
It allows to configure default directory mode using LocaMode entry in cas.openebs.io/config.
Does this PR require any upgrade changes?: No
If the changes in this PR are manually verified, list down the scenarios covered::
check the file mode of the created folder (0770 instead of 0777)
Any additional information for your reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs
Checklist:
<type>(<scope>): <subject>
PLEASE REMOVE BELOW INFORMATION BEFORE SUBMITTING
IMPORTANT: Please review the CONTRIBUTING.md file for detailed contributing guidelines.