Create runner.RunInForeground function in github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint/runner package (#448)

This is part of the effort of consolidating Mountpoint related
functionalities in `github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint`
package.

---

By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.

Signed-off-by: Burak Varlı <burakvar@amazon.co.uk>

Author: unexge

cmd/aws-s3-csi-mounter/csimounter/csimounter.go

@@ -1,17 +1,15 @@
 package csimounter
 
 import (
-	"bytes"
 	"fmt"
-	"io"
 	"io/fs"
 	"os"
-	"os/exec"
 
 	"k8s.io/klog/v2"
 
 	"github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint"
 	"github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint/mountoptions"
+	"github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint/runner"
 )
 
 var mountErrorFileperm = fs.FileMode(0600) // only owner readable and writeable
@@ -20,75 +18,43 @@ var mountErrorFileperm = fs.FileMode(0600) // only owner readable and writeable
 // so Kubernetes doesn't have to restart it and transition the Pod into `Succeeded` state.
 const successExitCode = 0
 
-// A CmdRunner is responsible for running given `cmd` until completion and returning its exit code and its error (if any).
-// This is mainly exposed for mocking in tests, `DefaultCmdRunner` is always used in non-test environments.
-type CmdRunner func(cmd *exec.Cmd) (int, error)
-
-// DefaultCmdRunner is a real CmdRunner implementation that runs given `cmd`.
-func DefaultCmdRunner(cmd *exec.Cmd) (int, error) {
-	err := cmd.Run()
-	if err != nil {
-		return 0, err
-	}
-	return cmd.ProcessState.ExitCode(), nil
-}
+// restartExitCode is the exit code returned from `aws-s3-csi-mounter` to indicate an error exit,
+// so Kubernetes would restart it. This is the default exit code if `mount.exit` is not present,
+// meaning the CSI Driver Node Pod didn't requested a clean exit.
+const restartExitCode = 1
 
 // An Options represents options to use while mounting Mountpoint.
 type Options struct {
 	MountpointPath string
 	MountExitPath  string
 	MountErrPath   string
 	MountOptions   mountoptions.Options
-	CmdRunner      CmdRunner
+	CmdRunner      runner.CmdRunner
 }
 
 // Run runs Mountpoint with given options until completion and returns its exit code and its error (if any).
 func Run(options Options) (int, error) {
-	if options.CmdRunner == nil {
-		options.CmdRunner = DefaultCmdRunner
-	}
-
 	mountOptions := options.MountOptions
-
-	fuseDev := os.NewFile(uintptr(mountOptions.Fd), "/dev/fuse")
-	if fuseDev == nil {
-		return 0, fmt.Errorf("passed file descriptor %d is invalid", mountOptions.Fd)
-	}
-
 	mountpointArgs := mountpoint.ParseArgs(mountOptions.Args)
 
-	// 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.
-	mountpointArgs.Set(mountpoint.ArgForeground, mountpoint.ArgNoValue)
-
 	// TODO: This is a temporary workaround to create a cache folder if caching is enabled,
 	// ideally we should create a volume (`emptyDir` by default) in the Mountpoint Pod and use that.
 	mountpointArgs, err := createCacheDir(mountpointArgs)
 	if err != nil {
 		return 0, fmt.Errorf("failed to create cache dir: %w", err)
 	}
 
-	args := append([]string{
-		mountOptions.BucketName,
-		// We pass FUSE fd using `ExtraFiles`, and each entry becomes as file descriptor 3+i.
-		"/dev/fd/3",
-	}, mountpointArgs.SortedList()...)
-
-	cmd := exec.Command(options.MountpointPath, args...)
-	cmd.ExtraFiles = []*os.File{fuseDev}
-	cmd.Env = options.MountOptions.Env
-
-	var stderrBuf bytes.Buffer
-
-	// Connect Mountpoint's stdout/stderr to this commands stdout/stderr,
-	// so Mountpoint logs can be viewable with `kubectl logs`.
-	cmd.Stdout = os.Stdout
-	cmd.Stderr = io.MultiWriter(os.Stderr, &stderrBuf)
-
-	exitCode, err := options.CmdRunner(cmd)
+	exitCode, stdErr, err := runner.RunInForeground(runner.ForegroundOptions{
+		BinaryPath: options.MountpointPath,
+		BucketName: mountOptions.BucketName,
+		Fd:         mountOptions.Fd,
+		Args:       mountpointArgs,
+		Env:        mountOptions.Env,
+		CmdRunner:  options.CmdRunner,
+	})
 	if err != nil {
 		// If Mountpoint fails, write it to `options.MountErrPath` to let `PodMounter` running in the same node know.
-		if writeErr := os.WriteFile(options.MountErrPath, stderrBuf.Bytes(), mountErrorFileperm); writeErr != nil {
+		if writeErr := os.WriteFile(options.MountErrPath, stdErr, mountErrorFileperm); writeErr != nil {
 			klog.Errorf("Failed to write mount error logs to %s: %v\n", options.MountErrPath, err)
 		}
 		return exitCode, err
@@ -100,7 +66,7 @@ func Run(options Options) (int, error) {
 		return successExitCode, nil
 	}
 
-	return exitCode, nil
+	return restartExitCode, nil
 }
 
 // checkIfFileExists checks whether given `path` exists.

cmd/aws-s3-csi-mounter/csimounter/csimounter_test.go

@@ -15,6 +15,9 @@ import (
 	"github.com/awslabs/aws-s3-csi-driver/pkg/util/testutil/assert"
 )
 
+const successExitCode = 0
+const restartExitCode = 1
+
 func TestRunningMountpoint(t *testing.T) {
 	mountpointPath := filepath.Join(t.TempDir(), "mount-s3")
 
@@ -37,7 +40,7 @@ func TestRunningMountpoint(t *testing.T) {
 			CmdRunner: runner,
 		})
 		assert.NoError(t, err)
-		assert.Equals(t, 0, exitCode)
+		assert.Equals(t, restartExitCode, exitCode)
 	})
 
 	t.Run("Passes bucket name", func(t *testing.T) {
@@ -56,7 +59,7 @@ func TestRunningMountpoint(t *testing.T) {
 			CmdRunner: runner,
 		})
 		assert.NoError(t, err)
-		assert.Equals(t, 0, exitCode)
+		assert.Equals(t, restartExitCode, exitCode)
 	})
 
 	t.Run("Passes environment variables", func(t *testing.T) {
@@ -70,13 +73,14 @@ func TestRunningMountpoint(t *testing.T) {
 		exitCode, err := csimounter.Run(csimounter.Options{
 			MountpointPath: mountpointPath,
 			MountOptions: mountoptions.Options{
-				Fd:  int(mountertest.OpenDevNull(t).Fd()),
-				Env: env,
+				Fd:         int(mountertest.OpenDevNull(t).Fd()),
+				BucketName: "test-bucket",
+				Env:        env,
 			},
 			CmdRunner: runner,
 		})
 		assert.NoError(t, err)
-		assert.Equals(t, 0, exitCode)
+		assert.Equals(t, restartExitCode, exitCode)
 	})
 
 	t.Run("Adds `--foreground` argument if not passed", func(t *testing.T) {
@@ -98,7 +102,7 @@ func TestRunningMountpoint(t *testing.T) {
 			CmdRunner: runner,
 		})
 		assert.NoError(t, err)
-		assert.Equals(t, 0, exitCode)
+		assert.Equals(t, restartExitCode, exitCode)
 
 		exitCode, err = csimounter.Run(csimounter.Options{
 			MountpointPath: mountpointPath,
@@ -110,7 +114,7 @@ func TestRunningMountpoint(t *testing.T) {
 			CmdRunner: runner,
 		})
 		assert.NoError(t, err)
-		assert.Equals(t, 0, exitCode)
+		assert.Equals(t, restartExitCode, exitCode)
 	})
 
 	t.Run("Fails if file descriptor is invalid", func(t *testing.T) {
@@ -139,7 +143,7 @@ func TestRunningMountpoint(t *testing.T) {
 			_, err := c.Stderr.Write([]byte(mountpointErr.Error()))
 			assert.NoError(t, err)
 
-			return 1, mountpointErr
+			return restartExitCode, mountpointErr
 		}
 
 		exitCode, err := csimounter.Run(csimounter.Options{
@@ -152,7 +156,7 @@ func TestRunningMountpoint(t *testing.T) {
 			CmdRunner: runner,
 		})
 		assert.Equals(t, mountpointErr, err)
-		assert.Equals(t, 1, exitCode)
+		assert.Equals(t, restartExitCode, exitCode)
 
 		errMsg, err := os.ReadFile(mountErrPath)
 		assert.NoError(t, err)
@@ -185,7 +189,7 @@ func TestRunningMountpoint(t *testing.T) {
 			CmdRunner: runner,
 		})
 		assert.NoError(t, err)
-		// Should be `0` even though Mountpoint exited with a different exit code
-		assert.Equals(t, 0, exitCode)
+		// Should be `successExitCode` even though Mountpoint exited with a different exit code
+		assert.Equals(t, successExitCode, exitCode)
 	})
 }

