command: Add option to spawn a command with a pgid

Processes can be spawned with pgid [1], which can then be used to kill a
process and all child processes that were spawned with that pid as a pgid.

This allows us to, in a later commit, tie all Git commands to the
main Gitaly process. When a Gitaly process exits, a sys admin, a script,
or Gitlab Omnibus can do a `kill -9 -<old gitaly pid>` to kill any process
that has <old gitaly pid> as its pgid.

Add an option to allow commands to be instantiated with a parent pid.

1. https://pkg.go.dev/syscall#SysProcAttr

5 files changed, 94 insertions, 3 deletions

diff --git a/internal/command/command.go b/internal/command/command.go
index 2881cf86f..8862fe3eb 100644
--- a/internal/command/command.go
+++ b/internal/command/command.go
@@ -201,6 +201,16 @@ func New(ctx context.Context, nameAndArgs []string, opts ...Option) (*Command, e
 
 	cmd := exec.Command(nameAndArgs[0], nameAndArgs[1:]...)
 
+	if featureflag.RunCmdsInProcessGroup.IsEnabled(ctx) {
+		cmd.SysProcAttr = &syscall.SysProcAttr{
+			Setpgid: true,
+			Pgid:    cfg.parentPid,
+		}
+	} else {
+		// Start the command in its own process group (nice for signalling)
+		cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	}
+
 	command := &Command{
 		cmd:             cmd,
 		startTime:       time.Now(),
@@ -222,9 +232,6 @@ func New(ctx context.Context, nameAndArgs []string, opts ...Option) (*Command, e
 	// And finally inject environment variables required for tracing into the command.
 	cmd.Env = envInjector(ctx, cmd.Env)
 
-	// Start the command in its own process group (nice for signalling)
-	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
-
 	// Three possible values for stdin:
 	//   * nil - Go implicitly uses /dev/null
 	//   * stdinSentinel - configure with cmd.StdinPipe(), allowing Write() to work
diff --git a/internal/command/command_test.go b/internal/command/command_test.go
index e10dadd4e..784b52e85 100644
--- a/internal/command/command_test.go
+++ b/internal/command/command_test.go
@@ -7,9 +7,11 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"os"
 	"path/filepath"
 	"runtime"
 	"strings"
+	"syscall"
 	"testing"
 	"time"
 
@@ -21,6 +23,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/git/repository"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/metadata/featureflag"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
 )
 
@@ -512,3 +515,64 @@ func TestCommand_withFinalizer(t *testing.T) {
 		}
 	})
 }
+
+func TestCommand_withPgid(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+	r, w := io.Pipe()
+	parentCmd, err := New(ctx, []string{"cat", "/dev/stdin"}, WithStdin(r))
+	require.NoError(t, err)
+
+	errCh := make(chan error)
+	go func() {
+		errCh <- parentCmd.Wait()
+	}()
+
+	parentPid := parentCmd.cmd.Process.Pid
+
+	childCmd, err := New(
+		ctx,
+		[]string{"cat", "/dev/stdin"},
+		WithStdin(r),
+		WithParent(parentPid),
+	)
+	require.NoError(t, err)
+	go func() {
+		errCh <- childCmd.Wait()
+	}()
+
+	childPid := childCmd.cmd.Process.Pid
+
+	// check that both processes are running
+	for _, pid := range []int{parentPid, childPid} {
+		process, err := os.FindProcess(pid)
+		require.NoError(t, err)
+		require.NoError(t, process.Signal(syscall.Signal(0)))
+	}
+
+	// check that killing the pgid kills the childpid as well
+	if featureflag.RunCmdsInProcessGroup.IsEnabled(ctx) {
+		require.NoError(t, syscall.Kill(-parentPid, syscall.SIGKILL))
+	} else {
+		require.NoError(t, syscall.Kill(parentPid, syscall.SIGKILL))
+		require.NoError(t, syscall.Kill(childPid, syscall.SIGKILL))
+	}
+
+	w.Close()
+
+	err = <-errCh
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "signal: killed")
+
+	err = <-errCh
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "signal: killed")
+
+	// check that both processes are not running
+	for _, pid := range []int{parentPid, childPid} {
+		process, err := os.FindProcess(pid)
+		require.NoError(t, err)
+		require.Error(t, process.Signal(syscall.Signal(0)))
+	}
+}
diff --git a/internal/command/option.go b/internal/command/option.go
index cca8e62f1..b91ff8dbd 100644
--- a/internal/command/option.go
+++ b/internal/command/option.go
@@ -21,6 +21,8 @@ type config struct {
 
 	cgroupsManager CgroupsManager
 	cgroupsRepo    repository.GitRepo
+
+	parentPid int
 }
 
 // Option is an option that can be passed to `New()` for controlling how the command is being
@@ -108,3 +110,10 @@ func WithFinalizer(finalizer func(*Command)) Option {
 		cfg.finalizer = finalizer
 	}
 }
+
+// WithParent sets a parent pid under which the command will be spawned
+func WithParent(pid int) Option {
+	return func(cfg *config) {
+		cfg.parentPid = pid
+	}
+}
diff --git a/internal/metadata/featureflag/ff_run_cmds_in_pgroup.go b/internal/metadata/featureflag/ff_run_cmds_in_pgroup.go
new file mode 100644
index 000000000..055ec6798
--- /dev/null
+++ b/internal/metadata/featureflag/ff_run_cmds_in_pgroup.go
@@ -0,0 +1,10 @@
+package featureflag
+
+// RunCmdsInProcessGroup will run all git commands under the Gitaly pid as the
+// pgid.
+var RunCmdsInProcessGroup = NewFeatureFlag(
+	"run_cmds_in_process_group",
+	"v15.5.0",
+	"https://gitlab.com/gitlab-org/gitaly/-/issues/4494",
+	false,
+)
diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go
index 02440e7f0..63ef64cad 100644
--- a/internal/testhelper/testhelper.go
+++ b/internal/testhelper/testhelper.go
@@ -196,6 +196,7 @@ func ContextWithoutCancel(opts ...ContextOpt) context.Context {
 	// deep in the call stack, so almost every test function would have to inject it into its
 	// context.
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.RunCommandsInCGroup, true)
+	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.RunCmdsInProcessGroup, rnd.Int()%2 == 0)
 	// PraefectGeneratedReplicaPaths affects many tests as it changes the repository creation logic.
 	// Randomly enable the flag to exercise both paths to some extent.
 	ctx = featureflag.ContextWithFeatureFlag(ctx, featureflag.PraefectGeneratedReplicaPaths, rnd.Int()%2 == 0)