cgroups: Inject pid into cgroup mananger

To make testing easier, inject the current running pid into the cgroup
manager instead of having the cgroup manager call the global
os.Getpid(). This way, tests become safer from flakes.

7 files changed, 29 insertions, 21 deletions

diff --git a/cmd/gitaly/main.go b/cmd/gitaly/main.go
index a6032e247..9a276607f 100644
--- a/cmd/gitaly/main.go
+++ b/cmd/gitaly/main.go
@@ -117,7 +117,7 @@ func configure(configPath string) (config.Cfg, error) {
 
 	glog.Configure(glog.Loggers, cfg.Logging.Format, cfg.Logging.Level)
 
-	if err := cgroups.NewManager(cfg.Cgroups).Setup(); err != nil {
+	if err := cgroups.NewManager(cfg.Cgroups, os.Getpid()).Setup(); err != nil {
 		return config.Cfg{}, fmt.Errorf("failed setting up cgroups: %w", err)
 	}
 
@@ -376,7 +376,7 @@ func run(cfg config.Cfg) error {
 	defer shutdownWorkers()
 
 	defer func() {
-		if err := cgroups.NewManager(cfg.Cgroups).Cleanup(); err != nil {
+		if err := cgroups.NewManager(cfg.Cgroups, os.Getpid()).Cleanup(); err != nil {
 			log.WithError(err).Warn("error cleaning up cgroups")
 		}
 	}()
diff --git a/internal/cgroups/cgroups.go b/internal/cgroups/cgroups.go
index b2209b509..df68ef16a 100644
--- a/internal/cgroups/cgroups.go
+++ b/internal/cgroups/cgroups.go
@@ -24,9 +24,9 @@ type Manager interface {
 }
 
 // NewManager returns the appropriate Cgroups manager
-func NewManager(cfg cgroups.Config) Manager {
+func NewManager(cfg cgroups.Config, pid int) Manager {
 	if cfg.Repositories.Count > 0 {
-		return newV1Manager(cfg)
+		return newV1Manager(cfg, pid)
 	}
 
 	return &NoopManager{}
diff --git a/internal/cgroups/cgroups_linux_test.go b/internal/cgroups/cgroups_linux_test.go
index 98d469731..9f59a9a25 100644
--- a/internal/cgroups/cgroups_linux_test.go
+++ b/internal/cgroups/cgroups_linux_test.go
@@ -18,5 +18,5 @@ func TestNewManager(t *testing.T) {
 	cfg := cgroups.Config{Repositories: cgroups.Repositories{Count: 10}}
 
 	require.IsType(t, &CGroupV1Manager{}, &CGroupV1Manager{cfg: cfg})
-	require.IsType(t, &NoopManager{}, NewManager(cgroups.Config{}))
+	require.IsType(t, &NoopManager{}, NewManager(cgroups.Config{}, 1))
 }
diff --git a/internal/cgroups/v1.go b/internal/cgroups/v1.go
index 545f27bed..eb19db799 100644
--- a/internal/cgroups/v1.go
+++ b/internal/cgroups/v1.go
@@ -7,6 +7,6 @@ import (
 )
 
 // For systems other than Linux, we return a noop manager if cgroups was enabled.
-func newV1Manager(cfg cgroups.Config) *NoopManager {
+func newV1Manager(cfg cgroups.Config, pid int) *NoopManager {
 	return &NoopManager{}
 }
diff --git a/internal/cgroups/v1_linux.go b/internal/cgroups/v1_linux.go
index 17065ba20..57552230c 100644
--- a/internal/cgroups/v1_linux.go
+++ b/internal/cgroups/v1_linux.go
@@ -3,7 +3,6 @@ package cgroups
 import (
 	"fmt"
 	"hash/crc32"
-	"os"
 	"strings"
 
 	"github.com/containerd/cgroups"
@@ -21,11 +20,13 @@ type CGroupV1Manager struct {
 	hierarchy                            func() ([]cgroups.Subsystem, error)
 	memoryReclaimAttemptsTotal, cpuUsage *prometheus.GaugeVec
 	procs                                *prometheus.GaugeVec
+	pid                                  int
 }
 
-func newV1Manager(cfg cgroupscfg.Config) *CGroupV1Manager {
+func newV1Manager(cfg cgroupscfg.Config, pid int) *CGroupV1Manager {
 	return &CGroupV1Manager{
 		cfg: cfg,
+		pid: pid,
 		hierarchy: func() ([]cgroups.Subsystem, error) {
 			return defaultSubsystems(cfg.Mountpoint)
 		},
@@ -218,7 +219,7 @@ func (cg *CGroupV1Manager) repoPath(groupID int) string {
 }
 
 func (cg *CGroupV1Manager) currentProcessCgroup() string {
-	return fmt.Sprintf("/%s/gitaly-%d", cg.cfg.HierarchyRoot, os.Getpid())
+	return fmt.Sprintf("/%s/gitaly-%d", cg.cfg.HierarchyRoot, cg.pid)
 }
 
 func defaultSubsystems(root string) ([]cgroups.Subsystem, error) {
diff --git a/internal/cgroups/v1_linux_test.go b/internal/cgroups/v1_linux_test.go
index efa15b0bf..ad19884e7 100644
--- a/internal/cgroups/v1_linux_test.go
+++ b/internal/cgroups/v1_linux_test.go
@@ -38,22 +38,24 @@ func defaultCgroupsConfig() cgroups.Config {
 func TestSetup(t *testing.T) {
 	mock := newMock(t)
 
+	pid := 1
 	v1Manager := &CGroupV1Manager{
 		cfg:       defaultCgroupsConfig(),
 		hierarchy: mock.hierarchy,
+		pid:       pid,
 	}
 	require.NoError(t, v1Manager.Setup())
 
 	for i := 0; i < 3; i++ {
 		memoryPath := filepath.Join(
-			mock.root, "memory", "gitaly", fmt.Sprintf("gitaly-%d", os.Getpid()), fmt.Sprintf("repos-%d", i), "memory.limit_in_bytes",
+			mock.root, "memory", "gitaly", fmt.Sprintf("gitaly-%d", pid), fmt.Sprintf("repos-%d", i), "memory.limit_in_bytes",
 		)
 		memoryContent := readCgroupFile(t, memoryPath)
 
 		require.Equal(t, string(memoryContent), "1024000")
 
 		cpuPath := filepath.Join(
-			mock.root, "cpu", "gitaly", fmt.Sprintf("gitaly-%d", os.Getpid()), fmt.Sprintf("repos-%d", i), "cpu.shares",
+			mock.root, "cpu", "gitaly", fmt.Sprintf("gitaly-%d", pid), fmt.Sprintf("repos-%d", i), "cpu.shares",
 		)
 		cpuContent := readCgroupFile(t, cpuPath)
 
@@ -74,9 +76,11 @@ func TestAddCommand(t *testing.T) {
 	config.Repositories.MemoryBytes = 1024
 	config.Repositories.CPUShares = 16
 
+	pid := 1
 	v1Manager1 := &CGroupV1Manager{
 		cfg:       config,
 		hierarchy: mock.hierarchy,
+		pid:       pid,
 	}
 	require.NoError(t, v1Manager1.Setup())
 	ctx := testhelper.Context(t)
@@ -88,6 +92,7 @@ func TestAddCommand(t *testing.T) {
 	v1Manager2 := &CGroupV1Manager{
 		cfg:       config,
 		hierarchy: mock.hierarchy,
+		pid:       pid,
 	}
 
 	t.Run("without a repository", func(t *testing.T) {
@@ -99,13 +104,13 @@ func TestAddCommand(t *testing.T) {
 
 		for _, s := range mock.subsystems {
 			path := filepath.Join(mock.root, string(s.Name()), "gitaly",
-				fmt.Sprintf("gitaly-%d", os.Getpid()), fmt.Sprintf("repos-%d", groupID), "cgroup.procs")
+				fmt.Sprintf("gitaly-%d", pid), fmt.Sprintf("repos-%d", groupID), "cgroup.procs")
 			content := readCgroupFile(t, path)
 
-			pid, err := strconv.Atoi(string(content))
+			cmdPid, err := strconv.Atoi(string(content))
 			require.NoError(t, err)
 
-			require.Equal(t, cmd2.Pid(), pid)
+			require.Equal(t, cmd2.Pid(), cmdPid)
 		}
 	})
 
@@ -121,13 +126,13 @@ func TestAddCommand(t *testing.T) {
 
 		for _, s := range mock.subsystems {
 			path := filepath.Join(mock.root, string(s.Name()), "gitaly",
-				fmt.Sprintf("gitaly-%d", os.Getpid()), fmt.Sprintf("repos-%d", groupID), "cgroup.procs")
+				fmt.Sprintf("gitaly-%d", pid), fmt.Sprintf("repos-%d", groupID), "cgroup.procs")
 			content := readCgroupFile(t, path)
 
-			pid, err := strconv.Atoi(string(content))
+			cmdPid, err := strconv.Atoi(string(content))
 			require.NoError(t, err)
 
-			require.Equal(t, cmd2.Pid(), pid)
+			require.Equal(t, cmd2.Pid(), cmdPid)
 		}
 	})
 }
@@ -135,16 +140,18 @@ func TestAddCommand(t *testing.T) {
 func TestCleanup(t *testing.T) {
 	mock := newMock(t)
 
+	pid := 1
 	v1Manager := &CGroupV1Manager{
 		cfg:       defaultCgroupsConfig(),
 		hierarchy: mock.hierarchy,
+		pid:       pid,
 	}
 	require.NoError(t, v1Manager.Setup())
 	require.NoError(t, v1Manager.Cleanup())
 
 	for i := 0; i < 3; i++ {
-		memoryPath := filepath.Join(mock.root, "memory", "gitaly", fmt.Sprintf("gitaly-%d", os.Getpid()), fmt.Sprintf("repos-%d", i))
-		cpuPath := filepath.Join(mock.root, "cpu", "gitaly", fmt.Sprintf("gitaly-%d", os.Getpid()), fmt.Sprintf("repos-%d", i))
+		memoryPath := filepath.Join(mock.root, "memory", "gitaly", fmt.Sprintf("gitaly-%d", pid), fmt.Sprintf("repos-%d", i))
+		cpuPath := filepath.Join(mock.root, "cpu", "gitaly", fmt.Sprintf("gitaly-%d", pid), fmt.Sprintf("repos-%d", i))
 
 		require.NoDirExists(t, memoryPath)
 		require.NoDirExists(t, cpuPath)
@@ -165,7 +172,7 @@ func TestMetrics(t *testing.T) {
 	config.Repositories.MemoryBytes = 1048576
 	config.Repositories.CPUShares = 16
 
-	v1Manager1 := newV1Manager(config)
+	v1Manager1 := newV1Manager(config, 1)
 	v1Manager1.hierarchy = mock.hierarchy
 
 	mock.setupMockCgroupFiles(t, v1Manager1, 2)
diff --git a/internal/git/command_factory.go b/internal/git/command_factory.go
index 210288dd4..fb41037f1 100644
--- a/internal/git/command_factory.go
+++ b/internal/git/command_factory.go
@@ -127,7 +127,7 @@ func NewExecCommandFactory(cfg config.Cfg, opts ...ExecCommandFactoryOption) (_
 		cfg:            cfg,
 		execEnvs:       execEnvs,
 		locator:        config.NewLocator(cfg),
-		cgroupsManager: cgroups.NewManager(cfg.Cgroups),
+		cgroupsManager: cgroups.NewManager(cfg.Cgroups, os.Getpid()),
 		invalidCommandsMetric: prometheus.NewCounterVec(
 			prometheus.CounterOpts{
 				Name: "gitaly_invalid_commands_total",