Make vtctldclient mount command more standard (#14281)

Signed-off-by: Matt Lord <mattalord@gmail.com>
vitessio · Oct 15, 2023 · 0f751fb · 0f751fb
1 parent 2663df2
commit 0f751fb
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 31 deletions.
diff --git a/go/cmd/vtctldclient/command/vreplication/mount/mount.go b/go/cmd/vtctldclient/command/vreplication/mount/mount.go
@@ -29,8 +29,8 @@ import (
 )
 
 var (
-	// mount is the base command for all actions related to the mount action.
-	mount = &cobra.Command{
+	// base is the base command for all actions related to the mount action.
+	base = &cobra.Command{
 		Use:                   "Mount [command] [command-flags]",
 		Short:                 "Mount is used to link an external Vitess cluster in order to migrate data from it.",
 		DisableFlagsInUseLine: true,
@@ -40,6 +40,7 @@ var (
 )
 
 var mountOptions struct {
+	Name       string
 	TopoType   string
 	TopoServer string
 	TopoRoot   string
@@ -48,21 +49,21 @@ var mountOptions struct {
 var register = &cobra.Command{
 	Use:                   "register",
 	Short:                 "Register an external Vitess Cluster.",
-	Example:               `vtctldclient --server localhost:15999 mount register --topo-type etcd2 --topo-server localhost:12379 --topo-root /vitess/global ext1`,
+	Example:               `vtctldclient --server localhost:15999 mount register --name ext1 --topo-type etcd2 --topo-server localhost:12379 --topo-root /vitess/global`,
 	DisableFlagsInUseLine: true,
 	Aliases:               []string{"Register"},
-	Args:                  cobra.ExactArgs(1),
+	Args:                  cobra.NoArgs,
 	RunE:                  commandRegister,
 }
 
 func commandRegister(cmd *cobra.Command, args []string) error {
 	cli.FinishedParsing(cmd)
 
 	req := &vtctldatapb.MountRegisterRequest{
+		Name:       mountOptions.Name,
 		TopoType:   mountOptions.TopoType,
 		TopoServer: mountOptions.TopoServer,
 		TopoRoot:   mountOptions.TopoRoot,
-		Name:       cmd.Flags().Arg(0),
 	}
 	_, err := common.GetClient().MountRegister(common.GetCommandCtx(), req)
 	if err != nil {
@@ -75,18 +76,18 @@ func commandRegister(cmd *cobra.Command, args []string) error {
 var unregister = &cobra.Command{
 	Use:                   "unregister",
 	Short:                 "Unregister a previously mounted external Vitess Cluster.",
-	Example:               `vtctldclient --server localhost:15999 mount unregister ext1`,
+	Example:               `vtctldclient --server localhost:15999 mount unregister --name ext1`,
 	DisableFlagsInUseLine: true,
 	Aliases:               []string{"Unregister"},
-	Args:                  cobra.ExactArgs(1),
+	Args:                  cobra.NoArgs,
 	RunE:                  commandUnregister,
 }
 
 func commandUnregister(cmd *cobra.Command, args []string) error {
 	cli.FinishedParsing(cmd)
 
 	req := &vtctldatapb.MountUnregisterRequest{
-		Name: args[0],
+		Name: mountOptions.Name,
 	}
 	_, err := common.GetClient().MountUnregister(common.GetCommandCtx(), req)
 	if err != nil {
@@ -99,18 +100,18 @@ func commandUnregister(cmd *cobra.Command, args []string) error {
 var show = &cobra.Command{
 	Use:                   "show",
 	Short:                 "Show attributes of a previously mounted external Vitess Cluster.",
-	Example:               `vtctldclient --server localhost:15999 Mount Show ext1`,
+	Example:               `vtctldclient --server localhost:15999 mount show --name ext1`,
 	DisableFlagsInUseLine: true,
 	Aliases:               []string{"Show"},
-	Args:                  cobra.ExactArgs(1),
+	Args:                  cobra.NoArgs,
 	RunE:                  commandShow,
 }
 
 func commandShow(cmd *cobra.Command, args []string) error {
 	cli.FinishedParsing(cmd)
 
 	req := &vtctldatapb.MountShowRequest{
-		Name: args[0],
+		Name: mountOptions.Name,
 	}
 	resp, err := common.GetClient().MountShow(common.GetCommandCtx(), req)
 	if err != nil {
@@ -154,21 +155,27 @@ func commandList(cmd *cobra.Command, args []string) error {
 }
 
 func registerCommands(root *cobra.Command) {
-	root.AddCommand(mount)
-	addRegisterFlags(register)
-	mount.AddCommand(register)
-	mount.AddCommand(unregister)
-	mount.AddCommand(show)
-	mount.AddCommand(list)
-}
-
-func addRegisterFlags(cmd *cobra.Command) {
-	cmd.Flags().StringVar(&mountOptions.TopoType, "topo-type", "", "Topo server implementation to use.")
-	cmd.Flags().StringVar(&mountOptions.TopoServer, "topo-server", "", "Topo server address.")
-	cmd.Flags().StringVar(&mountOptions.TopoRoot, "topo-root", "", "Topo server root path.")
-	cmd.MarkFlagRequired("topo-type")
-	cmd.MarkFlagRequired("topo-server")
-	cmd.MarkFlagRequired("topo-root")
+	root.AddCommand(base)
+
+	register.Flags().StringVar(&mountOptions.Name, "name", "", "Name to use for the mount.")
+	register.MarkFlagRequired("name")
+	register.Flags().StringVar(&mountOptions.TopoType, "topo-type", "", "Topo server implementation to use.")
+	register.MarkFlagRequired("topo-type")
+	register.Flags().StringVar(&mountOptions.TopoServer, "topo-server", "", "Topo server address.")
+	register.MarkFlagRequired("topo-server")
+	register.Flags().StringVar(&mountOptions.TopoRoot, "topo-root", "", "Topo server root path.")
+	register.MarkFlagRequired("topo-root")
+	base.AddCommand(register)
+
+	unregister.Flags().StringVar(&mountOptions.Name, "name", "", "Name of the mount.")
+	unregister.MarkFlagRequired("name")
+	base.AddCommand(unregister)
+
+	show.Flags().StringVar(&mountOptions.Name, "name", "", "Name of the mount.")
+	show.MarkFlagRequired("name")
+	base.AddCommand(show)
+
+	base.AddCommand(list)
 }
 
 func init() {

diff --git a/go/test/endtoend/vreplication/migrate_test.go b/go/test/endtoend/vreplication/migrate_test.go
@@ -221,8 +221,8 @@ func TestVtctldMigrate(t *testing.T) {
 	var output, expected string
 
 	t.Run("mount external cluster", func(t *testing.T) {
-		output, err := vc.VtctldClient.ExecuteCommandWithOutput("Mount", "register", "--topo-type=etcd2",
-			fmt.Sprintf("--topo-server=localhost:%d", extVc.ClusterConfig.topoPort), "--topo-root=/vitess/global", "ext1")
+		output, err := vc.VtctldClient.ExecuteCommandWithOutput("Mount", "register", "--name=ext1", "--topo-type=etcd2",
+			fmt.Sprintf("--topo-server=localhost:%d", extVc.ClusterConfig.topoPort), "--topo-root=/vitess/global")
 		require.NoError(t, err, "Mount Register command failed with %s", output)
 
 		output, err = vc.VtctldClient.ExecuteCommandWithOutput("Mount", "list")
@@ -231,7 +231,7 @@ func TestVtctldMigrate(t *testing.T) {
 		names := gjson.Get(output, "names")
 		require.Equal(t, 1, len(names.Array()))
 		require.Equal(t, "ext1", names.Array()[0].String())
-		output, err = vc.VtctldClient.ExecuteCommandWithOutput("Mount", "show", "ext1")
+		output, err = vc.VtctldClient.ExecuteCommandWithOutput("Mount", "show", "--name=ext1")
 		require.NoError(t, err, "Mount command failed with %s\n", output)
 
 		require.Equal(t, "etcd2", gjson.Get(output, "topo_type").String())
@@ -302,15 +302,15 @@ func TestVtctldMigrate(t *testing.T) {
 	})
 
 	t.Run("unmount external cluster", func(t *testing.T) {
-		output, err = vc.VtctldClient.ExecuteCommandWithOutput("Mount", "unregister", "ext1")
+		output, err = vc.VtctldClient.ExecuteCommandWithOutput("Mount", "unregister", "--name=ext1")
 		require.NoError(t, err, "Mount command failed with %s\n", output)
 
 		output, err = vc.VtctldClient.ExecuteCommandWithOutput("Mount", "list")
 		require.NoError(t, err, "Mount command failed with %+v : %s\n", output)
 		expected = "{}\n"
 		require.Equal(t, expected, output)
 
-		output, err = vc.VtctldClient.ExecuteCommandWithOutput("Mount", "show", "ext1")
+		output, err = vc.VtctldClient.ExecuteCommandWithOutput("Mount", "show", "--name=ext1")
 		require.Errorf(t, err, "there is no vitess cluster named ext1")
 	})
 }