From 6cac3d13cbd89e31f69b17f3166d2efca6360daa 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 Upstream-commit: 123f0bfd9894be11cb2da401dad513fd7373d41c Component: cli --- .../cli/command/stack/deploy_composefile.go | 5 +++-- components/cli/cli/compose/convert/service.go | 19 +++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/components/cli/cli/command/stack/deploy_composefile.go b/components/cli/cli/command/stack/deploy_composefile.go index abe0b0ea2c..1b2ca1d6f0 100644 --- a/components/cli/cli/command/stack/deploy_composefile.go +++ b/components/cli/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/components/cli/cli/compose/convert/service.go b/components/cli/cli/compose/convert/service.go index 79c6555276..8b0dd311e0 100644 --- a/components/cli/cli/compose/convert/service.go +++ b/components/cli/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 f257d783a4735be84a7262e25e4941d5743ef934 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 Upstream-commit: 341703d21e88c55d3489404f9b4fbdcb1e97f98a Component: cli --- .../cli/command/stack/deploy_composefile.go | 29 ++++------ .../command/stack/deploy_composefile_test.go | 57 +++++++++++++++++++ .../cli/cli/internal/test/network/client.go | 56 ++++++++++++++++++ components/cli/scripts/test/watch | 2 +- 4 files changed, 126 insertions(+), 18 deletions(-) create mode 100644 components/cli/cli/internal/test/network/client.go diff --git a/components/cli/cli/command/stack/deploy_composefile.go b/components/cli/cli/command/stack/deploy_composefile.go index 1b2ca1d6f0..642867cba9 100644 --- a/components/cli/cli/command/stack/deploy_composefile.go +++ b/components/cli/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/components/cli/cli/command/stack/deploy_composefile_test.go b/components/cli/cli/command/stack/deploy_composefile_test.go index d5ef5463ff..6d811ed208 100644 --- a/components/cli/cli/command/stack/deploy_composefile_test.go +++ b/components/cli/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/components/cli/cli/internal/test/network/client.go b/components/cli/cli/internal/test/network/client.go new file mode 100644 index 0000000000..5f35cd4514 --- /dev/null +++ b/components/cli/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/components/cli/scripts/test/watch b/components/cli/scripts/test/watch index 61057e2430..3c2b46bea1 100755 --- a/components/cli/scripts/test/watch +++ b/components/cli/scripts/test/watch @@ -3,7 +3,7 @@ set -e filewatcher \ - -L 5 \ + -L 6 \ -x '**/*.swp' \ -x .git \ -x build \ From 74196507d22334e16d999bd129114e1156c368b0 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 Upstream-commit: d5b505ee8ceda32511359cff674472e349ea3a28 Component: cli --- components/cli/cli/compose/convert/service.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/components/cli/cli/compose/convert/service.go b/components/cli/cli/compose/convert/service.go index 8b0dd311e0..e465c32908 100644 --- a/components/cli/cli/compose/convert/service.go +++ b/components/cli/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) }