pkg/mountpoint/runner/foreground.go

@@ -0,0 +1,84 @@
+package runner
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"os/exec"
+
+	"github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint"
+)
+
+// ErrMissingBinaryPath is returned when Mountpoint binary path is empty.
+var ErrMissingBinaryPath = errors.New("runner: missing Mountpoint binary path")
+
+// ErrMissingBucketName is returned when S3 Bucket name is empty.
+var ErrMissingBucketName = errors.New("runner: missing S3 Bucket name")
+
+// A ForegroundOptions represents options for running Mountpoint in the foreground.
+type ForegroundOptions struct {
+	// Path to the Mountpoint binary `mount-s3`.
+	BinaryPath string
+	// Name of the S3 Bucket to mount.
+	BucketName string
+	// FUSE file descriptor for Mountpoint process to communicate with the kernel.
+	// Can be obtained using `github.com/awslabs/aws-s3-csi-driver/pkg/mountpoint/mounter` package.
+	Fd int
+	// Mountpoint arguments.
+	Args mountpoint.Args
+	// Mountpoint processes's environment variables.
+	Env []string
+	// Command runner to use, if nil, [DefaultCmdRunner] will be used.
+	CmdRunner CmdRunner
+}
+
+// RunInForeground runs Mountpoint in the foreground until completion.
+// It returns Mountpoint processes's exit code, standard error output (if Mountpunt ran and fail), and any error occurred.
+// It redirects Mountpoint processes's stdout and stderr to calling process.
+func RunInForeground(opts ForegroundOptions) (ExitCode, []byte, error) {
+	if opts.BinaryPath == "" {
+		return 0, nil, ErrMissingBinaryPath
+	}
+	if opts.BucketName == "" {
+		return 0, nil, ErrMissingBucketName
+	}
+	if opts.CmdRunner == nil {
+		opts.CmdRunner = DefaultCmdRunner
+	}
+
+	fuseDev := os.NewFile(uintptr(opts.Fd), "/dev/fuse")
+	if fuseDev == nil {
+		return 0, nil, fmt.Errorf("runner: passed file descriptor %d is not a valid FUSE file descriptor", opts.Fd)
+	}
+
+	mountpointArgs := opts.Args
+
+	// 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.
+	mountpointArgs.Set(mountpoint.ArgForeground, mountpoint.ArgNoValue)
+
+	args := append([]string{
+		opts.BucketName,
+		// We pass FUSE fd using `ExtraFiles`, and each entry becomes as file descriptor 3+i.
+		"/dev/fd/3",
+	}, mountpointArgs.SortedList()...)
+
+	cmd := exec.Command(opts.BinaryPath, args...)
+	cmd.ExtraFiles = []*os.File{fuseDev}
+	cmd.Env = opts.Env
+
+	var stderrBuf bytes.Buffer
+	// Connect Mountpoint's stdout/stderr to this commands stdout/stderr,
+	// as we're running Mountpoint in the foreground.
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = io.MultiWriter(os.Stderr, &stderrBuf)
+
+	exitCode, err := opts.CmdRunner(cmd)
+	if err != nil {
+		return exitCode, stderrBuf.Bytes(), err
+	}
+
+	return exitCode, nil, nil
+}

pkg/mountpoint/runner/runner.go

@@ -0,0 +1,20 @@
+// Package runner provides utilities for running Mountpoint instances.
+package runner
+
+import "os/exec"
+
+// An ExitCode represents exit code of a Mountpoint process.
+type ExitCode = int
+
+// A CmdRunner is responsible for running given `cmd` until completion and returning its exit code and its error (if any).
+// This is mainly exposed for mocking in tests, [DefaultCmdRunner] is always used in non-test environments.
+type CmdRunner func(cmd *exec.Cmd) (ExitCode, error)
+
+// DefaultCmdRunner is a real CmdRunner implementation that runs given `cmd`.
+func DefaultCmdRunner(cmd *exec.Cmd) (ExitCode, error) {
+	err := cmd.Run()
+	if err != nil {
+		return 0, err
+	}
+	return cmd.ProcessState.ExitCode(), nil
+}