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

add PVName as volume #157

Closed

Conversation

robkooper
Copy link

This relates to issue #149 and allows to use the PVName in the path pattern.

You can now use ${.PVC.namespace}-${.PVC.name}-${.PVC.volume} as the pathPattern to have the exact same volume name as the default.

This relates to issue kubernetes-sigs#149 and allows to use the PVName in the path pattern.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 12, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: robkooper
To complete the pull request process, please assign jackielii after the PR has been reviewed.
You can assign the PR to them by writing /assign @jackielii in a comment when ready.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 12, 2021
spiffxp added a commit to spiffxp/nfs-subdir-external-provisioner that referenced this pull request Nov 30, 2021
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
@robkooper
Copy link
Author

/assign @jackielii

@@ -119,7 +119,7 @@ To disable leader election, define an env variable named ENABLE_LEADER_ELECTION
| --------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :--------------------------------------------------------------: |
| onDelete | If it exists and has a delete value, delete the directory, if it exists and has a retain value, save the directory. | will be archived with name on the share: `archived-<volume.Name>` |
| archiveOnDelete | If it exists and has a false value, delete the directory. if `onDelete` exists, `archiveOnDelete` will be ignored. | will be archived with name on the share: `archived-<volume.Name>` |
| pathPattern | Specifies a template for creating a directory path via PVC metadata's such as labels, annotations, name or namespace. To specify metadata use `${.PVC.<metadata>}`. Example: If folder should be named like `<pvc-namespace>-<pvc-name>`, use `${.PVC.namespace}-${.PVC.name}` as pathPattern. | n/a |
| pathPattern | Specifies a template for creating a directory path via PVC metadata's such as labels, annotations, name or namespace. To specify metadata use `${.PVC.<metadata>}`. Example: If folder should be named like `<pvc-namespace>-<pvc-name>`, use `${.PVC.namespace}-${.PVC.name}` as pathPattern. To use the default name use `${.PVC.namespace}-${.PVC.name}-${.PVC.volume}` as pathPattern. | n/a |
Copy link
Contributor

Choose a reason for hiding this comment

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

i would say something more like The default pathPattern is ....

and i am wondering maybe it should really be the default instead of using strings.Join()

pvName := strings.Join([]string{pvcNamespace, pvcName, options.PVName}, "-")

🤔

Copy link
Author

Choose a reason for hiding this comment

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

@yonatankahana do you want me to see if I can make this change? My goal was to make as minimal a change as possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

@robkooper sorry for the long time delay.
maybe for now it's ok, you can leave it for another PR, let's just have more clear documentation about the default value and we can get this merged :)

Copy link
Author

Choose a reason for hiding this comment

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

I can change this as well as the pvname below. Let me see if I can get that done this week.

Copy link
Contributor

Choose a reason for hiding this comment

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

@robkooper thank you, that would be great!

humblec pushed a commit to humblec/nfs-subdir-external-provisioner that referenced this pull request May 11, 2022
@jsalatiel
Copy link

Any updates on this ?

@@ -93,6 +93,7 @@ func (p *nfsProvisioner) Provision(ctx context.Context, options controller.Provi
data: map[string]string{
"name": pvcName,
"namespace": pvcNamespace,
"volume": options.PVName,
Copy link
Contributor

Choose a reason for hiding this comment

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

btw, why volume and not pvname?

Copy link
Author

Choose a reason for hiding this comment

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

no real reason, mainly because binds it to a volume that is created by the storage manager. I'm open to change it to pvname since that clearly indicates that that part comes from the persistent volume name.

Copy link
Contributor

Choose a reason for hiding this comment

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

@robkooper
in the main README we writes Persistent volumes are provisioned as ${namespace}-${pvcName}-${pvName} which is wrong.
can you please update that as well?

@yonatankahana
Copy link
Contributor

/unassign @jackielii

@k8s-triage-robot
Copy link

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:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 29, 2022
@k8s-triage-robot
Copy link

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:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 28, 2022
@k8s-triage-robot
Copy link

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:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

@k8s-ci-robot
Copy link
Contributor

@k8s-triage-robot: Closed this PR.

In response to this:

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:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

  • Reopen this PR with /reopen
  • Mark this PR as fresh with /remove-lifecycle rotten
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants