FIX #2086 Add an error column to ls

Signed-off-by: David Gageot <david@gageot.net>

commands/ls.go

@@ -39,6 +39,7 @@ type HostListItem struct {
 	State        state.State
 	URL          string
 	SwarmOptions *swarm.Options
+	Error        string
 }
 
 func cmdLs(c CommandLine, api libmachine.API) error {
@@ -67,7 +68,7 @@ func cmdLs(c CommandLine, api libmachine.API) error {
 	swarmInfo := make(map[string]string)
 
 	w := tabwriter.NewWriter(os.Stdout, 5, 1, 3, ' ', 0)
-	fmt.Fprintln(w, "NAME\tACTIVE\tDRIVER\tSTATE\tURL\tSWARM")
+	fmt.Fprintln(w, "NAME\tACTIVE\tDRIVER\tSTATE\tURL\tSWARM\tERRORS")
 
 	for _, host := range hostList {
 		swarmOptions := host.HostOptions.SwarmOptions
@@ -98,8 +99,8 @@ func cmdLs(c CommandLine, api libmachine.API) error {
 				swarmInfo = fmt.Sprintf("%s (master)", swarmInfo)
 			}
 		}
-		fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\n",
+		fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\t%s\n",
-			item.Name, activeString, item.DriverName, item.State, item.URL, swarmInfo)
+			item.Name, activeString, item.DriverName, item.State, item.URL, swarmInfo, item.Error)
 	}
 
 	w.Flush()
