diff --git a/command/configtest.go b/command/configtest.go index fc35b79fe..a1de6d82e 100644 --- a/command/configtest.go +++ b/command/configtest.go @@ -1,18 +1,18 @@ package command import ( - "flag" "fmt" "strings" "github.com/hashicorp/consul/command/agent" - "github.com/mitchellh/cli" ) // ConfigTestCommand is a Command implementation that is used to // verify config files type ConfigTestCommand struct { - Ui cli.Ui + Meta + + configFiles []string } func (c *ConfigTestCommand) Help() string { @@ -27,34 +27,29 @@ Usage: consul configtest [options] Returns 0 if the configuration is valid, or 1 if there are problems. -Options: +` + c.Meta.Help() - -config-file=foo Path to a JSON file to read configuration from. - This can be specified multiple times. - -config-dir=foo Path to a directory to read configuration files - from. This will read every file ending in ".json" - as configuration in this directory in alphabetical - order. - ` return strings.TrimSpace(helpText) } func (c *ConfigTestCommand) Run(args []string) int { - var configFiles []string - cmdFlags := flag.NewFlagSet("configtest", flag.ContinueOnError) - cmdFlags.Usage = func() { c.Ui.Output(c.Help()) } - cmdFlags.Var((*agent.AppendSliceValue)(&configFiles), "config-file", "json file to read config from") - cmdFlags.Var((*agent.AppendSliceValue)(&configFiles), "config-dir", "directory of json files to read") - if err := cmdFlags.Parse(args); err != nil { + f := c.Meta.NewFlagSet(c) + f.Var((*agent.AppendSliceValue)(&c.configFiles), "config-file", + "Path to a JSON file to read configuration from. This can be specified multiple times.") + f.Var((*agent.AppendSliceValue)(&c.configFiles), "config-dir", + "Path to a directory to read configuration files from. This will read every file ending in "+ + ".json as configuration in this directory in alphabetical order.") + + if err := c.Meta.Parse(args); err != nil { return 1 } - if len(configFiles) <= 0 { + if len(c.configFiles) <= 0 { c.Ui.Error("Must specify config using -config-file or -config-dir") return 1 } - _, err := agent.ReadConfigPaths(configFiles) + _, err := agent.ReadConfigPaths(c.configFiles) if err != nil { c.Ui.Error(fmt.Sprintf("Config validation failed: %v", err.Error())) return 1 diff --git a/command/configtest_test.go b/command/configtest_test.go index 65e6e1753..f3ff3415c 100644 --- a/command/configtest_test.go +++ b/command/configtest_test.go @@ -9,6 +9,16 @@ import ( "github.com/mitchellh/cli" ) +func testConfigTestCommand(t *testing.T) (*cli.MockUi, *ConfigTestCommand) { + ui := new(cli.MockUi) + return ui, &ConfigTestCommand{ + Meta: Meta{ + Ui: ui, + Flags: FlagSetNone, + }, + } +} + func TestConfigTestCommand_implements(t *testing.T) { var _ cli.Command = &ConfigTestCommand{} } @@ -20,9 +30,7 @@ func TestConfigTestCommandFailOnEmptyFile(t *testing.T) { } defer os.RemoveAll(tmpFile.Name()) - cmd := &ConfigTestCommand{ - Ui: new(cli.MockUi), - } + _, cmd := testConfigTestCommand(t) args := []string{ "-config-file", tmpFile.Name(), @@ -40,16 +48,14 @@ func TestConfigTestCommandSucceedOnEmptyDir(t *testing.T) { } defer os.RemoveAll(td) - cmd := &ConfigTestCommand{ - Ui: new(cli.MockUi), - } + ui, cmd := testConfigTestCommand(t) args := []string{ "-config-dir", td, } if code := cmd.Run(args); code != 0 { - t.Fatalf("bad: %d", code) + t.Fatalf("bad: %d, %s", code, ui.ErrorWriter.String()) } } @@ -66,9 +72,7 @@ func TestConfigTestCommandSucceedOnMinimalConfigFile(t *testing.T) { t.Fatalf("err: %s", err) } - cmd := &ConfigTestCommand{ - Ui: new(cli.MockUi), - } + _, cmd := testConfigTestCommand(t) args := []string{ "-config-file", fp, @@ -91,9 +95,7 @@ func TestConfigTestCommandSucceedOnMinimalConfigDir(t *testing.T) { t.Fatalf("err: %s", err) } - cmd := &ConfigTestCommand{ - Ui: new(cli.MockUi), - } + _, cmd := testConfigTestCommand(t) args := []string{ "-config-dir", td, @@ -116,9 +118,7 @@ func TestConfigTestCommandSucceedOnConfigDirWithEmptyFile(t *testing.T) { t.Fatalf("err: %s", err) } - cmd := &ConfigTestCommand{ - Ui: new(cli.MockUi), - } + _, cmd := testConfigTestCommand(t) args := []string{ "-config-dir", td, diff --git a/command/force_leave.go b/command/force_leave.go index 4839135d6..bb23ae942 100644 --- a/command/force_leave.go +++ b/command/force_leave.go @@ -1,42 +1,37 @@ package command import ( - "flag" "fmt" - "github.com/mitchellh/cli" "strings" ) // ForceLeaveCommand is a Command implementation that tells a running Consul // to force a member to enter the "left" state. type ForceLeaveCommand struct { - Ui cli.Ui + Meta } func (c *ForceLeaveCommand) Run(args []string) int { - cmdFlags := flag.NewFlagSet("join", flag.ContinueOnError) - cmdFlags.Usage = func() { c.Ui.Output(c.Help()) } - rpcAddr := RPCAddrFlag(cmdFlags) - if err := cmdFlags.Parse(args); err != nil { + f := c.Meta.NewFlagSet(c) + if err := c.Meta.Parse(args); err != nil { return 1 } - nodes := cmdFlags.Args() + nodes := f.Args() if len(nodes) != 1 { - c.Ui.Error("A node name must be specified to force leave.") + c.Ui.Error("A single node name must be specified to force leave.") c.Ui.Error("") c.Ui.Error(c.Help()) return 1 } - client, err := RPCClient(*rpcAddr) + client, err := c.Meta.HTTPClient() if err != nil { c.Ui.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err)) return 1 } - defer client.Close() - err = client.ForceLeave(nodes[0]) + err = client.Agent().ForceLeave(nodes[0]) if err != nil { c.Ui.Error(fmt.Sprintf("Error force leaving: %s", err)) return 1 @@ -60,10 +55,7 @@ Usage: consul force-leave [options] name Consul will attempt to reconnect to those failed nodes for some period of time before eventually reaping them. -Options: +` + c.Meta.Help() - -rpc-addr=127.0.0.1:8400 RPC address of the Consul agent. - -` return strings.TrimSpace(helpText) } diff --git a/command/force_leave_test.go b/command/force_leave_test.go index d297380a8..fb6b55c4d 100644 --- a/command/force_leave_test.go +++ b/command/force_leave_test.go @@ -10,6 +10,16 @@ import ( "testing" ) +func testForceLeaveCommand(t *testing.T) (*cli.MockUi, *ForceLeaveCommand) { + ui := new(cli.MockUi) + return ui, &ForceLeaveCommand{ + Meta: Meta{ + Ui: ui, + Flags: FlagSetHTTP, + }, + } +} + func TestForceLeaveCommand_implements(t *testing.T) { var _ cli.Command = &ForceLeaveCommand{} } @@ -29,10 +39,9 @@ func TestForceLeaveCommandRun(t *testing.T) { // Forcibly shutdown a2 so that it appears "failed" in a1 a2.Shutdown() - ui := new(cli.MockUi) - c := &ForceLeaveCommand{Ui: ui} + ui, c := testForceLeaveCommand(t) args := []string{ - "-rpc-addr=" + a1.addr, + "-http-addr=" + a1.httpAddr, a2.config.NodeName, } @@ -57,8 +66,8 @@ func TestForceLeaveCommandRun(t *testing.T) { func TestForceLeaveCommandRun_noAddrs(t *testing.T) { ui := new(cli.MockUi) - c := &ForceLeaveCommand{Ui: ui} - args := []string{"-rpc-addr=foo"} + ui, c := testForceLeaveCommand(t) + args := []string{"-http-addr=foo"} code := c.Run(args) if code != 1 { diff --git a/command/lock.go b/command/lock.go index a31046226..e10e28c5e 100644 --- a/command/lock.go +++ b/command/lock.go @@ -83,7 +83,7 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { var childDone chan struct{} f := c.Meta.NewFlagSet(c) - f.IntVar(&c.limit, "limit", 1, + f.IntVar(&c.limit, "n", 1, "Optional limit on the number of concurrent lock holders. The underlying "+ "implementation switches from a lock to a semaphore when the value is "+ "greater than 1. The default value is 1.") @@ -128,6 +128,11 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { prefix = strings.TrimPrefix(prefix, "/") script := strings.Join(extra[1:], " ") + if c.timeout < 0 { + c.Ui.Error("Timeout must be positive") + return 1 + } + // Calculate a session name if none provided if c.name == "" { c.name = fmt.Sprintf("Consul lock for '%s' at '%s'", script, prefix) diff --git a/command/lock_test.go b/command/lock_test.go index 4d3800386..417212143 100644 --- a/command/lock_test.go +++ b/command/lock_test.go @@ -38,9 +38,8 @@ func argFail(t *testing.T, args []string, expected string) { } func TestLockCommand_BadArgs(t *testing.T) { - argFail(t, []string{"-try=blah", "test/prefix", "date"}, "parsing try timeout") - argFail(t, []string{"-try=0s", "test/prefix", "date"}, "timeout must be positive") - argFail(t, []string{"-try=-10s", "test/prefix", "date"}, "timeout must be positive") + argFail(t, []string{"-try=blah", "test/prefix", "date"}, "invalid duration") + argFail(t, []string{"-try=-10s", "test/prefix", "date"}, "Timeout must be positive") argFail(t, []string{"-monitor-retry=-5", "test/prefix", "date"}, "must be >= 0") } diff --git a/command/meta.go b/command/meta.go index 51fbc83d1..f60eaf4e2 100644 --- a/command/meta.go +++ b/command/meta.go @@ -23,7 +23,6 @@ type FlagSetFlags uint const ( FlagSetNone FlagSetFlags = iota << 1 FlagSetHTTP FlagSetFlags = iota << 1 - FlagSetRPC FlagSetFlags = iota << 1 ) type Meta struct { @@ -37,8 +36,6 @@ type Meta struct { datacenter string token string stale bool - - rpcAddr string } // HTTPClient returns a client with the parsed flags. It panics if the command @@ -85,35 +82,6 @@ func (m *Meta) httpFlags(f *flag.FlagSet) *flag.FlagSet { return f } -// RPCClient returns a client with the parsed flags. It panics if the command -// does not accept RPC flags or if the flags have not been parsed. -func (m *Meta) RPCClient() (*api.Client, error) { - if !m.hasRPC() { - panic("no rpc flags defined") - } - if !m.flagSet.Parsed() { - panic("flags have not been parsed") - } - - // TODO - return nil, nil -} - -// rpcFlags is the list of flags that apply to RPC connections. -func (m *Meta) rpcFlags(f *flag.FlagSet) *flag.FlagSet { - if f == nil { - f = flag.NewFlagSet("", flag.ContinueOnError) - } - - f.StringVar(&m.rpcAddr, "rpc-addr", "", - "Address and port to the Consul RPC agent. The value can be an IP "+ - "address or DNS address, but it must also include the port. This can "+ - "also be specified via the CONSUL_RPC_ADDR environment variable. The "+ - "default value is 127.0.0.1:8400.") - - return f -} - // NewFlagSet creates a new flag set for the given command. It automatically // generates help output and adds the appropriate API flags. func (m *Meta) NewFlagSet(c cli.Command) *flag.FlagSet { @@ -124,10 +92,6 @@ func (m *Meta) NewFlagSet(c cli.Command) *flag.FlagSet { m.httpFlags(f) } - if m.hasRPC() { - m.rpcFlags(f) - } - errR, errW := io.Pipe() errScanner := bufio.NewScanner(errR) go func() { @@ -157,40 +121,29 @@ func (m *Meta) hasHTTP() bool { return m.Flags&FlagSetHTTP != 0 } -// hasRPC returns true if this meta command contains RPC flags. -func (m *Meta) hasRPC() bool { - return m.Flags&FlagSetRPC != 0 -} - // helpFlagsFor visits all flags in the given flag set and prints formatted // help output. This function is sad because there's no "merging" of command // line flags. We explicitly pull out our "common" options into another section // by doing string comparisons :(. func (m *Meta) helpFlagsFor(f *flag.FlagSet) string { httpFlags := m.httpFlags(nil) - rpcFlags := m.rpcFlags(nil) var out bytes.Buffer - printTitle(&out, "Command Options") - f.VisitAll(func(f *flag.Flag) { - // Skip HTTP and RPC flags as they will be grouped separately - if flagContains(httpFlags, f) || flagContains(rpcFlags, f) { - return - } - printFlag(&out, f) - }) - - if m.hasHTTP() { - printTitle(&out, "HTTP API Options") - httpFlags.VisitAll(func(f *flag.Flag) { + if f.NFlag() > 0 { + printTitle(&out, "Command Options") + f.VisitAll(func(f *flag.Flag) { + // Skip HTTP flags as they will be grouped separately + if flagContains(httpFlags, f) { + return + } printFlag(&out, f) }) } - if m.hasRPC() { - printTitle(&out, "RPC API Options") - rpcFlags.VisitAll(func(f *flag.Flag) { + if m.hasHTTP() { + printTitle(&out, "HTTP API Options") + httpFlags.VisitAll(func(f *flag.Flag) { printFlag(&out, f) }) } diff --git a/commands.go b/commands.go index 9fbe0e261..1a646f3e5 100644 --- a/commands.go +++ b/commands.go @@ -31,7 +31,10 @@ func init() { "configtest": func() (cli.Command, error) { return &command.ConfigTestCommand{ - Ui: ui, + Meta: command.Meta{ + Flags: command.FlagSetNone, + Ui: ui, + }, }, nil }, @@ -50,7 +53,10 @@ func init() { "force-leave": func() (cli.Command, error) { return &command.ForceLeaveCommand{ - Ui: ui, + Meta: command.Meta{ + Flags: command.FlagSetHTTP, + Ui: ui, + }, }, nil },