Prune leftover runtime directories on startupsmh-fix-runtime-dir-cleanup

Gitaly can leave around stale runtime directories if it fails to run
its clean up when terminating. These runtime directories can pile up
as there's nothing cleaning them up later. This commit adds a pruning
logic where Gitaly will remove runtime directories that belonged to
process' that no longer exist. The process that created the runtime
directory is identified by the process id suffixed to the directory
name. The process is not perfect and can leave some directories
uncleaned if another process now has the same pid as the process that
created the directory.

Changelog: fixed

3 files changed, 152 insertions, 0 deletions

diff --git a/cmd/gitaly/main.go b/cmd/gitaly/main.go
index 781f5fd67..161095fbe 100644
--- a/cmd/gitaly/main.go
+++ b/cmd/gitaly/main.go
@@ -136,6 +136,12 @@ func run(cfg config.Cfg) error {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
+	if cfg.RuntimeDir != "" {
+		if err := config.PruneRuntimeDirectories(log.StandardLogger(), cfg.RuntimeDir); err != nil {
+			return fmt.Errorf("prune runtime directories: %w", err)
+		}
+	}
+
 	runtimeDir, err := config.SetupRuntimeDirectory(cfg, os.Getpid())
 	if err != nil {
 		return fmt.Errorf("setup runtime directory: %w", err)
diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go
index 95214ce8f..fad5c3a90 100644
--- a/internal/gitaly/config/config.go
+++ b/internal/gitaly/config/config.go
@@ -8,6 +8,7 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"strconv"
 	"strings"
 	"syscall"
 	"time"
@@ -517,6 +518,70 @@ func (cfg *Cfg) configurePackObjectsCache() error {
 	return nil
 }
 
+// PruneRuntimeDirectories removes leftover runtime directories that belonged to processes that
+// no longer exist. The removals are logged prior to being executed. Unexpected directory entries
+// are logged but not removed
+func PruneRuntimeDirectories(log log.FieldLogger, runtimeDir string) error {
+	entries, err := os.ReadDir(runtimeDir)
+	if err != nil {
+		return fmt.Errorf("list runtime directory: %w", err)
+	}
+
+	for _, entry := range entries {
+		if err := func() error {
+			log := log.WithField("path", filepath.Join(runtimeDir, entry.Name()))
+			if !entry.IsDir() {
+				// There should be no files, only the runtime directories.
+				log.Error("runtime directory contained an unexpected file")
+				return nil
+			}
+
+			components := strings.Split(entry.Name(), "-")
+			if len(components) != 2 || components[0] != "gitaly" {
+				// This directory does not match the runtime directory naming format
+				// of `gitaly-<process id>.
+				log.Error("runtime directory contained an unexpected directory")
+				return nil
+			}
+
+			processID, err := strconv.ParseInt(components[1], 10, 64)
+			if err != nil {
+				// This is not a runtime directory as the section after the hyphen is not a process id.
+				log.Error("runtime directory contained an unexpected directory")
+				return nil
+			}
+
+			process, err := os.FindProcess(int(processID))
+			if err != nil {
+				return fmt.Errorf("find process: %w", err)
+			}
+			defer func() {
+				if err := process.Release(); err != nil {
+					log.Error("failed releasing process")
+				}
+			}()
+
+			if err := process.Signal(syscall.Signal(0)); err != nil {
+				if !errors.Is(err, os.ErrProcessDone) {
+					return fmt.Errorf("signal: %w", err)
+				}
+
+				log.Info("removing leftover runtime directory")
+
+				if err := os.RemoveAll(filepath.Join(runtimeDir, entry.Name())); err != nil {
+					return fmt.Errorf("remove leftover runtime directory: %w", err)
+				}
+			}
+
+			return nil
+		}(); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // SetupRuntimeDirectory creates a new runtime directory. Runtime directory contains internal
 // runtime data generated by Gitaly such as the internal sockets. If cfg.RuntimeDir is set,
 // it's used as the parent directory for the runtime directory. Runtime directory owner process
diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go
index 4351e421c..b500d50a7 100644
--- a/internal/gitaly/config/config_test.go
+++ b/internal/gitaly/config/config_test.go
@@ -4,11 +4,13 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/gitaly/config/auth"
@@ -1263,3 +1265,82 @@ func TestSetupRuntimeDirectory(t *testing.T) {
 		}
 	})
 }
+
+func TestPruneRuntimeDirectories(t *testing.T) {
+	t.Run("no runtime directories", func(t *testing.T) {
+		require.NoError(t, PruneRuntimeDirectories(testhelper.NewDiscardingLogEntry(t), testhelper.TempDir(t)))
+	})
+
+	t.Run("unset runtime directory", func(t *testing.T) {
+		require.EqualError(t, PruneRuntimeDirectories(testhelper.NewDiscardingLogEntry(t), ""), "list runtime directory: open : no such file or directory")
+	})
+
+	t.Run("non-existent runtime directory", func(t *testing.T) {
+		require.EqualError(t, PruneRuntimeDirectories(testhelper.NewDiscardingLogEntry(t), "/path/does/not/exist"), "list runtime directory: open /path/does/not/exist: no such file or directory")
+	})
+
+	t.Run("invalid, stale and active runtime directories", func(t *testing.T) {
+		baseDir := testhelper.TempDir(t)
+		cfg := Cfg{RuntimeDir: baseDir}
+
+		// Setup a runtime directory for our process, it can't be stale as long as
+		// we are running.
+		ownRuntimeDir, err := SetupRuntimeDirectory(cfg, os.Getpid())
+		require.NoError(t, err)
+
+		expectedLogs := map[string]string{}
+
+		// Setup runtime directories for processes that have finished.
+		var prunableDirs []string
+		for i := 0; i < 2; i++ {
+			cmd := exec.Command("cat")
+			require.NoError(t, cmd.Run())
+
+			staleRuntimeDir, err := SetupRuntimeDirectory(cfg, cmd.Process.Pid)
+			require.NoError(t, err)
+
+			prunableDirs = append(prunableDirs, staleRuntimeDir)
+			expectedLogs[staleRuntimeDir] = "removing leftover runtime directory"
+		}
+
+		// Create an unexpected file in the runtime directory
+		unexpectedFilePath := filepath.Join(baseDir, "unexpected-file")
+		require.NoError(t, os.WriteFile(unexpectedFilePath, []byte(""), os.ModePerm))
+		expectedLogs[unexpectedFilePath] = "runtime directory contained an unexpected file"
+
+		nonPrunableDirs := []string{ownRuntimeDir}
+
+		// Setup some unexpected directories in the runtime directory
+		for _, dirName := range []string{
+			"nohyphen",
+			"too-many-hyphens",
+			"invalidprefix-3",
+			"gitaly-invalidpid",
+		} {
+			dirPath := filepath.Join(baseDir, dirName)
+			require.NoError(t, os.Mkdir(dirPath, os.ModePerm))
+			expectedLogs[dirPath] = "runtime directory contained an unexpected directory"
+			nonPrunableDirs = append(nonPrunableDirs, dirPath)
+		}
+
+		logger, hook := test.NewNullLogger()
+		require.NoError(t, PruneRuntimeDirectories(logger, cfg.RuntimeDir))
+
+		actualLogs := map[string]string{}
+		for _, entry := range hook.Entries {
+			actualLogs[entry.Data["path"].(string)] = entry.Message
+		}
+
+		require.Equal(t, expectedLogs, actualLogs)
+
+		require.FileExists(t, unexpectedFilePath)
+
+		for _, nonPrunableEntry := range nonPrunableDirs {
+			require.DirExists(t, nonPrunableEntry, nonPrunableEntry)
+		}
+
+		for _, prunableEntry := range prunableDirs {
+			require.NoDirExists(t, prunableEntry, prunableEntry)
+		}
+	})
+}