Add aws-s3-csi-mounter component
#291
Conversation
| @@ -0,0 +1,25 @@ | |||
| // Package assert provides utilities for making assertions during tests. | |||
There was a problem hiding this comment.
Is there any widely accepted testing repository that has assert equal/not equals we can use instead?
There was a problem hiding this comment.
https://github.com/stretchr/testify is the most common one as far as I know. I think some part of Go community discourages assert libraries. I'm not against it – I just didn't wanted to pull a big package for simple assertions. We also depend on https://github.com/onsi/gomega in our integration tests as it's used in Kubernetes testing framework.
I think I can create an issue to standardizing all assertions to one package in the codebase.
| ) | ||
|
|
||
| var mountSockPath = flag.String("mount-sock-path", "/sock/mount.sock", "Path of the Unix socket to receive mount options from.") | ||
| var mountSockRecvTimeout = flag.Duration("mount-sock-recv-timeout", 2*time.Minute, "Timeout for receiving mount options from passed Unix socket.") |
There was a problem hiding this comment.
2 minutes is a long time. Why this number?
There was a problem hiding this comment.
Kinda arbitrary. This is the time this component will wait until the CSI Driver Node Pod sends mount options to it. I didn't wanted to make it short and cause retry loop. I don't expect it to wait until the whole timeout duration unless we have a bug in one of our components.
|
|
||
| // parseUnixRights parses given socket control message to extract passed file descriptors. | ||
| func parseUnixRights(buf []byte) ([]int, error) { | ||
| socketControlMessages, err := syscall.ParseSocketControlMessage(buf) |
There was a problem hiding this comment.
I'm not sure what this is meant to do
There was a problem hiding this comment.
as the comment suggests "... parses given socket control message to extract passed file descriptors.". Maybe I can add references to "Ancillary messages" and "SCM_RIGHTS" from https://man7.org/linux/man-pages/man7/unix.7.html.
|
|
||
| // Recv receives passed mount options via `Send` function through given `sockPath`. | ||
| func Recv(ctx context.Context, sockPath string) (Options, error) { | ||
| l, err := net.Listen("unix", sockPath) |
There was a problem hiding this comment.
I've not looked through the protocol for this - is it documented anywhere?
There was a problem hiding this comment.
Do you mean the first parameter to net.Listen? If so, see https://pkg.go.dev/net#Listen
There was a problem hiding this comment.
I was more meaning the protocol for send/receive, but I can now see it's just json serialising some structure, which seems fine
|
|
||
| // WaitForUnixSocket waits for the duration of `timeout` until `path` exists by checking it every `interval`. | ||
| // It returns `ErrUnixSocketNotExists` if the socket won't appear within the timeout, otherwise it returns nil. | ||
| func WaitForUnixSocket(timeout time.Duration, interval time.Duration, path string) error { |
There was a problem hiding this comment.
Nit: Would it be more efficient to use something like https://github.com/fsnotify/fsnotify to receive a notification from the OS that the file exists?
There was a problem hiding this comment.
I think it would, but since this is not in the data path, and we don't need low-latency, I think it would complicate this function as well. Do you think we need it?
There was a problem hiding this comment.
That's fair, no need for it now and we can think about it later if we discover it's something we want
Signed-off-by: Burak Varlı <[email protected]>
Signed-off-by: Burak Varlı <[email protected]>
Signed-off-by: Burak Varlı <[email protected]>
Signed-off-by: Burak Varlı <[email protected]>
Signed-off-by: Burak Varlı <[email protected]>
Signed-off-by: Burak Varlı <[email protected]>
Signed-off-by: Burak Varlı <[email protected]>
unexge
force-pushed
the
unexge/podmounter
branch
from
fe4374d to
5eba97a
Compare
Signed-off-by: Burak Varlı <[email protected]>
| // By default Mountpoint runs in a detached mode. Here we want to monitor it by relaying its output, | ||
| // and also we want to wait until it terminates. We're passing `--foreground` to achieve this. | ||
| const foreground = "--foreground" | ||
| if !slices.Contains(args, foreground) { |
There was a problem hiding this comment.
We should probably also check for the presence of -f here as well
|
|
||
| // Recv receives passed mount options via `Send` function through given `sockPath`. | ||
| func Recv(ctx context.Context, sockPath string) (Options, error) { | ||
| l, err := net.Listen("unix", sockPath) |
There was a problem hiding this comment.
I was more meaning the protocol for send/receive, but I can now see it's just json serialising some structure, which seems fine
|
|
||
| // WaitForUnixSocket waits for the duration of `timeout` until `path` exists by checking it every `interval`. | ||
| // It returns `ErrUnixSocketNotExists` if the socket won't appear within the timeout, otherwise it returns nil. | ||
| func WaitForUnixSocket(timeout time.Duration, interval time.Duration, path string) error { |
There was a problem hiding this comment.
That's fair, no need for it now and we can think about it later if we discover it's something we want
This is part of #279.
This new component,
aws-s3-csi-mounter, will be the entry point for the container running Mountpoint. It will be responsible for receiving mount options and FUSE file descriptor and spawning Mountpoint process until completion.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.