From b249ccb1151a3c2c7e442b6c1a769821afb28fe4 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Tue, 31 Oct 2017 20:05:24 +0000 Subject: [PATCH] Update and use FindNetwork on Windows. Signed-off-by: Yong Tang --- api/server/router/network/network_routes.go | 4 +- daemon/container_operations_windows.go | 2 +- daemon/daemon_test.go | 2 +- daemon/errors.go | 17 ----- daemon/network.go | 11 ++-- daemon/oci_windows.go | 2 +- integration-cli/docker_cli_swarm_test.go | 73 --------------------- integration/service/create_test.go | 63 ++++++++++++++++++ 8 files changed, 73 insertions(+), 101 deletions(-) diff --git a/api/server/router/network/network_routes.go b/api/server/router/network/network_routes.go index 497ce0a502..f124813439 100644 --- a/api/server/router/network/network_routes.go +++ b/api/server/router/network/network_routes.go @@ -530,7 +530,7 @@ func (n *networkRouter) postNetworksPrune(ctx context.Context, w http.ResponseWr } // findUniqueNetwork will search network across different scopes (both local and swarm). -// NOTE: This findUniqueNetwork is differnt from FindUniqueNetwork from the daemon. +// NOTE: This findUniqueNetwork is different from FindUniqueNetwork in the daemon. // In case multiple networks have duplicate names, return error. // First find based on full ID, return immediately once one is found. // If a network appears both in swarm and local, assume it is in local first @@ -547,7 +547,7 @@ func (n *networkRouter) findUniqueNetwork(term string) (types.NetworkResource, e return *n.buildDetailedNetworkResources(network, false), nil } - if network.Name() == term { + if network.Name() == term && !network.Info().Ingress() { // No need to check the ID collision here as we are still in // local scope and the network ID is unique in this scope. listByFullName[network.ID()] = *n.buildDetailedNetworkResources(network, false) diff --git a/daemon/container_operations_windows.go b/daemon/container_operations_windows.go index 51762a2441..c443d2908d 100644 --- a/daemon/container_operations_windows.go +++ b/daemon/container_operations_windows.go @@ -170,7 +170,7 @@ func (daemon *Daemon) initializeNetworkingPaths(container *container.Container, if nc.NetworkSettings != nil { for n := range nc.NetworkSettings.Networks { - sn, err := daemon.FindNetwork(n) + sn, err := daemon.FindUniqueNetwork(n) if err != nil { continue } diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index b70e00e7d2..44bf3414c0 100644 --- a/daemon/daemon_test.go +++ b/daemon/daemon_test.go @@ -317,7 +317,7 @@ func TestValidateContainerIsolation(t *testing.T) { func TestFindNetworkErrorType(t *testing.T) { d := Daemon{} - _, err := d.FindNetwork("fakeNet") + _, err := d.FindUniqueNetwork("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 8e0756acfb..69c09fb83d 100644 --- a/daemon/errors.go +++ b/daemon/errors.go @@ -226,20 +226,3 @@ 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 9b5a66f36a..cf3d015b80 100644 --- a/daemon/network.go +++ b/daemon/network.go @@ -61,11 +61,6 @@ func (daemon *Daemon) FindUniqueNetwork(term string) (libnetwork.Network, error) return nil, libnetwork.ErrNoSuchNetwork(term) } -func isNoSuchNetworkError(err error) bool { - _, ok := err.(libnetwork.ErrNoSuchNetwork) - return ok -} - // GetNetworkByID function returns a network whose ID matches the given ID. // It fails with an error if no matching network is found. func (daemon *Daemon) GetNetworkByID(id string) (libnetwork.Network, error) { @@ -109,7 +104,11 @@ func (daemon *Daemon) GetNetworksByIDPrefix(partialID string) []libnetwork.Netwo // getAllNetworks returns a list containing all networks func (daemon *Daemon) getAllNetworks() []libnetwork.Network { - return daemon.netController.Networks() + c := daemon.netController + if c == nil { + return nil + } + return c.Networks() } type ingressJob struct { diff --git a/daemon/oci_windows.go b/daemon/oci_windows.go index c7c94f327a..d85cd269d9 100644 --- a/daemon/oci_windows.go +++ b/daemon/oci_windows.go @@ -159,7 +159,7 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { gwHNSID := "" if c.NetworkSettings != nil { for n := range c.NetworkSettings.Networks { - sn, err := daemon.FindNetwork(n) + sn, err := daemon.FindUniqueNetwork(n) if err != nil { continue } diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index 0eff6aacb2..283f576f57 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/integration-cli/docker_cli_swarm_test.go @@ -2137,76 +2137,3 @@ func (s *DockerSwarmSuite) TestSwarmClusterEventsConfig(c *check.C) { // filtered by config waitForEvent(c, d, t1, "-f type=config", "config remove "+id, defaultRetryCount) } - -func (s *DockerSwarmSuite) TestServiceCreateWithDuplicateNetworkNames(c *check.C) { - d := s.AddDaemon(c, true, true) - - name := "foo" - networkCreateRequest := types.NetworkCreateRequest{ - Name: name, - NetworkCreate: types.NetworkCreate{ - CheckDuplicate: false, - Driver: "bridge", - }, - } - - // Create networks with the same name, 2 in local scope and 1 in swarm scope - var n1 types.NetworkCreateResponse - status, body, err := d.SockRequest("POST", "/networks/create", networkCreateRequest) - c.Assert(err, checker.IsNil, check.Commentf(string(body))) - c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(body))) - c.Assert(json.Unmarshal(body, &n1), checker.IsNil) - - var n2 types.NetworkCreateResponse - status, body, err = d.SockRequest("POST", "/networks/create", networkCreateRequest) - c.Assert(err, checker.IsNil, check.Commentf(string(body))) - c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(body))) - c.Assert(json.Unmarshal(body, &n2), checker.IsNil) - - var n3 types.NetworkCreateResponse - // Dupliates with name but with different driver - networkCreateRequest.NetworkCreate.Driver = "overlay" - status, body, err = d.SockRequest("POST", "/networks/create", networkCreateRequest) - c.Assert(err, checker.IsNil, check.Commentf(string(body))) - c.Assert(status, checker.Equals, http.StatusCreated, check.Commentf(string(body))) - c.Assert(json.Unmarshal(body, &n3), checker.IsNil) - - // Create Service with the same name - d.CreateService(c, simpleTestService, func(s *swarm.Service) { - s.Spec.Name = "top" - s.Spec.TaskTemplate.Networks = []swarm.NetworkAttachmentConfig{ - {Target: name}, - } - }) - - // make sure task has been deployed. - waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1) - - result := icmd.RunCmd(d.Command("ps", "-a", "-q")) - result.Assert(c, icmd.Success) - containers := strings.Split(strings.TrimSpace(result.Stdout()), "\n") - c.Assert(len(containers), checker.Equals, 1) - - result = icmd.RunCmd(d.Command("inspect", "--format", `{{.NetworkSettings.Networks.foo.NetworkID}}`, containers[0])) - result.Assert(c, icmd.Success) - c.Assert(strings.TrimSpace(result.Stdout()), checker.Equals, n3.ID) - - // Remove Service - result = icmd.RunCmd(d.Command("service", "rm", "top")) - result.Assert(c, icmd.Success) - - // make sure task has been destroyed. - waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 0) - - result = icmd.RunCmd(d.Command("network", "rm", n1.ID)) - result.Assert(c, icmd.Success) - c.Assert(strings.TrimSpace(result.Stdout()), checker.Equals, n1.ID) - - result = icmd.RunCmd(d.Command("network", "rm", n2.ID)) - result.Assert(c, icmd.Success) - c.Assert(strings.TrimSpace(result.Stdout()), checker.Equals, n2.ID) - - result = icmd.RunCmd(d.Command("network", "rm", n3.ID)) - result.Assert(c, icmd.Success) - c.Assert(strings.TrimSpace(result.Stdout()), checker.Equals, n3.ID) -} diff --git a/integration/service/create_test.go b/integration/service/create_test.go index 6cfb27e820..a4373721f5 100644 --- a/integration/service/create_test.go +++ b/integration/service/create_test.go @@ -11,6 +11,7 @@ import ( "github.com/docker/docker/client" "github.com/docker/docker/integration-cli/request" "github.com/gotestyourself/gotestyourself/poll" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/context" ) @@ -80,6 +81,68 @@ func TestCreateServiceMultipleTimes(t *testing.T) { poll.WaitOn(t, networkIsRemoved(client, overlayID), poll.WithTimeout(1*time.Minute), poll.WithDelay(10*time.Second)) } +func TestCreateWithDuplicateNetworkNames(t *testing.T) { + defer setupTest(t)() + d := newSwarm(t) + defer d.Stop(t) + client, err := request.NewClientForHost(d.Sock()) + require.NoError(t, err) + + name := "foo" + networkCreate := types.NetworkCreate{ + CheckDuplicate: false, + Driver: "bridge", + } + + n1, err := client.NetworkCreate(context.Background(), name, networkCreate) + require.NoError(t, err) + + n2, err := client.NetworkCreate(context.Background(), name, networkCreate) + require.NoError(t, err) + + // Dupliates with name but with different driver + networkCreate.Driver = "overlay" + n3, err := client.NetworkCreate(context.Background(), name, networkCreate) + require.NoError(t, err) + + // Create Service with the same name + var instances uint64 = 1 + serviceSpec := swarmServiceSpec("top", instances) + + serviceSpec.TaskTemplate.Networks = append(serviceSpec.TaskTemplate.Networks, swarm.NetworkAttachmentConfig{Target: name}) + + service, err := client.ServiceCreate(context.Background(), serviceSpec, types.ServiceCreateOptions{}) + require.NoError(t, err) + + poll.WaitOn(t, serviceRunningTasksCount(client, service.ID, instances)) + + resp, _, err := client.ServiceInspectWithRaw(context.Background(), service.ID, types.ServiceInspectOptions{}) + require.NoError(t, err) + assert.Equal(t, n3.ID, resp.Spec.TaskTemplate.Networks[0].Target) + + // Remove Service + err = client.ServiceRemove(context.Background(), service.ID) + require.NoError(t, err) + + // Make sure task has been destroyed. + poll.WaitOn(t, serviceIsRemoved(client, service.ID)) + + // Remove networks + err = client.NetworkRemove(context.Background(), n3.ID) + require.NoError(t, err) + + err = client.NetworkRemove(context.Background(), n2.ID) + require.NoError(t, err) + + err = client.NetworkRemove(context.Background(), n1.ID) + require.NoError(t, err) + + // Make sure networks have been destroyed. + poll.WaitOn(t, networkIsRemoved(client, n3.ID), poll.WithTimeout(1*time.Minute), poll.WithDelay(10*time.Second)) + poll.WaitOn(t, networkIsRemoved(client, n2.ID), poll.WithTimeout(1*time.Minute), poll.WithDelay(10*time.Second)) + poll.WaitOn(t, networkIsRemoved(client, n1.ID), poll.WithTimeout(1*time.Minute), poll.WithDelay(10*time.Second)) +} + func swarmServiceSpec(name string, replicas uint64) swarm.ServiceSpec { return swarm.ServiceSpec{ Annotations: swarm.Annotations{