Browse code

Merge pull request #35634 from fcrisciani/fix-net-not-found

Restore error type in FindNetwork

Yong Tang authored on 2017/11/30 09:26: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) {