diff options
author | John Cai <jcai@gitlab.com> | 2021-12-15 00:03:52 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2021-12-15 00:03:54 +0300 |
commit | bbb50dfb1493226b44981006b6c8f50d416d917e (patch) | |
tree | 2b7818050bb33f725e453d029a8b6609b38544b5 /cmd/praefect | |
parent | 0e0aeb5ca4488903f41acd392911bd89d1ad3d6d (diff) |
praefect: Fix output of remove-repository
There was a bug in the logic so that we printed out "Did not remove"
even though we did remove the repository from disk. This change also
deletes some wrong code that checks for an error message that only
exists in praefect-but we are calling gitaly directly so this error
message will never appear.
Changelog: fixed
Diffstat (limited to 'cmd/praefect')
-rw-r--r-- | cmd/praefect/subcmd_remove_repository.go | 9 | ||||
-rw-r--r-- | cmd/praefect/subcmd_remove_repository_test.go | 33 |
2 files changed, 36 insertions, 6 deletions
diff --git a/cmd/praefect/subcmd_remove_repository.go b/cmd/praefect/subcmd_remove_repository.go index f6ee01358..49b049615 100644 --- a/cmd/praefect/subcmd_remove_repository.go +++ b/cmd/praefect/subcmd_remove_repository.go @@ -241,11 +241,7 @@ func (cmd *removeRepository) removeRepository(ctx context.Context, repo *gitalyp ctx = metadata.AppendToOutgoingContext(ctx, "client_name", removeRepositoryCmdName) repositoryClient := gitalypb.NewRepositoryServiceClient(conn) if _, err := repositoryClient.RemoveRepository(ctx, &gitalypb.RemoveRepositoryRequest{Repository: repo}); err != nil { - s, ok := status.FromError(err) - if !ok { - return false, fmt.Errorf("RemoveRepository: %w", err) - } - if !strings.Contains(s.Message(), fmt.Sprintf("get primary: repository %q/%q not found", cmd.virtualStorage, cmd.relativePath)) { + if _, ok := status.FromError(err); !ok { return false, fmt.Errorf("RemoveRepository: %w", err) } return false, nil @@ -312,8 +308,9 @@ func (cmd *removeRepository) removeNode( } else { if removed { fmt.Fprintf(cmd.w, "Successfully removed %s from %s\n", cmd.relativePath, node.Storage) + } else { + fmt.Fprintf(cmd.w, "Did not remove %s from %s\n", cmd.relativePath, node.Storage) } - fmt.Fprintf(cmd.w, "Did not remove %s from %s\n", cmd.relativePath, node.Storage) } logger.Debugf("repository removal call to gitaly %q completed", node.Storage) } diff --git a/cmd/praefect/subcmd_remove_repository_test.go b/cmd/praefect/subcmd_remove_repository_test.go index bf2ece389..6e920ec0e 100644 --- a/cmd/praefect/subcmd_remove_repository_test.go +++ b/cmd/praefect/subcmd_remove_repository_test.go @@ -4,6 +4,7 @@ import ( "bytes" "flag" "fmt" + "os" "path/filepath" "strings" "testing" @@ -158,6 +159,38 @@ func TestRemoveRepository_Exec(t *testing.T) { assert.Contains(t, out.String(), "Repository found on the following gitaly nodes:") assert.Contains(t, out.String(), fmt.Sprintf("Attempting to remove %s from the database, and delete it from all gitaly nodes...\n", repo.RelativePath)) assert.Contains(t, out.String(), fmt.Sprintf("Successfully removed %s from", repo.RelativePath)) + assert.NotContains(t, out.String(), fmt.Sprintf("Did not remove %s from", repo.RelativePath)) + + var repositoryRowExists bool + require.NoError(t, db.QueryRow( + `SELECT EXISTS(SELECT FROM repositories WHERE virtual_storage = $1 AND relative_path = $2)`, + cmd.virtualStorage, cmd.relativePath, + ).Scan(&repositoryRowExists)) + require.False(t, repositoryRowExists) + }) + + t.Run("repository doesnt exist on one gitaly", func(t *testing.T) { + var out bytes.Buffer + repo := createRepo(t, ctx, repoClient, praefectStorage, t.Name()) + + require.NoError(t, os.RemoveAll(filepath.Join(g2Cfg.Storages[0].Path, repo.RelativePath))) + + cmd := &removeRepository{ + logger: testhelper.NewDiscardingLogger(t), + virtualStorage: repo.StorageName, + relativePath: repo.RelativePath, + dialTimeout: time.Second, + apply: true, + w: &writer{w: &out}, + } + require.NoError(t, cmd.Exec(flag.NewFlagSet("", flag.PanicOnError), conf)) + + require.NoDirExists(t, filepath.Join(g1Cfg.Storages[0].Path, repo.RelativePath)) + require.NoDirExists(t, filepath.Join(g2Cfg.Storages[0].Path, repo.RelativePath)) + assert.Contains(t, out.String(), "Repository found in the database.\n") + assert.Contains(t, out.String(), "Repository found on the following gitaly nodes: gitaly-1") + assert.Contains(t, out.String(), fmt.Sprintf("Attempting to remove %s from the database, and delete it from all gitaly nodes...\n", repo.RelativePath)) + assert.Contains(t, out.String(), fmt.Sprintf("Successfully removed %s from gitaly-1", repo.RelativePath)) var repositoryRowExists bool require.NoError(t, db.QueryRow( |