Skip to content
Snippets Groups Projects
Commit 7d259a61 authored by Justin Tobler's avatar Justin Tobler
Browse files

Merge branch 'cherry-pick-5649e8da' into '16-1-stable'

Backport !6251 to 16-1-stable

See merge request gitlab-org/gitaly!6337



Merged-by: default avatarJustin Tobler <jtobler@gitlab.com>
Approved-by: default avatarJustin Tobler <jtobler@gitlab.com>
Co-authored-by: Patrick Steinhardt's avatarPatrick Steinhardt <psteinhardt@gitlab.com>
parents 262126b3 23bf5ded
No related branches found
No related tags found
1 merge request!6337Backport !6251 to 16-1-stable
Pipeline #1000159051 passed with warnings
......@@ -19,7 +19,8 @@ import (
const (
defaultDialTimeout = 10 * time.Second
paramVirtualStorage = "virtual-storage"
paramRelativePath = "repository"
paramRelativePath = "relative-path"
paramReplicaPath = "replica-path"
paramAuthoritativeStorage = "authoritative-storage"
)
......
......@@ -81,25 +81,25 @@ func TestAcceptDatalossSubcommand(t *testing.T) {
}{
{
desc: "missing virtual storage",
args: []string{"-repository", "test-repository-1", "-authoritative-storage", "test-physical-storage-2"},
args: []string{"-relative-path", "test-repository-1", "-authoritative-storage", "test-physical-storage-2"},
matchError: matchErrorMsg(`Required flag "virtual-storage" not set`),
expectedGenerations: startingGenerations,
},
{
desc: "missing repository",
args: []string{"-virtual-storage", "test-virtual-storage-1", "-authoritative-storage", "test-physical-storage-2"},
matchError: matchErrorMsg(`Required flag "repository" not set`),
matchError: matchErrorMsg(`Required flag "relative-path" not set`),
expectedGenerations: startingGenerations,
},
{
desc: "missing authoritative storage",
args: []string{"-virtual-storage", "test-virtual-storage-1", "-repository", "test-repository-1"},
args: []string{"-virtual-storage", "test-virtual-storage-1", "-relative-path", "test-repository-1"},
matchError: matchErrorMsg(`Required flag "authoritative-storage" not set`),
expectedGenerations: startingGenerations,
},
{
desc: "positional arguments",
args: []string{"-virtual-storage", "test-virtual-storage-1", "-repository", "test-repository-1", "-authoritative-storage", "test-physical-storage-2", "positional-arg"},
args: []string{"-virtual-storage", "test-virtual-storage-1", "-relative-path", "test-repository-1", "-authoritative-storage", "test-physical-storage-2", "positional-arg"},
matchError: func(t *testing.T, actual error) {
t.Helper()
require.Equal(t, unexpectedPositionalArgsError{Command: "accept-dataloss"}, actual)
......@@ -108,25 +108,25 @@ func TestAcceptDatalossSubcommand(t *testing.T) {
},
{
desc: "non-existent virtual storage",
args: []string{"-virtual-storage", "non-existent", "-repository", "test-repository-1", "-authoritative-storage", "test-physical-storage-2"},
args: []string{"-virtual-storage", "non-existent", "-relative-path", "test-repository-1", "-authoritative-storage", "test-physical-storage-2"},
matchError: matchErrorMsg(`set authoritative storage: rpc error: code = InvalidArgument desc = unknown virtual storage: "non-existent"`),
expectedGenerations: startingGenerations,
},
{
desc: "non-existent authoritative storage",
args: []string{"-virtual-storage", "test-virtual-storage-1", "-repository", "test-repository-1", "-authoritative-storage", "non-existent"},
args: []string{"-virtual-storage", "test-virtual-storage-1", "-relative-path", "test-repository-1", "-authoritative-storage", "non-existent"},
matchError: matchErrorMsg(`set authoritative storage: rpc error: code = InvalidArgument desc = unknown authoritative storage: "non-existent"`),
expectedGenerations: startingGenerations,
},
{
desc: "non-existent repository",
args: []string{"-virtual-storage", "test-virtual-storage-1", "-repository", "non-existent", "-authoritative-storage", "test-physical-storage-2"},
args: []string{"-virtual-storage", "test-virtual-storage-1", "-relative-path", "non-existent", "-authoritative-storage", "test-physical-storage-2"},
matchError: matchErrorMsg(`set authoritative storage: rpc error: code = InvalidArgument desc = repository does not exist on virtual storage`),
expectedGenerations: startingGenerations,
},
{
desc: "success",
args: []string{"-virtual-storage", "test-virtual-storage-1", "-repository", "test-repository-1", "-authoritative-storage", "test-physical-storage-2"},
args: []string{"-virtual-storage", "test-virtual-storage-1", "-relative-path", "test-repository-1", "-authoritative-storage", "test-physical-storage-2"},
matchError: func(t *testing.T, actual error) {
t.Helper()
require.NoError(t, actual)
......
......@@ -88,7 +88,7 @@ func TestRemoveRepositorySubcommand(t *testing.T) {
{
desc: "positional arguments",
args: func(*testing.T, *gitalypb.Repository, string) []string {
return []string{"-virtual-storage=vs", "-repository=r", "positional-arg"}
return []string{"-virtual-storage=vs", "-relative-path=r", "positional-arg"}
},
assertError: func(t *testing.T, err error, _ *gitalypb.Repository, _ string) {
assert.Equal(t, cli.Exit(unexpectedPositionalArgsError{Command: "remove-repository"}, 1), err)
......@@ -97,7 +97,7 @@ func TestRemoveRepositorySubcommand(t *testing.T) {
{
desc: "virtual-storage is not set",
args: func(*testing.T, *gitalypb.Repository, string) []string {
return []string{"-repository=r"}
return []string{"-relative-path=r"}
},
assertError: func(t *testing.T, err error, _ *gitalypb.Repository, _ string) {
assert.EqualError(t, err, `Required flag "virtual-storage" not set`)
......@@ -109,7 +109,7 @@ func TestRemoveRepositorySubcommand(t *testing.T) {
return []string{"-virtual-storage=vs"}
},
assertError: func(t *testing.T, err error, _ *gitalypb.Repository, _ string) {
assert.EqualError(t, err, `Required flag "repository" not set`)
assert.EqualError(t, err, `Required flag "relative-path" not set`)
},
},
{
......@@ -141,7 +141,7 @@ func TestRemoveRepositorySubcommand(t *testing.T) {
return writeConfigToFile(t, conf)
},
args: func(*testing.T, *gitalypb.Repository, string) []string {
return []string{"-virtual-storage=vs", "-repository=r"}
return []string{"-virtual-storage=vs", "-relative-path=r"}
},
assertError: func(t *testing.T, err error, _ *gitalypb.Repository, _ string) {
require.Contains(t, err.Error(), "connect to database: send ping: failed to connect to ")
......@@ -150,7 +150,7 @@ func TestRemoveRepositorySubcommand(t *testing.T) {
{
desc: "dry run",
args: func(_ *testing.T, repo *gitalypb.Repository, _ string) []string {
return []string{"-virtual-storage", repo.StorageName, "-repository", repo.RelativePath}
return []string{"-virtual-storage", repo.StorageName, "-relative-path", repo.RelativePath}
},
assertError: func(t *testing.T, err error, repo *gitalypb.Repository, replicaPath string) {
require.DirExists(t, filepath.Join(g1Cfg.Storages[0].Path, replicaPath))
......@@ -167,7 +167,7 @@ func TestRemoveRepositorySubcommand(t *testing.T) {
args: func(t *testing.T, repo *gitalypb.Repository, replicaPath string) []string {
require.DirExists(t, filepath.Join(g1Cfg.Storages[0].Path, replicaPath))
require.DirExists(t, filepath.Join(g2Cfg.Storages[0].Path, replicaPath))
return []string{"-virtual-storage", repo.StorageName, "-repository", repo.RelativePath, "-apply"}
return []string{"-virtual-storage", repo.StorageName, "-relative-path", repo.RelativePath, "-apply"}
},
assertError: func(t *testing.T, err error, repo *gitalypb.Repository, replicaPath string) {
require.NoError(t, err)
......@@ -184,7 +184,7 @@ func TestRemoveRepositorySubcommand(t *testing.T) {
{
desc: "db only",
args: func(t *testing.T, repo *gitalypb.Repository, _ string) []string {
return []string{"-virtual-storage", repo.StorageName, "-repository", repo.RelativePath, "-apply", "-db-only"}
return []string{"-virtual-storage", repo.StorageName, "-relative-path", repo.RelativePath, "-apply", "-db-only"}
},
assertError: func(t *testing.T, err error, repo *gitalypb.Repository, replicaPath string) {
require.NoError(t, err)
......@@ -209,7 +209,7 @@ func TestRemoveRepositorySubcommand(t *testing.T) {
desc: "repository doesnt exist on one gitaly",
args: func(t *testing.T, repo *gitalypb.Repository, replicaPath string) []string {
require.NoError(t, os.RemoveAll(filepath.Join(g2Cfg.Storages[0].Path, replicaPath)))
return []string{"-virtual-storage", repo.StorageName, "-repository", repo.RelativePath, "-apply"}
return []string{"-virtual-storage", repo.StorageName, "-relative-path", repo.RelativePath, "-apply"}
},
assertError: func(t *testing.T, err error, repo *gitalypb.Repository, replicaPath string) {
require.NoDirExists(t, filepath.Join(g1Cfg.Storages[0].Path, replicaPath))
......@@ -228,7 +228,7 @@ func TestRemoveRepositorySubcommand(t *testing.T) {
repoStore := datastore.NewPostgresRepositoryStore(db.DB, nil)
_, _, err = repoStore.DeleteRepository(ctx, repo.StorageName, repo.RelativePath)
require.NoError(t, err)
return []string{"-virtual-storage", repo.StorageName, "-repository", repo.RelativePath, "-apply"}
return []string{"-virtual-storage", repo.StorageName, "-relative-path", repo.RelativePath, "-apply"}
},
assertError: func(t *testing.T, err error, repo *gitalypb.Repository, replicaPath string) {
require.EqualError(t, err, "repository is not being tracked in Praefect")
......@@ -258,7 +258,7 @@ func TestRemoveRepositorySubcommand(t *testing.T) {
repo := createRepo(t, ctx, repoClient, praefectStorage, t.Name())
g2Srv.Shutdown()
replicaPath := gittest.GetReplicaPath(t, ctx, gitalycfg.Cfg{SocketPath: praefectServer.Address()}, repo)
stdout, stderr, err := runApp([]string{"-config", confPath, "remove-repository", "-virtual-storage", repo.StorageName, "-repository", repo.RelativePath, "-apply"})
stdout, stderr, err := runApp([]string{"-config", confPath, "remove-repository", "-virtual-storage", repo.StorageName, "-relative-path", repo.RelativePath, "-apply"})
assert.Empty(t, stderr)
require.NoError(t, err)
assert.Contains(t, stdout, "Repository removal completed.")
......
......@@ -30,53 +30,53 @@ func TestSetReplicationFactorSubcommand(t *testing.T) {
}{
{
desc: "unexpected positional arguments",
args: []string{"-virtual-storage=virtual-storage", "-repository=relative-path", "-replication-factor=1", "positonal-arg"},
args: []string{"-virtual-storage=virtual-storage", "-relative-path=relative-path", "-replication-factor=1", "positonal-arg"},
error: cli.Exit(unexpectedPositionalArgsError{Command: "set-replication-factor"}, 1),
},
{
desc: "missing virtual-storage",
args: []string{"-repository=relative-path", "-replication-factor=1"},
args: []string{"-relative-path=relative-path", "-replication-factor=1"},
error: errors.New(`Required flag "virtual-storage" not set`),
},
{
desc: "missing repository",
desc: "missing relative-path",
args: []string{"-virtual-storage=virtual-storage", "-replication-factor=1"},
error: errors.New(`Required flag "repository" not set`),
error: errors.New(`Required flag "relative-path" not set`),
},
{
desc: "missing replication-factor",
args: []string{"-virtual-storage=virtual-storage", "-repository=relative-path"},
args: []string{"-virtual-storage=virtual-storage", "-relative-path=relative-path"},
error: errors.New(`Required flag "replication-factor" not set`),
},
{
desc: "replication factor too small",
args: []string{"-virtual-storage=virtual-storage", "-repository=relative-path", "-replication-factor=0"},
args: []string{"-virtual-storage=virtual-storage", "-relative-path=relative-path", "-replication-factor=0"},
error: status.Error(codes.InvalidArgument, "set replication factor: attempted to set replication factor 0 but minimum is 1"),
},
{
desc: "replication factor too big",
args: []string{"-virtual-storage=virtual-storage", "-repository=relative-path", "-replication-factor=3"},
args: []string{"-virtual-storage=virtual-storage", "-relative-path=relative-path", "-replication-factor=3"},
error: status.Error(codes.InvalidArgument, "set replication factor: attempted to set replication factor 3 but virtual storage only contains 2 storages"),
},
{
desc: "virtual storage not found",
args: []string{"-virtual-storage=non-existent", "-repository=relative-path", "-replication-factor=2"},
args: []string{"-virtual-storage=non-existent", "-relative-path=relative-path", "-replication-factor=2"},
error: status.Error(codes.InvalidArgument, `set replication factor: virtual storage "non-existent" not found`),
},
{
desc: "repository not found",
args: []string{"-virtual-storage=virtual-storage", "-repository=non-existent", "-replication-factor=2"},
args: []string{"-virtual-storage=virtual-storage", "-relative-path=non-existent", "-replication-factor=2"},
error: status.Error(codes.InvalidArgument, `set replication factor: repository "virtual-storage"/"non-existent" not found`),
},
{
desc: "assignments are disabled",
args: []string{"-virtual-storage=virtual-storage", "-repository=relative-path", "-replication-factor=1"},
args: []string{"-virtual-storage=virtual-storage", "-relative-path=relative-path", "-replication-factor=1"},
store: praefect.NewDisabledAssignmentStore(nil),
error: status.Error(codes.Internal, `set replication factor: assignments are disabled`),
},
{
desc: "successfully set",
args: []string{"-virtual-storage=virtual-storage", "-repository=relative-path", "-replication-factor=2"},
args: []string{"-virtual-storage=virtual-storage", "-relative-path=relative-path", "-replication-factor=2"},
stdout: "current assignments: primary, secondary\n",
},
} {
......
......@@ -27,7 +27,8 @@ func newTrackRepositoriesCommand() *cli.Command {
"The -input-path flag must be the path of a file containing the details of the repositories\n" +
"to track as a list of newline-delimited JSON objects. Each line must contain the details for\n" +
"one and only one repository. Each item must contain the following keys:\n\n" +
" relative_path - The relative path of the repository on-disk.\n" +
" relative_path - The relative path on the virtual storage. Usually starts with '@hashed'\n" +
" replica_path - The relative path on physical storage. Can start with '@cluster' or match 'relative_path'.\n" +
" virtual_storage - The Praefect virtual storage name.\n" +
" authoritative_storage - Which storage to consider as the canonical copy of the repository.\n\n" +
"If -replicate-immediately is used, the command will attempt to replicate the repositories\n" +
......@@ -110,7 +111,12 @@ func trackRepositoriesAction(appCtx *cli.Context) error {
if request.RelativePath == "" {
badReq.errs = append(badReq.errs, requiredParameterError(paramRelativePath))
}
badReq.path = request.RelativePath
badReq.relativePath = request.RelativePath
if request.ReplicaPath == "" {
badReq.errs = append(badReq.errs, requiredParameterError(paramReplicaPath))
}
badReq.replicaPath = request.ReplicaPath
if request.VirtualStorage == "" {
badReq.errs = append(badReq.errs, requiredParameterError(paramVirtualStorage))
......@@ -187,9 +193,10 @@ func trackRepositoriesAction(appCtx *cli.Context) error {
}
type invalidRequest struct {
line int
path string
errs []error
line int
relativePath string
replicaPath string
errs []error
}
type dupPathError struct {
......@@ -205,7 +212,7 @@ func printInvalidRequests(w io.Writer, repoErrs []invalidRequest, pathLines map[
fmt.Fprintf(w, "Found %v invalid request(s) in %q:\n", len(repoErrs), inputPath)
for _, l := range repoErrs {
fmt.Fprintf(w, " line %v, relative_path: %q\n", l.line, l.path)
fmt.Fprintf(w, " line %v, relative_path: %q, replica_path: %q\n", l.line, l.relativePath, l.replicaPath)
for _, err := range l.errs {
if dup, ok := err.(*dupPathError); ok {
// The complete set of duplicate reqNums won't be known until input is
......
......@@ -5,6 +5,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"
"testing"
"time"
......@@ -108,9 +109,19 @@ func TestTrackRepositoriesSubcommand(t *testing.T) {
desc string
input string
relativePaths []string
replicaPaths []string
args func(inputPath string) []string
expectedOutput string
}{
{
desc: "replica path differs from relative path",
relativePaths: []string{"path/to/test/diff_repo1", "path/to/test/diff_repo2"},
replicaPaths: []string{"path/to/replica/diff_repo1", "path/to/replica/diff_repo2"},
args: func(inputPath string) []string {
return []string{"-replicate-immediately", "-input-path=" + inputPath}
},
expectedOutput: "Finished replicating repository to \"gitaly-2\".\n",
},
{
desc: "immediate replication",
relativePaths: []string{"path/to/test/repo1", "path/to/test/repo2"},
......@@ -136,18 +147,28 @@ func TestTrackRepositoriesSubcommand(t *testing.T) {
input, err := os.Create(inputPath)
require.NoError(t, err)
for _, path := range tc.relativePaths {
exists, err := repositoryStore.RepositoryExists(ctx, virtualStorageName, path)
if len(tc.replicaPaths) == 0 {
tc.replicaPaths = tc.relativePaths
}
for i, relPath := range tc.relativePaths {
exists, err := repositoryStore.RepositoryExists(ctx, virtualStorageName, relPath)
require.NoError(t, err)
require.False(t, exists)
replicaPath := tc.replicaPaths[i]
// create the repo on Gitaly without Praefect knowing
require.NoError(t, createRepoThroughGitaly1(path))
require.DirExists(t, filepath.Join(g1Cfg.Storages[0].Path, path))
require.NoDirExists(t, filepath.Join(g2Cfg.Storages[0].Path, path))
require.NoError(t, createRepoThroughGitaly1(replicaPath))
require.DirExists(t, filepath.Join(g1Cfg.Storages[0].Path, replicaPath))
require.NoDirExists(t, filepath.Join(g2Cfg.Storages[0].Path, replicaPath))
// Write repo details to input file
repoEntry, err := json.Marshal(trackRepositoryRequest{RelativePath: path, VirtualStorage: virtualStorageName, AuthoritativeStorage: authoritativeStorage})
repoEntry, err := json.Marshal(trackRepositoryRequest{
RelativePath: relPath,
ReplicaPath: replicaPath,
VirtualStorage: virtualStorageName,
AuthoritativeStorage: authoritativeStorage,
})
require.NoError(t, err)
_, err = fmt.Fprintf(input, string(repoEntry)+"\n")
require.NoError(t, err)
......@@ -160,8 +181,8 @@ func TestTrackRepositoriesSubcommand(t *testing.T) {
assert.Contains(t, stdout, tc.expectedOutput)
replicateImmediately := slices.Contains(tc.args(inputPath), "-replicate-immediately")
for _, path := range tc.relativePaths {
repositoryID, err := repositoryStore.GetRepositoryID(ctx, virtualStorageName, path)
for i, relPath := range tc.relativePaths {
repositoryID, err := repositoryStore.GetRepositoryID(ctx, virtualStorageName, relPath)
require.NoError(t, err)
assignments, err := assignmentStore.GetHostAssignments(ctx, virtualStorageName, repositoryID)
......@@ -170,7 +191,7 @@ func TestTrackRepositoriesSubcommand(t *testing.T) {
assert.Contains(t, assignments, g1Cfg.Storages[0].Name)
assert.Contains(t, assignments, g2Cfg.Storages[0].Name)
exists, err := repositoryStore.RepositoryExists(ctx, virtualStorageName, path)
exists, err := repositoryStore.RepositoryExists(ctx, virtualStorageName, relPath)
require.NoError(t, err)
assert.True(t, exists)
......@@ -179,9 +200,10 @@ func TestTrackRepositoriesSubcommand(t *testing.T) {
events, err := queue.Dequeue(ctx, virtualStorageName, g2Cfg.Storages[0].Name, 1)
require.NoError(t, err)
require.Len(t, events, 1)
assert.Equal(t, path, events[0].Job.RelativePath)
assert.Equal(t, relPath, events[0].Job.RelativePath)
} else {
require.DirExists(t, filepath.Join(g2Cfg.Storages[0].Path, path))
replicaPath := tc.replicaPaths[i]
require.DirExists(t, filepath.Join(g2Cfg.Storages[0].Path, replicaPath))
}
}
})
......@@ -207,6 +229,13 @@ func TestTrackRepositoriesSubcommand(t *testing.T) {
)
}
formatOutput := func(inputPath string, requestCt int, lines []string) string {
header := fmt.Sprintf("Validating repository information in %q\n", inputPath)
header += fmt.Sprintf("Found %v invalid request(s) in %q:\n", requestCt, inputPath)
rest := strings.Join(lines, "\n")
return header + rest + "\n"
}
const invalidEntryErr = "invalid entries found, aborting"
t.Run("fail", func(t *testing.T) {
......@@ -214,7 +243,8 @@ func TestTrackRepositoriesSubcommand(t *testing.T) {
desc string
args func(inputPath string) []string
input string
expectedOutput string
requestCt int
expectedOutput []string
expectedError string
trackedPath string
}{
......@@ -238,42 +268,78 @@ func TestTrackRepositoriesSubcommand(t *testing.T) {
expectedError: "no repository information found",
},
{
desc: "invalid JSON",
input: "@hashed/01/23/01234567890123456789.git",
expectedOutput: "invalid character '@' looking for beginning of value",
expectedError: invalidEntryErr,
desc: "invalid JSON",
input: "@hashed/01/23/01234567890123456789.git",
requestCt: 1,
expectedOutput: []string{
` line 1, relative_path: "", replica_path: ""`,
" invalid character '@' looking for beginning of value",
},
expectedError: invalidEntryErr,
},
{
desc: "missing path",
input: `{"virtual_storage":"praefect","authoritative_storage":"gitaly-1"}`,
expectedOutput: `"repository" is a required parameter`,
expectedError: invalidEntryErr,
desc: "missing relative path",
input: `{"replica_path":"r","virtual_storage":"praefect","authoritative_storage":"gitaly-1"}`,
requestCt: 1,
expectedOutput: []string{
` line 1, relative_path: "", replica_path: "r"`,
` "relative-path" is a required parameter`,
},
expectedError: invalidEntryErr,
},
{
desc: "missing replica path",
input: `{"relative_path":"r","virtual_storage":"praefect","authoritative_storage":"gitaly-1"}`,
requestCt: 1,
expectedOutput: []string{
` line 1, relative_path: "r", replica_path: ""`,
` "replica-path" is a required parameter`,
},
expectedError: invalidEntryErr,
},
{
desc: "invalid virtual storage",
input: `{"virtual_storage":"foo","relative_path":"bar","authoritative_storage":"gitaly-1"}`,
expectedOutput: `virtual storage "foo" not found`,
expectedError: invalidEntryErr,
desc: "invalid virtual storage",
input: `{"virtual_storage":"foo","relative_path":"bar","replica_path":"bar","authoritative_storage":"gitaly-1"}`,
requestCt: 1,
expectedOutput: []string{
` line 1, relative_path: "bar", replica_path: "bar"`,
` checking repository on disk: virtual storage "foo" not found`,
},
expectedError: invalidEntryErr,
},
{
desc: "repo does not exist",
input: `{"relative_path":"not_a_repo","virtual_storage":"praefect","authoritative_storage":"gitaly-1"}`,
expectedOutput: "not a valid git repository",
expectedError: invalidEntryErr,
desc: "repo does not exist",
input: `{"relative_path":"not_a_repo","replica_path":"foo","virtual_storage":"praefect","authoritative_storage":"gitaly-1"}`,
requestCt: 1,
expectedOutput: []string{
` line 1, relative_path: "not_a_repo", replica_path: "foo"`,
" not a valid git repository",
},
expectedError: invalidEntryErr,
},
{
desc: "duplicate path",
input: `{"virtual_storage":"praefect","relative_path":"duplicate","authoritative_storage":"gitaly-1"}
{"virtual_storage":"praefect","relative_path":"duplicate","authoritative_storage":"gitaly-1"}`,
expectedOutput: "duplicate entries for relative_path",
expectedError: invalidEntryErr,
desc: "duplicate relative path",
input: `{"virtual_storage":"praefect","relative_path":"duplicate","replica_path":"foo","authoritative_storage":"gitaly-1"}
{"virtual_storage":"praefect","relative_path":"duplicate","replica_path":"foo","authoritative_storage":"gitaly-1"}`,
requestCt: 2,
expectedOutput: []string{
` line 1, relative_path: "duplicate", replica_path: "foo"`,
" not a valid git repository",
` line 2, relative_path: "duplicate", replica_path: "foo"`,
" duplicate entries for relative_path, line [1 2]",
},
expectedError: invalidEntryErr,
},
{
desc: "repo is already tracked",
input: `{"relative_path":"already_tracked","virtual_storage":"praefect","authoritative_storage":"gitaly-1"}`,
expectedOutput: "repository is already tracked by Praefect",
expectedError: invalidEntryErr,
trackedPath: "already_tracked",
desc: "repo is already tracked",
input: `{"relative_path":"already_tracked","replica_path":"foo","virtual_storage":"praefect","authoritative_storage":"gitaly-1"}`,
requestCt: 1,
expectedOutput: []string{
` line 1, relative_path: "already_tracked", replica_path: "foo"`,
" repository is already tracked by Praefect",
},
expectedError: invalidEntryErr,
trackedPath: "already_tracked",
},
} {
tc := tc
......@@ -299,9 +365,10 @@ func TestTrackRepositoriesSubcommand(t *testing.T) {
assert.Empty(t, stderr)
require.Error(t, err)
if tc.expectedOutput != "" {
require.Contains(t, stdout, tc.expectedOutput)
if len(tc.expectedOutput) > 0 {
require.Equal(t, formatOutput(inputPath, tc.requestCt, tc.expectedOutput), stdout)
}
require.Contains(t, err.Error(), tc.expectedError)
})
}
......
......@@ -49,12 +49,17 @@ func newTrackRepositoryCommand() *cli.Command {
},
&cli.StringFlag{
Name: paramRelativePath,
Usage: "relative path to the repository",
Usage: "relative path of the repository on the virtual storage. Usually starts with '@hashed'",
Required: true,
},
&cli.StringFlag{
Name: paramReplicaPath,
Usage: "path of the repository on the physical storage. Can start with '@cluster' or match the relative path",
Required: true,
},
&cli.StringFlag{
Name: paramAuthoritativeStorage,
Usage: "storage with the repository to consider as authoritative",
Usage: "physical storage to consider as authoritative for the repository",
},
&cli.BoolFlag{
Name: "replicate-immediately",
......@@ -73,6 +78,7 @@ func newTrackRepositoryCommand() *cli.Command {
type trackRepositoryRequest struct {
RelativePath string `json:"relative_path"`
ReplicaPath string `json:"replica_path"`
VirtualStorage string `json:"virtual_storage"`
AuthoritativeStorage string `json:"authoritative_storage"`
}
......@@ -88,6 +94,7 @@ func trackRepositoryAction(appCtx *cli.Context) error {
virtualStorage := appCtx.String(paramVirtualStorage)
relativePath := appCtx.String(paramRelativePath)
replicaPath := appCtx.String(paramReplicaPath)
authoritativeStorage := appCtx.String(paramAuthoritativeStorage)
replicateImmediately := appCtx.Bool("replicate-immediately")
......@@ -109,6 +116,7 @@ func trackRepositoryAction(appCtx *cli.Context) error {
req := trackRepositoryRequest{
RelativePath: relativePath,
ReplicaPath: replicaPath,
AuthoritativeStorage: authoritativeStorage,
VirtualStorage: virtualStorage,
}
......@@ -129,6 +137,7 @@ func (req *trackRepositoryRequest) execRequest(ctx context.Context,
logger.WithFields(logrus.Fields{
"virtual_storage": req.VirtualStorage,
"relative_path": req.RelativePath,
"replica_path": req.ReplicaPath,
"authoritative_storage": req.AuthoritativeStorage,
}).Debug("track repository")
......@@ -229,6 +238,7 @@ func (req *trackRepositoryRequest) execRequest(ctx context.Context,
RepositoryID: repositoryID,
Change: datastore.UpdateRepo,
RelativePath: req.RelativePath,
ReplicaPath: req.ReplicaPath,
VirtualStorage: req.VirtualStorage,
SourceNodeStorage: primary,
TargetNodeStorage: secondary,
......@@ -287,7 +297,7 @@ func (req *trackRepositoryRequest) trackRepository(
repositoryID,
req.VirtualStorage,
req.RelativePath,
req.RelativePath,
req.ReplicaPath,
primary,
nil,
secondaries,
......@@ -325,14 +335,14 @@ func (req *trackRepositoryRequest) authoritativeRepositoryExists(ctx context.Con
for _, node := range vs.Nodes {
if node.Storage == nodeName {
logger.Debugf("check if repository %q exists on gitaly %q at %q", req.RelativePath, node.Storage, node.Address)
logger.Debugf("check if repository %q exists on gitaly %q at %q", req.ReplicaPath, node.Storage, node.Address)
repo := &gitalypb.Repository{
StorageName: node.Storage,
RelativePath: req.RelativePath,
RelativePath: req.ReplicaPath,
}
exists, err := repositoryExists(ctx, repo, node.Address, node.Token)
if err != nil {
fmt.Fprintf(w, "checking if repository exists %q, %q", node.Storage, req.RelativePath)
fmt.Fprintf(w, "checking if repository exists %q, %q", node.Storage, req.ReplicaPath)
return false, nil
}
return exists, nil
......
......@@ -109,30 +109,42 @@ func TestTrackRepositorySubcommand(t *testing.T) {
}{
{
name: "positional arguments",
args: []string{"-virtual-storage=v", "-repository=r", "-authoritative-storage=s", "positional-arg"},
args: []string{"-virtual-storage=v", "-relative-path=r", "-replica-path=r", "-authoritative-storage=s", "positional-arg"},
errorMsg: "track-repository doesn't accept positional arguments",
},
{
name: "virtual-storage is not set",
args: []string{
"-repository", "path/to/test/repo_1",
"-relative-path", "path/to/test/repo_1",
"-replica-path", "path/to/test/repo_1",
"-authoritative-storage", authoritativeStorage,
},
errorMsg: `Required flag "virtual-storage" not set`,
},
{
name: "repository is not set",
name: "relative-path is not set",
args: []string{
"-virtual-storage", virtualStorageName,
"-replica-path", "path/to/test/repo_1",
"-authoritative-storage", authoritativeStorage,
},
errorMsg: `Required flag "repository" not set`,
errorMsg: `Required flag "relative-path" not set`,
},
{
name: "replica-path is not set",
args: []string{
"-virtual-storage", virtualStorageName,
"-relative-path", "path/to/test/repo_1",
"-authoritative-storage", authoritativeStorage,
},
errorMsg: `Required flag "replica-path" not set`,
},
{
name: "authoritative-storage is not set",
args: []string{
"-virtual-storage", virtualStorageName,
"-repository", "path/to/test/repo_1",
"-relative-path", "path/to/test/repo_1",
"-replica-path", "path/to/test/repo_1",
},
errorMsg: `Required flag "authoritative-storage" not set`,
},
......@@ -140,7 +152,8 @@ func TestTrackRepositorySubcommand(t *testing.T) {
name: "repository does not exist",
args: []string{
"-virtual-storage", virtualStorageName,
"-repository", "path/to/test/repo_1",
"-relative-path", "path/to/test/repo_1",
"-replica-path", "path/to/test/repo_1",
"-authoritative-storage", authoritativeStorage,
},
errorMsg: "attempting to track repository in praefect database: authoritative repository does not exist",
......@@ -157,11 +170,19 @@ func TestTrackRepositorySubcommand(t *testing.T) {
t.Run("ok", func(t *testing.T) {
testCases := []struct {
relativePath string
replicaPath string
desc string
replicateImmediately bool
repositoryExists bool
expectedOutput []string
}{
{
desc: "replica path differs from relative path",
relativePath: "path/to/test/diff_repo",
replicaPath: "path/to/replica/diff_repo",
replicateImmediately: true,
expectedOutput: []string{"Finished replicating repository to \"gitaly-2\".\n"},
},
{
desc: "force replication",
relativePath: "path/to/test/repo1",
......@@ -203,20 +224,25 @@ func TestTrackRepositorySubcommand(t *testing.T) {
nodeMgr.Start(0, time.Hour)
defer nodeMgr.Stop()
if tc.replicaPath == "" {
tc.replicaPath = tc.relativePath
}
exists, err := repoDS.RepositoryExists(ctx, virtualStorageName, tc.relativePath)
require.NoError(t, err)
require.Equal(t, tc.repositoryExists, exists)
// create the repo on Gitaly without Praefect knowing
if !tc.repositoryExists {
require.NoError(t, createRepoThroughGitaly1(tc.relativePath))
require.DirExists(t, filepath.Join(g1Cfg.Storages[0].Path, tc.relativePath))
require.NoDirExists(t, filepath.Join(g2Cfg.Storages[0].Path, tc.relativePath))
require.NoError(t, createRepoThroughGitaly1(tc.replicaPath))
require.DirExists(t, filepath.Join(g1Cfg.Storages[0].Path, tc.replicaPath))
require.NoDirExists(t, filepath.Join(g2Cfg.Storages[0].Path, tc.replicaPath))
}
args := []string{
"-virtual-storage", virtualStorageName,
"-repository", tc.relativePath,
"-relative-path", tc.relativePath,
"-replica-path", tc.replicaPath,
"-authoritative-storage", authoritativeStorage,
}
if tc.replicateImmediately {
......@@ -270,7 +296,8 @@ func TestTrackRepositorySubcommand(t *testing.T) {
"-config", confPath,
trackRepositoryCmdName,
"-virtual-storage", virtualStorageName,
"-repository", relativePath,
"-relative-path", relativePath,
"-replica-path", relativePath,
"-authoritative-storage", authoritativeStorage,
})
require.NoError(t, err)
......@@ -280,7 +307,8 @@ func TestTrackRepositorySubcommand(t *testing.T) {
"-config", confPath,
trackRepositoryCmdName,
"-virtual-storage", virtualStorageName,
"-repository", relativePath,
"-relative-path", relativePath,
"-replica-path", relativePath,
"-authoritative-storage", authoritativeStorage,
})
require.NoError(t, err)
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment