From 7a6ca308c487862e8ded7252fa0e173b0b7889be Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 11 Feb 2016 13:52:32 -0800 Subject: [PATCH] FIX #3016 Add timeout to active command Signed-off-by: David Gageot --- commands/active.go | 9 ++++++++- commands/commands.go | 9 ++++++++- commands/ls.go | 20 +++++++++----------- commands/ls_test.go | 16 ++++++---------- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/commands/active.go b/commands/active.go index cdfa5d3be2..968a01964f 100644 --- a/commands/active.go +++ b/commands/active.go @@ -4,10 +4,16 @@ import ( "errors" "fmt" + "time" + "github.com/docker/machine/libmachine" "github.com/docker/machine/libmachine/persist" ) +const ( + activeDefaultTimeout = 10 +) + var ( errNoActiveHost = errors.New("No active host found") ) @@ -22,7 +28,8 @@ func cmdActive(c CommandLine, api libmachine.API) error { return fmt.Errorf("Error getting active host: %s", err) } - items := getHostListItems(hosts, hostsInError) + timeout := time.Duration(c.Int("timeout")) * time.Second + items := getHostListItems(hosts, hostsInError, timeout) active, err := activeHost(items) diff --git a/commands/commands.go b/commands/commands.go index 44b8b0a497..5a8afbe007 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -195,6 +195,13 @@ var Commands = []cli.Command{ Name: "active", Usage: "Print which machine is active", Action: runCommand(cmdActive), + Flags: []cli.Flag{ + cli.IntFlag{ + Name: "timeout, t", + Usage: fmt.Sprintf("Timeout in seconds, default to %ds", activeDefaultTimeout), + Value: activeDefaultTimeout, + }, + }, }, { Name: "config", @@ -281,7 +288,7 @@ var Commands = []cli.Command{ }, cli.IntFlag{ Name: "timeout, t", - Usage: fmt.Sprintf("Timeout in seconds, default to %s", stateTimeoutDuration), + Usage: fmt.Sprintf("Timeout in seconds, default to %ds", lsDefaultTimeout), Value: lsDefaultTimeout, }, cli.StringFlag{ diff --git a/commands/ls.go b/commands/ls.go index 85ca1e9471..7375c29070 100644 --- a/commands/ls.go +++ b/commands/ls.go @@ -32,8 +32,6 @@ const ( ) var ( - stateTimeoutDuration = lsDefaultTimeout * time.Second - headers = map[string]string{ "Name": "NAME", "Active": "ACTIVE", @@ -77,9 +75,6 @@ type FilterOptions struct { } func cmdLs(c CommandLine, api libmachine.API) error { - stateTimeoutDuration = time.Duration(c.Int("timeout")) * time.Second - log.Debugf("ls timeout set to %s", stateTimeoutDuration) - filters, err := parseFilters(c.StringSlice("filter")) if err != nil { return err @@ -119,7 +114,8 @@ func cmdLs(c CommandLine, api libmachine.API) error { w = os.Stdout } - items := getHostListItems(hostList, hostInError) + timeout := time.Duration(c.Int("timeout")) * time.Second + items := getHostListItems(hostList, hostInError, timeout) swarmMasters := make(map[string]string) swarmInfo := make(map[string]string) @@ -419,7 +415,7 @@ func attemptGetHostState(h *host.Host, stateQueryChan chan<- HostListItem) { } } -func getHostState(h *host.Host, hostListItemsChan chan<- HostListItem) { +func getHostState(h *host.Host, hostListItemsChan chan<- HostListItem, timeout time.Duration) { // This channel is used to communicate the properties we are querying // about the host in the case of a successful read. stateQueryChan := make(chan HostListItem) @@ -433,22 +429,24 @@ func getHostState(h *host.Host, hostListItemsChan chan<- HostListItem) { hostListItemsChan <- hli // Otherwise, give up after a predetermined duration. - case <-time.After(stateTimeoutDuration): + case <-time.After(timeout): hostListItemsChan <- HostListItem{ Name: h.Name, DriverName: h.Driver.DriverName(), State: state.Timeout, - ResponseTime: stateTimeoutDuration, + ResponseTime: timeout, } } } -func getHostListItems(hostList []*host.Host, hostsInError map[string]error) []HostListItem { +func getHostListItems(hostList []*host.Host, hostsInError map[string]error, timeout time.Duration) []HostListItem { + log.Debugf("timeout set to %s", timeout) + hostListItems := []HostListItem{} hostListItemsChan := make(chan HostListItem) for _, h := range hostList { - go getHostState(h, hostListItemsChan) + go getHostState(h, hostListItemsChan, timeout) } for range hostList { diff --git a/commands/ls_test.go b/commands/ls_test.go index 514dd00e5d..4509c68021 100644 --- a/commands/ls_test.go +++ b/commands/ls_test.go @@ -345,7 +345,7 @@ func TestGetHostListItems(t *testing.T) { {"foo", state.Running, true, "v1.9", ""}, } - items := getHostListItems(hosts, map[string]error{}) + items := getHostListItems(hosts, map[string]error{}, 10*time.Second) for i := range expected { assert.Equal(t, expected[i].name, items[i].Name) @@ -394,8 +394,7 @@ func TestGetHostListItemsEnvDockerHostUnset(t *testing.T) { "baz": {state.Saved, false}, } - // TEST - items := getHostListItems(hosts, map[string]error{}) + items := getHostListItems(hosts, map[string]error{}, 10*time.Second) for _, item := range items { expected := expected[item.Name] @@ -458,9 +457,6 @@ func TestIsSwarmActive(t *testing.T) { } func TestGetHostStateTimeout(t *testing.T) { - defer func(timeout time.Duration) { stateTimeoutDuration = timeout }(stateTimeoutDuration) - stateTimeoutDuration = 1 * time.Millisecond - hosts := []*host.Host{ { Name: "foo", @@ -470,12 +466,12 @@ func TestGetHostStateTimeout(t *testing.T) { }, } - hostItem := getHostListItems(hosts, nil)[0] + hostItem := getHostListItems(hosts, nil, time.Millisecond)[0] assert.Equal(t, "foo", hostItem.Name) assert.Equal(t, state.Timeout, hostItem.State) assert.Equal(t, "Driver", hostItem.DriverName) - assert.Equal(t, stateTimeoutDuration, hostItem.ResponseTime) + assert.Equal(t, time.Millisecond, hostItem.ResponseTime) } func TestGetHostStateError(t *testing.T) { @@ -488,7 +484,7 @@ func TestGetHostStateError(t *testing.T) { }, } - hostItem := getHostListItems(hosts, nil)[0] + hostItem := getHostListItems(hosts, nil, 10*time.Second)[0] assert.Equal(t, "foo", hostItem.Name) assert.Equal(t, state.Error, hostItem.State) @@ -514,7 +510,7 @@ func TestGetSomeHostInError(t *testing.T) { "bar": errors.New("invalid memory address or nil pointer dereference"), } - hostItems := getHostListItems(hosts, hostsInError) + hostItems := getHostListItems(hosts, hostsInError, 10*time.Second) assert.Equal(t, 2, len(hostItems)) hostItem := hostItems[0]