Merge pull request #331 from thaJeztah/17.09-backport-fix-net-not-found

[17.09] Restore error type in FindNetwork

components/engine/daemon/cluster/executor/container/controller.go

@@ -183,7 +183,7 @@ func (r *controller) Start(ctx context.Context) error {
 
 	for {
 		if err := r.adapter.start(ctx); err != nil {
-			if _, ok := err.(libnetwork.ErrNoSuchNetwork); ok {
+			if _, ok := errors.Cause(err).(libnetwork.ErrNoSuchNetwork); ok {
 				// Retry network creation again if we
 				// failed because some of the networks
 				// were not found.

components/engine/daemon/daemon_test.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/docker/docker/api/errdefs"
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/container"
 	_ "github.com/docker/docker/pkg/discovery/memory"
@@ -18,6 +19,9 @@ import (
 	"github.com/docker/docker/volume/local"
 	"github.com/docker/docker/volume/store"
 	"github.com/docker/go-connections/nat"
+	"github.com/docker/libnetwork"
+	"github.com/pkg/errors"
+	"github.com/stretchr/testify/assert"
 )
 
 //
@@ -304,3 +308,12 @@ func TestMerge(t *testing.T) {
 		}
 	}
 }
+
+func TestFindNetworkErrorType(t *testing.T) {
+	d := Daemon{}
+	_, err := d.FindNetwork("fakeNet")
+	_, ok := errors.Cause(err).(libnetwork.ErrNoSuchNetwork)
+	if !errdefs.IsNotFound(err) || !ok {
+		assert.Fail(t, "The FindNetwork method MUST always return an error that implements the NotFound interface and is ErrNoSuchNetwork")
+	}
+}

components/engine/daemon/errors.go

@@ -21,10 +21,6 @@ func volumeNotFound(id string) error {
 	return objNotFoundError{"volume", id}
 }
 
-func networkNotFound(id string) error {
-	return objNotFoundError{"network", id}
-}
-
 type objNotFoundError struct {
 	object string
 	id     string
@@ -214,3 +210,20 @@ func translateContainerdStartErr(cmd string, setExitCode func(int), err error) e
 	// TODO: it would be nice to get some better errors from containerd so we can return better errors here
 	return retErr
 }
+
+// TODO: cpuguy83 take care of it once the new library is ready
+type errNotFound struct{ error }
+
+func (errNotFound) NotFound() {}
+
+func (e errNotFound) Cause() error {
+	return e.error
+}
+
+// notFound is a helper to create an error of the class with the same name from any error type
+func notFound(err error) error {
+	if err == nil {
+		return nil
+	}
+	return errNotFound{err}
+}

components/engine/daemon/network.go

@@ -56,7 +56,9 @@ func (daemon *Daemon) GetNetworkByID(partialID string) (libnetwork.Network, erro
 	list := daemon.GetNetworksByID(partialID)
 
 	if len(list) == 0 {
-		return nil, errors.WithStack(networkNotFound(partialID))
+		// Be very careful to change the error type here, the libnetwork.ErrNoSuchNetwork error is used by the controller
+		// to retry the creation of the network as managed through the swarm manager
+		return nil, errors.WithStack(notFound(libnetwork.ErrNoSuchNetwork(partialID)))
 	}
 	if len(list) > 1 {
 		return nil, errors.WithStack(invalidIdentifier(partialID))

components/engine/integration-cli/docker_cli_netmode_test.go

@@ -49,7 +49,7 @@ func (s *DockerSuite) TestNetHostname(c *check.C) {
 	c.Assert(out, checker.Contains, "Invalid network mode: invalid container format container:<name|id>")
 
 	out, _ = dockerCmdWithFail(c, "run", "--net=weird", "busybox", "ps")
-	c.Assert(strings.ToLower(out), checker.Contains, "no such network")
+	c.Assert(strings.ToLower(out), checker.Contains, "not found")
 }
 
 func (s *DockerSuite) TestConflictContainerNetworkAndLinks(c *check.C) {