Convert configtest and force-leave commands to use Meta

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
+	f := c.Meta.NewFlagSet(c)
-	cmdFlags := flag.NewFlagSet("configtest", flag.ContinueOnError)
+	f.Var((*agent.AppendSliceValue)(&c.configFiles), "config-file",
-	cmdFlags.Usage = func() { c.Ui.Output(c.Help()) }
+		"Path to a JSON file to read configuration from. This can be specified multiple times.")
-	cmdFlags.Var((*agent.AppendSliceValue)(&configFiles), "config-file", "json file to read config from")
+	f.Var((*agent.AppendSliceValue)(&c.configFiles), "config-dir",
-	cmdFlags.Var((*agent.AppendSliceValue)(&configFiles), "config-dir", "directory of json files to read")
+		"Path to a directory  to read configuration files from. This will read every file ending in "+
-	if err := cmdFlags.Parse(args); err != nil {
+			".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

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{
+	_, cmd := testConfigTestCommand(t)
-		Ui: new(cli.MockUi),
-	}
 
 	args := []string{
 		"-config-file", tmpFile.Name(),
@@ -40,16 +48,14 @@ func TestConfigTestCommandSucceedOnEmptyDir(t *testing.T) {
 	}
 	defer os.RemoveAll(td)
 
-	cmd := &ConfigTestCommand{
+	ui, cmd := testConfigTestCommand(t)
-		Ui: new(cli.MockUi),
-	}
 
 	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{
+	_, cmd := testConfigTestCommand(t)
-		Ui: new(cli.MockUi),
-	}
 
 	args := []string{
 		"-config-file", fp,
@@ -91,9 +95,7 @@ func TestConfigTestCommandSucceedOnMinimalConfigDir(t *testing.T) {
 		t.Fatalf("err: %s", err)
 	}
 
-	cmd := &ConfigTestCommand{
+	_, cmd := testConfigTestCommand(t)
-		Ui: new(cli.MockUi),
-	}
 
 	args := []string{
 		"-config-dir", td,
@@ -116,9 +118,7 @@ func TestConfigTestCommandSucceedOnConfigDirWithEmptyFile(t *testing.T) {
 		t.Fatalf("err: %s", err)
 	}
 
-	cmd := &ConfigTestCommand{
+	_, cmd := testConfigTestCommand(t)
-		Ui: new(cli.MockUi),
-	}
 
 	args := []string{
 		"-config-dir", td,

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)
+	f := c.Meta.NewFlagSet(c)
-	cmdFlags.Usage = func() { c.Ui.Output(c.Help()) }
+	if err := c.Meta.Parse(args); err != nil {
-	rpcAddr := RPCAddrFlag(cmdFlags)
-	if err := cmdFlags.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)
 }

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)
+	ui, c := testForceLeaveCommand(t)
-	c := &ForceLeaveCommand{Ui: ui}
 	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}
+	ui, c := testForceLeaveCommand(t)
-	args := []string{"-rpc-addr=foo"}
+	args := []string{"-http-addr=foo"}
 
 	code := c.Run(args)
 	if code != 1 {

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)

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=blah", "test/prefix", "date"}, "invalid duration")
-	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=-10s", "test/prefix", "date"}, "timeout must be positive")
 	argFail(t, []string{"-monitor-retry=-5", "test/prefix", "date"}, "must be >= 0")
 }
 

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")
+	if f.NFlag() > 0 {
-	f.VisitAll(func(f *flag.Flag) {
+		printTitle(&out, "Command Options")
-		// Skip HTTP and RPC flags as they will be grouped separately
+		f.VisitAll(func(f *flag.Flag) {
-		if flagContains(httpFlags, f) || flagContains(rpcFlags, f) {
+			// Skip HTTP flags as they will be grouped separately
-			return
+			if flagContains(httpFlags, f) {
-		}
+				return
-		printFlag(&out, f)
+			}
-	})
-
-	if m.hasHTTP() {
-		printTitle(&out, "HTTP API Options")
-		httpFlags.VisitAll(func(f *flag.Flag) {
 			printFlag(&out, f)
 		})
 	}
 
-	if m.hasRPC() {
+	if m.hasHTTP() {
-		printTitle(&out, "RPC API Options")
+		printTitle(&out, "HTTP API Options")
-		rpcFlags.VisitAll(func(f *flag.Flag) {
+		httpFlags.VisitAll(func(f *flag.Flag) {
 			printFlag(&out, f)
 		})
 	}

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
 		},