Merge pull request #3042 from dgageot/3016-add-timeout-to-active-command

Add timeout to active command

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)
 

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{

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 {

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]