Keep origin remote and refs when creating an object pool

8 files changed, 20 insertions, 75 deletions

diff --git a/changelogs/unreleased/jc-object-create-keep-resf.yml b/changelogs/unreleased/jc-object-create-keep-resf.yml
new file mode 100644
index 000000000..53ac4f53c
--- /dev/null
+++ b/changelogs/unreleased/jc-object-create-keep-resf.yml
@@ -0,0 +1,5 @@
+---
+title: Keep origin remote and refs when creating an object pool
+merge_request: 1136
+author:
+type: changed
diff --git a/internal/git/objectpool/clone.go b/internal/git/objectpool/clone.go
index 19795a662..41110756f 100644
--- a/internal/git/objectpool/clone.go
+++ b/internal/git/objectpool/clone.go
@@ -2,7 +2,6 @@ package objectpool
 
 import (
 	"context"
-	"io"
 	"os"
 	"path"
 
@@ -34,30 +33,6 @@ func (o *ObjectPool) clone(ctx context.Context, repo *gitalypb.Repository) error
 	return cmd.Wait()
 }
 
-func (o *ObjectPool) removeRefs(ctx context.Context) error {
-	pipeReader, pipeWriter := io.Pipe()
-	defer pipeReader.Close()
-	defer pipeWriter.Close()
-
-	cmd, err := git.BareCommand(ctx, nil, pipeWriter, os.Stderr, nil, "--git-dir", o.FullPath(), "for-each-ref", "--format=delete %(refname)")
-	if err != nil {
-		return err
-	}
-
-	updateRefCmd, err := git.BareCommand(ctx, pipeReader, nil, os.Stderr, nil, "-C", o.FullPath(), "update-ref", "--stdin")
-	if err != nil {
-		return err
-	}
-
-	if err := cmd.Wait(); err != nil {
-		return err
-	}
-
-	pipeWriter.Close()
-
-	return updateRefCmd.Wait()
-}
-
 func (o *ObjectPool) removeHooksDir() error {
 	return os.RemoveAll(path.Join(o.FullPath(), "hooks"))
 }
diff --git a/internal/git/objectpool/clone_test.go b/internal/git/objectpool/clone_test.go
index 0c2a2b1d7..a79b1ac1c 100644
--- a/internal/git/objectpool/clone_test.go
+++ b/internal/git/objectpool/clone_test.go
@@ -26,24 +26,6 @@ func TestClone(t *testing.T) {
 	require.DirExists(t, path.Join(pool.FullPath(), "objects"))
 }
 
