praefect: Migrate metadata sub-command

Migration of the 'metadata' sub-command from the old
implementation to the new approach with usage of the
third party urfave/cli/v2 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, 114 insertions, 64 deletions

diff --git a/internal/cli/praefect/main.go b/internal/cli/praefect/main.go
index ed4f4de30..6c159b6b8 100644
--- a/internal/cli/praefect/main.go
+++ b/internal/cli/praefect/main.go
@@ -80,6 +80,7 @@ func NewApp() *cli.App {
 			newListUntrackedRepositoriesCommand(),
 			newTrackRepositoryCommand(),
 			newVerifyCommand(),
+			newMetadataCommand(),
 		},
 		Flags: []cli.Flag{
 			&cli.StringFlag{
diff --git a/internal/cli/praefect/subcmd.go b/internal/cli/praefect/subcmd.go
index f9c46719e..1c0011736 100644
--- a/internal/cli/praefect/subcmd.go
+++ b/internal/cli/praefect/subcmd.go
@@ -40,7 +40,6 @@ func subcommands(logger *logrus.Entry) map[string]subcmd {
 		setReplicationFactorCmdName: newSetReplicatioFactorSubcommand(os.Stdout),
 		removeRepositoryCmdName:     newRemoveRepository(logger, os.Stdout),
 		trackRepositoriesCmdName:    newTrackRepositories(logger, os.Stdout),
-		metadataCmdName:             newMetadataSubcommand(os.Stdout),
 	}
 }
 
diff --git a/internal/cli/praefect/subcmd_metadata.go b/internal/cli/praefect/subcmd_metadata.go
index 1cce06899..30ef2d516 100644
--- a/internal/cli/praefect/subcmd_metadata.go
+++ b/internal/cli/praefect/subcmd_metadata.go
@@ -1,96 +1,109 @@
 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 metadataCmdName = "metadata"
-
-type metadataSubcommand struct {
-	stdout         io.Writer
-	repositoryID   int64
-	virtualStorage string
-	relativePath   string
-}
-
-func newMetadataSubcommand(stdout io.Writer) *metadataSubcommand {
-	return &metadataSubcommand{stdout: stdout}
-}
-
-func (cmd *metadataSubcommand) FlagSet() *flag.FlagSet {
-	fs := flag.NewFlagSet(metadataCmdName, flag.ContinueOnError)
-	fs.Int64Var(&cmd.repositoryID, "repository-id", 0, "the repository's ID")
-	fs.StringVar(&cmd.virtualStorage, "virtual-storage", "", "the repository's virtual storage")
-	fs.StringVar(&cmd.relativePath, "relative-path", "", "the repository's relative path in the virtual storage")
-	return fs
-}
-
-func (cmd *metadataSubcommand) println(format string, args ...interface{}) {
-	fmt.Fprintf(cmd.stdout, format+"\n", args...)
+func newMetadataCommand() *cli.Command {
+	return &cli.Command{
+		Name:  "metadata",
+		Usage: "shows metadata information about repository",
+		Description: "The command provides metadata information about the repository. It includes " +
+			"identifier of the repository, path on the disk for it and it's replicas, information " +
+			"about replicas such as if it is assigned or not, its generation, health state, the storage, " +
+			"if it is a valid primary, etc. It can be invoked by providing repository identifier or " +
+			"virtual repository name and relative path.",
+		HideHelpCommand: true,
+		Action:          metadataAction,
+		Flags: []cli.Flag{
+			&cli.Int64Flag{
+				Name:  "repository-id",
+				Usage: "the repository's ID",
+			},
+			&cli.StringFlag{
+				Name:  paramVirtualStorage,
+				Usage: "the repository's virtual storage",
+			},
+			&cli.StringFlag{
+				Name:  "relative-path",
+				Usage: "the repository's relative path in the virtual storage",
+			},
+		},
+		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 *metadataSubcommand) Exec(flags *flag.FlagSet, cfg config.Config) error {
-	if flags.NArg() > 0 {
-		return unexpectedPositionalArgsError{Command: flags.Name()}
+func metadataAction(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)
+	relativePath := appCtx.String("relative-path")
+
 	var request gitalypb.GetRepositoryMetadataRequest
 	switch {
-	case cmd.repositoryID != 0:
-		if cmd.virtualStorage != "" || cmd.relativePath != "" {
+	case repositoryID != 0:
+		if virtualStorage != "" || relativePath != "" {
 			return errors.New("virtual storage and relative path can't be provided with a repository ID")
 		}
-		request.Query = &gitalypb.GetRepositoryMetadataRequest_RepositoryId{RepositoryId: cmd.repositoryID}
-	case cmd.virtualStorage != "" || cmd.relativePath != "":
-		if cmd.virtualStorage == "" {
+		request.Query = &gitalypb.GetRepositoryMetadataRequest_RepositoryId{RepositoryId: repositoryID}
+	case virtualStorage != "" || relativePath != "":
+		if virtualStorage == "" {
 			return errors.New("virtual storage is required with relative path")
-		} else if cmd.relativePath == "" {
+		} else if relativePath == "" {
 			return errors.New("relative path is required with virtual storage")
 		}
 		request.Query = &gitalypb.GetRepositoryMetadataRequest_Path_{
 			Path: &gitalypb.GetRepositoryMetadataRequest_Path{
-				VirtualStorage: cmd.virtualStorage,
-				RelativePath:   cmd.relativePath,
+				VirtualStorage: virtualStorage,
+				RelativePath:   relativePath,
 			},
 		}
 	default:
 		return errors.New("repository id or virtual storage and relative path 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)
+	conn, err := subCmdDial(appCtx.Context, nodeAddr, conf.Auth.Token, defaultDialTimeout)
 	if err != nil {
 		return fmt.Errorf("dial: %w", err)
 	}
 	defer conn.Close()
 
-	metadata, err := gitalypb.NewPraefectInfoServiceClient(conn).GetRepositoryMetadata(ctx, &request)
+	metadata, err := gitalypb.NewPraefectInfoServiceClient(conn).GetRepositoryMetadata(appCtx.Context, &request)
 	if err != nil {
 		return fmt.Errorf("get metadata: %w", err)
 	}
 
-	cmd.println("Repository ID: %d", metadata.RepositoryId)
-	cmd.println("Virtual Storage: %q", metadata.VirtualStorage)
-	cmd.println("Relative Path: %q", metadata.RelativePath)
-	cmd.println("Replica Path: %q", metadata.ReplicaPath)
-	cmd.println("Primary: %q", metadata.Primary)
-	cmd.println("Generation: %d", metadata.Generation)
-	cmd.println("Replicas:")
+	fmt.Fprintf(appCtx.App.Writer, "Repository ID: %d\n", metadata.RepositoryId)
+	fmt.Fprintf(appCtx.App.Writer, "Virtual Storage: %q\n", metadata.VirtualStorage)
+	fmt.Fprintf(appCtx.App.Writer, "Relative Path: %q\n", metadata.RelativePath)
+	fmt.Fprintf(appCtx.App.Writer, "Replica Path: %q\n", metadata.ReplicaPath)
+	fmt.Fprintf(appCtx.App.Writer, "Primary: %q\n", metadata.Primary)
+	fmt.Fprintf(appCtx.App.Writer, "Generation: %d\n", metadata.Generation)
+	fmt.Fprintf(appCtx.App.Writer, "Replicas:\n")
 	for _, replica := range metadata.Replicas {
-		cmd.println("- Storage: %q", replica.Storage)
-		cmd.println("  Assigned: %v", replica.Assigned)
+		fmt.Fprintf(appCtx.App.Writer, "- Storage: %q\n", replica.Storage)
+		fmt.Fprintf(appCtx.App.Writer, "  Assigned: %v\n", replica.Assigned)
 
 		generationText := fmt.Sprintf("%d, fully up to date", replica.Generation)
 		if replica.Generation == -1 {
@@ -104,10 +117,10 @@ func (cmd *metadataSubcommand) Exec(flags *flag.FlagSet, cfg config.Config) erro
 			verifiedAt = replica.VerifiedAt.AsTime().String()
 		}
 
-		cmd.println("  Generation: %s", generationText)
-		cmd.println("  Healthy: %v", replica.Healthy)
-		cmd.println("  Valid Primary: %v", replica.ValidPrimary)
-		cmd.println("  Verified At: %s", verifiedAt)
+		fmt.Fprintf(appCtx.App.Writer, "  Generation: %s\n", generationText)
+		fmt.Fprintf(appCtx.App.Writer, "  Healthy: %v\n", replica.Healthy)
+		fmt.Fprintf(appCtx.App.Writer, "  Valid Primary: %v\n", replica.ValidPrimary)
+		fmt.Fprintf(appCtx.App.Writer, "  Verified At: %s\n", verifiedAt)
 	}
 	return nil
 }
diff --git a/internal/cli/praefect/subcmd_metadata_test.go b/internal/cli/praefect/subcmd_metadata_test.go
index bfce0350d..05db30b7e 100644
--- a/internal/cli/praefect/subcmd_metadata_test.go
+++ b/internal/cli/praefect/subcmd_metadata_test.go
@@ -4,10 +4,12 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"io"
 	"testing"
 	"time"
 
 	"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"
@@ -22,7 +24,7 @@ func TestMetadataSubcommand(t *testing.T) {
 	ctx := testhelper.Context(t)
 
 	tx := testdb.New(t).Begin(t)
-	defer tx.Rollback(t)
+	t.Cleanup(func() { tx.Rollback(t) })
 
 	testdb.SetHealthyNodes(t, ctx, tx, map[string]map[string][]string{
 		"praefect": {"virtual-storage": {"primary", "secondary-1"}},
@@ -42,7 +44,23 @@ func TestMetadataSubcommand(t *testing.T) {
 	ln, clean := listenAndServe(t, []svcRegistrar{
 		registerPraefectInfoServer(info.NewServer(config.Config{}, rs, nil, nil, nil)),
 	})
-	defer clean()
+	t.Cleanup(clean)
+
+	conf := config.Config{
+		SocketPath: ln.Addr().String(),
+		VirtualStorages: []*config.VirtualStorage{
+			{
+				Name: "vs-1",
+				Nodes: []*config.Node{
+					{
+						Storage: "storage-1",
+						Address: "tcp://1.2.3.4",
+					},
+				},
+			},
+		},
+	}
+	confPath := writeConfigToFile(t, conf)
 
 	for _, tc := range []struct {
 		desc  string
@@ -50,6 +68,11 @@ func TestMetadataSubcommand(t *testing.T) {
 		error error
 	}{
 		{
+			desc:  "positional arguments",
+			args:  []string{"positional-arg"},
+			error: cli.Exit(unexpectedPositionalArgsError{Command: "metadata"}, 1),
+		},
+		{
 			desc:  "missing parameters fails",
 			error: errors.New("repository id or virtual storage and relative path required"),
 		},
@@ -87,14 +110,28 @@ func TestMetadataSubcommand(t *testing.T) {
 			args: []string{"-virtual-storage=virtual-storage", "-relative-path=relative-path"},
 		},
 	} {
+		tc := tc
 		t.Run(tc.desc, func(t *testing.T) {
-			stdout := &bytes.Buffer{}
-			cmd := newMetadataSubcommand(stdout)
+			t.Parallel()
 
-			fs := cmd.FlagSet()
-			require.NoError(t, fs.Parse(tc.args))
-			err := cmd.Exec(fs, config.Config{SocketPath: ln.Addr().String()})
-			testhelper.RequireGrpcError(t, tc.error, err)
+			var stdout bytes.Buffer
+			app := cli.App{
+				Reader:          bytes.NewReader(nil),
+				Writer:          &stdout,
+				ErrWriter:       io.Discard,
+				HideHelpCommand: true,
+				Commands: []*cli.Command{
+					newMetadataCommand(),
+				},
+				Flags: []cli.Flag{
+					&cli.StringFlag{
+						Name:  "config",
+						Value: confPath,
+					},
+				},
+			}
+			err := app.Run(append([]string{progname, "metadata"}, tc.args...))
+			require.Equal(t, tc.error, err)
 			if tc.error != nil {
 				return
 			}