Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Centralize command-line parsing #2412

Merged
merged 1 commit into from
Feb 8, 2017
Merged

Centralize command-line parsing #2412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
148 changes: 62 additions & 86 deletions command/lock.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package command

import (
"flag"
"fmt"
"os"
"path"
Expand All @@ -12,7 +11,6 @@ import (

"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/command/agent"
"github.com/mitchellh/cli"
)

const (
Expand All @@ -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 := `
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you consider putting the intro text into the meta object as well and then you'd just return c.Meta.Help() here? If we had markdown in here we could strip it for the CLI and generate the web versions with some magic :-)

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

Expand All @@ -93,81 +79,71 @@ 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]
prefix = strings.TrimPrefix(prefix, "/")
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
Expand All @@ -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))
Expand Down Expand Up @@ -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))
}
}()
Expand Down
34 changes: 18 additions & 16 deletions command/lock_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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}
Expand All @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down
Loading