From 6cac3d13cbd89e31f69b17f3166d2efca6360daa Mon Sep 17 00:00:00 2001 From: Madhu Venugopal Date: Thu, 25 May 2017 19:50:08 -0700 Subject: [PATCH 1/5] 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/5] 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/5] 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) } From 144f2f5310d02e512eceb21f2fcc1deca3e12d9c Mon Sep 17 00:00:00 2001 From: John Stephens Date: Sat, 27 May 2017 12:29:30 -0700 Subject: [PATCH 4/5] Fix stack compose bind-mount volumes for Windows For stack compose files, use filepath.IsAbs instead of path.IsAbs, for bind-mounted service volumes, because filepath.IsAbs handles Windows paths, while path.IsAbs does not. Signed-off-by: John Stephens Upstream-commit: 9043d39deaa9b8d6b64efd5b4eb0a2225d91dee2 Component: cli --- components/cli/cli/compose/loader/loader.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/components/cli/cli/compose/loader/loader.go b/components/cli/cli/compose/loader/loader.go index 0ed8ce87d7..f12a7f0eea 100644 --- a/components/cli/cli/compose/loader/loader.go +++ b/components/cli/cli/compose/loader/loader.go @@ -3,6 +3,7 @@ package loader import ( "fmt" "path" + "path/filepath" "reflect" "sort" "strings" @@ -371,7 +372,16 @@ func resolveVolumePaths(volumes []types.ServiceVolumeConfig, workingDir string, continue } - volume.Source = absPath(workingDir, expandUser(volume.Source, lookupEnv)) + filePath := expandUser(volume.Source, lookupEnv) + // Check for a Unix absolute path first, to handle a Windows client + // with a Unix daemon. This handles a Windows client connecting to a + // Unix daemon. Note that this is not required for Docker for Windows + // when specifying a local Windows path, because Docker for Windows + // translates the Windows path into a valid path within the VM. + if !path.IsAbs(filePath) { + filePath = absPath(workingDir, filePath) + } + volume.Source = filePath volumes[i] = volume } } @@ -492,11 +502,11 @@ func LoadConfigObjs(source map[string]interface{}, workingDir string) (map[strin return configs, nil } -func absPath(workingDir string, filepath string) string { - if path.IsAbs(filepath) { - return filepath +func absPath(workingDir string, filePath string) string { + if filepath.IsAbs(filePath) { + return filePath } - return path.Join(workingDir, filepath) + return filepath.Join(workingDir, filePath) } func transformMapStringString(data interface{}) (interface{}, error) { From e1f6bc3e4ffaf3ddb7c6ed280752a9866be53e9e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 1 Jun 2017 16:25:17 +0200 Subject: [PATCH 5/5] Allow --detach and --quiet flags when using --rollback Commit 78c204ef798c7380e11ba26e5cd231e04fc6efe4 added (f9bd8ec8b268581f93095c5a80679f0a8ff498bf in the moby repo) a validation to prevent `--rollback` from being used in combination with other flags that update the service spec. This validation was not taking into account that some flags only affect the CLI behavior, and are okay to be used when rolling back. This patch updates the validation, and adds `--quiet` and `--detach` to the list of allowed flags. Signed-off-by: Sebastiaan van Stijn Upstream-commit: f10f29df8d7ffede3faa29120a06c199e52a1d0c Component: cli --- components/cli/cli/command/service/update.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/cli/cli/command/service/update.go b/components/cli/cli/command/service/update.go index 85ac89aa52..5e3bc7c458 100644 --- a/components/cli/cli/command/service/update.go +++ b/components/cli/cli/command/service/update.go @@ -131,7 +131,7 @@ func runUpdate(dockerCli *command.DockerCli, flags *pflag.FlagSet, options *serv // Rollback can't be combined with other flags. otherFlagsPassed := false flags.VisitAll(func(f *pflag.Flag) { - if f.Name == "rollback" { + if f.Name == "rollback" || f.Name == "detach" || f.Name == "quiet" { return } if flags.Changed(f.Name) {