Merge branch 'better-cgroups-handling-with-upgrades-enabled' into 'master'

Fix missing cgroups after upgrading Gitaly

See merge request gitlab-org/gitaly!2914

3 files changed, 22 insertions, 23 deletions

diff --git a/changelogs/unreleased/better-cgroups-handling-with-upgrades-enabled.yml b/changelogs/unreleased/better-cgroups-handling-with-upgrades-enabled.yml
new file mode 100644
index 000000000..d4f0a00af
--- /dev/null
+++ b/changelogs/unreleased/better-cgroups-handling-with-upgrades-enabled.yml
@@ -0,0 +1,5 @@
+---
+title: Fix missing cgroups after upgrading Gitaly
+merge_request: 2914
+author:
+type: fixed
diff --git a/internal/cgroups/v1_linux.go b/internal/cgroups/v1_linux.go
index 436b4d97a..6c41a8d7f 100644
--- a/internal/cgroups/v1_linux.go
+++ b/internal/cgroups/v1_linux.go
@@ -3,6 +3,7 @@ package cgroups
 import (
 	"fmt"
 	"hash/crc32"
+	"os"
 	"strings"
 
 	"github.com/containerd/cgroups"
@@ -74,33 +75,26 @@ func (cg *CGroupV1Manager) AddCommand(cmd *command.Command) error {
 }
 
 func (cg *CGroupV1Manager) Cleanup() error {
-	var failures []string
+	processCgroupPath := cg.currentProcessCgroup()
 
-	for i := 0; i < int(cg.cfg.Count); i++ {
-		cgroupPath := cg.cgroupPath(i)
-		control, err := cgroups.Load(cg.hierarchy, cgroups.StaticPath(cgroupPath))
-		if err != nil {
-			failure := fmt.Sprintf("%s: %v", cgroupPath, err)
-			failures = append(failures, failure)
-			continue
-		}
-
-		if err := control.Delete(); err != nil {
-			failure := fmt.Sprintf("%s: %v", cgroupPath, err)
-			failures = append(failures, failure)
-			continue
-		}
+	control, err := cgroups.Load(cg.hierarchy, cgroups.StaticPath(processCgroupPath))
+	if err != nil {
+		return fmt.Errorf("failed loading cgroup %s: %w", processCgroupPath, err)
 	}
 
-	if len(failures) > 0 {
-		return fmt.Errorf("failed cleaning up cgroups: %s", strings.Join(failures, ","))
+	if err := control.Delete(); err != nil {
+		return fmt.Errorf("failed cleaning up cgroup %s: %w", processCgroupPath, err)
 	}
 
 	return nil
 }
 
 func (cg *CGroupV1Manager) cgroupPath(groupID int) string {
-	return fmt.Sprintf("/%s/gitaly-%d", cg.cfg.HierarchyRoot, groupID)
+	return fmt.Sprintf("/%s/shard-%d", cg.currentProcessCgroup(), groupID)
+}
+
+func (cg *CGroupV1Manager) currentProcessCgroup() string {
+	return fmt.Sprintf("/%s/gitaly-%d", cg.cfg.HierarchyRoot, os.Getpid())
 }
 
 func defaultSubsystems(root string) ([]cgroups.Subsystem, error) {
diff --git a/internal/cgroups/v1_linux_test.go b/internal/cgroups/v1_linux_test.go
index da0039a2b..c2bbd04bd 100644
--- a/internal/cgroups/v1_linux_test.go
+++ b/internal/cgroups/v1_linux_test.go
@@ -44,14 +44,14 @@ func TestSetup(t *testing.T) {
 
 	for i := 0; i < 3; i++ {
 		memoryPath := filepath.Join(
-			mock.root, "memory", "gitaly", fmt.Sprintf("gitaly-%d", i), "memory.limit_in_bytes",
+			mock.root, "memory", "gitaly", fmt.Sprintf("gitaly-%d", os.Getpid()), fmt.Sprintf("shard-%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", i), "cpu.shares",
+			mock.root, "cpu", "gitaly", fmt.Sprintf("gitaly-%d", os.Getpid()), fmt.Sprintf("shard-%d", i), "cpu.shares",
 		)
 		cpuContent := readCgroupFile(t, cpuPath)
 
@@ -88,7 +88,7 @@ func TestAddCommand(t *testing.T) {
 	groupID := uint(checksum) % config.Count
 
 	for _, s := range mock.subsystems {
-		path := filepath.Join(mock.root, string(s.Name()), "gitaly", fmt.Sprintf("gitaly-%d", groupID), "cgroup.procs")
+		path := filepath.Join(mock.root, string(s.Name()), "gitaly", fmt.Sprintf("gitaly-%d", os.Getpid()), fmt.Sprintf("shard-%d", groupID), "cgroup.procs")
 		content := readCgroupFile(t, path)
 
 		pid, err := strconv.Atoi(string(content))
@@ -110,8 +110,8 @@ func TestCleanup(t *testing.T) {
 	require.NoError(t, v1Manager.Cleanup())
 
 	for i := 0; i < 3; i++ {
-		memoryPath := filepath.Join(mock.root, "memory", "gitaly", fmt.Sprintf("gitaly-%d", i))
-		cpuPath := filepath.Join(mock.root, "cpu", "gitaly", fmt.Sprintf("gitaly-%d", i))
+		memoryPath := filepath.Join(mock.root, "memory", "gitaly", fmt.Sprintf("gitaly-%d", os.Getpid()), fmt.Sprintf("shard-%d", i))
+		cpuPath := filepath.Join(mock.root, "cpu", "gitaly", fmt.Sprintf("gitaly-%d", os.Getpid()), fmt.Sprintf("shard-%d", i))
 
 		require.NoDirExists(t, memoryPath)
 		require.NoDirExists(t, cpuPath)