Merge branch 'jc-fix-bundle' into 'master'

Call Cleanup before creating bundle

Closes gitlab#24626

See merge request gitlab-org/gitaly!2202

3 files changed, 30 insertions, 5 deletions

diff --git a/internal/service/repository/cleanup.go b/internal/service/repository/cleanup.go
index e18e9f401..1b44fe33e 100644
--- a/internal/service/repository/cleanup.go
+++ b/internal/service/repository/cleanup.go
@@ -40,7 +40,7 @@ func cleanupRepo(ctx context.Context, repo *gitalypb.Repository) error {
 	}
 
 	worktreeThreshold := time.Now().Add(-6 * time.Hour)
-	if err := cleanStaleWorktrees(repoPath, worktreeThreshold); err != nil {
+	if err := cleanStaleWorktrees(ctx, repo, repoPath, worktreeThreshold); err != nil {
 		return status.Errorf(codes.Internal, "Cleanup: cleanStaleWorktrees: %v", err)
 	}
 
@@ -105,7 +105,7 @@ func cleanPackedRefsLock(repoPath string, threshold time.Time) error {
 	return nil
 }
 
-func cleanStaleWorktrees(repoPath string, threshold time.Time) error {
+func cleanStaleWorktrees(ctx context.Context, repo *gitalypb.Repository, repoPath string, threshold time.Time) error {
 	worktreePath := filepath.Join(repoPath, worktreePrefix)
 
 	dirInfo, err := os.Stat(worktreePath)
@@ -126,10 +126,16 @@ func cleanStaleWorktrees(repoPath string, threshold time.Time) error {
 			continue
 		}
 
-		path := filepath.Join(worktreePath, info.Name())
-
 		if info.ModTime().Before(threshold) {
-			if err := os.RemoveAll(path); err != nil && !os.IsNotExist(err) {
+			cmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{
+				Name:  "worktree",
+				Flags: []git.Option{git.SubSubCmd{"remove"}, git.Flag{Name: "--force"}, git.SubSubCmd{info.Name()}},
+			})
+			if err != nil {
+				return err
+			}
+
+			if err = cmd.Wait(); err != nil {
 				return err
 			}
 		}
diff --git a/internal/service/repository/create_bundle.go b/internal/service/repository/create_bundle.go
index 4c0a42e3c..3f85ce522 100644
--- a/internal/service/repository/create_bundle.go
+++ b/internal/service/repository/create_bundle.go
@@ -4,6 +4,7 @@ import (
 	"io"
 
 	"gitlab.com/gitlab-org/gitaly/internal/git"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/gitaly/streamio"
 	"google.golang.org/grpc/codes"
@@ -18,6 +19,10 @@ func (s *server) CreateBundle(req *gitalypb.CreateBundleRequest, stream gitalypb
 
 	ctx := stream.Context()
 
+	if _, err := s.Cleanup(ctx, &gitalypb.CleanupRequest{Repository: req.GetRepository()}); err != nil {
+		return helper.ErrInternalf("running Cleanup on repository: %w", err)
+	}
+
 	cmd, err := git.SafeCmd(ctx, repo, nil, git.SubCmd{
 		Name:  "bundle",
 		Flags: []git.Option{git.SubSubCmd{"create"}, git.OutputToStdout, git.Flag{"--all"}},
diff --git a/internal/service/repository/create_bundle_test.go b/internal/service/repository/create_bundle_test.go
index 966eead17..2ad93a0f9 100644
--- a/internal/service/repository/create_bundle_test.go
+++ b/internal/service/repository/create_bundle_test.go
@@ -4,7 +4,9 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/tempdir"
@@ -27,6 +29,18 @@ func TestSuccessfulCreateBundleRequest(t *testing.T) {
 	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
 	defer cleanupFn()
 
+	// create a work tree with a HEAD pointing to a commit that is missing.
+	// CreateBundle should clean this up before creating the bundle
+	sha, branchName := testhelper.CreateCommitOnNewBranch(t, testRepoPath)
+
+	require.NoError(t, os.MkdirAll(filepath.Join(testRepoPath, "gitlab-worktree"), 0755))
+
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "worktree", "add", "gitlab-worktree/worktree1", sha)
+	os.Chtimes(filepath.Join(testRepoPath, "gitlab-worktree", "worktree1"), time.Now().Add(-7*time.Hour), time.Now().Add(-7*time.Hour))
+
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", "-D", branchName)
+	require.NoError(t, os.Remove(filepath.Join(testRepoPath, "objects", sha[0:2], sha[2:])))
+
 	request := &gitalypb.CreateBundleRequest{Repository: testRepo}
 
 	c, err := client.CreateBundle(ctx, request)