Browse code

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 <flavio.crisciani@docker.com>

Flavio Crisciani authored on 2017/11/29 10:06:26
Showing 5 changed files
... ...
@@ -183,7 +183,7 @@ func (r *controller) Start(ctx context.Context) error {
183 183
 
184 184
 	for {
185 185
 		if err := r.adapter.start(ctx); err != nil {
186
-			if _, ok := err.(libnetwork.ErrNoSuchNetwork); ok {
186
+			if _, ok := errors.Cause(err).(libnetwork.ErrNoSuchNetwork); ok {
187 187
 				// Retry network creation again if we
188 188
 				// failed because some of the networks
189 189
 				// were not found.
... ...
@@ -7,6 +7,7 @@ import (
7 7
 	"runtime"
8 8
 	"testing"
9 9
 
10
+	"github.com/docker/docker/api/errdefs"
10 11
 	containertypes "github.com/docker/docker/api/types/container"
11 12
 	"github.com/docker/docker/container"
12 13
 	_ "github.com/docker/docker/pkg/discovery/memory"
... ...
@@ -17,6 +18,8 @@ import (
17 17
 	"github.com/docker/docker/volume/local"
18 18
 	"github.com/docker/docker/volume/store"
19 19
 	"github.com/docker/go-connections/nat"
20
+	"github.com/docker/libnetwork"
21
+	"github.com/pkg/errors"
20 22
 	"github.com/stretchr/testify/assert"
21 23
 )
22 24
 
... ...
@@ -311,3 +314,12 @@ func TestValidateContainerIsolation(t *testing.T) {
311 311
 	_, err := d.verifyContainerSettings(runtime.GOOS, &containertypes.HostConfig{Isolation: containertypes.Isolation("invalid")}, nil, false)
312 312
 	assert.EqualError(t, err, "invalid isolation 'invalid' on "+runtime.GOOS)
313 313
 }
314
+
315
+func TestFindNetworkErrorType(t *testing.T) {
316
+	d := Daemon{}
317
+	_, err := d.FindNetwork("fakeNet")
318
+	_, ok := errors.Cause(err).(libnetwork.ErrNoSuchNetwork)
319
+	if !errdefs.IsNotFound(err) || !ok {
320
+		assert.Fail(t, "The FindNetwork method MUST always return an error that implements the NotFound interface and is ErrNoSuchNetwork")
321
+	}
322
+}
... ...
@@ -21,10 +21,6 @@ func volumeNotFound(id string) error {
21 21
 	return objNotFoundError{"volume", id}
22 22
 }
23 23
 
24
-func networkNotFound(id string) error {
25
-	return objNotFoundError{"network", id}
26
-}
27
-
28 24
 type objNotFoundError struct {
29 25
 	object string
30 26
 	id     string
... ...
@@ -230,3 +226,20 @@ func translateContainerdStartErr(cmd string, setExitCode func(int), err error) e
230 230
 	// TODO: it would be nice to get some better errors from containerd so we can return better errors here
231 231
 	return retErr
232 232
 }
233
+
234
+// TODO: cpuguy83 take care of it once the new library is ready
235
+type errNotFound struct{ error }
236
+
237
+func (errNotFound) NotFound() {}
238
+
239
+func (e errNotFound) Cause() error {
240
+	return e.error
241
+}
242
+
243
+// notFound is a helper to create an error of the class with the same name from any error type
244
+func notFound(err error) error {
245
+	if err == nil {
246
+		return nil
247
+	}
248
+	return errNotFound{err}
249
+}
... ...
@@ -45,7 +45,9 @@ func (daemon *Daemon) FindNetwork(idName string) (libnetwork.Network, error) {
45 45
 	// 3. match by ID prefix
46 46
 	list := daemon.GetNetworksByIDPrefix(idName)
47 47
 	if len(list) == 0 {
48
-		return nil, errors.WithStack(networkNotFound(idName))
48
+		// Be very careful to change the error type here, the libnetwork.ErrNoSuchNetwork error is used by the controller
49
+		// to retry the creation of the network as managed through the swarm manager
50
+		return nil, errors.WithStack(notFound(libnetwork.ErrNoSuchNetwork(idName)))
49 51
 	}
50 52
 	if len(list) > 1 {
51 53
 		return nil, errors.WithStack(invalidIdentifier(idName))
... ...
@@ -49,7 +49,7 @@ func (s *DockerSuite) TestNetHostname(c *check.C) {
49 49
 	c.Assert(out, checker.Contains, "Invalid network mode: invalid container format container:<name|id>")
50 50
 
51 51
 	out, _ = dockerCmdWithFail(c, "run", "--net=weird", "busybox", "ps")
52
-	c.Assert(strings.ToLower(out), checker.Contains, "no such network")
52
+	c.Assert(strings.ToLower(out), checker.Contains, "not found")
53 53
 }
54 54
 
55 55
 func (s *DockerSuite) TestConflictContainerNetworkAndLinks(c *check.C) {