Merge branch 'fix/cgroup-pid-0-deletion' into 'master'

temp_dir: Delete gitaly-0 runtime directory

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5217

Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Christian Couder <chriscool@tuxfamily.org>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Co-authored-by: Steve Azzopardi <sazzopardi@gitlab.com>

3 files changed, 43 insertions, 0 deletions

diff --git a/internal/cgroups/cgroups_linux_test.go b/internal/cgroups/cgroups_linux_test.go
index a921bc778..5f621a6e1 100644
--- a/internal/cgroups/cgroups_linux_test.go
+++ b/internal/cgroups/cgroups_linux_test.go
@@ -120,6 +120,25 @@ func TestPruneOldCgroups(t *testing.T) {
 			},
 			expectedPruned: false,
 		},
+		{
+			desc: "gitaly-0 directory is deleted",
+			cfg: cgroups.Config{
+				Mountpoint:    testhelper.TempDir(t),
+				HierarchyRoot: "gitaly",
+				Repositories: cgroups.Repositories{
+					Count:       10,
+					MemoryBytes: 10 * 1024 * 1024,
+					CPUShares:   1024,
+				},
+			},
+			setup: func(t *testing.T, cfg cgroups.Config) int {
+				cgroupManager := NewManager(cfg, 0)
+				require.NoError(t, cgroupManager.Setup())
+
+				return 0
+			},
+			expectedPruned: true,
+		},
 	}
 
 	for _, tc := range testCases {
diff --git a/internal/gitaly/config/temp_dir.go b/internal/gitaly/config/temp_dir.go
index d27100edf..01f6a55dc 100644
--- a/internal/gitaly/config/temp_dir.go
+++ b/internal/gitaly/config/temp_dir.go
@@ -44,6 +44,17 @@ func PruneOldGitalyProcessDirectories(log log.FieldLogger, directory string) err
 				return errors.New("gitaly process directory contains an unexpected directory")
 			}
 
+			// When the pid is 0 it might be from a previous failed run, so
+			// just delete it.
+			if processID == 0 {
+				if err := os.RemoveAll(filepath.Join(directory, entry.Name())); err != nil {
+					return fmt.Errorf("removing leftover gitaly process directory: %w", err)
+				}
+				log.Info("removed gitaly directory with no pid")
+
+				return nil
+			}
+
 			process, err := os.FindProcess(int(processID))
 			if err != nil {
 				return fmt.Errorf("could not find process: %w", err)
diff --git a/internal/gitaly/config/temp_dir_test.go b/internal/gitaly/config/temp_dir_test.go
index 9961278ce..e30d0890c 100644
--- a/internal/gitaly/config/temp_dir_test.go
+++ b/internal/gitaly/config/temp_dir_test.go
@@ -110,4 +110,17 @@ func TestPruneOldGitalyProcessDirectories(t *testing.T) {
 			require.NoDirExists(t, prunableEntry, prunableEntry)
 		}
 	})
+
+	t.Run("gitaly-0 directory exists", func(t *testing.T) {
+		baseDir := testhelper.TempDir(t)
+		cfg := Cfg{RuntimeDir: baseDir}
+
+		_, err := SetupRuntimeDirectory(cfg, 0)
+		require.NoError(t, err)
+
+		logger, hook := test.NewNullLogger()
+		require.NoError(t, PruneOldGitalyProcessDirectories(logger, cfg.RuntimeDir))
+		require.Len(t, hook.Entries, 1)
+		require.Equal(t, "removed gitaly directory with no pid", hook.LastEntry().Message)
+	})
 }