Generalize internal gitaly socket dir

Both gitaly-ruby and gitaly hooks will need the gitaly internal socket
directory, so we can pull it out into the config package. Also this
change starts an internal gitaly socket that listens for connections.

7 files changed, 80 insertions, 32 deletions

diff --git a/changelogs/unreleased/jc-gitaly-internal-socket.yml b/changelogs/unreleased/jc-gitaly-internal-socket.yml
new file mode 100644
index 000000000..267cab425
--- /dev/null
+++ b/changelogs/unreleased/jc-gitaly-internal-socket.yml
@@ -0,0 +1,5 @@
+---
+title: Use internal socket dir for internal gitaly socket
+merge_request: 1642
+author:
+type: other
diff --git a/cmd/gitaly/main.go b/cmd/gitaly/main.go
index 624c08747..f8860df2c 100644
--- a/cmd/gitaly/main.go
+++ b/cmd/gitaly/main.go
@@ -92,6 +92,7 @@ func run(b *bootstrap.Bootstrap) error {
 
 	for _, c := range []starter.Config{
 		{starter.Unix, config.Config.SocketPath},
+		{starter.Unix, config.GitalyInternalSocketPath()},
 		{starter.TCP, config.Config.ListenAddr},
 		{starter.TLS, config.Config.TLSListenAddr},
 	} {
diff --git a/internal/bootstrap/server_factory.go b/internal/bootstrap/server_factory.go
index ca6d870d1..c754fbf53 100644
--- a/internal/bootstrap/server_factory.go
+++ b/internal/bootstrap/server_factory.go
@@ -39,6 +39,7 @@ func (s *GitalyServerFactory) Stop() {
 	}
 
 	s.ruby.Stop()
+	server.CleanupInternalSocketDir()
 }
 
 // GracefulStop stops both the secure and insecure servers gracefully
diff --git a/internal/config/config.go b/internal/config/config.go
index d38616586..a814e315c 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -3,11 +3,13 @@ package config
 import (
 	"fmt"
 	"io"
+	"io/ioutil"
 	"net"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/BurntSushi/toml"
@@ -357,6 +359,52 @@ func validateToken() error {
 	return nil
 }
 
+var (
+	lazyInit                   sync.Once
+	generatedInternalSocketDir string
+)
+
+func generateSocketPath() {
+	// The socket path must be short-ish because listen(2) fails on long
+	// socket paths. We hope/expect that ioutil.TempDir 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
+
+	var err error
+	generatedInternalSocketDir, err = ioutil.TempDir("", "gitaly-internal")
+	if err != nil {
+		log.Fatalf("create ruby server socket directory: %v", err)
+	}
+}
+
+// InternalSocketDir will generate a temp dir for internal sockets if one is not provided in the config
+func InternalSocketDir() string {
+	if Config.InternalSocketDir != "" {
+		return Config.InternalSocketDir
+	}
+
+	if generatedInternalSocketDir == "" {
+		lazyInit.Do(generateSocketPath)
+	}
+
+	return generatedInternalSocketDir
+}
+
+// GitalyInternalSocketPath is the path to the internal gitaly socket
+func GitalyInternalSocketPath() string {
+	socketDir := InternalSocketDir()
+	if socketDir == "" {
+		panic("internal socket directory is missing")
+	}
+
+	return filepath.Join(socketDir, "internal.sock")
+}
+
+// GeneratedInternalSocketDir returns the path to the generated internal socket directory
+func GeneratedInternalSocketDir() string {
+	return generatedInternalSocketDir
+}
+
 func validateInternalSocketDir() error {
 	if Config.InternalSocketDir == "" {
 		return nil
@@ -372,6 +420,10 @@ func validateInternalSocketDir() error {
 		return fmt.Errorf("InternalSocketDir %s is not a directory", dir)
 	}
 
+	return trySocketCreation(dir)
+}
+
+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
 	// and thus fatal at boot time
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index fac19e48a..abe48934c 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -714,3 +714,14 @@ func TestValidateInternalSocketDir(t *testing.T) {
 		})
 	}
 }
