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.

Sign up for GitHub

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

Jump to bottom

Add missing pvname #250

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add missing pvname #250

wants to merge 6 commits into from
+59 −65

Conversation

robkooper
Copy link

This relates to issue #149 and allows to use the PVName in the path pattern. This adds the ability to use the pvname as part of the pathpattern, the same way that is done in the default pattern. The default path pattern is now a string that is parsed the same way as the user specified pathpattern.

This addresses the issues in PR #157. I decided to start clean to address those issues, hence a new PR.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 19, 2022
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 19, 2022
yonatankahana
yonatankahana suggested changes Nov 24, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
yonatankahana
yonatankahana reviewed Nov 25, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all 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:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this 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 Feb 23, 2023
@robkooper
Copy link
Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 23, 2023
@robkooper
Add missing pvname
b48ecac 
This adds the ability to use the pvname as part of the pathpattern. The defaultpath is now a string that is parsed the same way as the user specified pathpattern.
@robkooper
fix typo and space
7a1baca
@robkooper
fix tyopos
b47ac8e
@robkooper
Copy link
Author

updated branch to latest release and rebased code on top of that.

yonatankahana
yonatankahana reviewed Mar 17, 2023
View reviewed changes
README.md Outdated Show resolved Hide resolved
yonatankahana
yonatankahana reviewed Mar 17, 2023
View reviewed changes
README.md Show resolved Hide resolved
cmd/nfs-subdir-external-provisioner/provisioner.go
}

parsedPath := metadata.stringParser(pathPattern)
if parsedPath == "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

empty parsedPath means that the stringParser() necessarily failed? or pathPattern may exist but empty as well?
and what can cause stringParser to fail, pathPattern such as foo-${.nonexistingvar}? in such case stringParser() will return empty string?

Copy link
Author

Choose a reason for hiding this comment

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

the stringParser() will return "" only if none of the patterns match. The tricky part is that if the pattern is ${.nonexisting1}-${.nonexisting1} the parsedPath will be -, and will not be captured by the error and all nfs paths will be mapped to just -

@yonatankahana
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Mar 17, 2023
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all 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:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this 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 Jun 15, 2023
@jcpunk
Copy link

jcpunk commented Jun 20, 2023

It would be nice to see this merged in.

@robkooper
Copy link
Author

Will look at the comments, and fix the PR
/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 20, 2023
@robkooper
Merge remote-tracking branch 'upstream/master' into add-volume-parame…
92d3dd7 
…ter2
@robkooper
updates
a398074 
- add text to chart/readme
- empty pathPattern is same as default
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: robkooper
Once this PR has been reviewed and has the lgtm label, please assign kmova for approval. For more information see the Kubernetes Code Review Process.

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 removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 11, 2023
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 11, 2023
@dadurex
Copy link

dadurex commented Aug 24, 2023 •
edited

Is there plans to merge this change cause i'm very interested in having this variable accessible for setting pathPatern?

nfs-subdir-external-provisioner/cmd/nfs-subdir-external-provisioner/provisioner.go

Line 90 in a398074

"pvname": options.PVName,

@kijko
Copy link

kijko commented Aug 24, 2023

Is there plans to merge this change cause i'm very interested in having this variable accessible for setting pathPatern?

nfs-subdir-external-provisioner/cmd/nfs-subdir-external-provisioner/provisioner.go

Line 90 in a398074

"pvname": options.PVName,

also waiting for this one

@shaggy17goo
Copy link

Is there plans to merge this change cause i'm very interested in having this variable accessible for setting pathPatern?

nfs-subdir-external-provisioner/cmd/nfs-subdir-external-provisioner/provisioner.go

Line 90 in a398074

"pvname": options.PVName,

also waiting for this one

I'm also waiting, please merge ASAP

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all 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:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this 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 Mar 2, 2024
@robkooper
Copy link
Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 2, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all 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:

  • Mark this PR as fresh with /remove-lifecycle stale
  • Close this 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 May 31, 2024
@robkooper
Copy link
Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashishranjan738 ashishranjan738 Awaiting requested review from ashishranjan738

@kmova kmova Awaiting requested review from kmova

@yonatankahana yonatankahana Awaiting requested review from yonatankahana

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@robkooper @k8s-triage-robot @yonatankahana @jcpunk @k8s-ci-robot @dadurex @kijko @shaggy17goo