repository: Move cleanup of worktrees into housekeeping package

We want to allow for OptimizeRepository to be called without invoking an
RPC in order to allow us to call it in more contexts. As a preparatory
step, we thus have to move all functionality which is invoked by it into
a package that is independent of gRPC services.

Move the cleanup of worktrees into the housekeeping package as part of
this move.

7 files changed, 281 insertions, 248 deletions

diff --git a/internal/git/housekeeping/worktrees.go b/internal/git/housekeeping/worktrees.go
new file mode 100644
index 000000000..76ee8cb9c
--- /dev/null
+++ b/internal/git/housekeeping/worktrees.go
@@ -0,0 +1,159 @@
+package housekeeping
+
+//nolint:depguard
+import (
+	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+	"time"
+
+	"gitlab.com/gitlab-org/gitaly/v14/internal/command"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+const (
+	worktreePrefix = "gitlab-worktree"
+)
+
+// CleanupWorktrees cleans up stale and disconnected worktrees for the given repository.
+func CleanupWorktrees(ctx context.Context, repo *localrepo.Repo) error {
+	if _, err := repo.Path(); err != nil {
+		return err
+	}
+
+	worktreeThreshold := time.Now().Add(-6 * time.Hour)
+	if err := cleanStaleWorktrees(ctx, repo, worktreeThreshold); err != nil {
+		return status.Errorf(codes.Internal, "Cleanup: cleanStaleWorktrees: %v", err)
+	}
+
+	if err := cleanDisconnectedWorktrees(ctx, repo); err != nil {
+		return status.Errorf(codes.Internal, "Cleanup: cleanDisconnectedWorktrees: %v", err)
+	}
+
+	return nil
+}
+
+func cleanStaleWorktrees(ctx context.Context, repo *localrepo.Repo, threshold time.Time) error {
+	repoPath, err := repo.Path()
+	if err != nil {
+		return err
+	}
+
+	worktreePath := filepath.Join(repoPath, worktreePrefix)
+
+	dirInfo, err := os.Stat(worktreePath)
+	if err != nil {
+		if os.IsNotExist(err) || !dirInfo.IsDir() {
+			return nil
+		}
+		return err
+	}
+
+	worktreeEntries, err := ioutil.ReadDir(worktreePath)
+	if err != nil {
+		return err
+	}
+
+	for _, info := range worktreeEntries {
+		if !info.IsDir() || (info.Mode()&os.ModeSymlink != 0) {
+			continue
+		}
+
+		if info.ModTime().Before(threshold) {
+			err := removeWorktree(ctx, repo, info.Name())
+			switch {
+			case errors.Is(err, errUnknownWorktree):
+				// if git doesn't recognise the worktree then we can safely remove it
+				if err := os.RemoveAll(filepath.Join(worktreePath, info.Name())); err != nil {
+					return fmt.Errorf("worktree remove dir: %w", err)
+				}
+			case err != nil:
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
+// errUnknownWorktree indicates that git does not recognise the worktree
+var errUnknownWorktree = errors.New("unknown worktree")
+
+func removeWorktree(ctx context.Context, repo *localrepo.Repo, name string) error {
+	var stderr bytes.Buffer
+	err := repo.ExecAndWait(ctx, git.SubSubCmd{
+		Name:   "worktree",
+		Action: "remove",
+		Flags:  []git.Option{git.Flag{Name: "--force"}},
+		Args:   []string{name},
+	},
+		git.WithRefTxHook(repo),
+		git.WithStderr(&stderr),
+	)
+	if isExitWithCode(err, 128) && strings.HasPrefix(stderr.String(), "fatal: '"+name+"' is not a working tree") {
+		return errUnknownWorktree
+	} else if err != nil {
+		return fmt.Errorf("remove worktree: %w, stderr: %q", err, stderr.String())
+	}
+
+	return nil
+}
+
+func isExitWithCode(err error, code int) bool {
+	actual, ok := command.ExitStatus(err)
+	if !ok {
+		return false
+	}
+
+	return code == actual
+}
+
+func cleanDisconnectedWorktrees(ctx context.Context, repo *localrepo.Repo) error {
+	repoPath, err := repo.Path()
+	if err != nil {
+		return err
+	}
+
+	// Spawning a command is expensive. We thus try to avoid the overhead by first
+	// determining if there could possibly be any work to be done by git-worktree(1). We do so
+	// by reading the directory in which worktrees are stored, and if it's empty then we know
+	// that there aren't any worktrees in the first place.
+	worktreeEntries, err := os.ReadDir(filepath.Join(repoPath, "worktrees"))
+	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
+			return nil
+		}
+	}
+
+	hasWorktrees := false
+	for _, worktreeEntry := range worktreeEntries {
+		if !worktreeEntry.IsDir() {
+			continue
+		}
+
+		if worktreeEntry.Name() == "." || worktreeEntry.Name() == ".." {
+			continue
+		}
+
+		hasWorktrees = true
+		break
+	}
+
+	// There are no worktrees, so let's avoid spawning the Git command.
+	if !hasWorktrees {
+		return nil
+	}
+
+	return repo.ExecAndWait(ctx, git.SubSubCmd{
+		Name:   "worktree",
+		Action: "prune",
+	}, git.WithRefTxHook(repo))
+}
diff --git a/internal/git/housekeeping/worktrees_test.go b/internal/git/housekeeping/worktrees_test.go
new file mode 100644
index 000000000..e631e0c2c
--- /dev/null
+++ b/internal/git/housekeeping/worktrees_test.go
@@ -0,0 +1,104 @@
+package housekeeping
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
+)
+
+func TestCleanupDisconnectedWorktrees_doesNothingWithoutWorktrees(t *testing.T) {
+	t.Parallel()
+
+	ctx := testhelper.Context(t)
+	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+	worktreePath := filepath.Join(testhelper.TempDir(t), "worktree")
+
+	failingGitCmdFactory := gittest.NewInterceptingCommandFactory(ctx, t, cfg, func(git.ExecutionEnvironment) string {
+		return `#!/usr/bin/env bash
+		exit 15
+		`
+	})
+
+	repo := localrepo.New(config.NewLocator(cfg), failingGitCmdFactory, nil, repoProto)
+
+	// If this command did spawn git-worktree(1) we'd see an error. It doesn't though because it
+	// detects that there aren't any worktrees at all.
+	require.NoError(t, cleanDisconnectedWorktrees(ctx, repo))
+
+	gittest.AddWorktree(t, cfg, repoPath, worktreePath)
+
+	// We have now added a worktree now, so it should detect that there are worktrees and thus
+	// spawn the Git command. We thus expect the error code we inject via the failing Git
+	// command factory.
+	require.EqualError(t, cleanDisconnectedWorktrees(ctx, repo), "exit status 15")
+}
+
+func TestRemoveWorktree(t *testing.T) {
+	t.Parallel()
+
+	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
+
+	repo := localrepo.NewTestRepo(t, cfg, repoProto)
+
+	existingWorktreePath := filepath.Join(repoPath, worktreePrefix, "existing")
+	gittest.AddWorktree(t, cfg, repoPath, existingWorktreePath)
+
+	disconnectedWorktreePath := filepath.Join(repoPath, worktreePrefix, "disconnected")
+	gittest.AddWorktree(t, cfg, repoPath, disconnectedWorktreePath)
+	require.NoError(t, os.RemoveAll(disconnectedWorktreePath))
+
+	orphanedWorktreePath := filepath.Join(repoPath, worktreePrefix, "orphaned")
+	require.NoError(t, os.MkdirAll(orphanedWorktreePath, os.ModePerm))
+
+	for _, tc := range []struct {
+		worktree     string
+		errorIs      error
+		expectExists bool
+	}{
+		{
+			worktree:     "existing",
+			expectExists: false,
+		},
+		{
+			worktree:     "disconnected",
+			expectExists: false,
+		},
+		{
+			worktree:     "unknown",
+			errorIs:      errUnknownWorktree,
+			expectExists: false,
+		},
+		{
+			worktree:     "orphaned",
+			errorIs:      errUnknownWorktree,
+			expectExists: true,
+		},
+	} {
+		t.Run(tc.worktree, func(t *testing.T) {
+			ctx := testhelper.Context(t)
+
+			worktreePath := filepath.Join(repoPath, worktreePrefix, tc.worktree)
+
+			err := removeWorktree(ctx, repo, tc.worktree)
+			if tc.errorIs == nil {
+				require.NoError(t, err)
+			} else {
+				require.ErrorIs(t, err, tc.errorIs)
+			}
+
+			if tc.expectExists {
+				require.DirExists(t, worktreePath)
+			} else {
+				require.NoDirExists(t, worktreePath)
+			}
+		})
+	}
+}
diff --git a/internal/gitaly/service/repository/cleanup.go b/internal/gitaly/service/repository/cleanup.go
index 8755c053d..83d8514d7 100644
--- a/internal/gitaly/service/repository/cleanup.go
+++ b/internal/gitaly/service/repository/cleanup.go
@@ -1,169 +1,18 @@
 package repository
 
-//nolint:depguard
 import (
-	"bytes"
 	"context"
-	"errors"
-	"fmt"
-	"io/ioutil"
-	"os"
-	"path/filepath"
-	"strings"
-	"time"
 
-	"gitlab.com/gitlab-org/gitaly/v14/internal/command"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
+	"gitlab.com/gitlab-org/gitaly/v14/internal/git/housekeeping"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
-)
-
-const (
-	worktreePrefix = "gitlab-worktree"
 )
 
 func (s *server) Cleanup(ctx context.Context, in *gitalypb.CleanupRequest) (*gitalypb.CleanupResponse, error) {
 	repo := s.localrepo(in.GetRepository())
 
-	if err := cleanupWorktrees(ctx, repo); err != nil {
+	if err := housekeeping.CleanupWorktrees(ctx, repo); err != nil {
 		return nil, err
 	}
 
 	return &gitalypb.CleanupResponse{}, nil
 }
-
-func cleanupWorktrees(ctx context.Context, repo *localrepo.Repo) error {
-	if _, err := repo.Path(); err != nil {
-		return err
-	}
-
-	worktreeThreshold := time.Now().Add(-6 * time.Hour)
-	if err := cleanStaleWorktrees(ctx, repo, worktreeThreshold); err != nil {
-		return status.Errorf(codes.Internal, "Cleanup: cleanStaleWorktrees: %v", err)
-	}
-
-	if err := cleanDisconnectedWorktrees(ctx, repo); err != nil {
-		return status.Errorf(codes.Internal, "Cleanup: cleanDisconnectedWorktrees: %v", err)
-	}
-
-	return nil
-}
-
-func cleanStaleWorktrees(ctx context.Context, repo *localrepo.Repo, threshold time.Time) error {
-	repoPath, err := repo.Path()
-	if err != nil {
-		return err
-	}
-
-	worktreePath := filepath.Join(repoPath, worktreePrefix)
-
-	dirInfo, err := os.Stat(worktreePath)
-	if err != nil {
-		if os.IsNotExist(err) || !dirInfo.IsDir() {
-			return nil
-		}
-		return err
-	}
-
-	worktreeEntries, err := ioutil.ReadDir(worktreePath)
-	if err != nil {
-		return err
-	}
-
-	for _, info := range worktreeEntries {
-		if !info.IsDir() || (info.Mode()&os.ModeSymlink != 0) {
-			continue
-		}
-
-		if info.ModTime().Before(threshold) {
-			err := removeWorktree(ctx, repo, info.Name())
-			switch {
-			case errors.Is(err, errUnknownWorktree):
-				// if git doesn't recognise the worktree then we can safely remove it
-				if err := os.RemoveAll(filepath.Join(worktreePath, info.Name())); err != nil {
-					return fmt.Errorf("worktree remove dir: %w", err)
-				}
-			case err != nil:
-				return err
-			}
-		}
-	}
-
-	return nil
-}
-
-// errUnknownWorktree indicates that git does not recognise the worktree
-var errUnknownWorktree = errors.New("unknown worktree")
-
-func removeWorktree(ctx context.Context, repo *localrepo.Repo, name string) error {
-	var stderr bytes.Buffer
-	err := repo.ExecAndWait(ctx, git.SubSubCmd{
-		Name:   "worktree",
-		Action: "remove",
-		Flags:  []git.Option{git.Flag{Name: "--force"}},
-		Args:   []string{name},
-	},
-		git.WithRefTxHook(repo),
-		git.WithStderr(&stderr),
-	)
-	if isExitWithCode(err, 128) && strings.HasPrefix(stderr.String(), "fatal: '"+name+"' is not a working tree") {
-		return errUnknownWorktree
-	} else if err != nil {
-		return fmt.Errorf("remove worktree: %w, stderr: %q", err, stderr.String())
-	}
-
-	return nil
-}
-
-func isExitWithCode(err error, code int) bool {
-	actual, ok := command.ExitStatus(err)
-	if !ok {
-		return false
-	}
-
-	return code == actual
-}
-
-func cleanDisconnectedWorktrees(ctx context.Context, repo *localrepo.Repo) error {
-	repoPath, err := repo.Path()
-	if err != nil {
-		return err
-	}
-
-	// Spawning a command is expensive. We thus try to avoid the overhead by first
-	// determining if there could possibly be any work to be done by git-worktree(1). We do so
-	// by reading the directory in which worktrees are stored, and if it's empty then we know
-	// that there aren't any worktrees in the first place.
-	worktreeEntries, err := os.ReadDir(filepath.Join(repoPath, "worktrees"))
-	if err != nil {
-		if errors.Is(err, os.ErrNotExist) {
-			return nil
-		}
-	}
-
-	hasWorktrees := false
-	for _, worktreeEntry := range worktreeEntries {
-		if !worktreeEntry.IsDir() {
-			continue
-		}
-
-		if worktreeEntry.Name() == "." || worktreeEntry.Name() == ".." {
-			continue
-		}
-
-		hasWorktrees = true
-		break
-	}
-
-	// There are no worktrees, so let's avoid spawning the Git command.
-	if !hasWorktrees {
-		return nil
-	}
-
-	return repo.ExecAndWait(ctx, git.SubSubCmd{
-		Name:   "worktree",
-		Action: "prune",
-	}, git.WithRefTxHook(repo))
-}
diff --git a/internal/gitaly/service/repository/cleanup_test.go b/internal/gitaly/service/repository/cleanup_test.go
index 9975a22ab..373ae72cc 100644
--- a/internal/gitaly/service/repository/cleanup_test.go
+++ b/internal/gitaly/service/repository/cleanup_test.go
@@ -8,15 +8,15 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git/gittest"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/git/localrepo"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper"
-	"gitlab.com/gitlab-org/gitaly/v14/internal/testhelper/testcfg"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 )
 
+const (
+	worktreePrefix = "gitlab-worktree"
+)
+
 // TODO: replace emulated rebase RPC with actual
 // https://gitlab.com/gitlab-org/gitaly/issues/1750
 func TestCleanupDeletesStaleWorktrees(t *testing.T) {
@@ -142,92 +142,3 @@ func TestCleanupDisconnectedWorktrees(t *testing.T) {
 	// to checkout another worktree at the same path
 	gittest.AddWorktree(t, cfg, repoPath, worktreePath)
 }
-
-func TestCleanupDisconnectedWorktrees_doesNothingWithoutWorktrees(t *testing.T) {
-	t.Parallel()
-
-	ctx := testhelper.Context(t)
-	cfg, repoProto, repoPath, _ := setupRepositoryService(ctx, t)
-	worktreePath := filepath.Join(testhelper.TempDir(t), "worktree")
-
-	failingGitCmdFactory := gittest.NewInterceptingCommandFactory(ctx, t, cfg, func(git.ExecutionEnvironment) string {
-		return `#!/usr/bin/env bash
-		exit 15
-		`
-	})
-
-	repo := localrepo.New(config.NewLocator(cfg), failingGitCmdFactory, nil, repoProto)
-
-	// If this command did spawn git-worktree(1) we'd see an error. It doesn't though because it
-	// detects that there aren't any worktrees at all.
-	require.NoError(t, cleanDisconnectedWorktrees(ctx, repo))
-
-	gittest.AddWorktree(t, cfg, repoPath, worktreePath)
-
-	// We have now added a worktree now, so it should detect that there are worktrees and thus
-	// spawn the Git command. We thus expect the error code we inject via the failing Git
-	// command factory.
-	require.EqualError(t, cleanDisconnectedWorktrees(ctx, repo), "exit status 15")
-}
-
-func TestRemoveWorktree(t *testing.T) {
-	t.Parallel()
-
-	cfg, repoProto, repoPath := testcfg.BuildWithRepo(t)
-
-	repo := localrepo.NewTestRepo(t, cfg, repoProto)
-
-	existingWorktreePath := filepath.Join(repoPath, worktreePrefix, "existing")
-	gittest.AddWorktree(t, cfg, repoPath, existingWorktreePath)
-
-	disconnectedWorktreePath := filepath.Join(repoPath, worktreePrefix, "disconnected")
-	gittest.AddWorktree(t, cfg, repoPath, disconnectedWorktreePath)
-	require.NoError(t, os.RemoveAll(disconnectedWorktreePath))
-
-	orphanedWorktreePath := filepath.Join(repoPath, worktreePrefix, "orphaned")
-	require.NoError(t, os.MkdirAll(orphanedWorktreePath, os.ModePerm))
-
-	for _, tc := range []struct {
-		worktree     string
-		errorIs      error
-		expectExists bool
-	}{
-		{
-			worktree:     "existing",
-			expectExists: false,
-		},
-		{
-			worktree:     "disconnected",
-			expectExists: false,
-		},
-		{
-			worktree:     "unknown",
-			errorIs:      errUnknownWorktree,
-			expectExists: false,
-		},
-		{
-			worktree:     "orphaned",
-			errorIs:      errUnknownWorktree,
-			expectExists: true,
-		},
-	} {
-		t.Run(tc.worktree, func(t *testing.T) {
-			ctx := testhelper.Context(t)
-
-			worktreePath := filepath.Join(repoPath, worktreePrefix, tc.worktree)
-
-			err := removeWorktree(ctx, repo, tc.worktree)
-			if tc.errorIs == nil {
-				require.NoError(t, err)
-			} else {
-				require.ErrorIs(t, err, tc.errorIs)
-			}
-
-			if tc.expectExists {
-				require.DirExists(t, worktreePath)
-			} else {
-				require.NoDirExists(t, worktreePath)
-			}
-		})
-	}
-}
diff --git a/internal/gitaly/service/repository/create_bundle_from_ref_list.go b/internal/gitaly/service/repository/create_bundle_from_ref_list.go
index 6bc15efd1..0f8ae2f21 100644
--- a/internal/gitaly/service/repository/create_bundle_from_ref_list.go
+++ b/internal/gitaly/service/repository/create_bundle_from_ref_list.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"io"
 
+	"gitlab.com/gitlab-org/gitaly/v14/internal/command"
 	"gitlab.com/gitlab-org/gitaly/v14/internal/git"
 	"gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/v14/streamio"
@@ -77,3 +78,12 @@ func (s *server) CreateBundleFromRefList(stream gitalypb.RepositoryService_Creat
 
 	return nil
 }
+
+func isExitWithCode(err error, code int) bool {
+	actual, ok := command.ExitStatus(err)
+	if !ok {
+		return false
+	}
+
+	return code == actual
+}
diff --git a/internal/gitaly/service/repository/gc.go b/internal/gitaly/service/repository/gc.go
index 17bb15473..13adfc326 100644
--- a/internal/gitaly/service/repository/gc.go
+++ b/internal/gitaly/service/repository/gc.go
@@ -29,7 +29,7 @@ func (s *server) GarbageCollect(ctx context.Context, in *gitalypb.GarbageCollect
 
 	repo := s.localrepo(in.GetRepository())
 
-	if err := cleanupWorktrees(ctx, repo); err != nil {
+	if err := housekeeping.CleanupWorktrees(ctx, repo); err != nil {
 		return nil, err
 	}
 
diff --git a/internal/gitaly/service/repository/optimize.go b/internal/gitaly/service/repository/optimize.go
index 8295756b8..8640333b5 100644
--- a/internal/gitaly/service/repository/optimize.go
+++ b/internal/gitaly/service/repository/optimize.go
@@ -68,7 +68,7 @@ func optimizeRepository(ctx context.Context, repo *localrepo.Repo, txManager tra
 		return fmt.Errorf("could not execute houskeeping: %w", err)
 	}
 
-	if err := cleanupWorktrees(ctx, repo); err != nil {
+	if err := housekeeping.CleanupWorktrees(ctx, repo); err != nil {
 		return fmt.Errorf("could not clean up worktrees: %w", err)
 	}