Re: [kubernetes/utils] Move mount code from kubernetes/kubernetes for reusing from CSI plugins (#38)
Michelle Au
/assign
cc @kubernetes/sig-storage-pr-reviews
—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
Michelle Au
@msau42 commented on this pull request.
In mount/mount.go:
> + // Will operate in the host mount namespace if kubelet is running in a container. + // Error is returned on any other error than "file not found". + ExistsPath(pathname string) (bool, error) + // CleanSubPaths removes any bind-mounts created by PrepareSafeSubpath in given + // pod volume directory. + CleanSubPaths(podDir string, volumeName string) error + // PrepareSafeSubpath does everything that's necessary to prepare a subPath + // that's 1) inside given volumePath and 2) immutable after this call. + // + // newHostPath - location of prepared subPath. It should be used instead of + // hostName when running the container. + // cleanupAction - action to run when the container is running or it failed to start. + // + // CleanupAction must be called immediately after the container with given + // subpath starts. On the other hand, Interface.CleanSubPaths must be called + // when the pod finishes.
I think CleanSubPaths, SafeMakeDir, and PrepareSafeSubpath should not be included in the mount util interface. Those are Kubernetes specific operations and not general purpose mount operations.
In mount/mount.go:
> + // that's 1) inside given volumePath and 2) immutable after this call. + // + // newHostPath - location of prepared subPath. It should be used instead of + // hostName when running the container. + // cleanupAction - action to run when the container is running or it failed to start. + // + // CleanupAction must be called immediately after the container with given + // subpath starts. On the other hand, Interface.CleanSubPaths must be called + // when the pod finishes. + PrepareSafeSubpath(subPath Subpath) (newHostPath string, cleanupAction func(), err error) + // GetMountRefs finds all mount references to the path, returns a + // list of paths. Path could be a mountpoint path, device or a normal + // directory (for bind mount). + GetMountRefs(pathname string) ([]string, error) + // GetFSGroup returns FSGroup of the path. + GetFSGroup(pathname string) (int64, error)
FSGroup is a kubernetes concept so should not belong here either.
GetSELinuxSupport() and GetMode() are also not used by any plugins either.
Michelle Au
cc @jsafrane
do the mount propagation and exec interfaces need to be here too?
Jan Šafránek
Mount interface has grown from a simple mounting tool to generic OS interface. It's worth splitting mount.Interface into smaller chunks and leave only those that are generic enough to be used by other consumers (e.g. real mounting), but the Kubernetes parts should remain in Kubernetes.
So mount propagation, NsenterMounter and ExecMounter should remain in Kubernetes, they have very little use outside.
Michelle Au
Maybe it would be easier to restructure the mount interface in Kubernetes core first. Then pull the generic mount utils out.
fejta-bot
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
fejta-bot
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
fejta-bot
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
Closed #38.
k8s-ci-robot
@fejta-bot: Closed this PR.
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.