@@ -231,36 +232,19 @@ func matchesName(host *host.Host, names []string) bool {
 }
 
 func attemptGetHostState(h *host.Host, stateQueryChan chan<- HostListItem) {
-	stateCh := make(chan state.State)
+	url := ""
-	urlCh := make(chan string)
+	hostError := ""
 
-	go func() {
 	currentState, err := h.Driver.GetState()
+	if err == nil {
+		url, err = h.GetURL()
+	}
 	if err != nil {
-			log.Errorf("error getting state for host %s: %s", h.Name, err)
+		hostError = err.Error()
 	}
-
+	if hostError == drivers.ErrHostIsNotRunning.Error() {
-		stateCh <- currentState
+		hostError = ""
-	}()
-
-	go func() {
-		url, err := h.GetURL()
-		if err != nil {
-			if err.Error() == drivers.ErrHostIsNotRunning.Error() {
-				url = ""
-			} else {
-				log.Errorf("error getting URL for host %s: %s", h.Name, err)
 	}
-		}
-
-		urlCh <- url
-	}()
-
-	currentState := <-stateCh
-	url := <-urlCh
-
-	close(stateCh)
-	close(urlCh)
 
 	stateQueryChan <- HostListItem{
 		Name:         h.Name,
@@ -269,6 +253,7 @@ func attemptGetHostState(h *host.Host, stateQueryChan chan<- HostListItem) {
 		State:        currentState,
 		URL:          url,
 		SwarmOptions: h.HostOptions.SwarmOptions,
+		Error:        hostError,
 	}
 }
 

commands/ls_test.go

@@ -276,6 +276,7 @@ func TestFilterHostsDifferentFlagsProduceAND(t *testing.T) {
 
 	assert.EqualValues(t, filterHosts(hosts, opts), expected)
 }
+
 func captureStdout() (chan string, *os.File) {
 	r, w, _ := os.Pipe()
 	os.Stdout = w
@@ -297,67 +298,57 @@ func TestGetHostListItems(t *testing.T) {
 	hosts := []*host.Host{
 		{
 			Name: "foo",
-			DriverName: "fakedriver",
 			Driver: &fakedriver.Driver{
 				MockState: state.Running,
 			},
 			HostOptions: &host.Options{
-				SwarmOptions: &swarm.Options{
+				SwarmOptions: &swarm.Options{},
-					Master:    false,
-					Address:   "",
-					Discovery: "",
-				},
 			},
 		},
 		{
 			Name: "bar",
-			DriverName: "fakedriver",
 			Driver: &fakedriver.Driver{
 				MockState: state.Stopped,
 			},
 			HostOptions: &host.Options{
-				SwarmOptions: &swarm.Options{
+				SwarmOptions: &swarm.Options{},
-					Master:    false,
-					Address:   "",
-					Discovery: "",
-				},
 			},
 		},
 		{
 			Name: "baz",
-			DriverName: "fakedriver",
 			Driver: &fakedriver.Driver{
-				MockState: state.Running,
+				MockState: state.Error,
 			},
 			HostOptions: &host.Options{
-				SwarmOptions: &swarm.Options{
+				SwarmOptions: &swarm.Options{},
-					Master:    false,
-					Address:   "",
-					Discovery: "",
-				},
 			},
 		},
 	}
 
-	expected := map[string]state.State{
+	expected := map[string]struct {
-		"foo": state.Running,
+		state  state.State
-		"bar": state.Stopped,
+		active bool
-		"baz": state.Running,
+		error  string
+	}{
+		"foo": {state.Running, true, ""},
+		"bar": {state.Stopped, false, ""},
+		"baz": {state.Error, false, "Unable to get url"},
 	}
 
 	items := []HostListItem{}
 	for _, host := range hosts {
 		go getHostState(host, hostListItemsChan)
 	}
-
 	for i := 0; i < len(hosts); i++ {
 		items = append(items, <-hostListItemsChan)
 	}
 
 	for _, item := range items {
-		if expected[item.Name] != item.State {
+		expected := expected[item.Name]
-			t.Fatal("Expected state did not match for item", item)
+
-		}
+		assert.Equal(t, expected.state, item.State)
+		assert.Equal(t, expected.active, item.Active)
+		assert.Equal(t, expected.error, item.Error)
 	}
 }
 
@@ -377,7 +368,6 @@ func TestGetHostListItemsEnvDockerHostUnset(t *testing.T) {
 	hosts := []*host.Host{
 		{
 			Name: "foo",
-			DriverName: "fakedriver",
 			Driver: &fakedriver.Driver{
 				MockState: state.Running,
 				MockURL:   "tcp://120.0.0.1:2376",
@@ -392,7 +382,6 @@ func TestGetHostListItemsEnvDockerHostUnset(t *testing.T) {
 		},
 		{
 			Name: "bar",
-			DriverName: "fakedriver",
 			Driver: &fakedriver.Driver{
 				MockState: state.Stopped,
 			},
@@ -406,7 +395,6 @@ func TestGetHostListItemsEnvDockerHostUnset(t *testing.T) {
 		},
 		{
 			Name: "baz",
-			DriverName: "fakedriver",
 			Driver: &fakedriver.Driver{
 				MockState: state.Saved,
 			},
@@ -433,18 +421,15 @@ func TestGetHostListItemsEnvDockerHostUnset(t *testing.T) {
 	for _, host := range hosts {
 		go getHostState(host, hostListItemsChan)
 	}
-
 	for i := 0; i < len(hosts); i++ {
 		items = append(items, <-hostListItemsChan)
 	}
 
 	for _, item := range items {
-		if expected[item.Name].state != item.State {
+		expected := expected[item.Name]
-			t.Fatal("Expected state did not match for item", item)
+
-		}
+		assert.Equal(t, expected.state, item.State)
-		if expected[item.Name].active != item.Active {
+		assert.Equal(t, expected.active, item.Active)
-			t.Fatal("Expected active flag did not match for item", item)
-		}
 	}
 }
 

drivers/fakedriver/fakedriver.go

@@ -1,6 +1,8 @@
 package fakedriver
 
 import (
+	"fmt"
+
 	"github.com/docker/machine/libmachine/drivers"
 	"github.com/docker/machine/libmachine/mcnflag"
 	"github.com/docker/machine/libmachine/state"
@@ -27,6 +29,12 @@ func (d *Driver) SetConfigFromFlags(flags drivers.DriverOptions) error {
 }
 
 func (d *Driver) GetURL() (string, error) {
+	if d.MockState == state.Error {
+		return "", fmt.Errorf("Unable to get url")
+	}
+	if d.MockState != state.Running {
+		return "", drivers.ErrHostIsNotRunning
+	}
 	return d.MockURL, nil
 }