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

subPathPattern has no effect in AccessPoint final path #1113

Closed
tpzumezawa opened this issue Aug 24, 2023 · 16 comments
Closed

subPathPattern has no effect in AccessPoint final path #1113

tpzumezawa opened this issue Aug 24, 2023 · 16 comments
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@tpzumezawa
Copy link

tpzumezawa commented Aug 24, 2023

/kind bug

What happened?
Set the options:
basePath: "/dynamic_provisioning2"
subPathPattern: "${.PVC.namespace}/${.PVC.name}"
ensureUniqueDirectory: "false"

With:
.PVC.namespace : "prometheus"
.PVC.name: "efs-claim"

Driver created a directory:
/dynamic_provisioning2/pvc-74f1003f-0d95-49f3-add9-5f8f803761aa

What you expected to happen?
Driver should create a directory similar to:
/dynamic_provisioning2/prometheus/efs-claim/pvc-74f1003f-0d95-49f3-add9-5f8f803761aa

How to reproduce it (as minimally and precisely as possible)?
kind: Namespace
apiVersion: v1
metadata:
name: prometheus
labels:
name: prometheus

kind: StorageClass
apiVersion: storage.k8s.io/v1
metadata:
name: efs-sc
provisioner: efs.csi.aws.com
parameters:
provisioningMode: efs-ap
fileSystemId: fs-0f445a7a9e0f9d70e
directoryPerms: "700"
basePath: "/dynamic_provisioning2"
subPathPattern: "${.PVC.namespace}/${.PVC.name}"
ensureUniqueDirectory: "false" # optional

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
name: efs-claim
namespace: prometheus
spec:
accessModes:
- ReadWriteMany
storageClassName: efs-sc
resources:
requests:
storage: 5Gi

apiVersion: v1
kind: Pod
metadata:
name: efs-app
namespace: prometheus
spec:
containers:
- name: app
image: centos
command: ["/bin/sh"]
args: ["-c", "while true; do echo $(date -u) >> /data/out; sleep 5; done"]
volumeMounts:
- name: persistent-storage
mountPath: /data
volumes:
- name: persistent-storage
persistentVolumeClaim:
claimName: efs-claim

Anything else we need to know?:

Environment

  • Kubernetes version (use kubectl version):
    Client Version: version.Info{Major:"1", Minor:"24+", GitVersion:"v1.24.7-eks-fb459a0", GitCommit:"c240013134c03a740781ffa1436ba2688b50b494", GitTreeState:"clean", BuildDate:"2022-10-24T20:40:13Z", GoVersion:"go1.18.7", Compiler:"gc", Platform:"linux/amd64"}
    Kustomize Version: v4.5.4
    Server Version: version.Info{Major:"1", Minor:"27+", GitVersion:"v1.27.4-eks-2d98532", GitCommit:"3d90c097c72493c2f1a9dd641e4a22d24d15be68", GitTreeState:"clean", BuildDate:"2023-07-28T16:51:44Z", GoVersion:"go1.20.6", Compiler:"gc", Platform:"linux/amd64"}

  • Driver version:

  • Chart aws-efs-csi-driver-2.4.9

  • app version: 1.6.0

Please also attach debug logs to help us better diagnose

  • Instructions to gather debug logs can be found here
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Aug 24, 2023
@mskanth972
Copy link
Contributor

mskanth972 commented Aug 25, 2023

Hi @tpzumezawa, that's expected behavior the access point name will always be of the form pvc-id. However, if they specify a base path, the access point folder should be created under the base path.
You can refer here
#640 (comment)

@tpzumezawa
Copy link
Author

Hi @tpzumezawa, that's expected behavior the access point name will always be of the form pvc-id. However, if they specify a base path, the access point folder should be created under the base path. You can refer here #640 (comment)

I am not doubting the functionality of "basePath" but the funcionality of "subPathPattern" parameter and even the ensureUniqueDirectory parameter will not seem to work properly because I set it to false so I expect no PV ID in the final path

@mskanth972
Copy link
Contributor

Sorry for the above, it seems the feature is merged to the master but not to the release branch. We are planning to implement this in coming release.

@gurusimohan
Copy link

@mskanth972 , Meanwhile, Do we have any other options to create the image out master branch and use it till you release the next version?

@mskanth972
Copy link
Contributor

Unfortunately no. But you can clone the master branch code and build image out of it which could help you leverage the feature.

@lpythoud
Copy link

+1, this feature is very important for us. Any idea when this new version could be released ? thx

@mskanth972
Copy link
Contributor

There are couple of features we are planning with this feature, most probably will be done in the next week.

@andrewhharmon
Copy link

im trying to mount2 eks clusters to the same basepath so the can share the same data. am i understanding that's not possible bc they will each have their own uniquely named pvc which means i can't get the access points to both point to the same / dir?

@mskanth972
Copy link
Contributor

Hi @andrewhharmon, PR just adds the control over the base paths but does not include the feature of reusing Access Points. But we are also planning to include that feature where you can access the same access point to two different clusters, most probably we will release this in coming version.

@andrewhharmon
Copy link

does that mean with this PR though i can just set the 2 access points to the same base path? or does EFS not allow that?

@mskanth972
Copy link
Contributor

mskanth972 commented Sep 12, 2023

No, every access point has a client token which is unique for each AP. This PR does not restrict in creating Access Point and use the old one when we are looking to re-use that. So when you use different cluster here, it creates a new Access Point rather than using the old one despite of using the same base path names.
But this workaround can be done by this PR and we are testing and if everything looks good we are planning to release it also

@ajaysinghh
Copy link

There are couple of features we are planning with this feature, most probably will be done in the next week.

Hi @mskanth972, is there any ETA for the release?

@runningman84
Copy link

it was already released yesterday and it seems to work for me...

@ajaysinghh
Copy link

it was already released yesterday and it seems to work for me...

Thank you, I have tested the feature and it is working fine.

@mskanth972
Copy link
Contributor

Closing the issue, please feel free to open if you are facing the issue still
/close

@k8s-ci-robot
Copy link
Contributor

@mskanth972: Closing this issue.

In response to this:

Closing the issue, please feel free to open if you are facing the issue still
/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
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

8 participants