diff --git a/go/cmd/mysqlctl/command/start.go b/go/cmd/mysqlctl/command/start.go index 872443fe2e6..8adeb185a35 100644 --- a/go/cmd/mysqlctl/command/start.go +++ b/go/cmd/mysqlctl/command/start.go @@ -62,7 +62,7 @@ func commandStart(cmd *cobra.Command, args []string) error { func init() { utils.SetFlagDurationVar(Start.Flags(), &startArgs.WaitTime, "wait-time", startArgs.WaitTime, "How long to wait for mysqld startup.") - Start.Flags().Var(&startArgs.MySQLdArgs, "mysqld_args", "List of comma-separated flags to pass additionally to mysqld.") + Start.Flags().Var(&startArgs.MySQLdArgs, "mysqld-args", "List of comma-separated flags to pass additionally to mysqld.") Root.AddCommand(Start) } diff --git a/go/cmd/mysqlctl/mysqlctl.go b/go/cmd/mysqlctl/mysqlctl.go index 72198c2c8c0..846d80dd47f 100644 --- a/go/cmd/mysqlctl/mysqlctl.go +++ b/go/cmd/mysqlctl/mysqlctl.go @@ -20,9 +20,12 @@ package main import ( "vitess.io/vitess/go/cmd/mysqlctl/command" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { + command.Root.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) + if err := command.Root.Execute(); err != nil { log.Exit(err) } diff --git a/go/cmd/mysqlctld/mysqlctld.go b/go/cmd/mysqlctld/mysqlctld.go index 5843c5a15e1..5bdfc7a84b8 100644 --- a/go/cmd/mysqlctld/mysqlctld.go +++ b/go/cmd/mysqlctld/mysqlctld.go @@ -22,9 +22,12 @@ package main import ( "vitess.io/vitess/go/cmd/mysqlctld/cli" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) + if err := cli.Main.Execute(); err != nil { log.Exit(err) } diff --git a/go/cmd/rulesctl/main.go b/go/cmd/rulesctl/main.go index aedc3c66078..9c7a44414f7 100644 --- a/go/cmd/rulesctl/main.go +++ b/go/cmd/rulesctl/main.go @@ -8,10 +8,12 @@ import ( vtlog "vitess.io/vitess/go/vt/log" "vitess.io/vitess/go/vt/logutil" "vitess.io/vitess/go/vt/servenv" + "vitess.io/vitess/go/vt/utils" ) func main() { rootCmd := cmd.Main() + rootCmd.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) vtlog.RegisterFlags(rootCmd.PersistentFlags()) logutil.RegisterFlags(rootCmd.PersistentFlags()) acl.RegisterFlags(rootCmd.PersistentFlags()) diff --git a/go/cmd/topo2topo/topo2topo.go b/go/cmd/topo2topo/topo2topo.go index c1276ebf504..f9019460898 100644 --- a/go/cmd/topo2topo/topo2topo.go +++ b/go/cmd/topo2topo/topo2topo.go @@ -20,11 +20,13 @@ import ( "vitess.io/vitess/go/cmd/topo2topo/cli" "vitess.io/vitess/go/exit" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { defer exit.RecoverAll() + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := cli.Main.Execute(); err != nil { log.Exitf("%s", err) } diff --git a/go/cmd/vtaclcheck/cli/vtactlcheck.go b/go/cmd/vtaclcheck/cli/vtactlcheck.go index ebac94131e8..36aceb55585 100644 --- a/go/cmd/vtaclcheck/cli/vtactlcheck.go +++ b/go/cmd/vtaclcheck/cli/vtactlcheck.go @@ -22,6 +22,7 @@ import ( "vitess.io/vitess/go/acl" "vitess.io/vitess/go/vt/logutil" "vitess.io/vitess/go/vt/servenv" + "vitess.io/vitess/go/vt/utils" "vitess.io/vitess/go/vt/vtaclcheck" ) @@ -58,6 +59,8 @@ func run(cmd *cobra.Command, args []string) error { } func init() { + Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) + servenv.MoveFlagsToCobraCommand(Main) Main.Flags().StringVar(&aclFile, "acl-file", aclFile, "The path of the JSON ACL file to check") diff --git a/go/cmd/vtadmin/main.go b/go/cmd/vtadmin/main.go index 07d36cf2e35..d58fa69a152 100644 --- a/go/cmd/vtadmin/main.go +++ b/go/cmd/vtadmin/main.go @@ -227,6 +227,7 @@ func registerFlags() { func main() { registerFlags() + rootCmd.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := rootCmd.Execute(); err != nil { log.Fatal(err) } diff --git a/go/cmd/vtbackup/vtbackup.go b/go/cmd/vtbackup/vtbackup.go index 37dcadc9b19..a69afd8dc97 100644 --- a/go/cmd/vtbackup/vtbackup.go +++ b/go/cmd/vtbackup/vtbackup.go @@ -20,11 +20,13 @@ import ( "vitess.io/vitess/go/cmd/vtbackup/cli" "vitess.io/vitess/go/exit" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { defer exit.Recover() + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := cli.Main.Execute(); err != nil { log.Error(err) exit.Return(1) diff --git a/go/cmd/vtbench/vtbench.go b/go/cmd/vtbench/vtbench.go index 0d8bb85b536..75e221d6fe5 100644 --- a/go/cmd/vtbench/vtbench.go +++ b/go/cmd/vtbench/vtbench.go @@ -20,11 +20,13 @@ import ( "vitess.io/vitess/go/cmd/vtbench/cli" "vitess.io/vitess/go/exit" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { defer exit.Recover() + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := cli.Main.Execute(); err != nil { log.Exit(err) } diff --git a/go/cmd/vtclient/vtclient.go b/go/cmd/vtclient/vtclient.go index ccfd31a0ac3..5df798dfd6d 100644 --- a/go/cmd/vtclient/vtclient.go +++ b/go/cmd/vtclient/vtclient.go @@ -19,10 +19,13 @@ package main import ( "vitess.io/vitess/go/cmd/vtclient/cli" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { cli.InitializeFlags() + + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := cli.Main.Execute(); err != nil { log.Exit(err) } diff --git a/go/cmd/vtcombo/main.go b/go/cmd/vtcombo/main.go index f5de215b617..8736714b605 100644 --- a/go/cmd/vtcombo/main.go +++ b/go/cmd/vtcombo/main.go @@ -25,11 +25,13 @@ import ( "vitess.io/vitess/go/cmd/vtcombo/cli" "vitess.io/vitess/go/exit" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { defer exit.Recover() + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := cli.Main.Execute(); err != nil { log.Error(err) exit.Return(1) diff --git a/go/cmd/vtctl/vtctl.go b/go/cmd/vtctl/vtctl.go index 78fd403deec..5564cfa91a0 100644 --- a/go/cmd/vtctl/vtctl.go +++ b/go/cmd/vtctl/vtctl.go @@ -91,6 +91,8 @@ func init() { fs.BoolVar(&detachedMode, "detach", detachedMode, "detached mode - run vtcl detached from the terminal") acl.RegisterFlags(fs) + + fs.SetNormalizeFunc(utils.NormalizeUnderscoresToDashes) }) } diff --git a/go/cmd/vtctlclient/main.go b/go/cmd/vtctlclient/main.go index 420d3e4939b..872728cd4c7 100644 --- a/go/cmd/vtctlclient/main.go +++ b/go/cmd/vtctlclient/main.go @@ -52,6 +52,8 @@ func init() { fs.StringVar(&server, "server", server, "server to use for connection") acl.RegisterFlags(fs) + + fs.SetNormalizeFunc(utils.NormalizeUnderscoresToDashes) }) } diff --git a/go/cmd/vtctld/main.go b/go/cmd/vtctld/main.go index 46ce01e409f..06329733fdd 100644 --- a/go/cmd/vtctld/main.go +++ b/go/cmd/vtctld/main.go @@ -19,9 +19,11 @@ package main import ( "vitess.io/vitess/go/cmd/vtctld/cli" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := cli.Main.Execute(); err != nil { log.Fatal(err) } diff --git a/go/cmd/vtctldclient/main.go b/go/cmd/vtctldclient/main.go index 4f31204ca8c..8ee11ff0753 100644 --- a/go/cmd/vtctldclient/main.go +++ b/go/cmd/vtctldclient/main.go @@ -33,6 +33,7 @@ import ( "vitess.io/vitess/go/vt/vttablet/tmclient" _flag "vitess.io/vitess/go/internal/flag" + flagUtils "vitess.io/vitess/go/vt/utils" ) func main() { @@ -55,6 +56,7 @@ func main() { // hack to get rid of an "ERROR: logging before flag.Parse" _flag.TrickGlog() + command.Root.SetGlobalNormalizationFunc(flagUtils.NormalizeUnderscoresToDashes) // back to your regularly scheduled cobra programming if err := command.Root.Execute(); err != nil { log.Error(err) diff --git a/go/cmd/vtexplain/vtexplain.go b/go/cmd/vtexplain/vtexplain.go index 37774076382..35a29c1ca5a 100644 --- a/go/cmd/vtexplain/vtexplain.go +++ b/go/cmd/vtexplain/vtexplain.go @@ -21,11 +21,13 @@ import ( "vitess.io/vitess/go/cmd/vtexplain/cli" "vitess.io/vitess/go/exit" + "vitess.io/vitess/go/vt/utils" ) func main() { defer exit.RecoverAll() + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := cli.Main.Execute(); err != nil { fmt.Printf("ERROR: %s\n", err) exit.Return(1) diff --git a/go/cmd/vtgate/cli/cli.go b/go/cmd/vtgate/cli/cli.go index 3261e703fb3..dc0eba48752 100644 --- a/go/cmd/vtgate/cli/cli.go +++ b/go/cmd/vtgate/cli/cli.go @@ -213,7 +213,5 @@ func init() { Main.Flags().StringVar(&plannerName, "planner-version", plannerName, "Sets the default planner to use when the session has not changed it. Valid values are: Gen4, Gen4Greedy, Gen4Left2Right") // Support both variants until v25 - // Main.MarkFlagRequired("tablet-types-to-wait") - Main.MarkFlagsOneRequired("tablet-types-to-wait", "tablet_types_to_wait") - + Main.MarkFlagsOneRequired("tablet-types-to-wait") } diff --git a/go/cmd/vtgate/vtgate.go b/go/cmd/vtgate/vtgate.go index fd81fe85a68..085565704a7 100644 --- a/go/cmd/vtgate/vtgate.go +++ b/go/cmd/vtgate/vtgate.go @@ -19,9 +19,11 @@ package main import ( "vitess.io/vitess/go/cmd/vtgate/cli" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := cli.Main.Execute(); err != nil { log.Exit(err) } diff --git a/go/cmd/vtgateclienttest/main.go b/go/cmd/vtgateclienttest/main.go index 313b27de04a..76d64dd46e2 100644 --- a/go/cmd/vtgateclienttest/main.go +++ b/go/cmd/vtgateclienttest/main.go @@ -20,11 +20,13 @@ import ( "vitess.io/vitess/go/cmd/vtgateclienttest/cli" "vitess.io/vitess/go/exit" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { defer exit.Recover() + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := cli.Main.Execute(); err != nil { log.Exitf("%s", err) } diff --git a/go/cmd/vtorc/main.go b/go/cmd/vtorc/main.go index c85d51e9325..b4d12d41950 100644 --- a/go/cmd/vtorc/main.go +++ b/go/cmd/vtorc/main.go @@ -19,12 +19,14 @@ package main import ( "vitess.io/vitess/go/cmd/vtorc/cli" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) // main is the application's entry point. It will spawn an HTTP interface. func main() { // TODO: viperutil.BindFlags() + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := cli.Main.Execute(); err != nil { log.Exit(err) } diff --git a/go/cmd/vttablet/cli/cli_test.go b/go/cmd/vttablet/cli/cli_test.go index 3785e881f53..305e736f4c4 100644 --- a/go/cmd/vttablet/cli/cli_test.go +++ b/go/cmd/vttablet/cli/cli_test.go @@ -25,7 +25,6 @@ import ( "vitess.io/vitess/go/vt/topo" "vitess.io/vitess/go/vt/topo/memorytopo" - "vitess.io/vitess/go/vt/utils" ) // TestRunFailsToStartTabletManager tests the code path in 'run' where we fail to start the TabletManager @@ -43,15 +42,16 @@ func TestRunFailsToStartTabletManager(t *testing.T) { os.Args = append([]string{}, args...) }) - flags := make(map[string]string) - utils.SetFlagVariantsForTests(flags, "--topo-implementation", "test") - utils.SetFlagVariantsForTests(flags, "--topo-global-server-address", "localhost") - utils.SetFlagVariantsForTests(flags, "--topo-global-root", "cell") - utils.SetFlagVariantsForTests(flags, "--db-host", "localhost") - utils.SetFlagVariantsForTests(flags, "--db-port", "3306") - utils.SetFlagVariantsForTests(flags, "--init-keyspace", "ks") - utils.SetFlagVariantsForTests(flags, "--init-shard", "0") - utils.SetFlagVariantsForTests(flags, "--init-tablet-type", "replica") + flags := map[string]string{ + "--topo-implementation": "test", + "--topo-global-server-address": "localhost", + "--topo-global-root": "cell", + "--db-host": "localhost", + "--db-port": "3306", + "--init-keyspace": "ks", + "--init-shard": "0", + "--init-tablet-type": "replica", + } var flagArgs []string for flag, value := range flags { diff --git a/go/cmd/vttablet/vttablet.go b/go/cmd/vttablet/vttablet.go index 0f91f48b649..ad65dd54990 100644 --- a/go/cmd/vttablet/vttablet.go +++ b/go/cmd/vttablet/vttablet.go @@ -20,9 +20,11 @@ package main import ( "vitess.io/vitess/go/cmd/vttablet/cli" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := cli.Main.Execute(); err != nil { log.Exit(err) } diff --git a/go/cmd/vttestserver/cli/main_test.go b/go/cmd/vttestserver/cli/main_test.go index 3387eba20f8..c7fba2dc2a5 100644 --- a/go/cmd/vttestserver/cli/main_test.go +++ b/go/cmd/vttestserver/cli/main_test.go @@ -38,7 +38,6 @@ import ( "vitess.io/vitess/go/vt/log" "vitess.io/vitess/go/vt/logutil" "vitess.io/vitess/go/vt/tlstest" - "vitess.io/vitess/go/vt/utils" "vitess.io/vitess/go/vt/vtctl/vtctlclient" "vitess.io/vitess/go/vt/vttest" @@ -191,7 +190,7 @@ func TestNoScatter(t *testing.T) { conf := config defer resetConfig(conf) - cluster, err := startCluster(utils.GetFlagVariantForTests("--no-scatter")) + cluster, err := startCluster("--no-scatter") require.NoError(t, err) defer cluster.TearDown() @@ -303,14 +302,14 @@ func TestMtlsAuth(t *testing.T) { // When cluster starts it will apply SQL and VSchema migrations in the configured schema-dir folder // With mtls authorization enabled, the authorized CN must match the certificate's CN cluster, err := startCluster( - utils.GetFlagVariantForTests("--grpc-auth-mode")+"=mtls", - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--grpc-key"), key), - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--grpc-cert"), cert), - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--grpc-ca"), caCert), - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--vtctld-grpc-key"), clientKey), - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--vtctld-grpc-cert"), clientCert), - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--vtctld-grpc-ca"), caCert), - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--grpc-auth-mtls-allowed-substrings"), "CN=ClientApp")) + "--grpc-auth-mode=mtls", + fmt.Sprintf("%s=%s", "--grpc-key", key), + fmt.Sprintf("%s=%s", "--grpc-cert", cert), + fmt.Sprintf("%s=%s", "--grpc-ca", caCert), + fmt.Sprintf("%s=%s", "--vtctld-grpc-key", clientKey), + fmt.Sprintf("%s=%s", "--vtctld-grpc-cert", clientCert), + fmt.Sprintf("%s=%s", "--vtctld-grpc-ca", caCert), + fmt.Sprintf("%s=%s", "--grpc-auth-mtls-allowed-substrings", "CN=ClientApp")) require.NoError(t, err) defer func() { cluster.PersistentMode = false // Cleanup the tmpdir as we're done @@ -345,13 +344,13 @@ func TestMtlsAuthUnauthorizedFails(t *testing.T) { // For mtls authorization failure by providing a client certificate with different CN thant the // authorized in the configuration cluster, err := startCluster( - utils.GetFlagVariantForTests("--grpc-auth-mode")+"=mtls", - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--grpc-key"), key), - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--grpc-cert"), cert), - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--grpc-ca"), caCert), - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--vtctld-grpc-key"), clientKey), - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--vtctld-grpc-cert"), clientCert), - fmt.Sprintf("%s=%s", utils.GetFlagVariantForTests("--vtctld-grpc-ca"), caCert), + "--grpc-auth-mode=mtls", + fmt.Sprintf("%s=%s", "--grpc-key", key), + fmt.Sprintf("%s=%s", "--grpc-cert", cert), + fmt.Sprintf("%s=%s", "--grpc-ca", caCert), + fmt.Sprintf("%s=%s", "--vtctld-grpc-key", clientKey), + fmt.Sprintf("%s=%s", "--vtctld-grpc-cert", clientCert), + fmt.Sprintf("%s=%s", "--vtctld-grpc-ca", caCert), fmt.Sprintf("--grpc-auth-mtls-allowed-substrings=%s", "CN=ClientApp")) defer cluster.TearDown() @@ -376,8 +375,8 @@ var clusterKeyspaces = []string{ func startCluster(flags ...string) (cluster vttest.LocalCluster, err error) { args := []string{"vttestserver"} - schemaDirArg := utils.GetFlagVariantForTests("--schema-dir") + "=data/schema" - tabletHostname := fmt.Sprintf("%s=localhost", utils.GetFlagVariantForTests("--tablet-hostname")) + schemaDirArg := "--schema-dir=data/schema" + tabletHostname := fmt.Sprintf("%s=localhost", "--tablet-hostname") keyspaceArg := "--keyspaces=" + strings.Join(clusterKeyspaces, ",") numShardsArg := "--num-shards=2,2" vschemaDDLAuthorizedUsers := "--vschema-ddl-authorized-users=%" diff --git a/go/cmd/vttestserver/main.go b/go/cmd/vttestserver/main.go index 95e63fa8019..08a7d247784 100644 --- a/go/cmd/vttestserver/main.go +++ b/go/cmd/vttestserver/main.go @@ -19,10 +19,13 @@ package main import ( "vitess.io/vitess/go/cmd/vttestserver/cli" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { - if err := cli.New().Execute(); err != nil { + cmd := cli.New() + cmd.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) + if err := cmd.Execute(); err != nil { log.Fatal(err) } } diff --git a/go/cmd/vttlstest/vttlstest.go b/go/cmd/vttlstest/vttlstest.go index 8b98687c7a8..7360c287569 100644 --- a/go/cmd/vttlstest/vttlstest.go +++ b/go/cmd/vttlstest/vttlstest.go @@ -22,11 +22,13 @@ import ( "vitess.io/vitess/go/cmd/vttlstest/cli" "vitess.io/vitess/go/exit" "vitess.io/vitess/go/vt/logutil" + "vitess.io/vitess/go/vt/utils" ) func main() { defer exit.Recover() defer logutil.Flush() + cli.Root.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) cobra.CheckErr(cli.Root.Execute()) } diff --git a/go/cmd/zk/zkcmd.go b/go/cmd/zk/zkcmd.go index f0c39d6b0f8..e1fe11c65cd 100644 --- a/go/cmd/zk/zkcmd.go +++ b/go/cmd/zk/zkcmd.go @@ -24,6 +24,7 @@ import ( "vitess.io/vitess/go/cmd/zk/command" "vitess.io/vitess/go/exit" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { @@ -38,6 +39,7 @@ func main() { cancel() }() + command.Root.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) // Run the command. if err := command.Root.ExecuteContext(ctx); err != nil { log.Error(err) diff --git a/go/cmd/zkctl/zkctl.go b/go/cmd/zkctl/zkctl.go index b00e3eb4812..a86ba25db44 100644 --- a/go/cmd/zkctl/zkctl.go +++ b/go/cmd/zkctl/zkctl.go @@ -20,11 +20,13 @@ import ( "vitess.io/vitess/go/cmd/zkctl/command" "vitess.io/vitess/go/exit" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { defer exit.Recover() + command.Root.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := command.Root.Execute(); err != nil { log.Error(err) exit.Return(1) diff --git a/go/cmd/zkctld/zkctld.go b/go/cmd/zkctld/zkctld.go index 211b63325eb..cf745337c45 100644 --- a/go/cmd/zkctld/zkctld.go +++ b/go/cmd/zkctld/zkctld.go @@ -23,10 +23,12 @@ import ( "vitess.io/vitess/go/cmd/zkctld/cli" "vitess.io/vitess/go/exit" "vitess.io/vitess/go/vt/log" + "vitess.io/vitess/go/vt/utils" ) func main() { defer exit.Recover() + cli.Main.SetGlobalNormalizationFunc(utils.NormalizeUnderscoresToDashes) if err := cli.Main.Execute(); err != nil { log.Error(err) exit.Return(1) diff --git a/go/test/endtoend/vtorc/readtopologyinstance/main_test.go b/go/test/endtoend/vtorc/readtopologyinstance/main_test.go index 581f37c4684..6a783da3447 100644 --- a/go/test/endtoend/vtorc/readtopologyinstance/main_test.go +++ b/go/test/endtoend/vtorc/readtopologyinstance/main_test.go @@ -27,7 +27,6 @@ import ( topodatapb "vitess.io/vitess/go/vt/proto/topodata" "vitess.io/vitess/go/vt/servenv" - vtutils "vitess.io/vitess/go/vt/utils" "vitess.io/vitess/go/vt/vtorc/config" "vitess.io/vitess/go/vt/vtorc/inst" "vitess.io/vitess/go/vt/vtorc/logic" @@ -58,10 +57,6 @@ func TestReadTopologyInstanceBufferable(t *testing.T) { "--topo-global-root": clusterInfo.ClusterInstance.VtctldClientProcess.TopoGlobalRoot, } - vtutils.SetFlagVariantsForTests(args, "--topo-global-server-address", clusterInfo.ClusterInstance.VtctldClientProcess.TopoGlobalAddress) - vtutils.SetFlagVariantsForTests(args, "--topo-implementation", clusterInfo.ClusterInstance.VtctldClientProcess.TopoImplementation) - vtutils.SetFlagVariantsForTests(args, "--topo-global-root", clusterInfo.ClusterInstance.VtctldClientProcess.TopoGlobalRoot) - os.Args = []string{"vtorc"} for k, v := range args { os.Args = append(os.Args, k, v) diff --git a/go/vt/grpcclient/client_test.go b/go/vt/grpcclient/client_test.go index 336b4f16769..7b680252c0b 100644 --- a/go/vt/grpcclient/client_test.go +++ b/go/vt/grpcclient/client_test.go @@ -31,7 +31,6 @@ import ( vtgatepb "vitess.io/vitess/go/vt/proto/vtgate" vtgateservicepb "vitess.io/vitess/go/vt/proto/vtgateservice" - "vitess.io/vitess/go/vt/utils" ) func TestDialErrors(t *testing.T) { @@ -94,13 +93,14 @@ func TestRegisterGRPCClientFlags(t *testing.T) { require.Equal(t, "", credsFile) // Use SetFlagVariantsForTests to randomly pick dashed or underscored keys. - flagMap := map[string]string{} - utils.SetFlagVariantsForTests(flagMap, "--grpc-keepalive-time", "5s") - utils.SetFlagVariantsForTests(flagMap, "--grpc-keepalive-timeout", "5s") - utils.SetFlagVariantsForTests(flagMap, "--grpc-initial-conn-window-size", "10") - utils.SetFlagVariantsForTests(flagMap, "--grpc-initial-window-size", "10") - utils.SetFlagVariantsForTests(flagMap, "--grpc-compression", "not-snappy") - utils.SetFlagVariantsForTests(flagMap, "--grpc-auth-static-client-creds", "tempfile") + flagMap := map[string]string{ + "--grpc-keepalive-time": "5s", + "--grpc-keepalive-timeout": "5s", + "--grpc-initial-window-size": "10", + "--grpc-initial-conn-window-size": "10", + "--grpc-compression": "not-snappy", + "--grpc-auth-static-client-creds": "tempfile", + } // Test setting flags from command-line arguments args := []string{"test"} diff --git a/go/vt/utils/flags.go b/go/vt/utils/flags.go index a58dde3a36f..acc2b2ce812 100644 --- a/go/vt/utils/flags.go +++ b/go/vt/utils/flags.go @@ -19,6 +19,7 @@ package utils import ( "fmt" "math/rand/v2" + "os" "strings" "time" @@ -52,15 +53,11 @@ func flagVariants(name string) (underscored, dashed string) { func setFlagVar[T any](fs *pflag.FlagSet, p *T, name string, def T, usage string, setFunc func(fs *pflag.FlagSet, p *T, name string, def T, usage string)) { - underscored, dashed := flagVariants(name) - if name == underscored { + if strings.Contains(name, "_") { fmt.Printf("[WARNING] Please use flag names with dashes instead of underscores, preparing for deprecation of underscores in flag names") } - setFunc(fs, p, dashed, def, usage) - setFunc(fs, p, underscored, def, "") - _ = fs.MarkHidden(underscored) - _ = fs.MarkDeprecated(underscored, fmt.Sprintf("use %s instead", dashed)) + setFunc(fs, p, name, def, usage) } func SetFlagIntVar(fs *pflag.FlagSet, p *int, name string, def int, usage string) { @@ -111,14 +108,10 @@ func SetFlagFloat64Var(fs *pflag.FlagSet, p *float64, name string, def float64, // using both the dashed and underscored versions of the flag name. // The underscored version is hidden and marked as deprecated. func SetFlagVar(fs *pflag.FlagSet, value pflag.Value, name, usage string) { - underscored, dashed := flagVariants(name) - if name == underscored { + if strings.Contains(name, "_") { fmt.Printf("[WARNING] Please use flag names with dashes instead of underscores, preparing for deprecation of underscores in flag names") } - fs.Var(value, dashed, usage) - fs.Var(value, underscored, "") - _ = fs.MarkHidden(underscored) - _ = fs.MarkDeprecated(underscored, fmt.Sprintf("use %s instead", dashed)) + fs.Var(value, name, usage) } // SetFlagVariantsForTests randomly assigns either the underscored or dashed version of the flag name to the map. @@ -150,3 +143,30 @@ func GetFlagVariantForTestsByVersion(flagName string, majorVersion int) string { } return underscored } + +var ( + deprecationWarningsEmitted = make(map[string]bool) +) + +// Translate flag names from underscores to dashes and print a deprecation warning. +func NormalizeUnderscoresToDashes(f *pflag.FlagSet, name string) pflag.NormalizedName { + // `log_dir`, `log_link` and `log_backtrace_at` are exceptions because they are used by glog. + if name == "log_dir" || name == "log_link" || name == "log_backtrace_at" { + return pflag.NormalizedName(name) + } + + // We only want to normalize flags that purely use underscores. + if !strings.Contains(name, "_") || strings.Contains(name, "-") { + return pflag.NormalizedName(name) + } + + normalizedName := strings.ReplaceAll(name, "_", "-") + + // Only emit a warning if we haven't emitted one yet + if !deprecationWarningsEmitted[name] { + deprecationWarningsEmitted[name] = true + fmt.Fprintf(os.Stderr, "Flag --%s has been deprecated, use --%s instead \n", name, normalizedName) + } + + return pflag.NormalizedName(normalizedName) +} diff --git a/go/vt/utils/flags_test.go b/go/vt/utils/flags_test.go index 8d47065505c..030e2e47a5c 100644 --- a/go/vt/utils/flags_test.go +++ b/go/vt/utils/flags_test.go @@ -17,10 +17,11 @@ limitations under the License. package utils import ( - "fmt" "testing" "github.com/spf13/pflag" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestFlagVariants(t *testing.T) { @@ -51,51 +52,20 @@ func testFlagVar[T any](t *testing.T, name string, def T, usage string, setter f fs := pflag.NewFlagSet("test", pflag.ContinueOnError) var value T setter(fs, &value, name, def, usage) - underscored, dashed := flagVariants(name) // Verify the primary (dashed) flag. - fDashed := fs.Lookup(dashed) - if fDashed == nil { - t.Fatalf("Expected flag %q to be registered", dashed) - } else { - if fDashed.Usage != usage { - t.Errorf("Expected usage %q for flag %q, got %q", usage, dashed, fDashed.Usage) - } - if fDashed.Hidden { - t.Errorf("Flag %q should not be hidden", dashed) - } - } - - // Verify the (deprecated )alias (underscored) flag. - fUnderscored := fs.Lookup(underscored) - if fUnderscored == nil { - t.Fatalf("Expected flag %q to be registered", underscored) - } else { - if fUnderscored.Usage != "" { - t.Errorf("Expected empty usage for flag %q, got %q", underscored, fUnderscored.Usage) - } - if !fUnderscored.Hidden { - t.Errorf("Flag %q should be hidden", underscored) - } - expectedDep := fmt.Sprintf("use %s instead", dashed) - if fUnderscored.Deprecated != expectedDep { - t.Errorf("Expected deprecated message %q for flag %q, got %q", expectedDep, underscored, fUnderscored.Deprecated) - } - } -} - -func testFlagVars[T any](t *testing.T, name string, def T, usage string, setter func(fs *pflag.FlagSet, p *T, name string, def T, usage string)) { - underscored, dashed := flagVariants(name) - testFlagVar(t, dashed, def, usage, setter) - testFlagVar(t, underscored, def, "", setter) + flag := fs.Lookup(name) + require.NotNilf(t, flag, "Expected flag %q to be registered", name) + assert.Equal(t, usage, flag.Usage) + assert.Falsef(t, flag.Hidden, "Flag %q should not be hidden", name) } func TestSetFlagIntVar(t *testing.T) { - testFlagVars(t, "int-flag", 42, "an integer flag", SetFlagIntVar) + testFlagVar(t, "int-flag", 42, "an integer flag", SetFlagIntVar) } func TestSetFlagBoolVar(t *testing.T) { - testFlagVars(t, "bool-flag", true, "a boolean flag", SetFlagBoolVar) + testFlagVar(t, "bool-flag", true, "a boolean flag", SetFlagBoolVar) } func TestSetFlagVariantsForTests(t *testing.T) { diff --git a/go/vt/vtgate/buffer/flags_test.go b/go/vt/vtgate/buffer/flags_test.go index 488cf326860..cbed66cecac 100644 --- a/go/vt/vtgate/buffer/flags_test.go +++ b/go/vt/vtgate/buffer/flags_test.go @@ -21,8 +21,6 @@ import ( "testing" "github.com/spf13/pflag" - - "vitess.io/vitess/go/vt/utils" ) func TestVerifyFlags(t *testing.T) { @@ -42,16 +40,16 @@ func TestVerifyFlags(t *testing.T) { // Verify that the non-allowed (non-trivial) flag combinations are caught. defer resetFlagsForTesting() - parse([]string{utils.GetFlagVariantForTests("--buffer-keyspace-shards"), "ks1/0"}) + parse([]string{"--buffer-keyspace-shards", "ks1/0"}) if err := verifyFlags(); err == nil || !strings.Contains(err.Error(), "also requires that") { - t.Fatalf("List of shards requires --enable_buffer. err: %v", err) + t.Fatalf("List of shards requires --enable-buffer. err: %v", err) } resetFlagsForTesting() parse([]string{ - "--enable_buffer", - utils.GetFlagVariantForTests("--enable-buffer-dry-run"), + "--enable-buffer", + "--enable-buffer-dry-run", }) if err := verifyFlags(); err == nil || !strings.Contains(err.Error(), "To avoid ambiguity") { t.Fatalf("Dry-run and non-dry-run mode together require an explicit list of shards for actual buffering. err: %v", err) @@ -60,8 +58,8 @@ func TestVerifyFlags(t *testing.T) { resetFlagsForTesting() parse([]string{ - "--enable_buffer", - utils.GetFlagVariantForTests("--buffer-keyspace-shards"), "ks1//0", + "--enable-buffer", + "--buffer-keyspace-shards", "ks1//0", }) if err := verifyFlags(); err == nil || !strings.Contains(err.Error(), "invalid shard path") { t.Fatalf("Invalid shard names are not allowed. err: %v", err) @@ -70,8 +68,8 @@ func TestVerifyFlags(t *testing.T) { resetFlagsForTesting() parse([]string{ - "--enable_buffer", - utils.GetFlagVariantForTests("--buffer-keyspace-shards"), "ks1,ks1/0", + "--enable-buffer", + "--buffer-keyspace-shards", "ks1,ks1/0", }) if err := verifyFlags(); err == nil || !strings.Contains(err.Error(), "has overlapping entries") { t.Fatalf("Listed keyspaces and shards must not overlap. err: %v", err) diff --git a/go/vt/vtgate/grpcvtgateconn/conn_rpc_test.go b/go/vt/vtgate/grpcvtgateconn/conn_rpc_test.go index ab440edfd65..21a58e1cf5d 100644 --- a/go/vt/vtgate/grpcvtgateconn/conn_rpc_test.go +++ b/go/vt/vtgate/grpcvtgateconn/conn_rpc_test.go @@ -29,7 +29,6 @@ import ( "vitess.io/vitess/go/vt/grpcclient" "vitess.io/vitess/go/vt/servenv" - "vitess.io/vitess/go/vt/utils" "vitess.io/vitess/go/vt/vtgate/grpcvtgateservice" "vitess.io/vitess/go/vt/vtgate/vtgateconn" ) @@ -102,15 +101,14 @@ func TestGRPCVTGateConnAuth(t *testing.T) { grpcclient.RegisterFlags(fs) grpcclient.ResetStaticAuth() - authStaticClientCredsFlag := utils.GetFlagVariantForTests("--grpc-auth-static-client-creds") // Parse the flag using the chosen variant err = fs.Parse([]string{ - authStaticClientCredsFlag, + "--grpc-auth-static-client-creds", f.Name(), }) - require.NoError(t, err, "failed to set `%s=%s`", authStaticClientCredsFlag, f.Name()) + require.NoError(t, err, "failed to set `%s=%s`", "--grpc-auth-static-client-creds", f.Name()) client, err := dial(ctx, listener.Addr().String()) require.NoError(t, err) RegisterTestDialProtocol(client) @@ -140,13 +138,12 @@ func TestGRPCVTGateConnAuth(t *testing.T) { grpcclient.RegisterFlags(fs) grpcclient.ResetStaticAuth() - authStaticClientCredsFlag = utils.GetFlagVariantForTests("--grpc-auth-static-client-creds") err = fs.Parse([]string{ - authStaticClientCredsFlag, + "--grpc-auth-static-client-creds", f.Name(), }) - require.NoError(t, err, "failed to set `%s=%s`", authStaticClientCredsFlag, f.Name()) + require.NoError(t, err, "failed to set `%s=%s`", "--grpc-auth-static-client-creds", f.Name()) client, err = dial(ctx, listener.Addr().String()) if err != nil { diff --git a/go/vt/vttablet/tabletserver/tabletenv/config.go b/go/vt/vttablet/tabletserver/tabletenv/config.go index aadad63d860..db8409c1749 100644 --- a/go/vt/vttablet/tabletserver/tabletenv/config.go +++ b/go/vt/vttablet/tabletserver/tabletenv/config.go @@ -161,8 +161,8 @@ func registerTabletEnvFlags(fs *pflag.FlagSet) { utils.SetFlagBoolVar(fs, ¤tConfig.TrackSchemaVersions, "track-schema-versions", false, "When enabled, vttablet will store versions of schemas at each position that a DDL is applied and allow retrieval of the schema corresponding to a position") fs.Int64Var(¤tConfig.SchemaVersionMaxAgeSeconds, "schema-version-max-age-seconds", 0, "max age of schema version records to kept in memory by the vreplication historian") - _ = fs.Bool("twopc_enable", true, "TwoPC is enabled") - _ = fs.MarkDeprecated("twopc_enable", "TwoPC is always enabled, the transaction abandon age can be configured") + _ = fs.Bool("twopc-enable", true, "TwoPC is enabled") + _ = fs.MarkDeprecated("twopc-enable", "TwoPC is always enabled, the transaction abandon age can be configured") utils.SetFlagFloatDurationVar(fs, ¤tConfig.TwoPCAbandonAge, "twopc-abandon-age", defaultConfig.TwoPCAbandonAge, "Any unresolved transaction older than this time will be sent to the coordinator to be resolved. NOTE: Providing time as seconds (float64) is deprecated. Use time.Duration format (e.g., '1s', '2m', '1h').")