-
Notifications
You must be signed in to change notification settings - Fork 767
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
set mode and permissions from PVC annotation (nfs.io/createMode,nfs/createUID,nfs/createGID) #121
Conversation
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
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. I understand the commands that are listed here. |
Welcome @lorenzofaresin! |
Please verify again my CLA signature |
…o/createUID,nfs.io/createGID)
703ee23
to
cb203b4
Compare
Please verify again my CLA signature |
if err := os.MkdirAll(fullPath, 0777); err != nil { | ||
createMode := os.FileMode(0777) | ||
annotationCreateMode, exists := metadata.annotations["nfs.io/createMode"] | ||
if exists { |
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.
please format this file with gofmt
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.
fixed
createMode := os.FileMode(0777) | ||
annotationCreateMode, exists := metadata.annotations["nfs.io/createMode"] | ||
if exists { | ||
annotationCreateModeUInt, _ := strconv.ParseUint(annotationCreateMode, 8, 32) |
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.
why is the error here ignored?
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.
fixed
createMode = os.FileMode(annotationCreateModeUInt) | ||
} | ||
|
||
createUID := "0" |
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.
root:root is really the default everywhere?
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.
it looks like the previous pull request behavior
5489de6 Merge pull request kubernetes-sigs#174 from mauriciopoppe/bump-kind-version 0c675d4 Bump kind version to v0.11.1 ef69a88 Merge pull request kubernetes-sigs#173 from nick5616/add-ws2022 44c710c added WS2022 to build platforms 0883be4 Merge pull request kubernetes-sigs#171 from pohly/example-commands 02cda51 build.make: support binaries outside of cmd, with optional go.mod 65922ea Merge pull request kubernetes-sigs#170 from pohly/canary-snapshot-controller c0bdfb3 prow.sh: deploy canary snapshot-controller in canary jobs 0438f15 Merge pull request kubernetes-sigs#167 from c0va23/feature/release-armv7-image 4786f4d Merge pull request kubernetes-sigs#168 from msau42/update-release-prereq 6a2dc64 Remove requirement to be top-level approver. Only maintainers membership is required to do a release 30a4f7b Release armv7 image ac8108f Merge pull request kubernetes-sigs#165 from consideRatio/pr/update-github-links-ref-to-master-to-HEAD 999b483 docs: make github links reference HEAD instead of main fd67069 docs: make github links reference HEAD instead of master c0a4fb1 Merge pull request kubernetes-sigs#164 from anubha-v-ardhan/patch-1 9c6a6c0 Master to main cleanup 682c686 Merge pull request kubernetes-sigs#162 from pohly/pod-name-via-shell-command 36a29f5 Merge pull request kubernetes-sigs#163 from pohly/remove-bazel 68e43ca prow.sh: remove Bazel build support c5f59c5 prow.sh: allow shell commands in CSI_PROW_SANITY_POD 71c810a Merge pull request kubernetes-sigs#161 from pohly/mock-test-fixes 9e438f8 prow.sh: fix mock testing d7146c7 Merge pull request kubernetes-sigs#160 from pohly/kind-update 4b6aa60 prow.sh: update to KinD v0.11.0 7cdc76f Merge pull request kubernetes-sigs#159 from pohly/fix-deployment-selection ef8bd33 prow.sh: more flexible CSI_PROW_DEPLOYMENT, part II 204bc89 Merge pull request kubernetes-sigs#158 from pohly/fix-deployment-selection 61538bb prow.sh: more flexible CSI_PROW_DEPLOYMENT 2b0e6db Merge pull request kubernetes-sigs#157 from humblec/csi-release a2fcd6d Adding myself to csi reviewers group f325590 Merge pull request kubernetes-sigs#149 from pohly/cluster-logs 4b03b30 Merge pull request kubernetes-sigs#155 from pohly/owners a6453c8 owners: introduce aliases ad83def Merge pull request kubernetes-sigs#153 from pohly/fix-image-builds 5561780 build.make: fix image publishng 29bd39b Merge pull request kubernetes-sigs#152 from pohly/bump-csi-test bc42793 prow.sh: use csi-test v4.2.0 b546baa Merge pull request kubernetes-sigs#150 from mauriciopoppe/windows-multiarch-args bfbb6f3 add parameter base_image and addon_image to BUILD_PARAMETERS 2d61d3b Merge pull request kubernetes-sigs#151 from humblec/cm 48e71f0 Replace `which` command ( non standard) with `command -v` builtin feb20e2 prow.sh: collect cluster logs 7b96bea Merge pull request kubernetes-sigs#148 from dobsonj/add-checkpathcmd-to-prow 2d2e03b prow.sh: enable -csi.checkpathcmd option in csi-sanity 09d4151 Merge pull request kubernetes-sigs#147 from pohly/mock-testing 74cfbc9 prow.sh: support mock tests 4a3f110 prow.sh: remove obsolete test suppression 6616a6b Merge pull request kubernetes-sigs#146 from pohly/kubernetes-1.21 510fb0f prow.sh: support Kubernetes 1.21 c63c61b prow.sh: add CSI_PROW_DEPLOYMENT_SUFFIX 51ac11c Merge pull request kubernetes-sigs#144 from pohly/pull-jobs dd54c92 pull-test.sh: test importing csi-release-tools into other repo 7d2643a Merge pull request kubernetes-sigs#143 from pohly/path-setup 6880b0c prow.sh: avoid creating paths unless really running tests bc0504a Merge pull request kubernetes-sigs#140 from jsafrane/remove-unused-k8s-libs 5b1de1a go-get-kubernetes.sh: remove unused k8s libs 49b4269 Merge pull request kubernetes-sigs#120 from pohly/add-kubernetes-release a1e1127 Merge pull request kubernetes-sigs#139 from pohly/kind-for-kubernetes-latest 1c0fb09 prow.sh: use KinD main for latest Kubernetes 1d77cfc Merge pull request kubernetes-sigs#138 from pohly/kind-update-0.10 bff2fb7 prow.sh: KinD 0.10.0 95eac33 Merge pull request kubernetes-sigs#137 from pohly/fix-go-version-check 437e431 verify-go-version.sh: fix check after removal of travis.yml 1748b16 Merge pull request kubernetes-sigs#136 from pohly/go-1.16 ec844ea remove travis.yml, Go 1.16 df76aba Merge pull request kubernetes-sigs#134 from andyzhangx/add-build-arg e314a56 add build-arg ARCH for building multi-arch images, e.g. ARG ARCH FROM k8s.gcr.io/build-image/debian-base-${ARCH}:v2.1.3 7bc70e5 Merge pull request kubernetes-sigs#129 from pohly/squash-documentation e0b02e7 README.md: document usage of --squash 316cb95 Merge pull request kubernetes-sigs#132 from yiyang5055/bugfix/boilerplate 26e2ab1 fix: default boilerplate path 1add8c1 Merge pull request kubernetes-sigs#133 from pohly/kubernetes-1.20-tag 3e811d6 prow.sh: fix "on-master" prow jobs 1d60e77 Merge pull request kubernetes-sigs#131 from pohly/kubernetes-1.20-tag 9f10459 prow.sh: support building Kubernetes for a specific version f7e7ee4 docs: steps for adding testing against new Kubernetes release fe1f284 Merge pull request kubernetes-sigs#121 from kvaps/namespace-check 8fdf0f7 Merge pull request kubernetes-sigs#128 from fengzixu/master 1c94220 fix: fix a bug of csi-sanity a4c41e6 Merge pull request kubernetes-sigs#127 from pohly/fix-boilerplate ece0f50 check namespace for snapshot-controller dbd8967 verify-boilerplate.sh: fix path to script 9289fd1 Merge pull request kubernetes-sigs#125 from sachinkumarsingh092/optional-spelling-boilerplate-checks ad29307 Make the spelling and boilerplate checks optional 5f06d02 Merge pull request kubernetes-sigs#124 from sachinkumarsingh092/fix-spellcheck-boilerplate-tests 48186eb Fix spelling and boilerplate errors 71690af Merge pull request kubernetes-sigs#122 from sachinkumarsingh092/include-spellcheck-boilerplate-tests 981be3f Adding spelling and boilerplate checks. 2bb7525 Merge pull request kubernetes-sigs#117 from fengzixu/master 4ab8b15 use the tag to replace commit of csi-test 5d74e45 change the csi-test import path to v4 7dcd0a9 upgrade csi-test to v4.0.2 git-subtree-dir: release-tools git-subtree-split: 5489de6
Please review |
How ist the status of this PR? Would like to see this merged as i have currently permissio issues with nextcloud du to the 777 permission flag |
Check namespace for snapshot-controller
The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages issues and PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
@k8s-triage-robot: Closed this PR. 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. |
+1 for @terrificsoysauce taking over this PR. |
/reopen |
@lorenzofaresin: Reopened this PR. 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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lorenzofaresin 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 |
a963782
to
70f1a83
Compare
This might be too much work, but could these options be set as parameters to the storage class instead? And then utilize the same templating support that Maybe those parameters can be set with sane defaults, but this would allow flexibility for an cluster admin to ensure that all directories provisioned are a specific mode, or owned by a particular UID/GID, etc. |
Hi @nogweii thanks for your feedback but your request is OOT from current implementation. Please open a new issue if you want. |
@kmova @wongma7 @ashishranjan738 @yonatankahana please review |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
@k8s-triage-robot: Closed this PR. 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. |
I think this is still useful, could someone reopen this and review? |
/reopen |
@yonatankahana: Reopened this PR. 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. |
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
@k8s-triage-robot: Closed this PR. 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. |
Hey everyone, What's prevented this PR from being merged? @yonatankahana Thanks! 😊👍 |
What this PR does / why we need it:
Needed for #41
Which issue(s) this PR fixes (optional, using fixes #(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Needed for #41