Merge branch 'sh-add-cleanup' into 'master'

Support RPC for Repository cleanup

See merge request gitlab-org/gitaly!648

3 files changed, 202 insertions, 140 deletions

diff --git a/internal/service/repository/cleanup.go b/internal/service/repository/cleanup.go
index cd93171ea..c3e99daf2 100644
--- a/internal/service/repository/cleanup.go
+++ b/internal/service/repository/cleanup.go
@@ -7,17 +7,25 @@ import (
 	"strings"
 	"time"
 
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
-
-	pb "gitlab.com/gitlab-org/gitaly-proto/go"
-
 	"golang.org/x/net/context"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
-func (server) Cleanup(ctx context.Context, in *pb.CleanupRequest) (*pb.CleanupResponse, error) {
-	return nil, helper.Unimplemented
+func (server) Cleanup(_ctx context.Context, in *pb.CleanupRequest) (*pb.CleanupResponse, error) {
+	repoPath, err := helper.GetRepoPath(in.GetRepository())
+	if err != nil {
+		return nil, err
+	}
+
+	if err := cleanupRepo(repoPath); err != nil {
+		return nil, err
+	}
+
+	return &pb.CleanupResponse{}, nil
 }
 
 func cleanupRepo(repoPath string) error {
diff --git a/internal/service/repository/cleanup_test.go b/internal/service/repository/cleanup_test.go
new file mode 100644
index 000000000..70de2fab8
--- /dev/null
+++ b/internal/service/repository/cleanup_test.go
@@ -0,0 +1,187 @@
+package repository
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+)
+
+func TestCleanupDeletesRefsLocks(t *testing.T) {
+	server, serverSocketPath := runRepoServer(t)
+	defer server.Stop()
+
+	client, conn := newRepositoryClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	req := &pb.CleanupRequest{Repository: testRepo}
+	refsPath := filepath.Join(testRepoPath, "refs")
+
+	keepRefPath := filepath.Join(refsPath, "heads", "keepthis")
+	createFileWithTimes(keepRefPath, freshTime)
+	keepOldRefPath := filepath.Join(refsPath, "heads", "keepthisalso")
+	createFileWithTimes(keepOldRefPath, oldTime)
+	keepDeceitfulRef := filepath.Join(refsPath, "heads", " .lock.not-actually-a-lock.lock ")
+	createFileWithTimes(keepDeceitfulRef, oldTime)
+
+	keepLockPath := filepath.Join(refsPath, "heads", "keepthis.lock")
+	createFileWithTimes(keepLockPath, freshTime)
+
+	deleteLockPath := filepath.Join(refsPath, "heads", "deletethis.lock")
+	createFileWithTimes(deleteLockPath, oldTime)
+
+	c, err := client.Cleanup(ctx, req)
+	assert.NoError(t, err)
+	assert.NotNil(t, c)
+
+	// Sanity checks
+	assert.FileExists(t, keepRefPath)
+	assert.FileExists(t, keepOldRefPath)
+	assert.FileExists(t, keepDeceitfulRef)
+
+	assert.FileExists(t, keepLockPath)
+
+	testhelper.AssertFileNotExists(t, deleteLockPath)
+}
+
+func TestCleanupDeletesPackedRefsLock(t *testing.T) {
+	server, serverSocketPath := runRepoServer(t)
+	defer server.Stop()
+
+	client, conn := newRepositoryClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testCases := []struct {
+		desc        string
+		lockTime    *time.Time
+		shouldExist bool
+	}{
+		{
+			desc:        "with a recent lock",
+			lockTime:    &freshTime,
+			shouldExist: true,
+		},
+		{
+			desc:        "with an old lock",
+			lockTime:    &oldTime,
+			shouldExist: false,
+		},
+		{
+			desc:        "with a non-existing lock",
+			lockTime:    nil,
+			shouldExist: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+			defer cleanupFn()
+
+			// Force the packed-refs file to have an old time to test that even
+			// in that case it doesn't get deleted
+			packedRefsPath := filepath.Join(testRepoPath, "packed-refs")
+			os.Chtimes(packedRefsPath, oldTime, oldTime)
+
+			req := &pb.CleanupRequest{Repository: testRepo}
+			lockPath := filepath.Join(testRepoPath, "packed-refs.lock")
+
+			if tc.lockTime != nil {
+				createFileWithTimes(lockPath, *tc.lockTime)
+			}
+
+			ctx, cancel := testhelper.Context()
+			defer cancel()
+
+			c, err := client.Cleanup(ctx, req)
+
+			// Sanity checks
+			assert.FileExists(t, filepath.Join(testRepoPath, "HEAD")) // For good measure
+			assert.FileExists(t, packedRefsPath)
+
+			if tc.shouldExist {
+				assert.FileExists(t, lockPath)
+			} else {
+				assert.NoError(t, err)
+				assert.NotNil(t, c)
+
+				testhelper.AssertFileNotExists(t, lockPath)
+			}
+		})
+	}
+}
+
+func TestCleanupDeletesStaleWorktrees(t *testing.T) {
+	server, serverSocketPath := runRepoServer(t)
+	defer server.Stop()
+
+	client, conn := newRepositoryClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testCases := []struct {
+		desc         string
+		worktreeTime time.Time
+		shouldExist  bool
+	}{
+		{
+			desc:         "with a recent worktree",
+			worktreeTime: freshTime,
+			shouldExist:  true,
+		},
+		{
+			desc:         "with a slightly old worktree",
+			worktreeTime: oldTime,
+			shouldExist:  true,
+		},
+		{
+			desc:         "with an old worktree",
+			worktreeTime: oldTreeTime,
+			shouldExist:  false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+			defer cleanupFn()
+
+			req := &pb.CleanupRequest{Repository: testRepo}
+
+			testhelper.AddWorktree(t, testRepoPath, "test-worktree")
+			basePath := filepath.Join(testRepoPath, "worktrees")
+			worktreePath := filepath.Join(basePath, "test-worktree")
+
+			require.NoError(t, os.Chtimes(worktreePath, tc.worktreeTime, tc.worktreeTime))
+
+			ctx, cancel := testhelper.Context()
+			defer cancel()
+
+			c, err := client.Cleanup(ctx, req)
+
+			// Sanity check
+			assert.FileExists(t, filepath.Join(testRepoPath, "HEAD")) // For good measure
+
+			if tc.shouldExist {
+				assert.DirExists(t, basePath)
+				assert.DirExists(t, worktreePath)
+			} else {
+				assert.NoError(t, err)
+				assert.NotNil(t, c)
+
+				testhelper.AssertFileNotExists(t, worktreePath)
+			}
+		})
+	}
+}
diff --git a/internal/service/repository/gc_test.go b/internal/service/repository/gc_test.go
index f1bbe87ad..951f983c4 100644
--- a/internal/service/repository/gc_test.go
+++ b/internal/service/repository/gc_test.go
@@ -13,7 +13,6 @@ import (
 	"google.golang.org/grpc/codes"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
@@ -127,139 +126,7 @@ func TestGarbageCollectDeletesRefsLocks(t *testing.T) {
 
 	assert.FileExists(t, keepLockPath)
 
-	// There's assert.FileExists but no assert.NotFileExists ¯\_(ツ)_/¯
-	_, err = os.Stat(deleteLockPath)
-	assert.True(t, os.IsNotExist(err))
-}
-
-func TestGarbageCollectDeletesPackedRefsLock(t *testing.T) {
-	server, serverSocketPath := runRepoServer(t)
-	defer server.Stop()
-
-	client, conn := newRepositoryClient(t, serverSocketPath)
-	defer conn.Close()
-
-	testCases := []struct {
-		desc        string
-		lockTime    *time.Time
-		shouldExist bool
-	}{
-		{
-			desc:        "with a recent lock",
-			lockTime:    &freshTime,
-			shouldExist: true,
-		},
-		{
-			desc:        "with an old lock",
-			lockTime:    &oldTime,
-			shouldExist: false,
-		},
-		{
-			desc:        "with a non-existing lock",
-			lockTime:    nil,
-			shouldExist: false,
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.desc, func(t *testing.T) {
-			testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
-			defer cleanupFn()
-
-			// Force the packed-refs file to have an old time to test that even
-			// in that case it doesn't get deleted
-			packedRefsPath := filepath.Join(testRepoPath, "packed-refs")
-			os.Chtimes(packedRefsPath, oldTime, oldTime)
-
-			req := &pb.GarbageCollectRequest{Repository: testRepo}
-			lockPath := filepath.Join(testRepoPath, "packed-refs.lock")
-
-			if tc.lockTime != nil {
-				createFileWithTimes(lockPath, *tc.lockTime)
-			}
-
-			ctx, cancel := testhelper.Context()
-			defer cancel()
-
-			c, err := client.GarbageCollect(ctx, req)
-
-			// Sanity checks
-			assert.FileExists(t, filepath.Join(testRepoPath, "HEAD")) // For good measure
-			assert.FileExists(t, packedRefsPath)
-
-			if tc.shouldExist {
-				assert.FileExists(t, lockPath)
-			} else {
-				assert.NoError(t, err)
-				assert.NotNil(t, c)
-
-				testhelper.AssertFileNotExists(t, lockPath)
-			}
-		})
-	}
-}
-
-func TestGarbageCollectDeletesStaleWorktrees(t *testing.T) {
-	server, serverSocketPath := runRepoServer(t)
-	defer server.Stop()
-
-	client, conn := newRepositoryClient(t, serverSocketPath)
-	defer conn.Close()
-
-	testCases := []struct {
-		desc         string
-		worktreeTime time.Time
-		shouldExist  bool
-	}{
-		{
-			desc:         "with a recent worktree",
-			worktreeTime: freshTime,
-			shouldExist:  true,
-		},
-		{
-			desc:         "with a slightly old worktree",
-			worktreeTime: oldTime,
-			shouldExist:  true,
-		},
-		{
-			desc:         "with an old worktree",
-			worktreeTime: oldTreeTime,
-			shouldExist:  false,
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.desc, func(t *testing.T) {
-			testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
-			defer cleanupFn()
-
-			req := &pb.GarbageCollectRequest{Repository: testRepo}
-
-			testhelper.AddWorktree(t, testRepoPath, "test-worktree")
-			basePath := filepath.Join(testRepoPath, "worktrees")
-			worktreePath := filepath.Join(basePath, "test-worktree")
-
-			require.NoError(t, os.Chtimes(worktreePath, tc.worktreeTime, tc.worktreeTime))
-
-			ctx, cancel := testhelper.Context()
-			defer cancel()
-
-			c, err := client.GarbageCollect(ctx, req)
-
-			// Sanity check
-			assert.FileExists(t, filepath.Join(testRepoPath, "HEAD")) // For good measure
-
-			if tc.shouldExist {
-				assert.DirExists(t, basePath)
-				assert.DirExists(t, worktreePath)
-			} else {
-				assert.NoError(t, err)
-				assert.NotNil(t, c)
-
-				testhelper.AssertFileNotExists(t, worktreePath)
-			}
-		})
-	}
+	testhelper.AssertFileNotExists(t, deleteLockPath)
 }
 
 func TestGarbageCollectFailure(t *testing.T) {