Merge branch 'jt-replicate-object-pools' into 'master'

repository: Replicate object pools during `ReplicateRepository` RPC

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/6123

Merged-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: Patrick Steinhardt <psteinhardt@gitlab.com>
Reviewed-by: karthik nayak <knayak@gitlab.com>
Co-authored-by: Justin Tobler <jtobler@gitlab.com>

2 files changed, 43 insertions, 23 deletions

diff --git a/internal/gitaly/service/repository/replicate.go b/internal/gitaly/service/repository/replicate.go
index 309435586..4f1be41f9 100644
--- a/internal/gitaly/service/repository/replicate.go
+++ b/internal/gitaly/service/repository/replicate.go
@@ -154,16 +154,16 @@ func (s *server) create(ctx context.Context, in *gitalypb.ReplicateRepositoryReq
 		ctxlogrus.Extract(ctx).WithField("repo_path", repoPath).Warn("removed invalid repository")
 	}
 
-	if err := s.createFromSnapshot(ctx, in); err != nil {
+	if err := s.createFromSnapshot(ctx, in.GetSource(), in.GetRepository()); err != nil {
 		return fmt.Errorf("could not create repository from snapshot: %w", err)
 	}
 
 	return nil
 }
 
-func (s *server) createFromSnapshot(ctx context.Context, in *gitalypb.ReplicateRepositoryRequest) error {
-	if err := repoutil.Create(ctx, s.locator, s.gitCmdFactory, s.txManager, in.GetRepository(), func(repo *gitalypb.Repository) error {
-		if err := s.extractSnapshot(ctx, in.GetSource(), repo); err != nil {
+func (s *server) createFromSnapshot(ctx context.Context, source, target *gitalypb.Repository) error {
+	if err := repoutil.Create(ctx, s.locator, s.gitCmdFactory, s.txManager, target, func(repo *gitalypb.Repository) error {
+		if err := s.extractSnapshot(ctx, source, repo); err != nil {
 			return fmt.Errorf("extracting snapshot: %w", err)
 		}
 
@@ -452,16 +452,24 @@ func (s *server) syncObjectPool(ctx context.Context, sourceRepoProto, targetRepo
 	switch {
 	case errors.Is(err, objectpool.ErrInvalidPoolRepository):
 		// In the case where the source repository does link to an object pool, but the object pool
-		// does not yet exist on the target storage, the object pool must be replicated as well.
-		// This scenario is not handled yet and instead object pool replication is not performed.
-		return nil
+		// does not yet exist on the target storage, the object pool must be replicated as well. To
+		// accomplish this, a snapshot of the source object pool is extracted onto the target
+		// storage.
+		if err := s.createFromSnapshot(ctx, sourcePoolProto.GetRepository(), targetPoolProto.GetRepository()); err != nil {
+			return fmt.Errorf("replicate object pool: %w", err)
+		}
+
+		targetPool, err = objectpool.FromProto(s.locator, s.gitCmdFactory, s.catfileCache, s.txManager, s.housekeepingManager, targetPoolProto)
+		if err != nil {
+			return fmt.Errorf("get replicated object pool: %w", err)
+		}
 	case err != nil:
-		return fmt.Errorf("get target repository object pool: %w", err)
+		return fmt.Errorf("get target object pool: %w", err)
 	}
 
 	// Link the replicated repository to the object pool.
 	if err := targetPool.Link(ctx, targetRepo); err != nil {
-		return fmt.Errorf("link target repository object pool: %w", err)
+		return fmt.Errorf("link target object pool: %w", err)
 	}
 
 	return nil
diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go
index f2498dd6c..152b53196 100644
--- a/internal/gitaly/service/repository/replicate_test.go
+++ b/internal/gitaly/service/repository/replicate_test.go
@@ -490,10 +490,8 @@ func testReplicateRepository(t *testing.T, ctx context.Context) {
 					LinkRepositoryToObjectPool: true,
 				})
 
-				// Repository replication is currently not able to replicate object pool
-				// repositories. For the Git alternates file to be recreated on the target storage
-				// the required target object pool must already exist on the target storage. This
-				// limitation will go away once proper object pool replication is implemented.
+				// If the required object pool already exists on the target storage, object pool
+				// replication is skipped and the repository is linked to the existing object pool.
 				gittest.CreateObjectPool(t, ctx, cfg, targetProto, gittest.CreateObjectPoolConfig{
 					RelativePath:               sourcePool.GetRepository().GetRelativePath(),
 					LinkRepositoryToObjectPool: false,
@@ -515,21 +513,37 @@ func testReplicateRepository(t *testing.T, ctx context.Context) {
 			},
 		},
 		{
-			desc: "target link not replicated due to missing object pool",
+			desc: "source object pool replicated to target",
+			// Object pool replication is not currently supported by Praefect. Existing object pool
+			// repositories cannot be located because Praefect rewrites repository messages.
+			// Consequently, this test case is executed with Praefect disabled.
+			serverOpts: []testserver.GitalyServerOpt{testserver.WithDisablePraefect()},
 			setup: func(t *testing.T, cfg config.Cfg) setupData {
-				sourceProto, _, targetProto, _ := setupSourceAndTarget(t, cfg, false)
+				sourceProto, sourcePath, targetProto, _ := setupSourceAndTarget(t, cfg, false)
 
-				// If the required object pool does not exist on the target node, the target
-				// repository will not be linked to anything.
-				gittest.CreateObjectPool(t, ctx, cfg, sourceProto, gittest.CreateObjectPoolConfig{
+				// If the required object pool does not exist on the target storage, a snapshot copy
+				// of the object pool from the source storage will be extracted onto the target.
+				_, sourcePoolPath := gittest.CreateObjectPool(t, ctx, cfg, sourceProto, gittest.CreateObjectPoolConfig{
 					LinkRepositoryToObjectPool: true,
 				})
 
+				// Write commit to object pool to ensure that the pool and its contents are copied
+				// to the target storage.
+				commitID := gittest.WriteCommit(t, cfg, sourcePoolPath)
+
+				expectedAltInfo, err := stats.AlternatesInfoForRepository(sourcePath)
+				require.NoError(t, err)
+
+				if featureflag.ReplicateRepositoryObjectPool.IsDisabled(ctx) {
+					expectedAltInfo = stats.AlternatesInfo{Exists: false}
+				}
+
 				return setupData{
 					source:              sourceProto,
 					target:              targetProto,
 					replicateObjectPool: true,
-					expectedAltInfo:     stats.AlternatesInfo{Exists: false},
+					expectedAltInfo:     expectedAltInfo,
+					expectedObjects:     []string{commitID.String()},
 				}
 			},
 		},
@@ -544,10 +558,8 @@ func testReplicateRepository(t *testing.T, ctx context.Context) {
 					LinkRepositoryToObjectPool: true,
 				})
 
-				// Repository replication is currently not able to replicate object pool
-				// repositories. For the Git alternates file to be recreated on the target storage
-				// the required target object pool must already exist on the target storage. This
-				// limitation will go away once proper object pool replication is implemented.
+				// Create the required object pool repository on the target storage, to validate
+				// that it does not get linked even though it exists.
 				gittest.CreateObjectPool(t, ctx, cfg, targetProto, gittest.CreateObjectPoolConfig{
 					RelativePath:               sourcePool.GetRepository().GetRelativePath(),
 					LinkRepositoryToObjectPool: false,