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

Make EFS path creation behavior configurable #63

Closed
leakingtapan opened this issue Jul 25, 2019 · 29 comments
Closed

Make EFS path creation behavior configurable #63

leakingtapan opened this issue Jul 25, 2019 · 29 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@leakingtapan
Copy link
Contributor

leakingtapan commented Jul 25, 2019

Is your feature request related to a problem? Please describe.
With the subpath feature implemented, user will be able to mount a subpath of EFS into container as persistent volume. However, user must create each subpath beforehand before they consume the PV from container. This creates a hard user experience to use the feature.

Describe the solution you'd like in detail
Similar to hostPath volume, add a optional type field to the PV's volumeAttributes:

apiVersion: v1
kind: PersistentVolume
metadata:
  name: efs-pv
spec:
 ...
  csi:
    driver: efs.csi.aws.com
    volumeHandle: fs-c2a43e69:/a/b
    volumeAttributes:
      type: "Directory"

volumeAttributes.type usage table:

Value Behavior
Empty string (default) is for backward compatibility, which means that no checks will be performed before mounting the EFS volume
DirectoryOrCreate If nothing exists at the given path, an empty directory will be created there as needed with permission set to 0755, having the same group and ownership with Pod.
Directory A directory must exist at the given path

/cc @wongma7

Additional Information

We will need to consider leveraging Pod Info on Mount to populate pod uid/gid to the driver when creating the directory dynamically.

@wongma7
Copy link
Contributor

wongma7 commented Jul 25, 2019

Good point about the permissions. It looks like Pod Info on Mount won't give us anything useful though, the UID there is just the pod unique UID: https://github.com/kubernetes/kubernetes/blob/665e76d97623447d13bb3b33b68591a985b49c9d/pkg/volume/csi/csi_mounter.go#L324.

We can't leverage fsgroup to change the GID owner of the directory on mount either, because EFS is RWX not RWO so fsgroup will have no effect (imagine if pod A mounted it then pod B mounted it and pod A lost permission).

EFS has no root squashing so on vanilla clusters where containers run as root (i.e. PSP is not configured with runAsUser: MustRunAsNonRoot https://kubernetes.io/docs/concepts/policy/pod-security-policy/#users-and-groups), all containers will have full access. https://docs.aws.amazon.com/efs/latest/ug/accessing-fs-nfs-permissions.html#accessing-fs-nfs-permissions-root-user

@wreed4
Copy link

wreed4 commented Aug 7, 2019

To be clear, this was the original intent of #46 . Requiring the directory to exist prior to volume creation is really not helpful from a kubernetes standpoint.

@leakingtapan
Copy link
Contributor Author

@wreed4 Yep. We will address the issue here. Speaking of creating the directory dynamically. There are several use cases I can think of:

  1. User manually create the PV and specify the path in the spec.csi.VolumeAttributes and path will be created dynamically even if it is not on EFS filesystem yet.
  2. User creates PVC which will trigger PV creation dynamically. And the path will be set to a random UUID (eg. PV name) and the path will be created dynamically if it is missing.

Which one is your original intent?

Also for creating the path dynamically, what's the expected uid/gid and file permission mode for the directory?

@Vedrillan
Copy link

I think the original intent is to mimic the NFS behavior and simply be able to set a path to mount in the target https://github.com/kubernetes/examples/blob/master/staging/volumes/nfs/nfs-pv.yaml, which is option 1 in your comment @leakingtapan

I suppose uid/gid and mode should be 0:0 0777, otherwise I do not see how it could work.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/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 Dec 24, 2019
@leakingtapan
Copy link
Contributor 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 Dec 26, 2019
@wreed4
Copy link

wreed4 commented Jan 13, 2020

@leakingtapan I actually believe both use-cases you outline above are useful and should both be possible, where the second is a special case of the first.

You bring up a really good point with the permissions though. We will be mounting these volumes as different customers, so it is important that custA cannot read custB's data. However we intend to solve this problem at the k8s level with PVCs and namespaces (ie, pods from custA cannot mount custB's PVC because it's in a different namespace). Granted it's theoretically possible for pvA to point to the same underlying information as pvB, but I think as long as we don't do that and can demonstrate how we avoid that situation, we'd be okay.

The ability to create two PVs pointing at the same underlying data may actually end up proving useful to us as it allows us to be able to copy files between two environments (namespaces) of the same customer in a single pod. If we couldn't do this, we'd have to launch a pod in ns1, mount PVC1, copy files to a transport PV, then launch pod2 in ns2, mount PVC2 and copy files from the transport PV into pvc2.

@leakingtapan leakingtapan removed this from the 0.3 milestone Mar 7, 2020
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/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 5, 2020
@olfway
Copy link

olfway commented Jun 19, 2020

/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 19, 2020
@nomopo45
Copy link

nomopo45 commented Jul 8, 2020

Hello,

I just want to know if there are news regarding the fact that the user must create each subpath beforehand before they consume the PV from container, is it possible to do an auto creation of the directory or not yet ?

thank you

@lknaubkonrad
Copy link

I have the same question.

@jjayeshgohil
Copy link

Hi,

We are also looking for this feature.

Thanks

@sppwf
Copy link

sppwf commented Sep 3, 2020

Hi,

Waiting the feature to be implemented.

Thanks
Sergiu

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/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 Dec 2, 2020
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/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 Jan 1, 2021
@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/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.

@MarcusNoble
Copy link

Can we get this issue reopened? Currently waiting on this before switching from the deprecated efs-provisioner.

@leakingtapan
Copy link
Contributor Author

/reopen
/lifecycle frozen

@k8s-ci-robot
Copy link
Contributor

@leakingtapan: Reopened this issue.

In response to this:

/reopen
/lifecycle frozen

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.

@k8s-ci-robot k8s-ci-robot reopened this Feb 1, 2021
@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. labels Feb 1, 2021
@pre
Copy link

pre commented Mar 29, 2021

Needing to manually create the Access Points (directories) beforehand in order to use subPath was a hard unexpected surprise 👾

@gabegorelick
Copy link

User creates PVC which will trigger PV creation dynamically. And the path will be set to a random UUID (eg. PV name) and the path will be created dynamically if it is missing.

This seems similar to NFS Subdir External Provisioner's pathPattern. This is an old issue, but I'm not aware of any way to do this in the EFS CSI driver today. Dynamic provisioning using access points will be preferred for most users, but it's not exactly the same.

@edijsdrezovs
Copy link

Auto creation of subpath is very missing feature

@paalkr
Copy link

paalkr commented Apr 1, 2022

Ref: #538

@Preen
Copy link

Preen commented Jan 15, 2023

+1

@AlissonRS
Copy link

Also looking for this.

@RyanStan
Copy link
Contributor

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label May 15, 2023
@moorthy156
Copy link

Any update on this, looking for this feature as well

RomanBednar pushed a commit to RomanBednar/aws-efs-csi-driver that referenced this issue Jan 16, 2024
…t-ocp

OCPBUGS-24492: UPSTREAM: 1217: set results count for listing access points
@seanzatzdev-amazon
Copy link
Contributor

subPathPattern in recent releases should take care of this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests