From 51cea0a53c2fd36832277402e9faac81bfb4abd4 Mon Sep 17 00:00:00 2001 From: Flavio Crisciani Date: Tue, 28 Nov 2017 17:06:26 -0800 Subject: [PATCH] Restore error type in FindNetwork The error type libnetwork.ErrNoSuchNetwork is used in the controller to retry the network creation as a managed network though the manager. The change of the type was breaking the logic causing the network to not being created anymore so that no new container on that network was able to be launched Added unit test Signed-off-by: Flavio Crisciani --- .../cluster/executor/container/controller.go | 2 +- daemon/daemon_test.go | 12 +++++++++++ daemon/errors.go | 21 +++++++++++++++---- daemon/network.go | 4 +++- integration-cli/docker_cli_netmode_test.go | 2 +- 5 files changed, 34 insertions(+), 7 deletions(-) diff --git a/daemon/cluster/executor/container/controller.go b/daemon/cluster/executor/container/controller.go index dda12591a9..df14a7a76d 100644 --- a/daemon/cluster/executor/container/controller.go +++ b/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. diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index 422be1fd77..b70e00e7d2 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -7,6 +7,7 @@ import ( "runtime" "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" @@ -17,6 +18,8 @@ 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" ) @@ -311,3 +314,12 @@ func TestValidateContainerIsolation(t *testing.T) { _, err := d.verifyContainerSettings(runtime.GOOS, &containertypes.HostConfig{Isolation: containertypes.Isolation("invalid")}, nil, false) assert.EqualError(t, err, "invalid isolation 'invalid' on "+runtime.GOOS) } + +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") + } +} diff --git a/daemon/errors.go b/daemon/errors.go index 889261fa35..8e0756acfb 100644 --- a/daemon/errors.go +++ b/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 @@ -230,3 +226,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} +} diff --git a/daemon/network.go b/daemon/network.go index 61548c5b5e..0420caaad5 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -45,7 +45,9 @@ func (daemon *Daemon) FindNetwork(idName string) (libnetwork.Network, error) { // 3. match by ID prefix list := daemon.GetNetworksByIDPrefix(idName) if len(list) == 0 { - return nil, errors.WithStack(networkNotFound(idName)) + // 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(idName))) } if len(list) > 1 { return nil, errors.WithStack(invalidIdentifier(idName)) diff --git a/integration-cli/docker_cli_netmode_test.go b/integration-cli/docker_cli_netmode_test.go index abf1ff2cf7..2b134d4de1 100644 --- a/integration-cli/docker_cli_netmode_test.go +++ b/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:") 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) {