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

Add missing pvname #250

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

Add missing pvname #250

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b48ecac
Add missing pvname
robkooper Nov 19, 2022
7a1baca
fix typo and space
robkooper Nov 24, 2022
b47ac8e
fix tyopos
robkooper Nov 25, 2022
faed42f
Update README.md
robkooper Mar 17, 2023
92d3dd7
Merge remote-tracking branch 'upstream/master' into add-volume-parame…
robkooper Aug 11, 2023
a398074
updates
robkooper Aug 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ $ oc adm policy add-scc-to-user hostmount-anyuid system:serviceaccount:$NAMESPAC

If you would like to use a custom built nfs-subdir-external-provisioner image, you must edit the provisioner's deployment file to specify the correct location of your `nfs-client-provisioner` container image.

Next you must edit the provisioner's deployment file to add connection information for your NFS server. Edit `deploy/deployment.yaml` and replace the two occurences of <YOUR NFS SERVER HOSTNAME> with your server's hostname.
Next you must edit the provisioner's deployment file to add connection information for your NFS server. Edit `deploy/deployment.yaml` and replace the two occurrences of <YOUR NFS SERVER HOSTNAME> with your server's hostname.

```yaml
kind: Deployment
Expand Down Expand Up @@ -234,11 +234,11 @@ To disable leader election, define an env variable named ENABLE_LEADER_ELECTION

**_Parameters:_**

| Name | Description | Default |
| --------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :--------------------------------------------------------------: |
| 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 |
| Name | Description | Default |
| --------------- | ------------------------------------------------------------ | :----------------------------------------------------------: |
| 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. The default pattern will add the pvname to folder, which will result in a unique folder name, combining namespace, name and an unique id. | will create a folder with the pattern: `${.PVC.namespace}-${.PVC.name}-${.PVC.pvname}` |
robkooper marked this conversation as resolved.
Show resolved Hide resolved

This is `deploy/class.yaml` which defines the NFS subdir external provisioner's Kubernetes Storage Class:

Expand Down
34 changes: 14 additions & 20 deletions cmd/nfs-subdir-external-provisioner/provisioner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ import (

const (
provisionerNameKey = "PROVISIONER_NAME"
defaultPathPattern = "${.PVC.namespace}-${.PVC.name}-${.PVC.pvname}"
mountPath = "/persistentvolumes"
)

type nfsProvisioner struct {
Expand Down Expand Up @@ -73,10 +75,6 @@ func (meta *pvcMetadata) stringParser(str string) string {
return str
}

const (
mountPath = "/persistentvolumes"
)

var _ controller.Provisioner = &nfsProvisioner{}

func (p *nfsProvisioner) Provision(ctx context.Context, options controller.ProvisionOptions) (*v1.PersistentVolume, controller.ProvisioningState, error) {
Expand All @@ -85,31 +83,27 @@ func (p *nfsProvisioner) Provision(ctx context.Context, options controller.Provi
}
glog.V(4).Infof("nfs provisioner: VolumeOptions %v", options)

pvcNamespace := options.PVC.Namespace
pvcName := options.PVC.Name

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

metadata := &pvcMetadata{
data: map[string]string{
"name": pvcName,
"namespace": pvcNamespace,
"name": options.PVC.Name,
"namespace": options.PVC.Namespace,
"pvname": options.PVName,
},
labels: options.PVC.Labels,
annotations: options.PVC.Annotations,
}

fullPath := filepath.Join(mountPath, pvName)
path := filepath.Join(p.path, pvName)

pathPattern, exists := options.StorageClass.Parameters["pathPattern"]
if exists {
customPath := metadata.stringParser(pathPattern)
if customPath != "" {
path = filepath.Join(p.path, customPath)
fullPath = filepath.Join(mountPath, customPath)
}
if !exists {
robkooper marked this conversation as resolved.
Show resolved Hide resolved
pathPattern = defaultPathPattern
}

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 -

return nil, controller.ProvisioningFinished, errors.New("unable to parse pathPattern: " + pathPattern)
}
path := filepath.Join(p.path, parsedPath)
fullPath := filepath.Join(mountPath, parsedPath)

glog.V(4).Infof("creating path %s", fullPath)
if err := os.MkdirAll(fullPath, 0o777); err != nil {
Expand Down