Colocate internal socket dir creation with runtime dir creation

The internal socket directory is expected to be present in the
runtime directory. The config loading process is currently creating
the directory when setting defaults. As we are about to move the
runtime directory creation outside of the config loading process,
let's colocate the internal socket directory creation with the runtime
directory creation as it depends on it. Along with this logic, the
validation logic is colocated with the runtime directory creation
logic as it's not possible to validate the internal socket before the
runtime directory is setup.

Some test cases have been removed as they no longer make sense, such
as the non-existent internal socket dir tests. SetupRuntimeDirectory
always creates the directory.

2 files changed, 23 insertions, 50 deletions

diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go
index c269eabd1..8e32468aa 100644
--- a/internal/gitaly/config/config.go
+++ b/internal/gitaly/config/config.go
@@ -206,7 +206,6 @@ func (cfg *Cfg) Validate() error {
 		cfg.ConfigureRuby,
 		cfg.validateBinDir,
 		cfg.validateRuntimeDir,
-		cfg.validateInternalSocketDir,
 		cfg.validateMaintenance,
 		cfg.validateCgroups,
 		cfg.configurePackObjectsCache,
@@ -238,14 +237,6 @@ func (cfg *Cfg) setDefaults() error {
 	}
 	cfg.RuntimeDir = runtimeDir
 
-	// The socket path must be short-ish because listen(2) fails on long
-	// socket paths. We hope/expect that os.MkdirTemp creates a directory
-	// that is not too deep. We need a directory, not a tempfile, because we
-	// will later want to set its permissions to 0700
-	if err := os.Mkdir(cfg.InternalSocketDir(), 0o700); err != nil {
-		return fmt.Errorf("create internal socket directory: %w", err)
-	}
-
 	if reflect.DeepEqual(cfg.DailyMaintenance, DailyJob{}) {
 		cfg.DailyMaintenance = defaultMaintenanceWindow(cfg.Storages)
 	}
@@ -441,18 +432,6 @@ func (cfg *Cfg) validateToken() error {
 	return nil
 }
 
-func (cfg *Cfg) validateInternalSocketDir() error {
-	if err := validateIsDirectory(cfg.InternalSocketDir(), "internal_socket_dir"); err != nil {
-		return err
-	}
-
-	if err := trySocketCreation(cfg.InternalSocketDir()); err != nil {
-		return fmt.Errorf("failed creating internal test socket: %w", err)
-	}
-
-	return nil
-}
-
 func trySocketCreation(dir string) error {
 	// To validate the socket can actually be created, we open and close a socket.
 	// Any error will be assumed persistent for when the gitaly-ruby sockets are created
@@ -576,8 +555,9 @@ func (cfg *Cfg) configurePackObjectsCache() error {
 // it's used as the parent directory for the runtime directory. Runtime directory owner process
 // can be identified by the suffix process ID suffixed in the directory name. If a directory already
 // exists for this process' ID, it's removed and recreated. If cfg.RuntimeDir is not set, a temporary
-// directory is used instead. SetupRuntimeDirectory returns the absolute path to the created runtime
-// directory.
+// directory is used instead. A directory is created for the internal socket as well since it is
+// expected to be present in the runtime directory. SetupRuntimeDirectory returns the absolute path
+// to the created runtime directory.
 func SetupRuntimeDirectory(cfg Cfg, processID int) (string, error) {
 	var runtimeDir string
 	if cfg.RuntimeDir == "" {
@@ -618,5 +598,21 @@ func SetupRuntimeDirectory(cfg Cfg, processID int) (string, error) {
 		}
 	}
 
+	// Set the runtime dir in the config as the internal socket helpers
+	// rely on it.
+	cfg.RuntimeDir = runtimeDir
+
+	// The socket path must be short-ish because listen(2) fails on long
+	// socket paths. We hope/expect that os.MkdirTemp creates a directory
+	// that is not too deep. We need a directory, not a tempfile, because we
+	// will later want to set its permissions to 0700
+	if err := os.Mkdir(cfg.InternalSocketDir(), 0o700); err != nil {
+		return "", fmt.Errorf("create internal socket directory: %w", err)
+	}
+
+	if err := trySocketCreation(cfg.InternalSocketDir()); err != nil {
+		return "", fmt.Errorf("failed creating internal test socket: %w", err)
+	}
+
 	return runtimeDir, nil
 }
diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go
index 2967fd398..ce73e9c8a 100644
--- a/internal/gitaly/config/config_test.go
+++ b/internal/gitaly/config/config_test.go
@@ -601,9 +601,9 @@ dir = '%s'`, gitlabShellDir))
 	}, cfg.Hooks)
 }
 
-func TestValidateInternalSocketDir(t *testing.T) {
+func TestSetupRuntimeDirectory_validateInternalSocket(t *testing.T) {
 	verifyPathDoesNotExist := func(t *testing.T, runtimeDir string, actualErr error) {
-		require.Equal(t, fmt.Errorf("internal_socket_dir: path doesn't exist: %q", filepath.Join(runtimeDir, "sock.d")), actualErr)
+		require.EqualError(t, actualErr, fmt.Sprintf("creating runtime directory: mkdir %s/gitaly-%d: no such file or directory", runtimeDir, os.Getpid()))
 	}
 
 	testCases := []struct {
@@ -612,13 +612,6 @@ func TestValidateInternalSocketDir(t *testing.T) {
 		verify func(t *testing.T, runtimeDir string, actualErr error)
 	}{
 		{
-			desc: "unconfigured runtime directory",
-			setup: func(t *testing.T) string {
-				return ""
-			},
-			verify: verifyPathDoesNotExist,
-		},
-		{
 			desc: "non existing directory",
 			setup: func(t *testing.T) string {
 				return "/path/does/not/exist"
@@ -626,22 +619,6 @@ func TestValidateInternalSocketDir(t *testing.T) {
 			verify: verifyPathDoesNotExist,
 		},
 		{
-			desc: "runtime directory missing sock.d",
-			setup: func(t *testing.T) string {
-				runtimeDir := testhelper.TempDir(t)
-				return runtimeDir
-			},
-			verify: verifyPathDoesNotExist,
-		},
-		{
-			desc: "runtime directory with valid sock.d",
-			setup: func(t *testing.T) string {
-				runtimeDir := testhelper.TempDir(t)
-				require.NoError(t, os.Mkdir(filepath.Join(runtimeDir, "sock.d"), os.ModePerm))
-				return runtimeDir
-			},
-		},
-		{
 			desc: "symlinked runtime directory",
 			setup: func(t *testing.T) string {
 				runtimeDir := testhelper.TempDir(t)
@@ -690,11 +667,11 @@ func TestValidateInternalSocketDir(t *testing.T) {
 				RuntimeDir: runtimeDir,
 			}
 
-			actualErr := cfg.validateInternalSocketDir()
+			_, actualErr := SetupRuntimeDirectory(cfg, os.Getpid())
 			if tc.verify == nil {
 				require.NoError(t, actualErr)
 			} else {
-				tc.verify(t, runtimeDir, actualErr)
+				tc.verify(t, cfg.RuntimeDir, actualErr)
 			}
 		})
 	}