Display driver in error without crashing

Signed-off-by: Jean-Laurent de Morlhon <jeanlaurent@morlhon.net>

commands/active.go

@@ -18,12 +18,12 @@ func cmdActive(c CommandLine, api libmachine.API) error {
 		return errTooManyArguments
 	}
 
-	hosts, err := persist.LoadAllHosts(api)
+	hosts, hostsInError, err := persist.LoadAllHosts(api)
 	if err != nil {
 		return fmt.Errorf("Error getting active host: %s", err)
 	}
 
-	items := getHostListItems(hosts)
+	items := getHostListItems(hosts, hostsInError)
 
 	for _, item := range items {
 		if item.Active {

commands/commands.go

@@ -64,9 +64,14 @@ func (c *contextCommandLine) Application() *cli.App {
 }
 
 func runAction(actionName string, c CommandLine, api libmachine.API) error {
-	hosts, err := persist.LoadHosts(api, c.Args())
-	if err != nil {
-		return err
+	hosts, hostsInError := persist.LoadHosts(api, c.Args())
+
+	if len(hostsInError) > 0 {
+		errs := []error{}
+		for _, err := range hostsInError {
+			errs = append(errs, err)
+		}
+		return consolidateErrs(errs)
 	}
 
 	if len(hosts) == 0 {

commands/ls.go

@@ -49,7 +49,7 @@ func cmdLs(c CommandLine, api libmachine.API) error {
 		return err
 	}
 
-	hostList, err := persist.LoadAllHosts(api)
+	hostList, hostInError, err := persist.LoadAllHosts(api)
 	if err != nil {
 		return err
 	}
@@ -81,7 +81,7 @@ func cmdLs(c CommandLine, api libmachine.API) error {
 		}
 	}
 
-	items := getHostListItems(hostList)
+	items := getHostListItems(hostList, hostInError)
 
 	for _, item := range items {
 		activeString := "-"
@@ -283,7 +283,7 @@ func getHostState(h *host.Host, hostListItemsChan chan<- HostListItem) {
 	}
 }
 
-func getHostListItems(hostList []*host.Host) []HostListItem {
+func getHostListItems(hostList []*host.Host, hostsInError map[string]error) []HostListItem {
 	hostListItems := []HostListItem{}
 	hostListItemsChan := make(chan HostListItem)
 
@@ -297,6 +297,15 @@ func getHostListItems(hostList []*host.Host) []HostListItem {
 
 	close(hostListItemsChan)
 
+	for name, err := range hostsInError {
+		itemInError := HostListItem{}
+		itemInError.Name = name
+		itemInError.DriverName = "not found"
+		itemInError.State = state.Error
+		itemInError.Error = err.Error()
+		hostListItems = append(hostListItems, itemInError)
+	}
+
 	sortHostListItemsByName(hostListItems)
 	return hostListItems
 }

commands/ls_test.go

@@ -6,6 +6,7 @@ import (
 
 	"time"
 
+	"errors"
 	"github.com/docker/machine/drivers/fakedriver"
 	"github.com/docker/machine/libmachine/host"
 	"github.com/docker/machine/libmachine/state"
@@ -323,7 +324,7 @@ func TestGetHostListItems(t *testing.T) {
 		{"foo", state.Running, true, ""},
 	}
 
-	items := getHostListItems(hosts)
+	items := getHostListItems(hosts, map[string]error{})
 
 	for i := range expected {
 		assert.Equal(t, expected[i].name, items[i].Name)
@@ -398,7 +399,7 @@ func TestGetHostListItemsEnvDockerHostUnset(t *testing.T) {
 		"baz": {state.Saved, false},
 	}
 
-	items := getHostListItems(hosts)
+	items := getHostListItems(hosts, map[string]error{})
 
 	for _, item := range items {
 		expected := expected[item.Name]
@@ -446,7 +447,7 @@ func TestGetHostStateTimeout(t *testing.T) {
 	}
 
 	stateTimeoutDuration = 1 * time.Second
-	hostItems := getHostListItems(hosts)
+	hostItems := getHostListItems(hosts, map[string]error{})
 	hostItem := hostItems[0]
 
 	assert.Equal(t, "foo", hostItem.Name)
@@ -466,7 +467,7 @@ func TestGetHostStateError(t *testing.T) {
 		},
 	}
 
-	hostItems := getHostListItems(hosts)
+	hostItems := getHostListItems(hosts, map[string]error{})
 	hostItem := hostItems[0]
 
 	assert.Equal(t, "foo", hostItem.Name)
@@ -476,3 +477,34 @@ func TestGetHostStateError(t *testing.T) {
 	assert.Equal(t, "Unable to get ip", hostItem.Error)
 	assert.Nil(t, hostItem.SwarmOptions)
 }
+
+func TestGetSomeHostInEror(t *testing.T) {
+	hosts := []*host.Host{
+		{
+			Name: "foo",
+			Driver: &fakedriver.Driver{
+				MockState: state.Running,
+			},
+		},
+	}
+	hostsInError := map[string]error{
+		"bar": errors.New("invalid memory address or nil pointer dereference"),
+	}
+
+	hostItems := getHostListItems(hosts, hostsInError)
+	assert.Equal(t, 2, len(hostItems))
+
+	hostItem := hostItems[0]
+
+	assert.Equal(t, "bar", hostItem.Name)
+	assert.Equal(t, state.Error, hostItem.State)
+	assert.Equal(t, "not found", hostItem.DriverName)
+	assert.Empty(t, hostItem.URL)
+	assert.Equal(t, "invalid memory address or nil pointer dereference", hostItem.Error)
+	assert.Nil(t, hostItem.SwarmOptions)
+
+	hostItem = hostItems[1]
+
+	assert.Equal(t, "foo", hostItem.Name)
+	assert.Equal(t, state.Running, hostItem.State)
+}

libmachine/persist/store.go

@@ -1,6 +1,8 @@
 package persist
 
-import "github.com/docker/machine/libmachine/host"
+import (
+	"github.com/docker/machine/libmachine/host"
+)
 
 type Store interface {
 	// Exists returns whether a machine exists or not
@@ -19,27 +21,27 @@ type Store interface {
 	Save(host *host.Host) error
 }
 
-func LoadHosts(s Store, hostNames []string) ([]*host.Host, error) {
+func LoadHosts(s Store, hostNames []string) ([]*host.Host, map[string]error) {
 	loadedHosts := []*host.Host{}
+	errors := map[string]error{}
 
 	for _, hostName := range hostNames {
 		h, err := s.Load(hostName)
 		if err != nil {
-			// TODO: (nathanleclaire) Should these be bundled up
-			// into one error instead of exiting?
-			return nil, err
+			errors[hostName] = err
+		} else {
+			loadedHosts = append(loadedHosts, h)
 		}
-		loadedHosts = append(loadedHosts, h)
 	}
 
-	return loadedHosts, nil
+	return loadedHosts, errors
 }
 
-func LoadAllHosts(s Store) ([]*host.Host, error) {
+func LoadAllHosts(s Store) ([]*host.Host, map[string]error, error) {
 	hostNames, err := s.List()
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
-
-	return LoadHosts(s, hostNames)
+	loadedHosts, hostInError := LoadHosts(s, hostNames)
+	return loadedHosts, hostInError, nil
 }