From 123f0bfd9894be11cb2da401dad513fd7373d41c Mon Sep 17 00:00:00 2001 From: Madhu Venugopal Date: Thu, 25 May 2017 19:50:08 -0700 Subject: [PATCH 1/3] With the introduction of node-local network support, docker services can be attached to special networks such as host and bridge. This fix brings in the required changes to make sure the stack file accepts these networks as well. Signed-off-by: Madhu Venugopal --- cli/command/stack/deploy_composefile.go | 5 +++-- cli/compose/convert/service.go | 19 +++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/cli/command/stack/deploy_composefile.go b/cli/command/stack/deploy_composefile.go index abe0b0ea2ca1..1b2ca1d6f0ee 100644 --- a/cli/command/stack/deploy_composefile.go +++ b/cli/command/stack/deploy_composefile.go @@ -13,6 +13,7 @@ import ( "github.com/docker/cli/cli/compose/loader" composetypes "github.com/docker/cli/cli/compose/types" "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/swarm" apiclient "github.com/docker/docker/client" dockerclient "github.com/docker/docker/client" @@ -177,11 +178,11 @@ func validateExternalNetworks( network, err := client.NetworkInspect(ctx, networkName, false) if err != nil { if dockerclient.IsErrNetworkNotFound(err) { - return errors.Errorf("network %q is declared as external, but could not be found. You need to create the network before the stack is deployed (with overlay driver)", networkName) + return errors.Errorf("network %q is declared as external, but could not be found. You need to create a swarm-scoped network before the stack is deployed", networkName) } return err } - if network.Scope != "swarm" { + if container.NetworkMode(networkName).IsUserDefined() && network.Scope != "swarm" { return errors.Errorf("network %q is declared as external, but it is not in the right scope: %q instead of %q", networkName, network.Scope, "swarm") } } diff --git a/cli/compose/convert/service.go b/cli/compose/convert/service.go index 79c655527694..8b0dd311e0ce 100644 --- a/cli/compose/convert/service.go +++ b/cli/compose/convert/service.go @@ -214,18 +214,21 @@ func convertServiceNetworks( if !ok && networkName != defaultNetwork { return nil, errors.Errorf("undefined network %q", networkName) } - var aliases []string - if network != nil { - aliases = network.Aliases - } target := namespace.Scope(networkName) if networkConfig.External.External { target = networkConfig.External.Name } - nets = append(nets, swarm.NetworkAttachmentConfig{ - Target: target, - Aliases: append(aliases, name), - }) + netAttachConfig := swarm.NetworkAttachmentConfig{ + Target: target, + } + if container.NetworkMode(target).IsUserDefined() { + var aliases []string + if network != nil { + aliases = network.Aliases + } + netAttachConfig.Aliases = append(aliases, name) + } + nets = append(nets, netAttachConfig) } sort.Sort(byNetworkTarget(nets)) From 341703d21e88c55d3489404f9b4fbdcb1e97f98a Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 26 May 2017 12:15:23 -0400 Subject: [PATCH 2/3] Add tests for verifyExternalNetwork Signed-off-by: Daniel Nephin --- cli/command/stack/deploy_composefile.go | 29 +++++----- cli/command/stack/deploy_composefile_test.go | 57 ++++++++++++++++++++ cli/internal/test/network/client.go | 56 +++++++++++++++++++ scripts/test/watch | 2 +- 4 files changed, 126 insertions(+), 18 deletions(-) create mode 100644 cli/internal/test/network/client.go diff --git a/cli/command/stack/deploy_composefile.go b/cli/command/stack/deploy_composefile.go index 1b2ca1d6f0ee..642867cba9f0 100644 --- a/cli/command/stack/deploy_composefile.go +++ b/cli/command/stack/deploy_composefile.go @@ -65,7 +65,7 @@ func deployCompose(ctx context.Context, dockerCli command.Cli, opts deployOption serviceNetworks := getServicesDeclaredNetworks(config.Services) networks, externalNetworks := convert.Networks(namespace, config.Networks, serviceNetworks) - if err := validateExternalNetworks(ctx, dockerCli, externalNetworks); err != nil { + if err := validateExternalNetworks(ctx, dockerCli.Client(), externalNetworks); err != nil { return err } if err := createNetworks(ctx, dockerCli, namespace, networks); err != nil { @@ -76,7 +76,7 @@ func deployCompose(ctx context.Context, dockerCli command.Cli, opts deployOption if err != nil { return err } - if err := createSecrets(ctx, dockerCli, namespace, secrets); err != nil { + if err := createSecrets(ctx, dockerCli, secrets); err != nil { return err } @@ -84,7 +84,7 @@ func deployCompose(ctx context.Context, dockerCli command.Cli, opts deployOption if err != nil { return err } - if err := createConfigs(ctx, dockerCli, namespace, configs); err != nil { + if err := createConfigs(ctx, dockerCli, configs); err != nil { return err } @@ -170,30 +170,26 @@ func getConfigFile(filename string) (*composetypes.ConfigFile, error) { func validateExternalNetworks( ctx context.Context, - dockerCli command.Cli, - externalNetworks []string) error { - client := dockerCli.Client() - + client dockerclient.NetworkAPIClient, + externalNetworks []string, +) error { for _, networkName := range externalNetworks { network, err := client.NetworkInspect(ctx, networkName, false) - if err != nil { - if dockerclient.IsErrNetworkNotFound(err) { - return errors.Errorf("network %q is declared as external, but could not be found. You need to create a swarm-scoped network before the stack is deployed", networkName) - } + switch { + case dockerclient.IsErrNotFound(err): + return errors.Errorf("network %q is declared as external, but could not be found. You need to create a swarm-scoped network before the stack is deployed", networkName) + case err != nil: return err - } - if container.NetworkMode(networkName).IsUserDefined() && network.Scope != "swarm" { - return errors.Errorf("network %q is declared as external, but it is not in the right scope: %q instead of %q", networkName, network.Scope, "swarm") + case container.NetworkMode(networkName).IsUserDefined() && network.Scope != "swarm": + return errors.Errorf("network %q is declared as external, but it is not in the right scope: %q instead of \"swarm\"", networkName, network.Scope) } } - return nil } func createSecrets( ctx context.Context, dockerCli command.Cli, - namespace convert.Namespace, secrets []swarm.SecretSpec, ) error { client := dockerCli.Client() @@ -220,7 +216,6 @@ func createSecrets( func createConfigs( ctx context.Context, dockerCli command.Cli, - namespace convert.Namespace, configs []swarm.ConfigSpec, ) error { client := dockerCli.Client() diff --git a/cli/command/stack/deploy_composefile_test.go b/cli/command/stack/deploy_composefile_test.go index d5ef5463ff17..6d811ed208a1 100644 --- a/cli/command/stack/deploy_composefile_test.go +++ b/cli/command/stack/deploy_composefile_test.go @@ -5,9 +5,14 @@ import ( "path/filepath" "testing" + "github.com/docker/cli/cli/internal/test/network" + "github.com/docker/docker/api/types" + "github.com/docker/docker/pkg/testutil" "github.com/docker/docker/pkg/testutil/tempfile" + "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/net/context" ) func TestGetConfigDetails(t *testing.T) { @@ -26,3 +31,55 @@ services: assert.Len(t, details.ConfigFiles, 1) assert.Len(t, details.Environment, len(os.Environ())) } + +type notFound struct { + error +} + +func (n notFound) NotFound() bool { + return true +} + +func TestValidateExternalNetworks(t *testing.T) { + var testcases = []struct { + inspectResponse types.NetworkResource + inspectError error + expectedMsg string + network string + }{ + { + inspectError: notFound{}, + expectedMsg: "could not be found. You need to create a swarm-scoped network", + }, + { + inspectError: errors.New("Unexpected"), + expectedMsg: "Unexpected", + }, + { + network: "host", + }, + { + network: "user", + expectedMsg: "is not in the right scope", + }, + { + network: "user", + inspectResponse: types.NetworkResource{Scope: "swarm"}, + }, + } + + for _, testcase := range testcases { + fakeClient := &network.FakeClient{ + NetworkInspectFunc: func(_ context.Context, _ string, _ bool) (types.NetworkResource, error) { + return testcase.inspectResponse, testcase.inspectError + }, + } + networks := []string{testcase.network} + err := validateExternalNetworks(context.Background(), fakeClient, networks) + if testcase.expectedMsg == "" { + assert.NoError(t, err) + } else { + testutil.ErrorContains(t, err, testcase.expectedMsg) + } + } +} diff --git a/cli/internal/test/network/client.go b/cli/internal/test/network/client.go new file mode 100644 index 000000000000..5f35cd4514ad --- /dev/null +++ b/cli/internal/test/network/client.go @@ -0,0 +1,56 @@ +package network + +import ( + "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/filters" + "github.com/docker/docker/api/types/network" + "golang.org/x/net/context" +) + +// FakeClient is a fake NetworkAPIClient +type FakeClient struct { + NetworkInspectFunc func(ctx context.Context, networkID string, verbose bool) (types.NetworkResource, error) +} + +// NetworkConnect fakes connecting to a network +func (c *FakeClient) NetworkConnect(ctx context.Context, networkID, container string, config *network.EndpointSettings) error { + return nil +} + +// NetworkCreate fakes creating a network +func (c *FakeClient) NetworkCreate(ctx context.Context, name string, options types.NetworkCreate) (types.NetworkCreateResponse, error) { + return types.NetworkCreateResponse{}, nil +} + +// NetworkDisconnect fakes disconencting from a network +func (c *FakeClient) NetworkDisconnect(ctx context.Context, networkID, container string, force bool) error { + return nil +} + +// NetworkInspect fakes inspecting a network +func (c *FakeClient) NetworkInspect(ctx context.Context, networkID string, verbose bool) (types.NetworkResource, error) { + if c.NetworkInspectFunc != nil { + return c.NetworkInspectFunc(ctx, networkID, verbose) + } + return types.NetworkResource{}, nil +} + +// NetworkInspectWithRaw fakes inspecting a network with a raw response +func (c *FakeClient) NetworkInspectWithRaw(ctx context.Context, networkID string, verbose bool) (types.NetworkResource, []byte, error) { + return types.NetworkResource{}, nil, nil +} + +// NetworkList fakes listing networks +func (c *FakeClient) NetworkList(ctx context.Context, options types.NetworkListOptions) ([]types.NetworkResource, error) { + return nil, nil +} + +// NetworkRemove fakes removing networks +func (c *FakeClient) NetworkRemove(ctx context.Context, networkID string) error { + return nil +} + +// NetworksPrune fakes pruning networks +func (c *FakeClient) NetworksPrune(ctx context.Context, pruneFilter filters.Args) (types.NetworksPruneReport, error) { + return types.NetworksPruneReport{}, nil +} diff --git a/scripts/test/watch b/scripts/test/watch index 61057e2430b5..3c2b46bea1e1 100755 --- a/scripts/test/watch +++ b/scripts/test/watch @@ -3,7 +3,7 @@ set -e filewatcher \ - -L 5 \ + -L 6 \ -x '**/*.swp' \ -x .git \ -x build \ From d5b505ee8ceda32511359cff674472e349ea3a28 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 26 May 2017 14:25:20 -0400 Subject: [PATCH 3/3] Only set default aliases when the network is user defined. Signed-off-by: Daniel Nephin --- cli/compose/convert/service.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/cli/compose/convert/service.go b/cli/compose/convert/service.go index 8b0dd311e0ce..e465c32908f1 100644 --- a/cli/compose/convert/service.go +++ b/cli/compose/convert/service.go @@ -214,19 +214,22 @@ func convertServiceNetworks( if !ok && networkName != defaultNetwork { return nil, errors.Errorf("undefined network %q", networkName) } + var aliases []string + if network != nil { + aliases = network.Aliases + } target := namespace.Scope(networkName) if networkConfig.External.External { target = networkConfig.External.Name } netAttachConfig := swarm.NetworkAttachmentConfig{ - Target: target, + Target: target, + Aliases: aliases, } + // Only add default aliases to user defined networks. Other networks do + // not support aliases. if container.NetworkMode(target).IsUserDefined() { - var aliases []string - if network != nil { - aliases = network.Aliases - } - netAttachConfig.Aliases = append(aliases, name) + netAttachConfig.Aliases = append(netAttachConfig.Aliases, name) } nets = append(nets, netAttachConfig) }