+
+func TestInternalSocketDir(t *testing.T) {
+	defer func(internalSocketDir string) {
+		Config.InternalSocketDir = internalSocketDir
+	}(Config.InternalSocketDir)
+
+	Config.InternalSocketDir = ""
+	socketDir := InternalSocketDir()
+
+	require.NoError(t, trySocketCreation(socketDir))
+}
diff --git a/internal/rubyserver/rubyserver.go b/internal/rubyserver/rubyserver.go
index 5f55e3ff4..c95b6934d 100644
--- a/internal/rubyserver/rubyserver.go
+++ b/internal/rubyserver/rubyserver.go
@@ -3,7 +3,6 @@ package rubyserver
 import (
 	"context"
 	"fmt"
-	"io/ioutil"
 	"net"
 	"os"
 	"path"
@@ -14,7 +13,6 @@ import (
 
 	grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
 	grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
-	log "github.com/sirupsen/logrus"
 	"gitlab.com/gitlab-org/gitaly/internal/command"
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 	"gitlab.com/gitlab-org/gitaly/internal/git/hooks"
@@ -31,10 +29,6 @@ import (
 )
 
 var (
-	socketDir string
-
-	lazyInit sync.Once
-
 	// ConnectTimeout is the timeout for establishing a connection to the gitaly-ruby process.
 	ConnectTimeout = 40 * time.Second
 )
@@ -46,27 +40,10 @@ func init() {
 	}
 }
 
-func prepareSocketPath() {
-	if config.Config.InternalSocketDir != "" {
-		socketDir = config.Config.InternalSocketDir
-		return
-	}
-
-	// The socket path must be short-ish because listen(2) fails on long
-	// socket paths. We hope/expect that ioutil.TempDir 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. The permission change
-	// is done in the Ruby child process.
-	var err error
-	socketDir, err = ioutil.TempDir("", "gitaly-ruby")
-	if err != nil {
-		log.Fatalf("create ruby server socket directory: %v", err)
-	}
-}
-
 func socketPath(id int) string {
+	socketDir := config.InternalSocketDir()
 	if socketDir == "" {
-		panic("socketDir is not set")
+		panic("internal socket directory is missing")
 	}
 
 	return filepath.Join(socketDir, fmt.Sprintf("ruby.%d", id))
@@ -95,11 +72,6 @@ func (s *Server) Stop() {
 			w.Process.Stop()
 		}
 	}
-
-	// If we use the old gitaly-ruby temp dir, we should clean up
-	if config.Config.InternalSocketDir == "" && socketDir != "" {
-		os.RemoveAll(socketDir)
-	}
 }
 
 // Start spawns the Ruby server.
@@ -109,8 +81,6 @@ func (s *Server) Start() error {
 }
 
 func (s *Server) start() error {
-	lazyInit.Do(prepareSocketPath)
-
 	wd, err := os.Getwd()
 	if err != nil {
 		return err
diff --git a/internal/server/server.go b/internal/server/server.go
index 97013d48e..250749248 100644
--- a/internal/server/server.go
+++ b/internal/server/server.go
@@ -3,6 +3,7 @@ package server
 import (
 	"context"
 	"crypto/tls"
+	"os"
 
 	grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
 	grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
@@ -139,3 +140,10 @@ func NewInsecure(rubyServer *rubyserver.Server) *grpc.Server {
 func NewSecure(rubyServer *rubyserver.Server) *grpc.Server {
 	return createNewServer(rubyServer, true)
 }
+
+// CleanupInternalSocketDir will clean up the directory for internal sockets if it is a generated temp dir
+func CleanupInternalSocketDir() {
+	if tmpDir := config.GeneratedInternalSocketDir(); tmpDir != "" {
+		os.RemoveAll(tmpDir)
+	}
+}