-func TestRemoveRefs(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.clone(ctx, testRepo))
-	require.NoError(t, pool.removeRefs(ctx))
-
-	out := testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "for-each-ref")
-	require.Len(t, out, 0)
-}
-
 func TestCloneExistingPool(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
diff --git a/internal/git/objectpool/link_test.go b/internal/git/objectpool/link_test.go
index c3740f4f5..f9d2934ff 100644
--- a/internal/git/objectpool/link_test.go
+++ b/internal/git/objectpool/link_test.go
@@ -47,8 +47,8 @@ func TestLink(t *testing.T) {
 	require.Equal(t, content, newContent)
 
 	// Test if the remote is set
-	remotes := testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "remote")
-	assert.Equal(t, testRepo.GetGlRepository()+"\n", string(remotes))
+	remotes := strings.Split(string(testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "remote")), "\n")
+	assert.Contains(t, remotes, testRepo.GetGlRepository())
 }
 
 func TestUnlink(t *testing.T) {
diff --git a/internal/git/objectpool/pool.go b/internal/git/objectpool/pool.go
index 393a0bf65..67d22870e 100644
--- a/internal/git/objectpool/pool.go
+++ b/internal/git/objectpool/pool.go
@@ -7,7 +7,6 @@ import (
 	"path"
 
 	"gitlab.com/gitlab-org/gitaly-proto/go/gitalypb"
-	"gitlab.com/gitlab-org/gitaly/internal/git/remote"
 	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
@@ -79,14 +78,6 @@ func (o *ObjectPool) Create(ctx context.Context, repo *gitalypb.Repository) (err
 		return fmt.Errorf("remove hooks: %v", err)
 	}
 
-	if err := remote.Remove(ctx, o, "origin"); err != nil {
-		return fmt.Errorf("remove origin: %v", err)
-	}
-
-	if err := o.removeRefs(ctx); err != nil {
-		return fmt.Errorf("remove refs: %v", err)
-	}
-
 	if err := o.setConfig(ctx, "gc.auto", "0"); err != nil {
 		return fmt.Errorf("config gc.auto: %v", err)
 	}
diff --git a/internal/git/objectpool/pool_test.go b/internal/git/objectpool/pool_test.go
index d1fe05ba6..74d2c1c16 100644
--- a/internal/git/objectpool/pool_test.go
+++ b/internal/git/objectpool/pool_test.go
@@ -3,6 +3,7 @@ package objectpool
 import (
 	"os"
 	"path"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -22,9 +23,11 @@ func TestCreate(t *testing.T) {
 	ctx, cancel := testhelper.Context()
 	defer cancel()
 
-	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
 	defer cleanupFn()
 
+	masterSha := testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "show-ref", "master")
+
 	pool, err := NewObjectPool(testRepo.GetStorageName(), t.Name())
 	require.NoError(t, err)
 
@@ -34,18 +37,18 @@ func TestCreate(t *testing.T) {
 
 	require.True(t, pool.IsValid())
 
-	// No refs
-	out := testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "for-each-ref")
-	assert.Len(t, out, 0)
-
-	// No remotes
-	out = testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "remote")
-	assert.Len(t, out, 0)
-
 	// No hooks
 	_, err = os.Stat(path.Join(pool.FullPath(), "hooks"))
 	assert.True(t, os.IsNotExist(err))
 
+	// origin is set
+	out := testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "remote", "get-url", "origin")
+	assert.Equal(t, testRepoPath, strings.TrimRight(string(out), "\n"))
+
+	// refs exist
+	out = testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "show-ref", "refs/heads/master")
+	assert.Equal(t, masterSha, out)
+
 	// No problems
 	out = testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "cat-file", "-s", "55bc176024cfa3baaceb71db584c7e5df900ea65")
 	assert.Equal(t, "282\n", string(out))
diff --git a/internal/service/objectpool/create_test.go b/internal/service/objectpool/create_test.go
index a67b55978..f4bbe0ee0 100644
--- a/internal/service/objectpool/create_test.go
+++ b/internal/service/objectpool/create_test.go
@@ -42,20 +42,12 @@ func TestCreate(t *testing.T) {
 	// Checks if the underlying repository is valid
 	require.True(t, pool.IsValid())
 
-	// No refs
-	out := testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "for-each-ref")
-	assert.Len(t, out, 0)
-
-	// No remotes
-	out = testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "remote")
-	assert.Len(t, out, 0)
-
 	// No hooks
 	_, err = os.Stat(path.Join(pool.FullPath(), "hooks"))
 	assert.True(t, os.IsNotExist(err))
 
 	// No problems
-	out = testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "cat-file", "-s", "55bc176024cfa3baaceb71db584c7e5df900ea65")
+	out := testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "cat-file", "-s", "55bc176024cfa3baaceb71db584c7e5df900ea65")
 	assert.Equal(t, "282\n", string(out))
 
 	// No automatic GC
diff --git a/internal/service/objectpool/link_test.go b/internal/service/objectpool/link_test.go
index 609b93155..33404c9cd 100644
--- a/internal/service/objectpool/link_test.go
+++ b/internal/service/objectpool/link_test.go
@@ -209,9 +209,6 @@ func TestUnlink(t *testing.T) {
 			if tc.code == codes.OK {
 				_, err = log.GetCommit(ctx, testRepo, poolCommitID)
 				require.True(t, log.IsNotFound(err), "expected 'not found' error got %v", err)
-
-				remotes := testhelper.MustRunCommand(t, nil, "git", "-C", pool.FullPath(), "remote")
-				require.Len(t, remotes, 0)
 			}
 		})
 	}