Merge branch 'zj-objectpool-linking' into 'master'

Link and Unlink RPCs

Closes #1345 and #1349

See merge request gitlab-org/gitaly!986

6 files changed, 344 insertions, 9 deletions

diff --git a/changelogs/unreleased/zj-objectpool-linking.yml b/changelogs/unreleased/zj-objectpool-linking.yml
new file mode 100644
index 000000000..604cc5c69
--- /dev/null
+++ b/changelogs/unreleased/zj-objectpool-linking.yml
@@ -0,0 +1,5 @@
+---
+title: Link and Unlink RPCs
+merge_request: 986
+author:
+type: added
diff --git a/internal/git/objectpool/link.go b/internal/git/objectpool/link.go
index 106fc56b3..284556e76 100644
--- a/internal/git/objectpool/link.go
+++ b/internal/git/objectpool/link.go
@@ -2,16 +2,42 @@ package objectpool
 
 import (
 	"context"
+	"io/ioutil"
+	"os"
+	"path/filepath"
 
 	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
-// Link writes the alternate file
-func Link(ctx context.Context, pool, repository repository.GitRepo) error {
-	return nil
+// Link will write the relative path to the object pool from the repository that
+// is to join the pool. This does not trigger deduplication, which is the
+// responsibility of the caller.
+func (o *ObjectPool) Link(ctx context.Context, repo repository.GitRepo) error {
+	altPath, err := alternatesPath(repo)
+	if err != nil {
+		return err
+	}
+
+	repoPath, err := helper.GetRepoPath(repo)
+	if err != nil {
+		return err
+	}
+
+	relPath, err := filepath.Rel(filepath.Join(repoPath, "objects"), o.FullPath())
+	if err != nil {
+		return err
+	}
+
+	return ioutil.WriteFile(altPath, []byte(filepath.Join(relPath, "objects")), 0644)
 }
 
-// Unlink removes the alternate file
-func Unlink(ctx context.Context, pool, repository repository.GitRepo) error {
-	return nil
+// Unlink removes the alternates file, so Git won't look there anymore
+func Unlink(ctx context.Context, repo repository.GitRepo) error {
+	altPath, err := alternatesPath(repo)
+	if err != nil {
+		return err
+	}
+
+	return os.RemoveAll(altPath)
 }
diff --git a/internal/git/objectpool/link_test.go b/internal/git/objectpool/link_test.go
new file mode 100644
index 000000000..4402d4b73
--- /dev/null
+++ b/internal/git/objectpool/link_test.go
@@ -0,0 +1,71 @@
+package objectpool
+
+import (
+	"io/ioutil"
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+)
+
+func TestLink(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	pool, err := NewObjectPool(testRepo.GetStorageName(), t.Name())
+	require.NoError(t, err)
+	defer pool.Remove(ctx)
+
+	require.NoError(t, pool.Create(ctx, testRepo))
+
+	altPath, err := alternatesPath(testRepo)
+	require.NoError(t, err)
+	_, err = os.Stat(altPath)
+	require.True(t, os.IsNotExist(err))
+
+	require.NoError(t, pool.Link(ctx, testRepo))
+
+	_, err = os.Stat(altPath)
+	require.False(t, os.IsNotExist(err))
+
+	content, err := ioutil.ReadFile(altPath)
+	require.NoError(t, err)
+
+	require.NoError(t, pool.Link(ctx, testRepo))
+
+	newContent, err := ioutil.ReadFile(altPath)
+	require.NoError(t, err)
+
+	require.Equal(t, content, newContent)
+}
+
+func TestUnlink(t *testing.T) {
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	pool, err := NewObjectPool(testRepo.GetStorageName(), t.Name())
+	require.NoError(t, err)
+	defer pool.Remove(ctx)
+
+	// Without a pool on disk, this doesn't return an error
+	require.NoError(t, Unlink(ctx, testRepo))
+
+	altPath, err := alternatesPath(testRepo)
+	require.NoError(t, err)
+
+	require.NoError(t, pool.Create(ctx, testRepo))
+	require.NoError(t, pool.Link(ctx, testRepo))
+	_, err = os.Stat(altPath)
+	require.False(t, os.IsNotExist(err))
+
+	require.NoError(t, Unlink(ctx, testRepo))
+	_, err = os.Stat(altPath)
+	require.True(t, os.IsNotExist(err))
+}
diff --git a/internal/git/objectpool/path.go b/internal/git/objectpool/path.go
index 4f538c5a5..d7e2002e6 100644
--- a/internal/git/objectpool/path.go
+++ b/internal/git/objectpool/path.go
@@ -1,6 +1,11 @@
 package objectpool
 
-import "path/filepath"
+import (
+	"path/filepath"
+
+	"gitlab.com/gitlab-org/gitaly/internal/git/repository"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
+)
 
 // GetRelativePath will create the relative path to the ObjectPool from the
 // storage path.
@@ -18,3 +23,12 @@ func (o *ObjectPool) GetStorageName() string {
 func (o *ObjectPool) FullPath() string {
 	return filepath.Join(o.storagePath, o.GetRelativePath())
 }
+
+func alternatesPath(repo repository.GitRepo) (string, error) {
+	repoPath, err := helper.GetRepoPath(repo)
+	if err != nil {
+		return "", err
+	}
+
+	return filepath.Join(repoPath, "objects", "info", "alternates"), nil
+}
diff --git a/internal/service/objectpool/link.go b/internal/service/objectpool/link.go
index 20f0ef9d0..ef3d0c15d 100644
--- a/internal/service/objectpool/link.go
+++ b/internal/service/objectpool/link.go
@@ -4,13 +4,37 @@ import (
 	"context"
 
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
+	"gitlab.com/gitlab-org/gitaly/internal/git/objectpool"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 func (s *server) LinkRepositoryToObjectPool(ctx context.Context, req *gitalypb.LinkRepositoryToObjectPoolRequest) (*gitalypb.LinkRepositoryToObjectPoolResponse, error) {
-	return nil, helper.Unimplemented
+	if req.GetRepository() == nil {
+		return nil, status.Error(codes.InvalidArgument, "no repository")
+	}
+
+	pool, err := poolForRequest(req)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := pool.Link(ctx, req.GetRepository()); err != nil {
+		return nil, status.Error(codes.Internal, helper.SanitizeString(err.Error()))
+	}
+
+	return &gitalypb.LinkRepositoryToObjectPoolResponse{}, nil
 }
 
 func (s *server) UnlinkRepositoryFromObjectPool(ctx context.Context, req *gitalypb.UnlinkRepositoryFromObjectPoolRequest) (*gitalypb.UnlinkRepositoryFromObjectPoolResponse, error) {
-	return nil, helper.Unimplemented
+	if req.GetRepository() == nil {
+		return nil, status.Error(codes.InvalidArgument, "no repository")
+	}
+
+	if err := objectpool.Unlink(ctx, req.GetRepository()); err != nil {
+		return nil, err
+	}
+
+	return &gitalypb.UnlinkRepositoryFromObjectPoolResponse{}, nil
 }
diff --git a/internal/service/objectpool/link_test.go b/internal/service/objectpool/link_test.go
new file mode 100644
index 000000000..17a58e39c
--- /dev/null
+++ b/internal/service/objectpool/link_test.go
@@ -0,0 +1,195 @@
+package objectpool
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
+	"gitlab.com/gitlab-org/gitaly/internal/git/log"
+	"gitlab.com/gitlab-org/gitaly/internal/git/objectpool"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+	"google.golang.org/grpc/codes"
+)
+
+func TestLink(t *testing.T) {
+	server, serverSocketPath := runObjectPoolServer(t)
+	defer server.Stop()
+
+	client, conn := newObjectPoolClient(t, serverSocketPath)
+	defer conn.Close()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	pool, err := objectpool.NewObjectPool(testRepo.GetStorageName(), t.Name())
+	require.NoError(t, err)
+	defer pool.Remove(ctx)
+	require.NoError(t, pool.Create(ctx, testRepo))
+
+	// Mock object in the pool, which should be available to the pool members
+	// after linking
+	poolCommitID := testhelper.CreateCommit(t, pool.FullPath(), "pool-test-branch", nil)
+
+	testCases := []struct {
+		desc string
+		req  *gitalypb.LinkRepositoryToObjectPoolRequest
+		code codes.Code
+	}{
+		{
+			desc: "Repository does not exist",
+			req: &gitalypb.LinkRepositoryToObjectPoolRequest{
+				Repository: nil,
+				ObjectPool: pool.ToProto(),
+			},
+			code: codes.InvalidArgument,
+		},
+		{
+			desc: "Pool does not exist",
+			req: &gitalypb.LinkRepositoryToObjectPoolRequest{
+				Repository: testRepo,
+				ObjectPool: nil,
+			},
+			code: codes.InvalidArgument,
+		},
+		{
+			desc: "Successful request",
+			req: &gitalypb.LinkRepositoryToObjectPoolRequest{
+				Repository: testRepo,
+				ObjectPool: pool.ToProto(),
+			},
+			code: codes.OK,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			_, err := client.LinkRepositoryToObjectPool(ctx, tc.req)
+			require.Equal(t, tc.code, helper.GrpcCode(err))
+
+			if tc.code == codes.OK {
+				commit, err := log.GetCommit(ctx, testRepo, poolCommitID)
+				require.NoError(t, err)
+				require.NotNil(t, commit)
+				require.Equal(t, poolCommitID, commit.Id)
+			}
+		})
+	}
+}
+
+func TestLinkIdempotent(t *testing.T) {
+	server, serverSocketPath := runObjectPoolServer(t)
+	defer server.Stop()
+
+	client, conn := newObjectPoolClient(t, serverSocketPath)
+	defer conn.Close()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	pool, err := objectpool.NewObjectPool(testRepo.GetStorageName(), t.Name())
+	require.NoError(t, err)
+	defer pool.Remove(ctx)
+	require.NoError(t, pool.Create(ctx, testRepo))
+
+	request := &gitalypb.LinkRepositoryToObjectPoolRequest{
+		Repository: testRepo,
+		ObjectPool: pool.ToProto(),
+	}
+
+	_, err = client.LinkRepositoryToObjectPool(ctx, request)
+	require.NoError(t, err)
+
+	_, err = client.LinkRepositoryToObjectPool(ctx, request)
+	require.NoError(t, err)
+}
+
+func TestUnlink(t *testing.T) {
+	server, serverSocketPath := runObjectPoolServer(t)
+	defer server.Stop()
+
+	client, conn := newObjectPoolClient(t, serverSocketPath)
+	defer conn.Close()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	pool, err := objectpool.NewObjectPool(testRepo.GetStorageName(), t.Name())
+	require.NoError(t, err)
+	defer pool.Remove(ctx)
+	require.NoError(t, pool.Create(ctx, testRepo))
+	require.NoError(t, pool.Link(ctx, testRepo))
+
+	poolCommitID := testhelper.CreateCommit(t, pool.FullPath(), "pool-test-branch", nil)
+
+	testCases := []struct {
+		desc string
+		req  *gitalypb.UnlinkRepositoryFromObjectPoolRequest
+		code codes.Code
+	}{
+		{
+			desc: "Repository does not exist",
+			req: &gitalypb.UnlinkRepositoryFromObjectPoolRequest{
+				Repository: nil,
+			},
+			code: codes.InvalidArgument,
+		},
+		{
+			desc: "Successful request",
+			req: &gitalypb.UnlinkRepositoryFromObjectPoolRequest{
+				Repository: testRepo,
+			},
+			code: codes.OK,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			_, err := client.UnlinkRepositoryFromObjectPool(ctx, tc.req)
+			require.Equal(t, tc.code, helper.GrpcCode(err))
+
+			if tc.code == codes.OK {
+				commit, err := log.GetCommit(ctx, testRepo, poolCommitID)
+				require.NoError(t, err)
+				require.Nil(t, commit)
+			}
+		})
+	}
+}
+
+func TestUnlinkIdempotent(t *testing.T) {
+	server, serverSocketPath := runObjectPoolServer(t)
+	defer server.Stop()
+
+	client, conn := newObjectPoolClient(t, serverSocketPath)
+	defer conn.Close()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	pool, err := objectpool.NewObjectPool(testRepo.GetStorageName(), t.Name())
+	require.NoError(t, err)
+	defer pool.Remove(ctx)
+	require.NoError(t, pool.Create(ctx, testRepo))
+	require.NoError(t, pool.Link(ctx, testRepo))
+
+	request := &gitalypb.UnlinkRepositoryFromObjectPoolRequest{testRepo}
+
+	_, err = client.UnlinkRepositoryFromObjectPool(ctx, request)
+	require.NoError(t, err)
+
+	_, err = client.UnlinkRepositoryFromObjectPool(ctx, request)
+	require.NoError(t, err)
+}