praefect: Migrate verify sub-command

Migration of the 'verify' sub-command from the old
implementation to the new approach with usage of the
third party library. The invocation of the command remains
the same, but it has more descriptive representation when
help is invoked. Also, it is possible now to invoke help
only for the sub-command to see more details about it and
its usage.

Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/5001

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