From de1718a8a69b85bb89a10a19fdbb81fb1df6e820 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Sun, 9 Oct 2016 12:10:40 +0800 Subject: [PATCH 1/7] Centralize command-line parsing --- command/lock.go | 148 +++++++++++--------------- command/lock_test.go | 34 +++--- command/meta.go | 247 +++++++++++++++++++++++++++++++++++++++++++ commands.go | 5 +- 4 files changed, 331 insertions(+), 103 deletions(-) create mode 100644 command/meta.go diff --git a/command/lock.go b/command/lock.go index 72f4ec5830e1..a31046226b7d 100644 --- a/command/lock.go +++ b/command/lock.go @@ -1,7 +1,6 @@ package command import ( - "flag" "fmt" "os" "path" @@ -12,7 +11,6 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/agent" - "github.com/mitchellh/cli" ) const ( @@ -37,54 +35,42 @@ const ( // LockCommand is a Command implementation that is used to setup // a "lock" which manages lock acquisition and invokes a sub-process type LockCommand struct { + Meta + ShutdownCh <-chan struct{} - Ui cli.Ui child *os.Process childLock sync.Mutex - verbose bool + + limit int + monitorRetry int + name string + passStdin bool + timeout time.Duration + verbose bool } func (c *LockCommand) Help() string { helpText := ` Usage: consul lock [options] prefix child... - Acquires a lock or semaphore at a given path, and invokes a child - process when successful. The child process can assume the lock is - held while it executes. If the lock is lost or communication is - disrupted the child process will be sent a SIGTERM signal and given - time to gracefully exit. After the grace period expires the process - will be hard terminated. + Acquires a lock or semaphore at a given path, and invokes a child process + when successful. The child process can assume the lock is held while it + executes. If the lock is lost or communication is disrupted the child + process will be sent a SIGTERM signal and given time to gracefully exit. + After the grace period expires the process will be hard terminated. - For Consul agents on Windows, the child process is always hard - terminated with a SIGKILL, since Windows has no POSIX compatible - notion for SIGTERM. + For Consul agents on Windows, the child process is always hard terminated + with a SIGKILL, since Windows has no POSIX compatible notion for SIGTERM. - When -n=1, only a single lock holder or leader exists providing - mutual exclusion. Setting a higher value switches to a semaphore - allowing multiple holders to coordinate. + When -n=1, only a single lock holder or leader exists providing mutual + exclusion. Setting a higher value switches to a semaphore allowing multiple + holders to coordinate. The prefix provided must have write privileges. -Options: - - -http-addr=127.0.0.1:8500 HTTP address of the Consul agent. - -n=1 Maximum number of allowed lock holders. If this - value is one, it operates as a lock, otherwise - a semaphore is used. - -name="" Optional name to associate with lock session. - -token="" ACL token to use. Defaults to that of agent. - -pass-stdin Pass stdin to child process. - -try=timeout Attempt to acquire the lock up to the given - timeout (eg. "15s"). - -monitor-retry=n Retry up to n times if Consul returns a 500 error - while monitoring the lock. This allows riding out brief - periods of unavailability without causing leader - elections, but increases the amount of time required - to detect a lost lock in some cases. Defaults to 3, - with a 1s wait between retries. Set to 0 to disable. - -verbose Enables verbose output -` +` + c.Meta.Help() + return strings.TrimSpace(helpText) } @@ -93,40 +79,49 @@ func (c *LockCommand) Run(args []string) int { return c.run(args, &lu) } -// run exposes the underlying lock for testing. func (c *LockCommand) run(args []string, lu **LockUnlock) int { var childDone chan struct{} - var name, token string - var limit int - var passStdin bool - var try string - var retry int - cmdFlags := flag.NewFlagSet("watch", flag.ContinueOnError) - cmdFlags.Usage = func() { c.Ui.Output(c.Help()) } - cmdFlags.IntVar(&limit, "n", 1, "") - cmdFlags.StringVar(&name, "name", "", "") - cmdFlags.StringVar(&token, "token", "", "") - cmdFlags.BoolVar(&passStdin, "pass-stdin", false, "") - cmdFlags.StringVar(&try, "try", "", "") - cmdFlags.IntVar(&retry, "monitor-retry", defaultMonitorRetry, "") - cmdFlags.BoolVar(&c.verbose, "verbose", false, "") - httpAddr := HTTPAddrFlag(cmdFlags) - if err := cmdFlags.Parse(args); err != nil { + + f := c.Meta.NewFlagSet(c) + f.IntVar(&c.limit, "limit", 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.") + f.IntVar(&c.monitorRetry, "monitor-retry", defaultMonitorRetry, + "Number of times to retry Consul returns a 500 error while monitoring "+ + "the lock. This allows riding out brief periods of unavailability "+ + "without causing leader elections, but increases the amount of time "+ + "required to detect a lost lock in some cases. The default value is 3, "+ + "with a 1s wait between retries. Set this value to 0 to disable retires.") + f.StringVar(&c.name, "name", "", + "Optional name to associate with the lock session. It not provided, one "+ + "is generated based on the provided child command.") + f.BoolVar(&c.passStdin, "pass-stdin", false, + "Pass stdin to the child process.") + f.DurationVar(&c.timeout, "timeout", 0, + "Maximum amount of time to wait to acquire the lock, specified as a "+ + "timestamp like \"1s\" or \"3h\". The default value is 0.") + f.BoolVar(&c.verbose, "verbose", false, + "Enable verbose (debugging) output.") + + // Deprecations + f.DurationVar(&c.timeout, "try", 0, + "DEPRECATED. Use -timeout instead.") + + if err := c.Meta.Parse(args); err != nil { return 1 } // Check the limit - if limit <= 0 { + if c.limit <= 0 { c.Ui.Error(fmt.Sprintf("Lock holder limit must be positive")) return 1 } // Verify the prefix and child are provided - extra := cmdFlags.Args() + extra := f.Args() if len(extra) < 2 { c.Ui.Error("Key prefix and child command must be specified") - c.Ui.Error("") - c.Ui.Error(c.Help()) return 1 } prefix := extra[0] @@ -134,40 +129,21 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { script := strings.Join(extra[1:], " ") // Calculate a session name if none provided - if name == "" { - name = fmt.Sprintf("Consul lock for '%s' at '%s'", script, prefix) - } - - // Verify the duration if given. - oneshot := false - var wait time.Duration - if try != "" { - var err error - wait, err = time.ParseDuration(try) - if err != nil { - c.Ui.Error(fmt.Sprintf("Error parsing try timeout: %s", err)) - return 1 - } - - if wait <= 0 { - c.Ui.Error("Try timeout must be positive") - return 1 - } - - oneshot = true + if c.name == "" { + c.name = fmt.Sprintf("Consul lock for '%s' at '%s'", script, prefix) } + // Calculate oneshot + oneshot := c.timeout > 0 + // Check the retry parameter - if retry < 0 { + if c.monitorRetry < 0 { c.Ui.Error("Number for 'monitor-retry' must be >= 0") return 1 } // Create and test the HTTP client - conf := api.DefaultConfig() - conf.Address = *httpAddr - conf.Token = token - client, err := api.NewClient(conf) + client, err := c.Meta.HTTPClient() if err != nil { c.Ui.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err)) return 1 @@ -179,10 +155,10 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { } // Setup the lock or semaphore - if limit == 1 { - *lu, err = c.setupLock(client, prefix, name, oneshot, wait, retry) + if c.limit == 1 { + *lu, err = c.setupLock(client, prefix, c.name, oneshot, c.timeout, c.monitorRetry) } else { - *lu, err = c.setupSemaphore(client, limit, prefix, name, oneshot, wait, retry) + *lu, err = c.setupSemaphore(client, c.limit, prefix, c.name, oneshot, c.timeout, c.monitorRetry) } if err != nil { c.Ui.Error(fmt.Sprintf("Lock setup failed: %s", err)) @@ -214,7 +190,7 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { // Start the child process childDone = make(chan struct{}) go func() { - if err := c.startChild(script, childDone, passStdin); err != nil { + if err := c.startChild(script, childDone, c.passStdin); err != nil { c.Ui.Error(fmt.Sprintf("%s", err)) } }() diff --git a/command/lock_test.go b/command/lock_test.go index 34c859a51fd8..4d380038694c 100644 --- a/command/lock_test.go +++ b/command/lock_test.go @@ -12,13 +12,22 @@ import ( "github.com/mitchellh/cli" ) +func testLockCommand(t *testing.T) (*cli.MockUi, *LockCommand) { + ui := new(cli.MockUi) + return ui, &LockCommand{ + Meta: Meta{ + Ui: ui, + Flags: FlagSetHTTP, + }, + } +} + func TestLockCommand_implements(t *testing.T) { var _ cli.Command = &LockCommand{} } func argFail(t *testing.T, args []string, expected string) { - ui := new(cli.MockUi) - c := &LockCommand{Ui: ui} + ui, c := testLockCommand(t) if code := c.Run(args); code != 1 { t.Fatalf("expected return code 1, got %d", code) } @@ -40,8 +49,7 @@ func TestLockCommand_Run(t *testing.T) { defer a1.Shutdown() waitForLeader(t, a1.httpAddr) - ui := new(cli.MockUi) - c := &LockCommand{Ui: ui} + ui, c := testLockCommand(t) filePath := filepath.Join(a1.dir, "test_touch") touchCmd := fmt.Sprintf("touch '%s'", filePath) args := []string{"-http-addr=" + a1.httpAddr, "test/prefix", touchCmd} @@ -63,8 +71,7 @@ func TestLockCommand_Try_Lock(t *testing.T) { defer a1.Shutdown() waitForLeader(t, a1.httpAddr) - ui := new(cli.MockUi) - c := &LockCommand{Ui: ui} + ui, c := testLockCommand(t) filePath := filepath.Join(a1.dir, "test_touch") touchCmd := fmt.Sprintf("touch '%s'", filePath) args := []string{"-http-addr=" + a1.httpAddr, "-try=10s", "test/prefix", touchCmd} @@ -95,8 +102,7 @@ func TestLockCommand_Try_Semaphore(t *testing.T) { defer a1.Shutdown() waitForLeader(t, a1.httpAddr) - ui := new(cli.MockUi) - c := &LockCommand{Ui: ui} + ui, c := testLockCommand(t) filePath := filepath.Join(a1.dir, "test_touch") touchCmd := fmt.Sprintf("touch '%s'", filePath) args := []string{"-http-addr=" + a1.httpAddr, "-n=3", "-try=10s", "test/prefix", touchCmd} @@ -127,8 +133,7 @@ func TestLockCommand_MonitorRetry_Lock_Default(t *testing.T) { defer a1.Shutdown() waitForLeader(t, a1.httpAddr) - ui := new(cli.MockUi) - c := &LockCommand{Ui: ui} + ui, c := testLockCommand(t) filePath := filepath.Join(a1.dir, "test_touch") touchCmd := fmt.Sprintf("touch '%s'", filePath) args := []string{"-http-addr=" + a1.httpAddr, "test/prefix", touchCmd} @@ -160,8 +165,7 @@ func TestLockCommand_MonitorRetry_Semaphore_Default(t *testing.T) { defer a1.Shutdown() waitForLeader(t, a1.httpAddr) - ui := new(cli.MockUi) - c := &LockCommand{Ui: ui} + ui, c := testLockCommand(t) filePath := filepath.Join(a1.dir, "test_touch") touchCmd := fmt.Sprintf("touch '%s'", filePath) args := []string{"-http-addr=" + a1.httpAddr, "-n=3", "test/prefix", touchCmd} @@ -193,8 +197,7 @@ func TestLockCommand_MonitorRetry_Lock_Arg(t *testing.T) { defer a1.Shutdown() waitForLeader(t, a1.httpAddr) - ui := new(cli.MockUi) - c := &LockCommand{Ui: ui} + ui, c := testLockCommand(t) filePath := filepath.Join(a1.dir, "test_touch") touchCmd := fmt.Sprintf("touch '%s'", filePath) args := []string{"-http-addr=" + a1.httpAddr, "-monitor-retry=9", "test/prefix", touchCmd} @@ -226,8 +229,7 @@ func TestLockCommand_MonitorRetry_Semaphore_Arg(t *testing.T) { defer a1.Shutdown() waitForLeader(t, a1.httpAddr) - ui := new(cli.MockUi) - c := &LockCommand{Ui: ui} + ui, c := testLockCommand(t) filePath := filepath.Join(a1.dir, "test_touch") touchCmd := fmt.Sprintf("touch '%s'", filePath) args := []string{"-http-addr=" + a1.httpAddr, "-n=3", "-monitor-retry=9", "test/prefix", touchCmd} diff --git a/command/meta.go b/command/meta.go new file mode 100644 index 000000000000..51fbc83d17cc --- /dev/null +++ b/command/meta.go @@ -0,0 +1,247 @@ +package command + +import ( + "bufio" + "bytes" + "flag" + "fmt" + "io" + "strings" + + "github.com/hashicorp/consul/api" + "github.com/mitchellh/cli" + text "github.com/tonnerre/golang-text" +) + +// maxLineLength is the maximum width of any line. +const maxLineLength int = 72 + +// FlagSetFlags is an enum to define what flags are present in the +// default FlagSet returned. +type FlagSetFlags uint + +const ( + FlagSetNone FlagSetFlags = iota << 1 + FlagSetHTTP FlagSetFlags = iota << 1 + FlagSetRPC FlagSetFlags = iota << 1 +) + +type Meta struct { + Ui cli.Ui + Flags FlagSetFlags + + flagSet *flag.FlagSet + + // These are the options which correspond to the HTTP API options + httpAddr string + datacenter string + token string + stale bool + + rpcAddr string +} + +// HTTPClient returns a client with the parsed flags. It panics if the command +// does not accept HTTP flags or if the flags have not been parsed. +func (m *Meta) HTTPClient() (*api.Client, error) { + if !m.hasHTTP() { + panic("no http flags defined") + } + if !m.flagSet.Parsed() { + panic("flags have not been parsed") + } + + return api.NewClient(&api.Config{ + Datacenter: m.datacenter, + Address: m.httpAddr, + Token: m.token, + }) +} + +// httpFlags is the list of flags that apply to HTTP connections. +func (m *Meta) httpFlags(f *flag.FlagSet) *flag.FlagSet { + if f == nil { + f = flag.NewFlagSet("", flag.ContinueOnError) + } + + f.StringVar(&m.datacenter, "datacenter", "", + "Name of the datacenter to query. If unspecified, this will default to "+ + "the datacenter of the queried agent.") + f.StringVar(&m.httpAddr, "http-addr", "", + "Address and port to the Consul HTTP 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_HTTP_ADDR environment variable. The "+ + "default value is 127.0.0.1:8500.") + f.StringVar(&m.token, "token", "", + "ACL token to use in the request. This can also be specified via the "+ + "CONSUL_HTTP_TOKEN environment variable. If unspecified, the query will "+ + "default to the token of the Consul agent at the HTTP address.") + f.BoolVar(&m.stale, "stale", false, + "Permit any Consul server (non-leader) to respond to this request. This "+ + "allows for lower latency and higher throughput, but can result in "+ + "stale data. This option has no effect on non-read operations. The "+ + "default value is false.") + + 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 { + f := flag.NewFlagSet("", flag.ContinueOnError) + f.Usage = func() { m.Ui.Error(c.Help()) } + + if m.hasHTTP() { + m.httpFlags(f) + } + + if m.hasRPC() { + m.rpcFlags(f) + } + + errR, errW := io.Pipe() + errScanner := bufio.NewScanner(errR) + go func() { + for errScanner.Scan() { + m.Ui.Error(errScanner.Text()) + } + }() + f.SetOutput(errW) + + m.flagSet = f + + return f +} + +// Parse is used to parse the underlying flag set. +func (m *Meta) Parse(args []string) error { + return m.flagSet.Parse(args) +} + +// Help returns the help for this flagSet. +func (m *Meta) Help() string { + return m.helpFlagsFor(m.flagSet) +} + +// hasHTTP returns true if this meta command contains HTTP flags. +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) { + printFlag(&out, f) + }) + } + + if m.hasRPC() { + printTitle(&out, "RPC API Options") + rpcFlags.VisitAll(func(f *flag.Flag) { + printFlag(&out, f) + }) + } + + return strings.TrimRight(out.String(), "\n") +} + +// printTitle prints a consistently-formatted title to the given writer. +func printTitle(w io.Writer, s string) { + fmt.Fprintf(w, "%s\n\n", s) +} + +// printFlag prints a single flag to the given writer. +func printFlag(w io.Writer, f *flag.Flag) { + example, _ := flag.UnquoteUsage(f) + if example != "" { + fmt.Fprintf(w, " -%s=<%s>\n", f.Name, example) + } else { + fmt.Fprintf(w, " -%s\n", f.Name) + } + + indented := wrapAtLength(f.Usage, 5) + fmt.Fprintf(w, "%s\n\n", indented) +} + +// flagContains returns true if the given flag is contained in the given flag +// set or false otherwise. +func flagContains(fs *flag.FlagSet, f *flag.Flag) bool { + var skip bool + + fs.VisitAll(func(hf *flag.Flag) { + if skip { + return + } + + if f.Name == hf.Name && f.Usage == hf.Usage { + skip = true + return + } + }) + + return skip +} + +// wrapAtLength wraps the given text at the maxLineLength, taxing into account +// any provided left padding. +func wrapAtLength(s string, pad int) string { + wrapped := text.Wrap(s, maxLineLength-pad) + lines := strings.Split(wrapped, "\n") + for i, line := range lines { + lines[i] = strings.Repeat(" ", pad) + line + } + return strings.Join(lines, "\n") +} diff --git a/commands.go b/commands.go index 41f60b3c0960..3a3fcfe69a0d 100644 --- a/commands.go +++ b/commands.go @@ -104,7 +104,10 @@ func init() { "lock": func() (cli.Command, error) { return &command.LockCommand{ ShutdownCh: makeShutdownCh(), - Ui: ui, + Meta: command.Meta{ + Flags: command.FlagSetHTTP, + Ui: ui, + }, }, nil }, From e86ec5b54c55051271c181864046b691e1f29301 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Mon, 6 Feb 2017 13:52:43 -0500 Subject: [PATCH 2/7] vendor: Add golang-text dependency --- .../github.com/tonnerre/golang-text/License | 19 ++++ vendor/github.com/tonnerre/golang-text/Readme | 3 + vendor/github.com/tonnerre/golang-text/doc.go | 3 + .../github.com/tonnerre/golang-text/indent.go | 74 ++++++++++++++++ .../github.com/tonnerre/golang-text/wrap.go | 86 +++++++++++++++++++ vendor/vendor.json | 6 ++ 6 files changed, 191 insertions(+) create mode 100644 vendor/github.com/tonnerre/golang-text/License create mode 100644 vendor/github.com/tonnerre/golang-text/Readme create mode 100644 vendor/github.com/tonnerre/golang-text/doc.go create mode 100644 vendor/github.com/tonnerre/golang-text/indent.go create mode 100755 vendor/github.com/tonnerre/golang-text/wrap.go diff --git a/vendor/github.com/tonnerre/golang-text/License b/vendor/github.com/tonnerre/golang-text/License new file mode 100644 index 000000000000..480a32805999 --- /dev/null +++ b/vendor/github.com/tonnerre/golang-text/License @@ -0,0 +1,19 @@ +Copyright 2012 Keith Rarick + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in +all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +THE SOFTWARE. diff --git a/vendor/github.com/tonnerre/golang-text/Readme b/vendor/github.com/tonnerre/golang-text/Readme new file mode 100644 index 000000000000..7e6e7c0687b3 --- /dev/null +++ b/vendor/github.com/tonnerre/golang-text/Readme @@ -0,0 +1,3 @@ +This is a Go package for manipulating paragraphs of text. + +See http://go.pkgdoc.org/github.com/kr/text for full documentation. diff --git a/vendor/github.com/tonnerre/golang-text/doc.go b/vendor/github.com/tonnerre/golang-text/doc.go new file mode 100644 index 000000000000..cf4c198f9558 --- /dev/null +++ b/vendor/github.com/tonnerre/golang-text/doc.go @@ -0,0 +1,3 @@ +// Package text provides rudimentary functions for manipulating text in +// paragraphs. +package text diff --git a/vendor/github.com/tonnerre/golang-text/indent.go b/vendor/github.com/tonnerre/golang-text/indent.go new file mode 100644 index 000000000000..4ebac45c0925 --- /dev/null +++ b/vendor/github.com/tonnerre/golang-text/indent.go @@ -0,0 +1,74 @@ +package text + +import ( + "io" +) + +// Indent inserts prefix at the beginning of each non-empty line of s. The +// end-of-line marker is NL. +func Indent(s, prefix string) string { + return string(IndentBytes([]byte(s), []byte(prefix))) +} + +// IndentBytes inserts prefix at the beginning of each non-empty line of b. +// The end-of-line marker is NL. +func IndentBytes(b, prefix []byte) []byte { + var res []byte + bol := true + for _, c := range b { + if bol && c != '\n' { + res = append(res, prefix...) + } + res = append(res, c) + bol = c == '\n' + } + return res +} + +// Writer indents each line of its input. +type indentWriter struct { + w io.Writer + bol bool + pre [][]byte + sel int + off int +} + +// NewIndentWriter makes a new write filter that indents the input +// lines. Each line is prefixed in order with the corresponding +// element of pre. If there are more lines than elements, the last +// element of pre is repeated for each subsequent line. +func NewIndentWriter(w io.Writer, pre ...[]byte) io.Writer { + return &indentWriter{ + w: w, + pre: pre, + bol: true, + } +} + +// The only errors returned are from the underlying indentWriter. +func (w *indentWriter) Write(p []byte) (n int, err error) { + for _, c := range p { + if w.bol { + var i int + i, err = w.w.Write(w.pre[w.sel][w.off:]) + w.off += i + if err != nil { + return n, err + } + } + _, err = w.w.Write([]byte{c}) + if err != nil { + return n, err + } + n++ + w.bol = c == '\n' + if w.bol { + w.off = 0 + if w.sel < len(w.pre)-1 { + w.sel++ + } + } + } + return n, nil +} diff --git a/vendor/github.com/tonnerre/golang-text/wrap.go b/vendor/github.com/tonnerre/golang-text/wrap.go new file mode 100755 index 000000000000..ca8856515c33 --- /dev/null +++ b/vendor/github.com/tonnerre/golang-text/wrap.go @@ -0,0 +1,86 @@ +package text + +import ( + "bytes" + "math" +) + +var ( + nl = []byte{'\n'} + sp = []byte{' '} +) + +const defaultPenalty = 1e5 + +// Wrap wraps s into a paragraph of lines of length lim, with minimal +// raggedness. +func Wrap(s string, lim int) string { + return string(WrapBytes([]byte(s), lim)) +} + +// WrapBytes wraps b into a paragraph of lines of length lim, with minimal +// raggedness. +func WrapBytes(b []byte, lim int) []byte { + words := bytes.Split(bytes.Replace(bytes.TrimSpace(b), nl, sp, -1), sp) + var lines [][]byte + for _, line := range WrapWords(words, 1, lim, defaultPenalty) { + lines = append(lines, bytes.Join(line, sp)) + } + return bytes.Join(lines, nl) +} + +// WrapWords is the low-level line-breaking algorithm, useful if you need more +// control over the details of the text wrapping process. For most uses, either +// Wrap or WrapBytes will be sufficient and more convenient. +// +// WrapWords splits a list of words into lines with minimal "raggedness", +// treating each byte as one unit, accounting for spc units between adjacent +// words on each line, and attempting to limit lines to lim units. Raggedness +// is the total error over all lines, where error is the square of the +// difference of the length of the line and lim. Too-long lines (which only +// happen when a single word is longer than lim units) have pen penalty units +// added to the error. +func WrapWords(words [][]byte, spc, lim, pen int) [][][]byte { + n := len(words) + + length := make([][]int, n) + for i := 0; i < n; i++ { + length[i] = make([]int, n) + length[i][i] = len(words[i]) + for j := i + 1; j < n; j++ { + length[i][j] = length[i][j-1] + spc + len(words[j]) + } + } + + nbrk := make([]int, n) + cost := make([]int, n) + for i := range cost { + cost[i] = math.MaxInt32 + } + for i := n - 1; i >= 0; i-- { + if length[i][n-1] <= lim { + cost[i] = 0 + nbrk[i] = n + } else { + for j := i + 1; j < n; j++ { + d := lim - length[i][j-1] + c := d*d + cost[j] + if length[i][j-1] > lim { + c += pen // too-long lines get a worse penalty + } + if c < cost[i] { + cost[i] = c + nbrk[i] = j + } + } + } + } + + var lines [][][]byte + i := 0 + for i < n { + lines = append(lines, words[i:nbrk[i]]) + i = nbrk[i] + } + return lines +} diff --git a/vendor/vendor.json b/vendor/vendor.json index 25f3fd77137b..4626f5a10f20 100644 --- a/vendor/vendor.json +++ b/vendor/vendor.json @@ -740,6 +740,12 @@ "revision": "bb4de0191aa41b5507caa14b0650cdbddcd9280b", "revisionTime": "2016-09-30T03:27:40Z" }, + { + "checksumSHA1": "t24KnvC9jRxiANVhpw2pqFpmEu8=", + "path": "github.com/tonnerre/golang-text", + "revision": "048ed3d792f7104850acbc8cfc01e5a6070f4c04", + "revisionTime": "2013-09-25T19:58:46Z" + }, { "checksumSHA1": "9jjO5GjLa0XF/nfWihF02RoH4qc=", "path": "golang.org/x/net/context", From 8009432b1b3db60e5f2a69d6be12e247844a2c5a Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Mon, 6 Feb 2017 20:50:51 -0500 Subject: [PATCH 3/7] Convert configtest and force-leave commands to use Meta --- command/configtest.go | 33 ++++++++---------- command/configtest_test.go | 32 +++++++++--------- command/force_leave.go | 24 +++++-------- command/force_leave_test.go | 19 ++++++++--- command/lock.go | 7 +++- command/lock_test.go | 5 ++- command/meta.go | 67 ++++++------------------------------- commands.go | 10 ++++-- 8 files changed, 78 insertions(+), 119 deletions(-) diff --git a/command/configtest.go b/command/configtest.go index fc35b79fe46d..a1de6d82ead7 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 65e6e1753b77..f3ff3415c096 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 4839135d6196..bb23ae942a94 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 d297380a8601..fb6b55c4d8f9 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 a31046226b7d..e10e28c5e198 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 4d380038694c..4172121437b0 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 51fbc83d17cc..f60eaf4e23d9 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 9fbe0e261c4e..1a646f3e5a24 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 }, From edc353914def587f96e058f5467707086e03666c Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Mon, 6 Feb 2017 23:45:58 -0500 Subject: [PATCH 4/7] Fix the check for displaying the command options --- command/meta.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/command/meta.go b/command/meta.go index f60eaf4e23d9..7218074ba289 100644 --- a/command/meta.go +++ b/command/meta.go @@ -130,16 +130,18 @@ func (m *Meta) helpFlagsFor(f *flag.FlagSet) string { var out bytes.Buffer - 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) - }) - } + first := true + f.VisitAll(func(f *flag.Flag) { + // Skip HTTP flags as they will be grouped separately + if flagContains(httpFlags, f) { + return + } + if first { + printTitle(&out, "Command Options") + first = false + } + printFlag(&out, f) + }) if m.hasHTTP() { printTitle(&out, "HTTP API Options") From 49d2ce1c3d7187977357661ec1783e357c28c120 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Tue, 7 Feb 2017 19:16:33 -0500 Subject: [PATCH 5/7] Move command Meta to base.Command and split http options --- command/{meta.go => base/command.go} | 139 ++++++++++++------ command/configtest.go | 23 +-- command/configtest_test.go | 5 +- command/force_leave.go | 11 +- command/force_leave_test.go | 5 +- command/lock.go | 59 ++++---- command/lock_test.go | 5 +- commands.go | 13 +- .../commands/_http_api_options.html.markdown | 16 -- .../_http_api_options_client.html.markdown | 8 + .../_http_api_options_server.html.markdown | 7 + ...markdown => force-leave.html.markdown.erb} | 8 +- .../docs/commands/kv/delete.html.markdown.erb | 3 +- .../docs/commands/kv/export.html.markdown.erb | 3 +- .../docs/commands/kv/get.html.markdown.erb | 3 +- .../docs/commands/kv/import.html.markdown.erb | 3 +- .../docs/commands/kv/put.html.markdown.erb | 3 +- ...k.html.markdown => lock.html.markdown.erb} | 23 ++- .../commands/snapshot/agent.html.markdown.erb | 2 +- .../snapshot/restore.html.markdown.erb | 2 +- .../commands/snapshot/save.html.markdown.erb | 2 +- 21 files changed, 196 insertions(+), 147 deletions(-) rename command/{meta.go => base/command.go} (59%) delete mode 100644 website/source/docs/commands/_http_api_options.html.markdown create mode 100644 website/source/docs/commands/_http_api_options_client.html.markdown create mode 100644 website/source/docs/commands/_http_api_options_server.html.markdown rename website/source/docs/commands/{force-leave.html.markdown => force-leave.html.markdown.erb} (76%) rename website/source/docs/commands/{lock.html.markdown => lock.html.markdown.erb} (92%) diff --git a/command/meta.go b/command/base/command.go similarity index 59% rename from command/meta.go rename to command/base/command.go index 7218074ba289..032b8026333c 100644 --- a/command/meta.go +++ b/command/base/command.go @@ -1,4 +1,4 @@ -package command +package base import ( "bufio" @@ -21,11 +21,14 @@ const maxLineLength int = 72 type FlagSetFlags uint const ( - FlagSetNone FlagSetFlags = iota << 1 - FlagSetHTTP FlagSetFlags = iota << 1 + FlagSetNone FlagSetFlags = iota << 1 + FlagSetClientHTTP FlagSetFlags = iota << 1 + FlagSetServerHTTP FlagSetFlags = iota << 1 + + FlagSetHTTP = FlagSetClientHTTP | FlagSetServerHTTP ) -type Meta struct { +type Command struct { Ui cli.Ui Flags FlagSetFlags @@ -33,47 +36,64 @@ type Meta struct { // These are the options which correspond to the HTTP API options httpAddr string - datacenter string token string + datacenter string stale bool } // HTTPClient returns a client with the parsed flags. It panics if the command // does not accept HTTP flags or if the flags have not been parsed. -func (m *Meta) HTTPClient() (*api.Client, error) { - if !m.hasHTTP() { +func (c *Command) HTTPClient() (*api.Client, error) { + if !c.hasClientHTTP() && !c.hasServerHTTP() { panic("no http flags defined") } - if !m.flagSet.Parsed() { + if !c.flagSet.Parsed() { panic("flags have not been parsed") } - return api.NewClient(&api.Config{ - Datacenter: m.datacenter, - Address: m.httpAddr, - Token: m.token, - }) + config := api.DefaultConfig() + if c.datacenter != "" { + config.Datacenter = c.datacenter + } + if c.httpAddr != "" { + config.Address = c.httpAddr + } + if c.token != "" { + config.Token = c.token + } + c.Ui.Info(fmt.Sprintf("client http addr: %s", config.Address)) + return api.NewClient(config) } -// httpFlags is the list of flags that apply to HTTP connections. -func (m *Meta) httpFlags(f *flag.FlagSet) *flag.FlagSet { +// httpFlagsClient is the list of flags that apply to HTTP connections. +func (c *Command) httpFlagsClient(f *flag.FlagSet) *flag.FlagSet { if f == nil { f = flag.NewFlagSet("", flag.ContinueOnError) } - f.StringVar(&m.datacenter, "datacenter", "", - "Name of the datacenter to query. If unspecified, this will default to "+ - "the datacenter of the queried agent.") - f.StringVar(&m.httpAddr, "http-addr", "", + f.StringVar(&c.httpAddr, "http-addr", "", "Address and port to the Consul HTTP 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_HTTP_ADDR environment variable. The "+ "default value is 127.0.0.1:8500.") - f.StringVar(&m.token, "token", "", + f.StringVar(&c.token, "token", "", "ACL token to use in the request. This can also be specified via the "+ "CONSUL_HTTP_TOKEN environment variable. If unspecified, the query will "+ "default to the token of the Consul agent at the HTTP address.") - f.BoolVar(&m.stale, "stale", false, + + return f +} + +// httpFlagsServer is the list of flags that apply to HTTP connections. +func (c *Command) httpFlagsServer(f *flag.FlagSet) *flag.FlagSet { + if f == nil { + f = flag.NewFlagSet("", flag.ContinueOnError) + } + + f.StringVar(&c.datacenter, "datacenter", "", + "Name of the datacenter to query. If unspecified, this will default to "+ + "the datacenter of the queried agent.") + f.BoolVar(&c.stale, "stale", false, "Permit any Consul server (non-leader) to respond to this request. This "+ "allows for lower latency and higher throughput, but can result in "+ "stale data. This option has no effect on non-read operations. The "+ @@ -84,72 +104,95 @@ func (m *Meta) httpFlags(f *flag.FlagSet) *flag.FlagSet { // 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 { +func (c *Command) NewFlagSet(command cli.Command) *flag.FlagSet { f := flag.NewFlagSet("", flag.ContinueOnError) - f.Usage = func() { m.Ui.Error(c.Help()) } + f.Usage = func() { c.Ui.Error(command.Help()) } + + if c.hasClientHTTP() { + c.httpFlagsClient(f) + } - if m.hasHTTP() { - m.httpFlags(f) + if c.hasServerHTTP() { + c.httpFlagsServer(f) } errR, errW := io.Pipe() errScanner := bufio.NewScanner(errR) go func() { for errScanner.Scan() { - m.Ui.Error(errScanner.Text()) + c.Ui.Error(errScanner.Text()) } }() f.SetOutput(errW) - m.flagSet = f + c.flagSet = f return f } // Parse is used to parse the underlying flag set. -func (m *Meta) Parse(args []string) error { - return m.flagSet.Parse(args) +func (c *Command) Parse(args []string) error { + return c.flagSet.Parse(args) } // Help returns the help for this flagSet. -func (m *Meta) Help() string { - return m.helpFlagsFor(m.flagSet) +func (c *Command) Help() string { + return c.helpFlagsFor(c.flagSet) +} + +// hasClientHTTP returns true if this meta command contains client HTTP flags. +func (c *Command) hasClientHTTP() bool { + return c.Flags&FlagSetClientHTTP != 0 } -// hasHTTP returns true if this meta command contains HTTP flags. -func (m *Meta) hasHTTP() bool { - return m.Flags&FlagSetHTTP != 0 +// hasServerHTTP returns true if this meta command contains server HTTP flags. +func (c *Command) hasServerHTTP() bool { + return c.Flags&FlagSetServerHTTP != 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) +func (c *Command) helpFlagsFor(f *flag.FlagSet) string { + httpFlagsClient := c.httpFlagsClient(nil) + httpFlagsServer := c.httpFlagsServer(nil) var out bytes.Buffer - first := true + firstHTTP := true + if c.hasClientHTTP() { + if firstHTTP { + printTitle(&out, "HTTP API Options") + firstHTTP = false + } + httpFlagsClient.VisitAll(func(f *flag.Flag) { + printFlag(&out, f) + }) + } + if c.hasServerHTTP() { + if firstHTTP { + printTitle(&out, "HTTP API Options") + firstHTTP = false + } + httpFlagsServer.VisitAll(func(f *flag.Flag) { + printFlag(&out, f) + }) + } + + firstCommand := true f.VisitAll(func(f *flag.Flag) { // Skip HTTP flags as they will be grouped separately - if flagContains(httpFlags, f) { + if flagContains(httpFlagsClient, f) || flagContains(httpFlagsServer, f) { return } - if first { + if firstCommand { printTitle(&out, "Command Options") - first = false + firstCommand = false } printFlag(&out, f) }) - if m.hasHTTP() { - printTitle(&out, "HTTP API Options") - httpFlags.VisitAll(func(f *flag.Flag) { - printFlag(&out, f) - }) - } - return strings.TrimRight(out.String(), "\n") } @@ -190,7 +233,7 @@ func flagContains(fs *flag.FlagSet, f *flag.Flag) bool { return skip } -// wrapAtLength wraps the given text at the maxLineLength, taxing into account +// wrapAtLength wraps the given text at the maxLineLength, taking into account // any provided left padding. func wrapAtLength(s string, pad int) string { wrapped := text.Wrap(s, maxLineLength-pad) diff --git a/command/configtest.go b/command/configtest.go index a1de6d82ead7..81eafef512ea 100644 --- a/command/configtest.go +++ b/command/configtest.go @@ -5,14 +5,13 @@ import ( "strings" "github.com/hashicorp/consul/command/agent" + "github.com/hashicorp/consul/command/base" ) // ConfigTestCommand is a Command implementation that is used to // verify config files type ConfigTestCommand struct { - Meta - - configFiles []string + base.Command } func (c *ConfigTestCommand) Help() string { @@ -27,29 +26,31 @@ Usage: consul configtest [options] Returns 0 if the configuration is valid, or 1 if there are problems. -` + c.Meta.Help() +` + c.Command.Help() return strings.TrimSpace(helpText) } func (c *ConfigTestCommand) Run(args []string) int { - f := c.Meta.NewFlagSet(c) - f.Var((*agent.AppendSliceValue)(&c.configFiles), "config-file", + var configFiles []string + + f := c.Command.NewFlagSet(c) + f.Var((*agent.AppendSliceValue)(&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 "+ + f.Var((*agent.AppendSliceValue)(&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 { + if err := c.Command.Parse(args); err != nil { return 1 } - if len(c.configFiles) <= 0 { + if len(configFiles) <= 0 { c.Ui.Error("Must specify config using -config-file or -config-dir") return 1 } - _, err := agent.ReadConfigPaths(c.configFiles) + _, err := agent.ReadConfigPaths(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 f3ff3415c096..766ad594bf4b 100644 --- a/command/configtest_test.go +++ b/command/configtest_test.go @@ -6,15 +6,16 @@ import ( "path/filepath" "testing" + "github.com/hashicorp/consul/command/base" "github.com/mitchellh/cli" ) func testConfigTestCommand(t *testing.T) (*cli.MockUi, *ConfigTestCommand) { ui := new(cli.MockUi) return ui, &ConfigTestCommand{ - Meta: Meta{ + Command: base.Command{ Ui: ui, - Flags: FlagSetNone, + Flags: base.FlagSetNone, }, } } diff --git a/command/force_leave.go b/command/force_leave.go index bb23ae942a94..778cc9d46c71 100644 --- a/command/force_leave.go +++ b/command/force_leave.go @@ -2,18 +2,19 @@ package command import ( "fmt" + "github.com/hashicorp/consul/command/base" "strings" ) // ForceLeaveCommand is a Command implementation that tells a running Consul // to force a member to enter the "left" state. type ForceLeaveCommand struct { - Meta + base.Command } func (c *ForceLeaveCommand) Run(args []string) int { - f := c.Meta.NewFlagSet(c) - if err := c.Meta.Parse(args); err != nil { + f := c.Command.NewFlagSet(c) + if err := c.Command.Parse(args); err != nil { return 1 } @@ -25,7 +26,7 @@ func (c *ForceLeaveCommand) Run(args []string) int { return 1 } - client, err := c.Meta.HTTPClient() + client, err := c.Command.HTTPClient() if err != nil { c.Ui.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err)) return 1 @@ -55,7 +56,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. -` + c.Meta.Help() +` + c.Command.Help() return strings.TrimSpace(helpText) } diff --git a/command/force_leave_test.go b/command/force_leave_test.go index fb6b55c4d8f9..257ce1b2b378 100644 --- a/command/force_leave_test.go +++ b/command/force_leave_test.go @@ -3,6 +3,7 @@ package command import ( "errors" "fmt" + "github.com/hashicorp/consul/command/base" "github.com/hashicorp/consul/testutil" "github.com/hashicorp/serf/serf" "github.com/mitchellh/cli" @@ -13,9 +14,9 @@ import ( func testForceLeaveCommand(t *testing.T) (*cli.MockUi, *ForceLeaveCommand) { ui := new(cli.MockUi) return ui, &ForceLeaveCommand{ - Meta: Meta{ + Command: base.Command{ Ui: ui, - Flags: FlagSetHTTP, + Flags: base.FlagSetClientHTTP, }, } } diff --git a/command/lock.go b/command/lock.go index e10e28c5e198..08b53fc3c8aa 100644 --- a/command/lock.go +++ b/command/lock.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/command/agent" + "github.com/hashicorp/consul/command/base" ) const ( @@ -35,19 +36,14 @@ const ( // LockCommand is a Command implementation that is used to setup // a "lock" which manages lock acquisition and invokes a sub-process type LockCommand struct { - Meta + base.Command ShutdownCh <-chan struct{} child *os.Process childLock sync.Mutex - limit int - monitorRetry int - name string - passStdin bool - timeout time.Duration - verbose bool + verbose bool } func (c *LockCommand) Help() string { @@ -69,7 +65,7 @@ Usage: consul lock [options] prefix child... The prefix provided must have write privileges. -` + c.Meta.Help() +` + c.Command.Help() return strings.TrimSpace(helpText) } @@ -81,39 +77,44 @@ func (c *LockCommand) Run(args []string) int { func (c *LockCommand) run(args []string, lu **LockUnlock) int { var childDone chan struct{} - - f := c.Meta.NewFlagSet(c) - f.IntVar(&c.limit, "n", 1, + var limit int + var monitorRetry int + var name string + var passStdin bool + var timeout time.Duration + + f := c.Command.NewFlagSet(c) + f.IntVar(&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.") - f.IntVar(&c.monitorRetry, "monitor-retry", defaultMonitorRetry, - "Number of times to retry Consul returns a 500 error while monitoring "+ + f.IntVar(&monitorRetry, "monitor-retry", defaultMonitorRetry, + "Number of times to retry if Consul returns a 500 error while monitoring "+ "the lock. This allows riding out brief periods of unavailability "+ "without causing leader elections, but increases the amount of time "+ "required to detect a lost lock in some cases. The default value is 3, "+ "with a 1s wait between retries. Set this value to 0 to disable retires.") - f.StringVar(&c.name, "name", "", + f.StringVar(&name, "name", "", "Optional name to associate with the lock session. It not provided, one "+ "is generated based on the provided child command.") - f.BoolVar(&c.passStdin, "pass-stdin", false, + f.BoolVar(&passStdin, "pass-stdin", false, "Pass stdin to the child process.") - f.DurationVar(&c.timeout, "timeout", 0, + f.DurationVar(&timeout, "timeout", 0, "Maximum amount of time to wait to acquire the lock, specified as a "+ "timestamp like \"1s\" or \"3h\". The default value is 0.") f.BoolVar(&c.verbose, "verbose", false, "Enable verbose (debugging) output.") // Deprecations - f.DurationVar(&c.timeout, "try", 0, + f.DurationVar(&timeout, "try", 0, "DEPRECATED. Use -timeout instead.") - if err := c.Meta.Parse(args); err != nil { + if err := c.Command.Parse(args); err != nil { return 1 } // Check the limit - if c.limit <= 0 { + if limit <= 0 { c.Ui.Error(fmt.Sprintf("Lock holder limit must be positive")) return 1 } @@ -128,27 +129,27 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { prefix = strings.TrimPrefix(prefix, "/") script := strings.Join(extra[1:], " ") - if c.timeout < 0 { + if 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) + if name == "" { + name = fmt.Sprintf("Consul lock for '%s' at '%s'", script, prefix) } // Calculate oneshot - oneshot := c.timeout > 0 + oneshot := timeout > 0 // Check the retry parameter - if c.monitorRetry < 0 { + if monitorRetry < 0 { c.Ui.Error("Number for 'monitor-retry' must be >= 0") return 1 } // Create and test the HTTP client - client, err := c.Meta.HTTPClient() + client, err := c.Command.HTTPClient() if err != nil { c.Ui.Error(fmt.Sprintf("Error connecting to Consul agent: %s", err)) return 1 @@ -160,10 +161,10 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { } // Setup the lock or semaphore - if c.limit == 1 { - *lu, err = c.setupLock(client, prefix, c.name, oneshot, c.timeout, c.monitorRetry) + if limit == 1 { + *lu, err = c.setupLock(client, prefix, name, oneshot, timeout, monitorRetry) } else { - *lu, err = c.setupSemaphore(client, c.limit, prefix, c.name, oneshot, c.timeout, c.monitorRetry) + *lu, err = c.setupSemaphore(client, limit, prefix, name, oneshot, timeout, monitorRetry) } if err != nil { c.Ui.Error(fmt.Sprintf("Lock setup failed: %s", err)) @@ -195,7 +196,7 @@ func (c *LockCommand) run(args []string, lu **LockUnlock) int { // Start the child process childDone = make(chan struct{}) go func() { - if err := c.startChild(script, childDone, c.passStdin); err != nil { + if err := c.startChild(script, childDone, passStdin); err != nil { c.Ui.Error(fmt.Sprintf("%s", err)) } }() diff --git a/command/lock_test.go b/command/lock_test.go index 4172121437b0..efbf028e56f9 100644 --- a/command/lock_test.go +++ b/command/lock_test.go @@ -9,15 +9,16 @@ import ( "time" "github.com/hashicorp/consul/api" + "github.com/hashicorp/consul/command/base" "github.com/mitchellh/cli" ) func testLockCommand(t *testing.T) (*cli.MockUi, *LockCommand) { ui := new(cli.MockUi) return ui, &LockCommand{ - Meta: Meta{ + Command: base.Command{ Ui: ui, - Flags: FlagSetHTTP, + Flags: base.FlagSetHTTP, }, } } diff --git a/commands.go b/commands.go index 1a646f3e5a24..3a9ee39672cc 100644 --- a/commands.go +++ b/commands.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/consul/command" "github.com/hashicorp/consul/command/agent" + "github.com/hashicorp/consul/command/base" "github.com/hashicorp/consul/version" "github.com/mitchellh/cli" ) @@ -31,8 +32,8 @@ func init() { "configtest": func() (cli.Command, error) { return &command.ConfigTestCommand{ - Meta: command.Meta{ - Flags: command.FlagSetNone, + Command: base.Command{ + Flags: base.FlagSetNone, Ui: ui, }, }, nil @@ -53,8 +54,8 @@ func init() { "force-leave": func() (cli.Command, error) { return &command.ForceLeaveCommand{ - Meta: command.Meta{ - Flags: command.FlagSetHTTP, + Command: base.Command{ + Flags: base.FlagSetClientHTTP, Ui: ui, }, }, nil @@ -123,8 +124,8 @@ func init() { "lock": func() (cli.Command, error) { return &command.LockCommand{ ShutdownCh: makeShutdownCh(), - Meta: command.Meta{ - Flags: command.FlagSetHTTP, + Command: base.Command{ + Flags: base.FlagSetHTTP, Ui: ui, }, }, nil diff --git a/website/source/docs/commands/_http_api_options.html.markdown b/website/source/docs/commands/_http_api_options.html.markdown deleted file mode 100644 index ad5e02a0c34d..000000000000 --- a/website/source/docs/commands/_http_api_options.html.markdown +++ /dev/null @@ -1,16 +0,0 @@ -* `-http-addr=` - Address of the Consul agent with the port. This can be - an IP address or DNS address, but it must include the port. This can also be - specified via the CONSUL_HTTP_ADDR environment variable. The default value is - 127.0.0.1:8500. - -* `-datacenter=` - Name of the datacenter to query. If unspecified, the - query will default to the datacenter of the Consul agent at the HTTP address. - -* `-token=` - ACL token to use in the request. This can also be specified - via the `CONSUL_HTTP_TOKEN` environment variable. If unspecified, the query - will default to the token of the Consul agent at the HTTP address. - -* `-stale` - Permit any Consul server (non-leader) to respond to this request. - This allows for lower latency and higher throughput, but can result in stale - data. This option has no effect on non-read operations. The default value is - false. diff --git a/website/source/docs/commands/_http_api_options_client.html.markdown b/website/source/docs/commands/_http_api_options_client.html.markdown new file mode 100644 index 000000000000..ccdf5176b786 --- /dev/null +++ b/website/source/docs/commands/_http_api_options_client.html.markdown @@ -0,0 +1,8 @@ +* `-http-addr=` - Address of the Consul agent with the port. This can be + an IP address or DNS address, but it must include the port. This can also be + specified via the `CONSUL_HTTP_ADDR` environment variable. The default value is + 127.0.0.1:8500. + +* `-token=` - ACL token to use in the request. This can also be specified + via the `CONSUL_HTTP_TOKEN` environment variable. If unspecified, the query + will default to the token of the Consul agent at the HTTP address. diff --git a/website/source/docs/commands/_http_api_options_server.html.markdown b/website/source/docs/commands/_http_api_options_server.html.markdown new file mode 100644 index 000000000000..ab042f3cf622 --- /dev/null +++ b/website/source/docs/commands/_http_api_options_server.html.markdown @@ -0,0 +1,7 @@ +* `-datacenter=` - Name of the datacenter to query. If unspecified, the + query will default to the datacenter of the Consul agent at the HTTP address. + +* `-stale` - Permit any Consul server (non-leader) to respond to this request. + This allows for lower latency and higher throughput, but can result in stale + data. This option has no effect on non-read operations. The default value is + false. \ No newline at end of file diff --git a/website/source/docs/commands/force-leave.html.markdown b/website/source/docs/commands/force-leave.html.markdown.erb similarity index 76% rename from website/source/docs/commands/force-leave.html.markdown rename to website/source/docs/commands/force-leave.html.markdown.erb index 2c07c233c2cb..70a6c3d05b2c 100644 --- a/website/source/docs/commands/force-leave.html.markdown +++ b/website/source/docs/commands/force-leave.html.markdown.erb @@ -28,11 +28,7 @@ as it will be removed from the Raft quorum. Usage: `consul force-leave [options] node` -The following command-line options are available for this command. -Every option is optional: +#### API Options -* `-rpc-addr` - Address to the RPC server of the agent you want to contact - to send this command. If this isn't specified, the command checks the - `CONSUL_RPC_ADDR` env variable. If this isn't set, the default RPC - address will be set to `127.0.0.1:8400`. +<%= partial "docs/commands/http_api_options_client" %> diff --git a/website/source/docs/commands/kv/delete.html.markdown.erb b/website/source/docs/commands/kv/delete.html.markdown.erb index e716437cb7cb..05295cbf8d25 100644 --- a/website/source/docs/commands/kv/delete.html.markdown.erb +++ b/website/source/docs/commands/kv/delete.html.markdown.erb @@ -17,7 +17,8 @@ Usage: `consul kv delete [options] KEY_OR_PREFIX` #### API Options -<%= partial "docs/commands/http_api_options" %> +<%= partial "docs/commands/http_api_options_client" %> +<%= partial "docs/commands/http_api_options_server" %> #### KV Delete Options diff --git a/website/source/docs/commands/kv/export.html.markdown.erb b/website/source/docs/commands/kv/export.html.markdown.erb index 2c3887d0db67..06bed5fa4468 100644 --- a/website/source/docs/commands/kv/export.html.markdown.erb +++ b/website/source/docs/commands/kv/export.html.markdown.erb @@ -19,7 +19,8 @@ Usage: `consul kv export [PREFIX]` #### API Options -<%= partial "docs/commands/http_api_options" %> +<%= partial "docs/commands/http_api_options_client" %> +<%= partial "docs/commands/http_api_options_server" %> ## Examples diff --git a/website/source/docs/commands/kv/get.html.markdown.erb b/website/source/docs/commands/kv/get.html.markdown.erb index 49c9b6ca33fd..07ec0969e475 100644 --- a/website/source/docs/commands/kv/get.html.markdown.erb +++ b/website/source/docs/commands/kv/get.html.markdown.erb @@ -20,7 +20,8 @@ Usage: `consul kv get [options] [KEY_OR_PREFIX]` #### API Options -<%= partial "docs/commands/http_api_options" %> +<%= partial "docs/commands/http_api_options_client" %> +<%= partial "docs/commands/http_api_options_server" %> #### KV Get Options diff --git a/website/source/docs/commands/kv/import.html.markdown.erb b/website/source/docs/commands/kv/import.html.markdown.erb index f0f0d1bb3265..ec06b579e351 100644 --- a/website/source/docs/commands/kv/import.html.markdown.erb +++ b/website/source/docs/commands/kv/import.html.markdown.erb @@ -17,7 +17,8 @@ Usage: `consul kv import [DATA]` #### API Options -<%= partial "docs/commands/http_api_options" %> +<%= partial "docs/commands/http_api_options_client" %> +<%= partial "docs/commands/http_api_options_server" %> ## Examples diff --git a/website/source/docs/commands/kv/put.html.markdown.erb b/website/source/docs/commands/kv/put.html.markdown.erb index 562f75835a46..68189f7722d7 100644 --- a/website/source/docs/commands/kv/put.html.markdown.erb +++ b/website/source/docs/commands/kv/put.html.markdown.erb @@ -16,7 +16,8 @@ Usage: `consul kv put [options] KEY [DATA]` #### API Options -<%= partial "docs/commands/http_api_options" %> +<%= partial "docs/commands/http_api_options_client" %> +<%= partial "docs/commands/http_api_options_server" %> #### KV Put Options diff --git a/website/source/docs/commands/lock.html.markdown b/website/source/docs/commands/lock.html.markdown.erb similarity index 92% rename from website/source/docs/commands/lock.html.markdown rename to website/source/docs/commands/lock.html.markdown.erb index a9b10a12d846..c5069c8a0f16 100644 --- a/website/source/docs/commands/lock.html.markdown +++ b/website/source/docs/commands/lock.html.markdown.erb @@ -45,11 +45,18 @@ of 5 seconds, a `SIGKILL` will be used to force termination. For Consul agents on Windows, the child process is always terminated with a `SIGKILL`, since Windows has no POSIX compatible notion for `SIGTERM`. -The list of available flags are: +#### API Options -* `-http-addr` - Address to the HTTP server of the agent you want to contact - to send this command. If this isn't specified, the command will contact - "127.0.0.1:8500" which is the default HTTP address of a Consul agent. +<%= partial "docs/commands/http_api_options_client" %> +<%= partial "docs/commands/http_api_options_server" %> + +#### Command Options + +* `-monitor-retry` - Retry up to this number of times if Consul returns a 500 error + while monitoring the lock. This allows riding out brief periods of unavailability + without causing leader elections, but increases the amount of time required + to detect a lost lock in some cases. Defaults to 3, with a 1s wait between retries. + Set to 0 to disable. * `-n` - Optional, limit of lock holders. Defaults to 1. The underlying implementation switches from a lock to a semaphore when increased past @@ -58,20 +65,12 @@ The list of available flags are: * `-name` - Optional name to associate with the underlying session. If not provided, one is generated based on the child command. -* `-token` - ACL token to use. Defaults to that of agent. - * `-pass-stdin` - Pass stdin to child process. * `-try` - Attempt to acquire the lock up to the given timeout. The timeout is a positive decimal number, with unit suffix, such as "500ms". Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h". -* `-monitor-retry` - Retry up to this number of times if Consul returns a 500 error - while monitoring the lock. This allows riding out brief periods of unavailability - without causing leader elections, but increases the amount of time required - to detect a lost lock in some cases. Defaults to 3, with a 1s wait between retries. - Set to 0 to disable. - * `-verbose` - Enables verbose output. ## SHELL diff --git a/website/source/docs/commands/snapshot/agent.html.markdown.erb b/website/source/docs/commands/snapshot/agent.html.markdown.erb index d01afab04235..fdf34ec3dd79 100644 --- a/website/source/docs/commands/snapshot/agent.html.markdown.erb +++ b/website/source/docs/commands/snapshot/agent.html.markdown.erb @@ -60,7 +60,7 @@ Usage: `consul snapshot agent [options]` #### API Options -<%= partial "docs/commands/http_api_options" %> +<%= partial "docs/commands/http_api_options_client" %> #### Config File Options: diff --git a/website/source/docs/commands/snapshot/restore.html.markdown.erb b/website/source/docs/commands/snapshot/restore.html.markdown.erb index 1f5018f873c0..8a28440ad345 100644 --- a/website/source/docs/commands/snapshot/restore.html.markdown.erb +++ b/website/source/docs/commands/snapshot/restore.html.markdown.erb @@ -27,7 +27,7 @@ Usage: `consul snapshot restore [options] FILE` #### API Options -<%= partial "docs/commands/http_api_options" %> +<%= partial "docs/commands/http_api_options_client" %> ## Examples diff --git a/website/source/docs/commands/snapshot/save.html.markdown.erb b/website/source/docs/commands/snapshot/save.html.markdown.erb index bb353eb284d3..e6029172ad1f 100644 --- a/website/source/docs/commands/snapshot/save.html.markdown.erb +++ b/website/source/docs/commands/snapshot/save.html.markdown.erb @@ -22,7 +22,7 @@ Usage: `consul snapshot save [options] FILE` #### API Options -<%= partial "docs/commands/http_api_options" %> +<%= partial "docs/commands/http_api_options_client" %> ## Examples From 197dc10a7fe42d9913485b1c80b2bd205847b62f Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Tue, 7 Feb 2017 20:05:16 -0500 Subject: [PATCH 6/7] Add utility types to enable checking for unset flags --- command/base/command.go | 28 ++- command/base/config_util.go | 319 ++++++++++++++++++++++++++++++ command/base/config_util_test.go | 128 ++++++++++++ test/command/merge/a.json | 7 + test/command/merge/b.json | 7 + test/command/merge/empty/foo.json | 3 + test/command/merge/nope | 7 + test/command/merge/subdir/c.json | 7 + test/command/merge/zero.json | 0 9 files changed, 489 insertions(+), 17 deletions(-) create mode 100644 command/base/config_util.go create mode 100644 command/base/config_util_test.go create mode 100644 test/command/merge/a.json create mode 100644 test/command/merge/b.json create mode 100644 test/command/merge/empty/foo.json create mode 100644 test/command/merge/nope create mode 100644 test/command/merge/subdir/c.json create mode 100644 test/command/merge/zero.json diff --git a/command/base/command.go b/command/base/command.go index 032b8026333c..ba159afd19b5 100644 --- a/command/base/command.go +++ b/command/base/command.go @@ -35,10 +35,10 @@ type Command struct { flagSet *flag.FlagSet // These are the options which correspond to the HTTP API options - httpAddr string - token string - datacenter string - stale bool + httpAddr stringValue + token stringValue + datacenter stringValue + stale boolValue } // HTTPClient returns a client with the parsed flags. It panics if the command @@ -52,15 +52,9 @@ func (c *Command) HTTPClient() (*api.Client, error) { } config := api.DefaultConfig() - if c.datacenter != "" { - config.Datacenter = c.datacenter - } - if c.httpAddr != "" { - config.Address = c.httpAddr - } - if c.token != "" { - config.Token = c.token - } + c.httpAddr.Merge(&config.Address) + c.token.Merge(&config.Token) + c.datacenter.Merge(&config.Datacenter) c.Ui.Info(fmt.Sprintf("client http addr: %s", config.Address)) return api.NewClient(config) } @@ -71,12 +65,12 @@ func (c *Command) httpFlagsClient(f *flag.FlagSet) *flag.FlagSet { f = flag.NewFlagSet("", flag.ContinueOnError) } - f.StringVar(&c.httpAddr, "http-addr", "", + f.Var(&c.httpAddr, "http-addr", "Address and port to the Consul HTTP 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_HTTP_ADDR environment variable. The "+ "default value is 127.0.0.1:8500.") - f.StringVar(&c.token, "token", "", + f.Var(&c.token, "token", "ACL token to use in the request. This can also be specified via the "+ "CONSUL_HTTP_TOKEN environment variable. If unspecified, the query will "+ "default to the token of the Consul agent at the HTTP address.") @@ -90,10 +84,10 @@ func (c *Command) httpFlagsServer(f *flag.FlagSet) *flag.FlagSet { f = flag.NewFlagSet("", flag.ContinueOnError) } - f.StringVar(&c.datacenter, "datacenter", "", + f.Var(&c.datacenter, "datacenter", "Name of the datacenter to query. If unspecified, this will default to "+ "the datacenter of the queried agent.") - f.BoolVar(&c.stale, "stale", false, + f.Var(&c.stale, "stale", "Permit any Consul server (non-leader) to respond to this request. This "+ "allows for lower latency and higher throughput, but can result in "+ "stale data. This option has no effect on non-read operations. The "+ diff --git a/command/base/config_util.go b/command/base/config_util.go new file mode 100644 index 000000000000..84969925958d --- /dev/null +++ b/command/base/config_util.go @@ -0,0 +1,319 @@ +package base + +import ( + "fmt" + "reflect" + "strconv" + "time" + + "github.com/mitchellh/mapstructure" + "os" + "path/filepath" + "sort" +) + +// TODO (slackpad) - Trying out a different pattern here for config handling. +// These classes support the flag.Value interface but work in a manner where +// we can tell if they have been set. This lets us work with an all-pointer +// config structure and merge it in a clean-ish way. If this ends up being a +// good pattern we should pull this out into a reusable library. + +// configDecodeHook should be passed to mapstructure in order to decode into +// the *Value objects here. +var configDecodeHook = mapstructure.ComposeDecodeHookFunc( + boolToBoolValueFunc(), + stringToDurationValueFunc(), + stringToStringValueFunc(), + float64ToUintValueFunc(), +) + +// boolValue provides a flag value that's aware if it has been set. +type boolValue struct { + v *bool +} + +// See flag.Value. +func (b *boolValue) IsBoolFlag() bool { + return true +} + +// Merge will overlay this value if it has been set. +func (b *boolValue) Merge(onto *bool) { + if b.v != nil { + *onto = *(b.v) + } +} + +// See flag.Value. +func (b *boolValue) Set(v string) error { + if b.v == nil { + b.v = new(bool) + } + var err error + *(b.v), err = strconv.ParseBool(v) + return err +} + +// See flag.Value. +func (b *boolValue) String() string { + var current bool + if b.v != nil { + current = *(b.v) + } + return fmt.Sprintf("%v", current) +} + +// boolToBoolValueFunc is a mapstructure hook that looks for an incoming bool +// mapped to a boolValue and does the translation. +func boolToBoolValueFunc() mapstructure.DecodeHookFunc { + return func( + f reflect.Type, + t reflect.Type, + data interface{}) (interface{}, error) { + if f.Kind() != reflect.Bool { + return data, nil + } + + val := boolValue{} + if t != reflect.TypeOf(val) { + return data, nil + } + + val.v = new(bool) + *(val.v) = data.(bool) + return val, nil + } +} + +// durationValue provides a flag value that's aware if it has been set. +type durationValue struct { + v *time.Duration +} + +// Merge will overlay this value if it has been set. +func (d *durationValue) Merge(onto *time.Duration) { + if d.v != nil { + *onto = *(d.v) + } +} + +// See flag.Value. +func (d *durationValue) Set(v string) error { + if d.v == nil { + d.v = new(time.Duration) + } + var err error + *(d.v), err = time.ParseDuration(v) + return err +} + +// See flag.Value. +func (d *durationValue) String() string { + var current time.Duration + if d.v != nil { + current = *(d.v) + } + return current.String() +} + +// stringToDurationValueFunc is a mapstructure hook that looks for an incoming +// string mapped to a durationValue and does the translation. +func stringToDurationValueFunc() mapstructure.DecodeHookFunc { + return func( + f reflect.Type, + t reflect.Type, + data interface{}) (interface{}, error) { + if f.Kind() != reflect.String { + return data, nil + } + + val := durationValue{} + if t != reflect.TypeOf(val) { + return data, nil + } + if err := val.Set(data.(string)); err != nil { + return nil, err + } + return val, nil + } +} + +// stringValue provides a flag value that's aware if it has been set. +type stringValue struct { + v *string +} + +// Merge will overlay this value if it has been set. +func (s *stringValue) Merge(onto *string) { + if s.v != nil { + *onto = *(s.v) + } +} + +// See flag.Value. +func (s *stringValue) Set(v string) error { + if s.v == nil { + s.v = new(string) + } + *(s.v) = v + return nil +} + +// See flag.Value. +func (s *stringValue) String() string { + var current string + if s.v != nil { + current = *(s.v) + } + return current +} + +// stringToStringValueFunc is a mapstructure hook that looks for an incoming +// string mapped to a stringValue and does the translation. +func stringToStringValueFunc() mapstructure.DecodeHookFunc { + return func( + f reflect.Type, + t reflect.Type, + data interface{}) (interface{}, error) { + if f.Kind() != reflect.String { + return data, nil + } + + val := stringValue{} + if t != reflect.TypeOf(val) { + return data, nil + } + val.v = new(string) + *(val.v) = data.(string) + return val, nil + } +} + +// uintValue provides a flag value that's aware if it has been set. +type uintValue struct { + v *uint +} + +// Merge will overlay this value if it has been set. +func (u *uintValue) Merge(onto *uint) { + if u.v != nil { + *onto = *(u.v) + } +} + +// See flag.Value. +func (u *uintValue) Set(v string) error { + if u.v == nil { + u.v = new(uint) + } + parsed, err := strconv.ParseUint(v, 0, 64) + *(u.v) = (uint)(parsed) + return err +} + +// See flag.Value. +func (u *uintValue) String() string { + var current uint + if u.v != nil { + current = *(u.v) + } + return fmt.Sprintf("%v", current) +} + +// float64ToUintValueFunc is a mapstructure hook that looks for an incoming +// float64 mapped to a uintValue and does the translation. +func float64ToUintValueFunc() mapstructure.DecodeHookFunc { + return func( + f reflect.Type, + t reflect.Type, + data interface{}) (interface{}, error) { + if f.Kind() != reflect.Float64 { + return data, nil + } + + val := uintValue{} + if t != reflect.TypeOf(val) { + return data, nil + } + + fv := data.(float64) + if fv < 0 { + return nil, fmt.Errorf("value cannot be negative") + } + + // The standard guarantees at least this, and this is fine for + // values we expect to use in configs vs. being fancy with the + // machine's size for uint. + if fv > (1<<32 - 1) { + return nil, fmt.Errorf("value is too large") + } + + val.v = new(uint) + *(val.v) = (uint)(fv) + return val, nil + } +} + +// visitFn is a callback that gets a chance to visit each file found during a +// traversal with visit(). +type visitFn func(path string) error + +// visit will call the visitor function on the path if it's a file, or for each +// file in the path if it's a directory. Directories will not be recursed into, +// and files in the directory will be visited in alphabetical order. +func visit(path string, visitor visitFn) error { + f, err := os.Open(path) + if err != nil { + return fmt.Errorf("error reading %q: %v", path, err) + } + defer f.Close() + + fi, err := f.Stat() + if err != nil { + return fmt.Errorf("error checking %q: %v", path, err) + } + + if !fi.IsDir() { + if err := visitor(path); err != nil { + return fmt.Errorf("error in %q: %v", path, err) + } + return nil + } + + contents, err := f.Readdir(-1) + if err != nil { + return fmt.Errorf("error listing %q: %v", path, err) + } + + sort.Sort(dirEnts(contents)) + for _, fi := range contents { + if fi.IsDir() { + continue + } + + fullPath := filepath.Join(path, fi.Name()) + if err := visitor(fullPath); err != nil { + return fmt.Errorf("error in %q: %v", fullPath, err) + } + } + + return nil +} + +// dirEnts applies sort.Interface to directory entries for sorting by name. +type dirEnts []os.FileInfo + +// See sort.Interface. +func (d dirEnts) Len() int { + return len(d) +} + +// See sort.Interface. +func (d dirEnts) Less(i, j int) bool { + return d[i].Name() < d[j].Name() +} + +// See sort.Interface. +func (d dirEnts) Swap(i, j int) { + d[i], d[j] = d[j], d[i] +} diff --git a/command/base/config_util_test.go b/command/base/config_util_test.go new file mode 100644 index 000000000000..b8264751fb4c --- /dev/null +++ b/command/base/config_util_test.go @@ -0,0 +1,128 @@ +package base + +import ( + "bytes" + "encoding/json" + "fmt" + "strings" + "testing" + + "github.com/mitchellh/mapstructure" + "path" + "reflect" +) + +func TestConfigUtil_Values(t *testing.T) { + type config struct { + B boolValue `mapstructure:"bool"` + D durationValue `mapstructure:"duration"` + S stringValue `mapstructure:"string"` + U uintValue `mapstructure:"uint"` + } + + cases := []struct { + in string + success string + failure string + }{ + { + `{ }`, + `"false" "0s" "" "0"`, + "", + }, + { + `{ "bool": true, "duration": "2h", "string": "hello", "uint": 23 }`, + `"true" "2h0m0s" "hello" "23"`, + "", + }, + { + `{ "bool": "nope" }`, + "", + "got 'string'", + }, + { + `{ "duration": "nope" }`, + "", + "invalid duration nope", + }, + { + `{ "string": 123 }`, + "", + "got 'float64'", + }, + { + `{ "uint": -1 }`, + "", + "value cannot be negative", + }, + { + `{ "uint": 4294967296 }`, + "", + "value is too large", + }, + } + for i, c := range cases { + var raw interface{} + dec := json.NewDecoder(bytes.NewBufferString(c.in)) + if err := dec.Decode(&raw); err != nil { + t.Fatalf("(case %d) err: %v", i, err) + } + + var r config + msdec, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + DecodeHook: configDecodeHook, + Result: &r, + ErrorUnused: true, + }) + if err != nil { + t.Fatalf("(case %d) err: %v", i, err) + } + + err = msdec.Decode(raw) + if c.failure != "" { + if err == nil || !strings.Contains(err.Error(), c.failure) { + t.Fatalf("(case %d) err: %v", i, err) + } + continue + } + if err != nil { + t.Fatalf("(case %d) err: %v", i, err) + } + + actual := fmt.Sprintf("%q %q %q %q", + r.B.String(), + r.D.String(), + r.S.String(), + r.U.String()) + if actual != c.success { + t.Fatalf("(case %d) bad: %s", i, actual) + } + } +} + +func TestConfigUtil_Visit(t *testing.T) { + var trail []string + visitor := func(path string) error { + trail = append(trail, path) + return nil + } + + basePath := "../../test/command/merge" + if err := visit(basePath, visitor); err != nil { + t.Fatalf("err: %v", err) + } + if err := visit(path.Join(basePath, "subdir", "c.json"), visitor); err != nil { + t.Fatalf("err: %v", err) + } + + expected := []string{ + path.Join(basePath, "a.json"), + path.Join(basePath, "b.json"), + path.Join(basePath, "nope"), + path.Join(basePath, "zero.json"), + path.Join(basePath, "subdir", "c.json"), + } + if !reflect.DeepEqual(trail, expected) { + t.Fatalf("bad: %#v", trail) + } +} diff --git a/test/command/merge/a.json b/test/command/merge/a.json new file mode 100644 index 000000000000..50d6c60128da --- /dev/null +++ b/test/command/merge/a.json @@ -0,0 +1,7 @@ +{ + "snapshot_agent": { + "snapshot": { + "interval": "1h" + } + } +} diff --git a/test/command/merge/b.json b/test/command/merge/b.json new file mode 100644 index 000000000000..ddf3dff8570a --- /dev/null +++ b/test/command/merge/b.json @@ -0,0 +1,7 @@ +{ + "snapshot_agent": { + "snapshot": { + "interval": "2h" + } + } +} diff --git a/test/command/merge/empty/foo.json b/test/command/merge/empty/foo.json new file mode 100644 index 000000000000..756fa36b9baa --- /dev/null +++ b/test/command/merge/empty/foo.json @@ -0,0 +1,3 @@ +{ + "not_related": true +} diff --git a/test/command/merge/nope b/test/command/merge/nope new file mode 100644 index 000000000000..0c947af68aca --- /dev/null +++ b/test/command/merge/nope @@ -0,0 +1,7 @@ +{ + "snapshot_agent": { + "snapshot": { + "interval": "3h" + } + } +} diff --git a/test/command/merge/subdir/c.json b/test/command/merge/subdir/c.json new file mode 100644 index 000000000000..a72381acb045 --- /dev/null +++ b/test/command/merge/subdir/c.json @@ -0,0 +1,7 @@ +{ + "snapshot_agent": { + "snapshot": { + "interval": "5h" + } + } +} diff --git a/test/command/merge/zero.json b/test/command/merge/zero.json new file mode 100644 index 000000000000..e69de29bb2d1 From 0b85bbfed1545fb09740395ffae8d7822973b70a Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Tue, 7 Feb 2017 20:56:49 -0500 Subject: [PATCH 7/7] Small tweaks to base command --- command/base/command.go | 7 +++---- command/base/config_util.go | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/command/base/command.go b/command/base/command.go index ba159afd19b5..3d4020924fc9 100644 --- a/command/base/command.go +++ b/command/base/command.go @@ -21,9 +21,9 @@ const maxLineLength int = 72 type FlagSetFlags uint const ( - FlagSetNone FlagSetFlags = iota << 1 - FlagSetClientHTTP FlagSetFlags = iota << 1 - FlagSetServerHTTP FlagSetFlags = iota << 1 + FlagSetNone FlagSetFlags = 1 << iota + FlagSetClientHTTP FlagSetFlags = 1 << iota + FlagSetServerHTTP FlagSetFlags = 1 << iota FlagSetHTTP = FlagSetClientHTTP | FlagSetServerHTTP ) @@ -55,7 +55,6 @@ func (c *Command) HTTPClient() (*api.Client, error) { c.httpAddr.Merge(&config.Address) c.token.Merge(&config.Token) c.datacenter.Merge(&config.Datacenter) - c.Ui.Info(fmt.Sprintf("client http addr: %s", config.Address)) return api.NewClient(config) } diff --git a/command/base/config_util.go b/command/base/config_util.go index 84969925958d..7eac69cd27d2 100644 --- a/command/base/config_util.go +++ b/command/base/config_util.go @@ -2,14 +2,14 @@ package base import ( "fmt" + "os" + "path/filepath" "reflect" + "sort" "strconv" "time" "github.com/mitchellh/mapstructure" - "os" - "path/filepath" - "sort" ) // TODO (slackpad) - Trying out a different pattern here for config handling.