Merge branch 'ps-praefect-cli-verify' into 'master'

praefect: migrate verify sub-command

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

Merged-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Approved-by: Toon Claes <toon@gitlab.com>
Approved-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Reviewed-by: Quang-Minh Nguyen <qmnguyen@gitlab.com>
Co-authored-by: Pavlo Strokov <pstrokov@gitlab.com>

4 files changed, 93 insertions, 43 deletions

diff --git a/internal/cli/praefect/main.go b/internal/cli/praefect/main.go
index e25f41d59..ed4f4de30 100644
--- a/internal/cli/praefect/main.go
+++ b/internal/cli/praefect/main.go
@@ -79,6 +79,7 @@ func NewApp() *cli.App {
 			newListStoragesCommand(),
 			newListUntrackedRepositoriesCommand(),
 			newTrackRepositoryCommand(),
+			newVerifyCommand(),
 		},
 		Flags: []cli.Flag{
 			&cli.StringFlag{
diff --git a/internal/cli/praefect/subcmd.go b/internal/cli/praefect/subcmd.go
index 87df59b96..f9c46719e 100644
--- a/internal/cli/praefect/subcmd.go
+++ b/internal/cli/praefect/subcmd.go
@@ -41,7 +41,6 @@ func subcommands(logger *logrus.Entry) map[string]subcmd {
 		removeRepositoryCmdName:     newRemoveRepository(logger, os.Stdout),
 		trackRepositoriesCmdName:    newTrackRepositories(logger, os.Stdout),
 		metadataCmdName:             newMetadataSubcommand(os.Stdout),
-		verifyCmdName:               newVerifySubcommand(os.Stdout),
 	}
 }
 
diff --git a/internal/cli/praefect/subcmd_verify.go b/internal/cli/praefect/subcmd_verify.go
index 544353e2c..5e40e8a91 100644
--- a/internal/cli/praefect/subcmd_verify.go
+++ b/internal/cli/praefect/subcmd_verify.go
@@ -1,74 +1,93 @@
 package praefect
 
 import (
-	"context"
 	"errors"
-	"flag"
 	"fmt"
-	"io"
 
-	"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/config"
+	"github.com/urfave/cli/v2"
+	"gitlab.com/gitlab-org/gitaly/v16/internal/log"
 	"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 )
 
 const verifyCmdName = "verify"
 
-type verifySubcommand struct {
-	stdout         io.Writer
-	repositoryID   int64
-	virtualStorage string
-	storage        string
-}
-
-func newVerifySubcommand(stdout io.Writer) *verifySubcommand {
-	return &verifySubcommand{stdout: stdout}
-}
-
-func (cmd *verifySubcommand) FlagSet() *flag.FlagSet {
-	fs := flag.NewFlagSet(metadataCmdName, flag.ContinueOnError)
-	fs.Int64Var(&cmd.repositoryID, "repository-id", 0, "the id of the repository to verify")
-	fs.StringVar(&cmd.virtualStorage, "virtual-storage", "", "the virtual storage to verify")
-	fs.StringVar(&cmd.storage, "storage", "", "the storage to verify")
-	return fs
+func newVerifyCommand() *cli.Command {
+	return &cli.Command{
+		Name:  verifyCmdName,
+		Usage: "marks a discrete repository, or repositories of a storage, or repositories of a virtual storage to be verified",
+		Description: "The command marks a single repository if 'repository-id' flag is provided or a batch of\n" +
+			"repositories that belong to a particular storage or virtual storage to be checked they exist.\n" +
+			"The repository existence is confirmed if repository exists on the disk. That verification operation\n" +
+			"runs in background and executes verification asynchronously.",
+		HideHelpCommand: true,
+		Action:          verifyAction,
+		Flags: []cli.Flag{
+			&cli.Int64Flag{
+				Name:  "repository-id",
+				Usage: "the id of the repository to verify",
+			},
+			&cli.StringFlag{
+				Name:  paramVirtualStorage,
+				Usage: "the virtual storage to verify",
+			},
+			&cli.StringFlag{
+				Name:  "storage",
+				Usage: "the storage to verify",
+			},
+		},
+		Before: func(ctx *cli.Context) error {
+			if ctx.Args().Present() {
+				_ = cli.ShowSubcommandHelp(ctx)
+				return cli.Exit(unexpectedPositionalArgsError{Command: ctx.Command.Name}, 1)
+			}
+			return nil
+		},
+	}
 }
 
-func (cmd *verifySubcommand) Exec(flags *flag.FlagSet, cfg config.Config) error {
-	if flags.NArg() > 0 {
-		return unexpectedPositionalArgsError{Command: flags.Name()}
+func verifyAction(appCtx *cli.Context) error {
+	logger := log.Default()
+	conf, err := getConfig(logger, appCtx.String(configFlagName))
+	if err != nil {
+		return err
 	}
 
+	repositoryID := appCtx.Int64("repository-id")
+	virtualStorage := appCtx.String(paramVirtualStorage)
+	storage := appCtx.String("storage")
+
 	var request gitalypb.MarkUnverifiedRequest
 	switch {
-	case cmd.repositoryID != 0:
-		if cmd.virtualStorage != "" || cmd.storage != "" {
+	case repositoryID != 0:
+		if virtualStorage != "" || storage != "" {
 			return errors.New("virtual storage and storage can't be provided with a repository ID")
 		}
 
-		request.Selector = &gitalypb.MarkUnverifiedRequest_RepositoryId{RepositoryId: cmd.repositoryID}
-	case cmd.storage != "":
-		if cmd.virtualStorage == "" {
+		request.Selector = &gitalypb.MarkUnverifiedRequest_RepositoryId{RepositoryId: repositoryID}
+	case storage != "":
+		if virtualStorage == "" {
 			return errors.New("virtual storage must be passed with storage")
 		}
 
 		request.Selector = &gitalypb.MarkUnverifiedRequest_Storage_{
 			Storage: &gitalypb.MarkUnverifiedRequest_Storage{
-				VirtualStorage: cmd.virtualStorage,
-				Storage:        cmd.storage,
+				VirtualStorage: virtualStorage,
+				Storage:        storage,
 			},
 		}
-	case cmd.virtualStorage != "":
-		request.Selector = &gitalypb.MarkUnverifiedRequest_VirtualStorage{VirtualStorage: cmd.virtualStorage}
+	case virtualStorage != "":
+		request.Selector = &gitalypb.MarkUnverifiedRequest_VirtualStorage{VirtualStorage: virtualStorage}
 	default:
 		return errors.New("(repository id), (virtual storage) or (virtual storage, storage) required")
 	}
 
-	nodeAddr, err := getNodeAddress(cfg)
+	nodeAddr, err := getNodeAddress(conf)
 	if err != nil {
 		return fmt.Errorf("get node address: %w", err)
 	}
 
-	ctx := context.TODO()
-	conn, err := subCmdDial(ctx, nodeAddr, cfg.Auth.Token, defaultDialTimeout)
+	ctx := appCtx.Context
+	conn, err := subCmdDial(ctx, nodeAddr, conf.Auth.Token, defaultDialTimeout)
 	if err != nil {
 		return fmt.Errorf("dial: %w", err)
 	}
@@ -79,7 +98,7 @@ func (cmd *verifySubcommand) Exec(flags *flag.FlagSet, cfg config.Config) error
 		return fmt.Errorf("verify replicas: %w", err)
 	}
 
-	fmt.Fprintf(cmd.stdout, "%d replicas marked unverified\n", response.GetReplicasMarked())
+	fmt.Fprintf(appCtx.App.Writer, "%d replicas marked unverified\n", response.GetReplicasMarked())
 
 	return nil
 }
diff --git a/internal/cli/praefect/subcmd_verify_test.go b/internal/cli/praefect/subcmd_verify_test.go
index 19a1e5d9d..901579f17 100644
--- a/internal/cli/praefect/subcmd_verify_test.go
+++ b/internal/cli/praefect/subcmd_verify_test.go
@@ -4,9 +4,11 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"io"
 	"testing"
 
 	"github.com/stretchr/testify/require"
+	"github.com/urfave/cli/v2"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/config"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/datastore"
 	"gitlab.com/gitlab-org/gitaly/v16/internal/praefect/service/info"
@@ -50,6 +52,11 @@ func TestVerifySubcommand(t *testing.T) {
 		expectedState  state
 	}{
 		{
+			desc:  "positional arguments",
+			args:  []string{"positional-arg"},
+			error: cli.Exit(unexpectedPositionalArgsError{Command: "verify"}, 1),
+		},
+		{
 			desc:          "no selector given",
 			error:         errors.New("(repository id), (virtual storage) or (virtual storage, storage) required"),
 			expectedState: startingState,
@@ -187,12 +194,36 @@ func TestVerifySubcommand(t *testing.T) {
 			`)
 			require.NoError(t, err)
 
-			stdout := &bytes.Buffer{}
-			cmd := newVerifySubcommand(stdout)
+			conf := config.Config{
+				SocketPath: ln.Addr().String(),
+				VirtualStorages: []*config.VirtualStorage{
+					{
+						Name: "vs",
+						Nodes: []*config.Node{
+							{Address: "stub", Storage: "st"},
+						},
+					},
+				},
+			}
+			confPath := writeConfigToFile(t, conf)
 
-			fs := cmd.FlagSet()
-			require.NoError(t, fs.Parse(tc.args))
-			err = cmd.Exec(fs, config.Config{SocketPath: ln.Addr().String()})
+			var stdout bytes.Buffer
+			app := cli.App{
+				Reader:          bytes.NewReader(nil),
+				Writer:          &stdout,
+				ErrWriter:       io.Discard,
+				HideHelpCommand: true,
+				Commands: []*cli.Command{
+					newVerifyCommand(),
+				},
+				Flags: []cli.Flag{
+					&cli.StringFlag{
+						Name:  "config",
+						Value: confPath,
+					},
+				},
+			}
+			err = app.Run(append([]string{progname, verifyCmdName}, tc.args...))
 			testhelper.RequireGrpcError(t, tc.error, err)
 			if tc.error != nil {
 				return