From 0f751fbb7c64ca5280c5d4f58d038e1df5477c67 Mon Sep 17 00:00:00 2001 From: Matt Lord Date: Sun, 15 Oct 2023 16:41:15 -0400 Subject: [PATCH] Make vtctldclient mount command more standard (#14281) Signed-off-by: Matt Lord --- .../command/vreplication/mount/mount.go | 59 +++++++++++-------- go/test/endtoend/vreplication/migrate_test.go | 10 ++-- 2 files changed, 38 insertions(+), 31 deletions(-) diff --git a/go/cmd/vtctldclient/command/vreplication/mount/mount.go b/go/cmd/vtctldclient/command/vreplication/mount/mount.go index df7e801988c..95ce3961e71 100644 --- 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,10 +49,10 @@ 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, } @@ -59,10 +60,10 @@ 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,10 +76,10 @@ 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, } @@ -86,7 +87,7 @@ 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,10 +100,10 @@ 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, } @@ -110,7 +111,7 @@ 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 index 2ec738bf8c8..75ab6a3151b 100644 --- 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,7 +302,7 @@ 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") @@ -310,7 +310,7 @@ func TestVtctldMigrate(t *testing.T) { 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") }) }