Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api: allow NW name that is the prefix of a swarm NW ID #27938

Merged
merged 1 commit into from Dec 16, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/server/router/network/network_routes.go
Expand Up @@ -80,7 +80,7 @@ func (n *networkRouter) postNetworkCreate(ctx context.Context, w http.ResponseWr
return err
}

if _, err := n.clusterProvider.GetNetwork(create.Name); err == nil {
if nws, err := n.clusterProvider.GetNetworksByName(create.Name); err == nil && len(nws) > 0 {
return libnetwork.NetworkNameError(create.Name)
}

Expand Down
20 changes: 17 additions & 3 deletions daemon/cluster/cluster.go
Expand Up @@ -1375,8 +1375,7 @@ func (c *Cluster) GetNetwork(input string) (apitypes.NetworkResource, error) {
return convert.BasicNetworkFromGRPC(*network), nil
}

// GetNetworks returns all current cluster managed networks.
func (c *Cluster) GetNetworks() ([]apitypes.NetworkResource, error) {
func (c *Cluster) getNetworks(filters *swarmapi.ListNetworksRequest_Filters) ([]apitypes.NetworkResource, error) {
c.mu.RLock()
defer c.mu.RUnlock()

Expand All @@ -1388,7 +1387,7 @@ func (c *Cluster) GetNetworks() ([]apitypes.NetworkResource, error) {
ctx, cancel := c.getRequestContext()
defer cancel()

r, err := state.controlClient.ListNetworks(ctx, &swarmapi.ListNetworksRequest{})
r, err := state.controlClient.ListNetworks(ctx, &swarmapi.ListNetworksRequest{Filters: filters})
if err != nil {
return nil, err
}
Expand All @@ -1402,6 +1401,21 @@ func (c *Cluster) GetNetworks() ([]apitypes.NetworkResource, error) {
return networks, nil
}

// GetNetworks returns all current cluster managed networks.
func (c *Cluster) GetNetworks() ([]apitypes.NetworkResource, error) {
return c.getNetworks(nil)
}

// GetNetworksByName returns cluster managed networks by name.
// It is ok to have multiple networks here. #18864
func (c *Cluster) GetNetworksByName(name string) ([]apitypes.NetworkResource, error) {
// Note that swarmapi.GetNetworkRequest.Name is not functional.
// So we cannot just use that with c.GetNetwork.
return c.getNetworks(&swarmapi.ListNetworksRequest_Filters{
Names: []string{name},
})
}

func attacherKey(target, containerID string) string {
return containerID + ":" + target
}
Expand Down
47 changes: 47 additions & 0 deletions integration-cli/docker_cli_swarm_test.go
Expand Up @@ -1508,3 +1508,50 @@ func (s *DockerTrustedSwarmSuite) TestTrustedServiceUpdate(c *check.C) {
c.Assert(err, check.NotNil, check.Commentf(out))
c.Assert(string(out), checker.Contains, "Error: remote trust data does not exist", check.Commentf(out))
}

// Test case for issue #27866, which did not allow NW name that is the prefix of a swarm NW ID.
// e.g. if the ingress ID starts with "n1", it was impossible to create a NW named "n1".
func (s *DockerSwarmSuite) TestSwarmNetworkCreateIssue27866(c *check.C) {
d := s.AddDaemon(c, true, true)
out, err := d.Cmd("network", "inspect", "-f", "{{.Id}}", "ingress")
c.Assert(err, checker.IsNil, check.Commentf("out: %v", out))
ingressID := strings.TrimSpace(out)
c.Assert(ingressID, checker.Not(checker.Equals), "")

// create a network of which name is the prefix of the ID of an overlay network
// (ingressID in this case)
newNetName := ingressID[0:2]
out, err = d.Cmd("network", "create", "--driver", "overlay", newNetName)
// In #27866, it was failing because of "network with name %s already exists"
c.Assert(err, checker.IsNil, check.Commentf("out: %v", out))
out, err = d.Cmd("network", "rm", newNetName)
c.Assert(err, checker.IsNil, check.Commentf("out: %v", out))
}

// Test case for https://github.com/docker/docker/pull/27938#issuecomment-265768303
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be cool if there could also be a description on what it does 👼 (linking the issue is 👍, but it would be even better with a small explanation so the reader doesn't have to necessary open his browser to know why it's there 👼)

Same above for TestSwarmNetworkCreateIssue27866

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

// This test creates two networks with the same name sequentially, with various drivers.
// Since the operations in this test are done sequentially, the 2nd call should fail with
// "network with name FOO already exists".
// Note that it is to ok have multiple networks with the same name if the operations are done
// in parallel. (#18864)
func (s *DockerSwarmSuite) TestSwarmNetworkCreateDup(c *check.C) {
d := s.AddDaemon(c, true, true)
drivers := []string{"bridge", "overlay"}
for i, driver1 := range drivers {
nwName := fmt.Sprintf("network-test-%d", i)
for _, driver2 := range drivers {
c.Logf("Creating a network named %q with %q, then %q",
nwName, driver1, driver2)
out, err := d.Cmd("network", "create", "--driver", driver1, nwName)
c.Assert(err, checker.IsNil, check.Commentf("out: %v", out))
out, err = d.Cmd("network", "create", "--driver", driver2, nwName)
c.Assert(out, checker.Contains,
fmt.Sprintf("network with name %s already exists", nwName))
c.Assert(err, checker.NotNil)
c.Logf("As expected, the attempt to network %q with %q failed: %s",
nwName, driver2, out)
out, err = d.Cmd("network", "rm", nwName)
c.Assert(err, checker.IsNil, check.Commentf("out: %v", out))
}
}
}