From f92af346dfa89fcc0860a7d8148239ee224c80cd Mon Sep 17 00:00:00 2001 From: Chunzhu Li Date: Mon, 14 Jun 2021 22:42:37 -0500 Subject: [PATCH] dmctl: fix dmctl command to improve its usability (#1750) --- cmd/dm-ctl/main.go | 220 +-------------------- dm/ctl/common/config.go | 130 ++++++------ dm/ctl/ctl.go | 202 ++++++++++++++----- dm/ctl/master/get_config.go | 20 +- tests/_utils/env_variables | 1 + tests/_utils/get_leader | 2 +- tests/_utils/run_dm_ctl_cmd_mode | 37 ++++ tests/_utils/run_dm_ctl_with_rematch | 2 +- tests/_utils/run_dm_ctl_with_retry | 2 +- tests/_utils/wait_until_sync | 2 +- tests/dmctl_basic/check_list/dmctl.sh | 4 +- tests/dmctl_basic/check_list/get_config.sh | 1 + tests/dmctl_basic/run.sh | 13 ++ tests/dmctl_command/run.sh | 8 +- tests/handle_error/lib.sh | 8 +- tests/handle_error_2/lib.sh | 8 +- tests/handle_error_3/lib.sh | 8 +- 17 files changed, 319 insertions(+), 349 deletions(-) create mode 100755 tests/_utils/run_dm_ctl_cmd_mode diff --git a/cmd/dm-ctl/main.go b/cmd/dm-ctl/main.go index 1547094b73..9f28e189a8 100644 --- a/cmd/dm-ctl/main.go +++ b/cmd/dm-ctl/main.go @@ -14,72 +14,18 @@ package main import ( - "context" - "flag" "fmt" - "io" "os" "os/signal" - "strings" "syscall" - "github.com/chzyer/readline" - "github.com/pingcap/errors" - "github.com/pingcap/dm/dm/ctl" "github.com/pingcap/dm/dm/ctl/common" "github.com/pingcap/dm/pkg/log" "github.com/pingcap/dm/pkg/terror" - "github.com/pingcap/dm/pkg/utils" ) -// output: -// Usage: dmctl [global options] command [command options] [arguments...] -// -// Available Commands: -// ... -// query-status query-status [-s source ...] [task-name] -// ... -// -// Special Commands: -// --encrypt encrypt plaintext to ciphertext -// ... -// -// Global Options: -// --V prints version and exit -// ... -func helpUsage(cfg *common.Config) { - fmt.Println("Usage: dmctl [global options] command [command options] [arguments...]") - fmt.Println() - ctl.PrintUsage() - fmt.Println() - fmt.Println("Special Commands:") - f := cfg.FlagSet.Lookup(common.EncryptCmdName) - fmt.Printf(" --%s %s\n", f.Name, f.Usage) - f = cfg.FlagSet.Lookup(common.DecryptCmdName) - fmt.Printf(" --%s %s\n", f.Name, f.Usage) - fmt.Println() - fmt.Println("Global Options:") - cfg.FlagSet.VisitAll(func(flag2 *flag.Flag) { - if flag2.Name == common.EncryptCmdName || flag2.Name == common.DecryptCmdName { - return - } - fmt.Printf(" --%s %s\n", flag2.Name, flag2.Usage) - }) -} - func main() { - cfg := common.NewConfig() - args := os.Args[1:] - - // no arguments: print help message about dmctl - if len(args) == 0 { - helpUsage(cfg) - os.Exit(0) - } - - args = aliasArgs(args) - // now, we use checker in dmctl while it using some pkg which log some thing when running // to make dmctl output more clear, simply redirect log to file rather output to stdout err := log.InitLogger(&log.Config{ @@ -91,100 +37,6 @@ func main() { os.Exit(2) } - // try to split one task operation from dmctl command - // because we allow user put task operation at last with two restrictions - // 1. one command one task operation - // 2. put task operation at last - cmdArgs := extractSubCommand(args) - lenArgs := len(args) - lenCmdArgs := len(cmdArgs) - if lenCmdArgs > 0 { - lenArgs -= lenCmdArgs - } - - finished, err := cfg.Parse(args[:lenArgs]) - if finished { - os.Exit(0) - } - - switch errors.Cause(err) { - case nil: - case flag.ErrHelp: - if lenCmdArgs > 0 { - // print help message about special subCommand - ctl.PrintHelp(cmdArgs) - } else { - // print help message about dmctl - helpUsage(cfg) - } - os.Exit(0) - default: - // NOTE: when `--help` in cmdArgs we need to print out the help msg. - for _, cmd := range cmdArgs { - if cmd == "--help" { - ctl.PrintHelp(cmdArgs) - os.Exit(0) - } - } - common.PrintLinesf("parse cmd flags err: %s", terror.Message(err)) - os.Exit(2) - } - - err = cfg.Validate() - if err != nil { - common.PrintLinesf("flags are not validate: %s", terror.Message(err)) - os.Exit(2) - } - - err = ctl.Init(cfg) - if err != nil { - common.PrintLinesf("%v", terror.Message(err)) - os.Exit(2) - } - if lenCmdArgs > 0 { - if err = commandMode(cmdArgs); err != nil { - os.Exit(2) - } - } else { - interactionMode() - } -} - -func extractSubCommand(args []string) []string { - collectedArgs := make([]string, 0, len(args)) - subCommand := make([]string, 0, len(args)) - for i := 0; i < len(args); i++ { - // check whether has multiple commands - if ctl.HasCommand(strings.ToLower(args[i])) { - subCommand = append(subCommand, args[i]) - } - } - if len(subCommand) == 0 { - return collectedArgs - } - if len(subCommand) > 1 { - fmt.Printf("command mode only support one command at a time, find %d:", len(subCommand)) - fmt.Println(subCommand) - os.Exit(1) - } - - for i := 0; i < len(args); i++ { - if ctl.HasCommand(strings.ToLower(args[i])) { - collectedArgs = append(collectedArgs, args[i:]...) - break - } - } - return collectedArgs -} - -func commandMode(args []string) error { - return ctl.Start(args) -} - -func interactionMode() { - utils.PrintInfo2("dmctl") - fmt.Println() // print a separater - sc := make(chan os.Signal, 1) signal.Notify(sc, syscall.SIGHUP, @@ -194,7 +46,7 @@ func interactionMode() { go func() { sig := <-sc - fmt.Printf("got signal [%v] to exit", sig) + fmt.Printf("\nGot signal [%v] to exit.\n", sig) switch sig { case syscall.SIGTERM: os.Exit(0) @@ -203,73 +55,5 @@ func interactionMode() { } }() - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - go common.SyncMasterEndpoints(ctx) - - loop() - - fmt.Println("dmctl exit") -} - -func loop() { - l, err := readline.NewEx(&readline.Config{ - Prompt: "\033[31m»\033[0m ", - HistoryFile: "/tmp/dmctlreadline.tmp", - InterruptPrompt: "^C", - EOFPrompt: "^D", - }) - if err != nil { - panic(err) - } - - for { - line, err := l.Readline() - if err != nil { - if err == readline.ErrInterrupt { - break - } else if err == io.EOF { - break - } - continue - } - - line = strings.TrimSpace(line) - if line == "exit" { - l.Close() - os.Exit(0) - } else if line == "" { - continue - } - - args := strings.Fields(line) - args = aliasArgs(args) - err = ctl.Start(args) - if err != nil { - fmt.Println("fail to run:", args) - } - - syncErr := log.L().Sync() - if syncErr != nil { - fmt.Fprintln(os.Stderr, "sync log failed", syncErr) - } - } - l.Close() -} - -func aliasArgs(args []string) []string { - args = aliasGetTaskCfgCmd(args) - return args -} - -func aliasGetTaskCfgCmd(args []string) []string { - for i, arg := range args { - if arg == "get-task-config" { - args = append(args[:i+1], args[i:]...) - args[i] = "get-config" - args[i+1] = "task" - return args - } - } - return args + ctl.MainStart(common.AdjustArgumentsForPflags(os.Args[1:])) } diff --git a/dm/ctl/common/config.go b/dm/ctl/common/config.go index 4b47b1fb57..5b1783cc17 100644 --- a/dm/ctl/common/config.go +++ b/dm/ctl/common/config.go @@ -15,7 +15,6 @@ package common import ( "encoding/json" - "flag" "fmt" "net" "os" @@ -27,6 +26,7 @@ import ( "github.com/BurntSushi/toml" "github.com/pingcap/errors" + "github.com/spf13/pflag" ) const ( @@ -53,31 +53,74 @@ const ( DefaultWarnCnt = 10 ) +var argsNeedAdjust = [...]string{"-version", "-config", "-master-addr", "-rpc-timeout", "-ssl-ca", "-ssl-cert", "-ssl-key", "-" + EncryptCmdName, "-" + DecryptCmdName} + // NewConfig creates a new base config for dmctl. -func NewConfig() *Config { +func NewConfig(fs *pflag.FlagSet) *Config { cfg := &Config{} - cfg.FlagSet = flag.NewFlagSet("dmctl", flag.ContinueOnError) - - // ignore default help usage - cfg.FlagSet.Usage = func() {} - fs := cfg.FlagSet - - fs.BoolVar(&cfg.printVersion, "V", false, "Prints version and exit.") - fs.StringVar(&cfg.ConfigFile, "config", "", "Path to config file.") - fs.StringVar(&cfg.MasterAddr, "master-addr", "", "Master API server address, this parameter is required when interacting with the dm-master") - fs.StringVar(&cfg.RPCTimeoutStr, "rpc-timeout", defaultRPCTimeout, fmt.Sprintf("RPC timeout, default is %s.", defaultRPCTimeout)) - fs.StringVar(&cfg.encrypt, EncryptCmdName, "", "Encrypts plaintext to ciphertext.") - fs.StringVar(&cfg.SSLCA, "ssl-ca", "", "Path of file that contains list of trusted SSL CAs for connection.") - fs.StringVar(&cfg.SSLCert, "ssl-cert", "", "Path of file that contains X509 certificate in PEM format for connection.") - fs.StringVar(&cfg.SSLKey, "ssl-key", "", "Path of file that contains X509 key in PEM format for connection.") - fs.StringVar(&cfg.decrypt, DecryptCmdName, "", "Decrypts ciphertext to plaintext.") - + cfg.FlagSet = fs return cfg } +// DefineConfigFlagSet defines flag definitions for configs. +func DefineConfigFlagSet(fs *pflag.FlagSet) { + fs.BoolP("version", "V", false, "Prints version and exit.") + fs.String("config", "", "Path to config file.") + fs.String("master-addr", "", "Master API server address, this parameter is required when interacting with the dm-master") + fs.String("rpc-timeout", defaultRPCTimeout, fmt.Sprintf("RPC timeout, default is %s.", defaultRPCTimeout)) + fs.String("ssl-ca", "", "Path of file that contains list of trusted SSL CAs for connection.") + fs.String("ssl-cert", "", "Path of file that contains X509 certificate in PEM format for connection.") + fs.String("ssl-key", "", "Path of file that contains X509 key in PEM format for connection.") + fs.String(EncryptCmdName, "", "Encrypts plaintext to ciphertext.") + fs.String(DecryptCmdName, "", "Decrypts ciphertext to plaintext.") + _ = fs.MarkHidden(EncryptCmdName) + _ = fs.MarkHidden(DecryptCmdName) +} + +// AdjustArgumentsForPflags adjust flag format args to pflags format. +func AdjustArgumentsForPflags(args []string) []string { + for i, arg := range args { + arg = strings.TrimSpace(arg) + for _, adjustArg := range argsNeedAdjust { + // -master-addr 127.0.0.1:8261 and -master-addr=127.0.0.1:8261 + if arg == adjustArg || strings.HasPrefix(arg, adjustArg+"=") { + args[i] = "-" + arg + } + } + } + return args +} + +func (c *Config) getConfigFromFlagSet() error { + var err error + fs := c.FlagSet + c.ConfigFile, err = fs.GetString("config") + if err != nil { + return err + } + c.MasterAddr, err = fs.GetString("master-addr") + if err != nil { + return err + } + c.RPCTimeoutStr, err = fs.GetString("rpc-timeout") + if err != nil { + return err + } + c.SSLCA, err = fs.GetString("ssl-ca") + if err != nil { + return err + } + c.SSLCert, err = fs.GetString("ssl-cert") + if err != nil { + return err + } + c.SSLKey, err = fs.GetString("ssl-key") + return err +} + // Config is the configuration. type Config struct { - *flag.FlagSet `json:"-"` + *pflag.FlagSet `json:"-"` MasterAddr string `toml:"master-addr" json:"master-addr"` @@ -86,11 +129,7 @@ type Config struct { ConfigFile string `json:"config-file"` - printVersion bool - encrypt string // string need to be encrypted - config.Security - decrypt string // string need to be decrypted } func (c *Config) String() string { @@ -102,52 +141,25 @@ func (c *Config) String() string { return string(cfg) } -// Parse parses flag definitions from the argument list. -func (c *Config) Parse(arguments []string) (finish bool, err error) { - err = c.FlagSet.Parse(arguments) +// Adjust parses flag definitions from the argument list. +func (c *Config) Adjust() error { + err := c.getConfigFromFlagSet() if err != nil { - return false, errors.Trace(err) - } - - if c.printVersion { - fmt.Println(utils.GetRawInfo()) - return true, nil - } - - if len(c.encrypt) > 0 { - ciphertext, err1 := utils.Encrypt(c.encrypt) - if err1 != nil { - return true, err1 - } - fmt.Println(ciphertext) - return true, nil - } - - if len(c.decrypt) > 0 { - plaintext, err1 := utils.Decrypt(c.decrypt) - if err1 != nil { - return true, err1 - } - fmt.Println(plaintext) - return true, nil + return errors.Trace(err) } // Load config file if specified. if c.ConfigFile != "" { err = c.configFromFile(c.ConfigFile) if err != nil { - return false, errors.Trace(err) + return errors.Trace(err) } } // Parse again to replace with command line options. - err = c.FlagSet.Parse(arguments) + err = c.getConfigFromFlagSet() if err != nil { - return false, errors.Trace(err) - } - - if len(c.FlagSet.Args()) != 0 { - return false, errors.Errorf("'%s' is an invalid flag", c.FlagSet.Arg(0)) + return errors.Trace(err) } // try get master Addr from env "DM_MASTER_ADDR" if this flag is empty. @@ -155,10 +167,10 @@ func (c *Config) Parse(arguments []string) (finish bool, err error) { c.MasterAddr = os.Getenv("DM_MASTER_ADDR") } if c.MasterAddr == "" { - return false, errors.Errorf("--master-addr not provided, this parameter is required when interacting with the dm-master, you can also use environment variable 'DM_MASTER_ADDR' to specify the value. Use `dmtcl --help` to see more help messages") + return errors.Errorf("--master-addr not provided, this parameter is required when interacting with the dm-master, you can also use environment variable 'DM_MASTER_ADDR' to specify the value. Use `dmtcl --help` to see more help messages") } - return false, errors.Trace(c.adjust()) + return errors.Trace(c.adjust()) } // Validate check config is ready to execute command. diff --git a/dm/ctl/ctl.go b/dm/ctl/ctl.go index 9ffb0338b8..70d1474af9 100644 --- a/dm/ctl/ctl.go +++ b/dm/ctl/ctl.go @@ -15,21 +15,22 @@ package ctl import ( "fmt" + "io" "os" + "strings" "github.com/pingcap/dm/dm/ctl/common" "github.com/pingcap/dm/dm/ctl/master" "github.com/pingcap/dm/pkg/log" + "github.com/pingcap/dm/pkg/utils" + "github.com/chzyer/readline" "github.com/pingcap/errors" "github.com/spf13/cobra" "go.uber.org/zap/zapcore" ) -var ( - commandMasterFlags = CommandMasterFlags{} - rootCmd *cobra.Command -) +var commandMasterFlags = CommandMasterFlags{} // CommandMasterFlags are flags that used in all commands for dm-master. type CommandMasterFlags struct { @@ -41,16 +42,13 @@ func (c CommandMasterFlags) Reset() { c.workers = c.workers[:0] } -func init() { - rootCmd = NewRootCmd() -} - // NewRootCmd generates a new rootCmd. func NewRootCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "dmctl", - Short: "DM control", - SilenceUsage: true, + Use: "dmctl", + Short: "DM control", + SilenceUsage: true, + SilenceErrors: true, } // --worker worker1 -w worker2 --worker=worker3,worker4 -w=worker5,worker6 cmd.PersistentFlags().StringSliceVarP(&commandMasterFlags.workers, "source", "s", []string{}, "MySQL Source ID.") @@ -77,6 +75,8 @@ func NewRootCmd() *cobra.Command { master.NewTransferSourceCmd(), master.NewStartRelayCmd(), master.NewStopRelayCmd(), + newDecryptCmd(), + newEncryptCmd(), ) // copied from (*cobra.Command).InitDefaultHelpCmd helpCmd := &cobra.Command{ @@ -108,48 +108,162 @@ func Init(cfg *common.Config) error { return errors.Trace(common.InitUtils(cfg)) } -// PrintUsage prints usage. -func PrintUsage() { - maxCmdLen := 0 - for _, cmd := range rootCmd.Commands() { - if maxCmdLen < len(cmd.Name()) { - maxCmdLen = len(cmd.Name()) - } +// Start starts running a command. +func Start(args []string) (cmd *cobra.Command, err error) { + commandMasterFlags.Reset() + rootCmd := NewRootCmd() + rootCmd.SetArgs(args) + return rootCmd.ExecuteC() +} + +func loop() error { + l, err := readline.NewEx(&readline.Config{ + Prompt: "\033[31m»\033[0m ", + HistoryFile: "/tmp/dmctlreadline.tmp", + InterruptPrompt: "^C", + EOFPrompt: "^D", + }) + if err != nil { + return err } - fmt.Println("Available Commands:") - for _, cmd := range rootCmd.Commands() { - format := fmt.Sprintf(" %%-%ds\t%%s\n", maxCmdLen) - fmt.Printf(format, cmd.Name(), cmd.Use) + + for { + line, err := l.Readline() + if err != nil { + if err == readline.ErrInterrupt { + break + } else if err == io.EOF { + break + } + continue + } + + line = strings.TrimSpace(line) + if line == "exit" { + l.Close() + os.Exit(0) + } else if line == "" { + continue + } + + args := strings.Fields(line) + c, err := Start(args) + if err != nil { + fmt.Println("fail to run:", args) + fmt.Println("Error:", err) + if c.CalledAs() == "" { + fmt.Printf("Run '%v --help' for usage.\n", c.CommandPath()) + } + } + + syncErr := log.L().Sync() + if syncErr != nil { + fmt.Fprintln(os.Stderr, "sync log failed", syncErr) + } } + return l.Close() } -// HasCommand represent whether rootCmd has this command. -func HasCommand(name string) bool { - for _, cmd := range rootCmd.Commands() { - if name == cmd.Name() { - return true +// MainStart starts running a command. +func MainStart(args []string) { + rootCmd := NewRootCmd() + rootCmd.RunE = func(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return loop() } + return cmd.Help() + } + + rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + if printVersion, err := cmd.Flags().GetBool("version"); err != nil { + return errors.Trace(err) + } else if printVersion { + cmd.Println(utils.GetRawInfo()) + os.Exit(0) + } + + // Make it compatible to flags encrypt/decrypt + if encrypt, err := cmd.Flags().GetString(common.EncryptCmdName); err != nil { + return errors.Trace(err) + } else if encrypt != "" { + ciphertext, err := utils.Encrypt(encrypt) + if err != nil { + return errors.Trace(err) + } + fmt.Println(ciphertext) + os.Exit(0) + } + if decrypt, err := cmd.Flags().GetString(common.DecryptCmdName); err != nil { + return errors.Trace(err) + } else if decrypt != "" { + plaintext, err := utils.Decrypt(decrypt) + if err != nil { + return errors.Trace(err) + } + fmt.Println(plaintext) + os.Exit(0) + } + + if cmd.Name() == common.DecryptCmdName || cmd.Name() == common.EncryptCmdName { + return nil + } + + cfg := common.NewConfig(cmd.Flags()) + err := cfg.Adjust() + if err != nil { + return err + } + + err = cfg.Validate() + if err != nil { + return err + } + + return Init(cfg) + } + common.DefineConfigFlagSet(rootCmd.PersistentFlags()) + rootCmd.SetArgs(args) + if c, err := rootCmd.ExecuteC(); err != nil { + rootCmd.Println("Error:", err) + if c.CalledAs() == "" { + rootCmd.Printf("Run '%v --help' for usage.\n", c.CommandPath()) + } + os.Exit(1) } - return false } -// PrintHelp print help message for special subCommand. -func PrintHelp(args []string) { - cmd, _, err := rootCmd.Find(args) - if err != nil { - fmt.Println(err) - rootCmd.SetOut(os.Stdout) - common.PrintCmdUsage(rootCmd) - return +func newEncryptCmd() *cobra.Command { + return &cobra.Command{ + Use: "encrypt ", + Short: "Encrypts plain text to cipher text.", + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) != 1 { + return cmd.Help() + } + ciphertext, err := utils.Encrypt(args[0]) + if err != nil { + return errors.Trace(err) + } + fmt.Println(ciphertext) + return nil + }, } - cmd.SetOut(os.Stdout) - common.PrintCmdUsage(cmd) } -// Start starts running a command. -func Start(args []string) (err error) { - commandMasterFlags.Reset() - rootCmd = NewRootCmd() - rootCmd.SetArgs(args) - return rootCmd.Execute() +func newDecryptCmd() *cobra.Command { + return &cobra.Command{ + Use: "decrypt ", + Short: "Decrypts cipher text to plain text", + RunE: func(cmd *cobra.Command, args []string) error { + if len(args) != 1 { + return cmd.Help() + } + plaintext, err := utils.Decrypt(args[0]) + if err != nil { + return errors.Trace(err) + } + fmt.Println(plaintext) + return nil + }, + } } diff --git a/dm/ctl/master/get_config.go b/dm/ctl/master/get_config.go index b6d11862c4..c95abc7caa 100644 --- a/dm/ctl/master/get_config.go +++ b/dm/ctl/master/get_config.go @@ -26,12 +26,15 @@ import ( "github.com/pingcap/dm/dm/pb" ) +const cmdGetTaskConfig = "get-task-config" + // NewGetCfgCmd creates a getCfg command. func NewGetCfgCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "get-config [--file filename]", - Short: "Gets the configuration.", - RunE: getCfgFunc, + Use: "get-config [--file filename]", + Short: "Gets the configuration.", + RunE: getCfgFunc, + Aliases: []string{cmdGetTaskConfig}, } cmd.Flags().StringP("file", "f", "", "write config to file") return cmd @@ -53,21 +56,24 @@ func convertCfgType(t string) pb.CfgType { } // getCfgFunc gets config. -func getCfgFunc(cmd *cobra.Command, _ []string) error { - if len(cmd.Flags().Args()) != 2 { +func getCfgFunc(cmd *cobra.Command, args []string) error { + if cmd.CalledAs() == cmdGetTaskConfig { + args = append([]string{"task"}, args...) + } + if len(args) != 2 { cmd.SetOut(os.Stdout) common.PrintCmdUsage(cmd) return errors.New("please check output to see error") } - cfgType := cmd.Flags().Arg(0) + cfgType := args[0] tp := convertCfgType(cfgType) if tp == pb.CfgType_InvalidType { common.PrintLinesf("invalid config type '%s'", cfgType) return errors.New("please check output to see error") } - cfgName := cmd.Flags().Arg(1) + cfgName := args[1] filename, err := cmd.Flags().GetString("file") if err != nil { common.PrintLinesf("can not get filename") diff --git a/tests/_utils/env_variables b/tests/_utils/env_variables index efcef55b25..255b76917f 100755 --- a/tests/_utils/env_variables +++ b/tests/_utils/env_variables @@ -8,6 +8,7 @@ TIDB_PASSWORD=${TIDB_PASSWORD:-123456} TIDB_PORT=${TIDB_PORT:-4000} MASTER_PORT=8261 +MASTER_PEER_PORT=8291 MASTER_PORT1=8261 MASTER_PORT2=8361 MASTER_PORT3=8461 diff --git a/tests/_utils/get_leader b/tests/_utils/get_leader index c67e8e83e5..325c6b0b82 100755 --- a/tests/_utils/get_leader +++ b/tests/_utils/get_leader @@ -15,7 +15,7 @@ pid=$$ leader_name="" for ((k = 0; k < 30; k++)); do - leader_name=$($binary -test.coverprofile="$TEST_DIR/cov.$TEST_NAME.dmctl.$ts.$pid.out" DEVEL -master-addr=$master_addr list-member --leader | grep name | gawk 'match($2,/"(.*)",/,a) {print a[1]}') + leader_name=$($binary -test.coverprofile="$TEST_DIR/cov.$TEST_NAME.dmctl.$ts.$pid.out" DEVEL --master-addr=$master_addr list-member --leader | grep name | gawk 'match($2,/"(.*)",/,a) {print a[1]}') if [[ ! -z "$leader_name" ]]; then echo $leader_name break diff --git a/tests/_utils/run_dm_ctl_cmd_mode b/tests/_utils/run_dm_ctl_cmd_mode new file mode 100755 index 0000000000..dd89820e27 --- /dev/null +++ b/tests/_utils/run_dm_ctl_cmd_mode @@ -0,0 +1,37 @@ +#!/bin/bash +# tools to run dmctl from command line +# parameter 1: work directory +# parameter 2: master-addr port +# parameter 3: command +# parameter 4...: check output content and count + +workdir=$1 +master_addr=$2 +cmd=$3 + +shift 3 + +PWD=$(pwd) +binary=$PWD/bin/dmctl.test +ts=$(date +"%s") +dmctl_log=$workdir/dmctl.$ts.log +pid=$$ +echo "dmctl test cmd: \"$cmd\"" +$binary -test.coverprofile="$TEST_DIR/cov.$TEST_NAME.dmctl.$ts.$pid.out" DEVEL $cmd --master-addr=$master_addr >$dmctl_log 2>&1 + +for ((i = 1; i < $#; i += 2)); do + j=$((i + 1)) + value=${!i} + expected=${!j} + got=$(sed "s/$value/$value\n/g" $dmctl_log | grep -c "$value") + if [ "$got" != "$expected" ]; then + echo "command: $cmd $value count: $got != expected: $expected" + cat $dmctl_log + exit 1 + fi +done + +# gocovmerge doesn't support merge profiles with different modes, however atomic +# mode and count mode have the same profile format, so we need to unify cover +# mode before running gocovmerge. As coverage file is not generated synchronously, +# we will patch covermode before `make coverage` diff --git a/tests/_utils/run_dm_ctl_with_rematch b/tests/_utils/run_dm_ctl_with_rematch index 4c28002a45..b1eff2377c 100755 --- a/tests/_utils/run_dm_ctl_with_rematch +++ b/tests/_utils/run_dm_ctl_with_rematch @@ -21,7 +21,7 @@ echo "dmctl test cmd: \"$cmd\"" all_matched=true for ((k = 0; k < 10; k++)); do - echo "$cmd" | $binary -test.coverprofile="$TEST_DIR/cov.$TEST_NAME.dmctl.$ts.$pid.out" DEVEL -master-addr=$master_addr >$dmctl_log 2>&1 + echo "$cmd" | $binary -test.coverprofile="$TEST_DIR/cov.$TEST_NAME.dmctl.$ts.$pid.out" DEVEL --master-addr=$master_addr >$dmctl_log 2>&1 all_matched=true for ((i = 1; i < $#; i += 2)); do j=$((i + 1)) diff --git a/tests/_utils/run_dm_ctl_with_retry b/tests/_utils/run_dm_ctl_with_retry index 572367ea80..9f7d811325 100755 --- a/tests/_utils/run_dm_ctl_with_retry +++ b/tests/_utils/run_dm_ctl_with_retry @@ -20,7 +20,7 @@ pid=$$ all_matched=true echo "dmctl test cmd: \"$cmd\"" for ((k = 0; k < 10; k++)); do - echo "$cmd" | $binary -test.coverprofile="$TEST_DIR/cov.$TEST_NAME.dmctl.$ts.$pid.out" DEVEL -master-addr=$master_addr >$dmctl_log 2>&1 + echo "$cmd" | $binary -test.coverprofile="$TEST_DIR/cov.$TEST_NAME.dmctl.$ts.$pid.out" DEVEL --master-addr=$master_addr >$dmctl_log 2>&1 all_matched=true for ((i = 1; i < $#; i += 2)); do j=$((i + 1)) diff --git a/tests/_utils/wait_until_sync b/tests/_utils/wait_until_sync index 85dadf22ba..ed7db7a041 100755 --- a/tests/_utils/wait_until_sync +++ b/tests/_utils/wait_until_sync @@ -15,7 +15,7 @@ pid=$$ while true; do sleep 5 - echo "$cmd" | $binary -test.coverprofile="$TEST_DIR/cov.$TEST_NAME.dmctl.$ts.$pid.out" DEVEL -master-addr=$master_addr >$dmctl_log 2>&1 + echo "$cmd" | $binary -test.coverprofile="$TEST_DIR/cov.$TEST_NAME.dmctl.$ts.$pid.out" DEVEL --master-addr=$master_addr >$dmctl_log 2>&1 value="\"stage\": \"Paused\"" # below grep will print the count of $value, exit when not 0 diff --git a/tests/dmctl_basic/check_list/dmctl.sh b/tests/dmctl_basic/check_list/dmctl.sh index fd48c7e96b..1f335a2583 100644 --- a/tests/dmctl_basic/check_list/dmctl.sh +++ b/tests/dmctl_basic/check_list/dmctl.sh @@ -19,5 +19,7 @@ function dmctl_wrong_addrs() { } function dmctl_no_addr() { - $PWD/bin/dmctl.test list-member | grep 'master-addr not provided' + run_dm_ctl_cmd_mode $WORK_DIR "" \ + "list-member" \ + "master-addr not provided" 1 } diff --git a/tests/dmctl_basic/check_list/get_config.sh b/tests/dmctl_basic/check_list/get_config.sh index 98088cd03e..a5b64002a0 100644 --- a/tests/dmctl_basic/check_list/get_config.sh +++ b/tests/dmctl_basic/check_list/get_config.sh @@ -101,6 +101,7 @@ function get_config_to_file() { # restart master with get config ps aux | grep dm-master | awk '{print $2}' | xargs kill || true check_port_offline $MASTER_PORT1 20 + check_port_offline $MASTER_PEER_PORT 20 run_dm_master $WORK_DIR/master $MASTER_PORT $dm_master_conf check_rpc_alive $cur/../bin/check_master_online 127.0.0.1:$MASTER_PORT diff --git a/tests/dmctl_basic/run.sh b/tests/dmctl_basic/run.sh index 455b1c9e42..2209fd00dd 100755 --- a/tests/dmctl_basic/run.sh +++ b/tests/dmctl_basic/run.sh @@ -124,6 +124,9 @@ function run() { run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" "any command" \ "can't connect to 127.0.0.1:8261" 1 \ "Please check your network connection\." 1 + run_dm_ctl_cmd_mode $WORK_DIR "127.0.0.1:$MASTER_PORT" "any command" \ + "unknown command \"any\" for \"dmctl\"" 1 \ + "Run 'dmctl --help' for usage\." 1 run_dm_master $WORK_DIR/master $MASTER_PORT $dm_master_conf check_rpc_alive $cur/../bin/check_master_online 127.0.0.1:$MASTER_PORT @@ -207,6 +210,13 @@ function run() { "\"source\": \"$SOURCE_ID1\"" 1 \ "\"source\": \"$SOURCE_ID2\"" 1 \ "\"stage\": \"Running\"" 4 + # test whether put --master-addr to the end works + run_dm_ctl_cmd_mode $WORK_DIR "127.0.0.1:$MASTER_PORT" \ + "query-status -s $SOURCE_ID1,$SOURCE_ID2" \ + "\"result\": true" 3 \ + "\"source\": \"$SOURCE_ID1\"" 1 \ + "\"source\": \"$SOURCE_ID2\"" 1 \ + "\"stage\": \"Running\"" 4 # update_task_not_paused $TASK_CONF # stop relay because get_config_to_file will stop source @@ -221,6 +231,9 @@ function run() { run_dm_ctl_with_retry $WORK_DIR "127.0.0.1:$MASTER_PORT" \ "query-status test" \ "\"stage\": \"Running\"" 2 + run_dm_ctl_cmd_mode $WORK_DIR "127.0.0.1:$MASTER_PORT" \ + "query-status test" \ + "\"stage\": \"Running\"" 2 echo "show_ddl_locks_no_locks" show_ddl_locks_no_locks $TASK_NAME diff --git a/tests/dmctl_command/run.sh b/tests/dmctl_command/run.sh index 8eb4abb349..e77934263f 100644 --- a/tests/dmctl_command/run.sh +++ b/tests/dmctl_command/run.sh @@ -6,7 +6,7 @@ cur=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) source $cur/../_utils/test_prepare WORK_DIR=$TEST_DIR/$TEST_NAME -help_cnt=37 +help_cnt=46 function run() { # check dmctl output with help flag @@ -33,15 +33,15 @@ function run() { fi # check dmctl command start-task output with master-addr and unknown flag - # it should print parse cmd flags err: 'xxxx' is an invalid flag% + # it should print unknown command xxxx $PWD/bin/dmctl.test DEVEL --master-addr=:$MASTER_PORT xxxx start-task >$WORK_DIR/help.log 2>&1 && exit 1 || echo "exit code should be not zero" help_msg=$(cat $WORK_DIR/help.log) help_msg_cnt=$(echo "${help_msg}" | wc -l | xargs) - if [ "$help_msg_cnt" != 1 ]; then + if [ "$help_msg_cnt" -lt 1 ]; then echo "dmctl case 3 help failed: $help_msg" exit 1 fi - echo $help_msg | grep -q "parse cmd flags err: 'xxxx' is an invalid flag" + echo $help_msg | grep -q "unknown command \"xxxx\" for \"dmctl\"" if [ $? -ne 0 ]; then echo "dmctl case 3 help failed: $help_msg" exit 1 diff --git a/tests/handle_error/lib.sh b/tests/handle_error/lib.sh index 9ee9622bd9..06a217bc17 100644 --- a/tests/handle_error/lib.sh +++ b/tests/handle_error/lib.sh @@ -45,7 +45,7 @@ function clean_table() { } function get_start_location() { - location=$($PWD/bin/dmctl.test DEVEL -master-addr=$1 \ + location=$($PWD/bin/dmctl.test DEVEL --master-addr=$1 \ query-status test -s $2 | grep Location | gawk 'match($0,/startLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\], endLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\]/,a) {printf "%s:%s", a[1], a[2]}') @@ -53,7 +53,7 @@ function get_start_location() { } function get_end_location() { - location=$($PWD/bin/dmctl.test DEVEL -master-addr=$1 \ + location=$($PWD/bin/dmctl.test DEVEL --master-addr=$1 \ query-status test -s $2 | grep Location | gawk 'match($0,/startLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\], endLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\]/,a) {printf "%s:%s", a[4], a[5]}') @@ -61,7 +61,7 @@ function get_end_location() { } function get_start_pos() { - pos=$($PWD/bin/dmctl.test DEVEL -master-addr=$1 \ + pos=$($PWD/bin/dmctl.test DEVEL --master-addr=$1 \ query-status test -s $2 | grep Location | gawk 'match($0,/startLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\], endLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\]/,a) {print a[2]}') @@ -69,7 +69,7 @@ function get_start_pos() { } function get_start_name() { - pos=$($PWD/bin/dmctl.test DEVEL -master-addr=$1 \ + pos=$($PWD/bin/dmctl.test DEVEL --master-addr=$1 \ query-status test -s $2 | grep Location | gawk 'match($0,/startLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\], endLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\]/,a) {print a[1]}') diff --git a/tests/handle_error_2/lib.sh b/tests/handle_error_2/lib.sh index 9ee9622bd9..06a217bc17 100644 --- a/tests/handle_error_2/lib.sh +++ b/tests/handle_error_2/lib.sh @@ -45,7 +45,7 @@ function clean_table() { } function get_start_location() { - location=$($PWD/bin/dmctl.test DEVEL -master-addr=$1 \ + location=$($PWD/bin/dmctl.test DEVEL --master-addr=$1 \ query-status test -s $2 | grep Location | gawk 'match($0,/startLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\], endLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\]/,a) {printf "%s:%s", a[1], a[2]}') @@ -53,7 +53,7 @@ function get_start_location() { } function get_end_location() { - location=$($PWD/bin/dmctl.test DEVEL -master-addr=$1 \ + location=$($PWD/bin/dmctl.test DEVEL --master-addr=$1 \ query-status test -s $2 | grep Location | gawk 'match($0,/startLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\], endLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\]/,a) {printf "%s:%s", a[4], a[5]}') @@ -61,7 +61,7 @@ function get_end_location() { } function get_start_pos() { - pos=$($PWD/bin/dmctl.test DEVEL -master-addr=$1 \ + pos=$($PWD/bin/dmctl.test DEVEL --master-addr=$1 \ query-status test -s $2 | grep Location | gawk 'match($0,/startLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\], endLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\]/,a) {print a[2]}') @@ -69,7 +69,7 @@ function get_start_pos() { } function get_start_name() { - pos=$($PWD/bin/dmctl.test DEVEL -master-addr=$1 \ + pos=$($PWD/bin/dmctl.test DEVEL --master-addr=$1 \ query-status test -s $2 | grep Location | gawk 'match($0,/startLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\], endLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\]/,a) {print a[1]}') diff --git a/tests/handle_error_3/lib.sh b/tests/handle_error_3/lib.sh index 9ee9622bd9..77c4b94195 100644 --- a/tests/handle_error_3/lib.sh +++ b/tests/handle_error_3/lib.sh @@ -45,7 +45,7 @@ function clean_table() { } function get_start_location() { - location=$($PWD/bin/dmctl.test DEVEL -master-addr=$1 \ + location=$($PWD/bin/dmctl.test DEVEL -master-addr $1 \ query-status test -s $2 | grep Location | gawk 'match($0,/startLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\], endLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\]/,a) {printf "%s:%s", a[1], a[2]}') @@ -53,7 +53,7 @@ function get_start_location() { } function get_end_location() { - location=$($PWD/bin/dmctl.test DEVEL -master-addr=$1 \ + location=$($PWD/bin/dmctl.test DEVEL --master-addr=$1 \ query-status test -s $2 | grep Location | gawk 'match($0,/startLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\], endLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\]/,a) {printf "%s:%s", a[4], a[5]}') @@ -61,7 +61,7 @@ function get_end_location() { } function get_start_pos() { - pos=$($PWD/bin/dmctl.test DEVEL -master-addr=$1 \ + pos=$($PWD/bin/dmctl.test DEVEL --master-addr=$1 \ query-status test -s $2 | grep Location | gawk 'match($0,/startLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\], endLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\]/,a) {print a[2]}') @@ -69,7 +69,7 @@ function get_start_pos() { } function get_start_name() { - pos=$($PWD/bin/dmctl.test DEVEL -master-addr=$1 \ + pos=$($PWD/bin/dmctl.test DEVEL --master-addr=$1 \ query-status test -s $2 | grep Location | gawk 'match($0,/startLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\], endLocation: \[position: \((.*), (.*)\), gtid-set: (.*)\]/,a) {print a[1]}')