diff --git a/cli/command/service/create.go b/cli/command/service/create.go index 76b61f6c2e..0e77f73d32 100644 --- a/cli/command/service/create.go +++ b/cli/command/service/create.go @@ -63,7 +63,9 @@ func runCreate(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *service apiClient := dockerCli.Client() createOpts := types.ServiceCreateOptions{} - service, err := opts.ToService() + ctx := context.Background() + + service, err := opts.ToService(ctx, apiClient) if err != nil { return err } @@ -79,8 +81,6 @@ func runCreate(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *service } - ctx := context.Background() - if err := resolveServiceImageDigest(dockerCli, &service); err != nil { return err } diff --git a/cli/command/service/opts.go b/cli/command/service/opts.go index 47d417fb25..066436838c 100644 --- a/cli/command/service/opts.go +++ b/cli/command/service/opts.go @@ -2,17 +2,20 @@ package service import ( "fmt" + "sort" "strconv" "strings" "time" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/client" "github.com/docker/docker/opts" runconfigopts "github.com/docker/docker/runconfig/opts" shlex "github.com/flynn-archive/go-shlex" "github.com/pkg/errors" "github.com/spf13/pflag" + "golang.org/x/net/context" ) type int64Value interface { @@ -270,12 +273,17 @@ func (c *credentialSpecOpt) Value() *swarm.CredentialSpec { return c.value } -func convertNetworks(networks []string) []swarm.NetworkAttachmentConfig { +func convertNetworks(ctx context.Context, apiClient client.NetworkAPIClient, networks []string) ([]swarm.NetworkAttachmentConfig, error) { nets := []swarm.NetworkAttachmentConfig{} - for _, network := range networks { - nets = append(nets, swarm.NetworkAttachmentConfig{Target: network}) + for _, networkIDOrName := range networks { + network, err := apiClient.NetworkInspect(ctx, networkIDOrName, false) + if err != nil { + return nil, err + } + nets = append(nets, swarm.NetworkAttachmentConfig{Target: network.ID}) } - return nets + sort.Sort(byNetworkTarget(nets)) + return nets, nil } type endpointOptions struct { @@ -455,7 +463,7 @@ func (opts *serviceOptions) ToServiceMode() (swarm.ServiceMode, error) { return serviceMode, nil } -func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) { +func (opts *serviceOptions) ToService(ctx context.Context, apiClient client.APIClient) (swarm.ServiceSpec, error) { var service swarm.ServiceSpec envVariables, err := runconfigopts.ReadKVStrings(opts.envFile.GetAll(), opts.env.GetAll()) @@ -487,6 +495,11 @@ func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) { return service, err } + networks, err := convertNetworks(ctx, apiClient, opts.networks.GetAll()) + if err != nil { + return service, err + } + service = swarm.ServiceSpec{ Annotations: swarm.Annotations{ Name: opts.name, @@ -517,7 +530,7 @@ func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) { Secrets: nil, Healthcheck: healthConfig, }, - Networks: convertNetworks(opts.networks.GetAll()), + Networks: networks, Resources: opts.resources.ToResourceRequirements(), RestartPolicy: opts.restartPolicy.ToRestartPolicy(), Placement: &swarm.Placement{ @@ -526,7 +539,6 @@ func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) { }, LogDriver: opts.logDriver.toLogDriver(), }, - Networks: convertNetworks(opts.networks.GetAll()), Mode: serviceMode, UpdateConfig: opts.update.config(), RollbackConfig: opts.rollback.config(), @@ -666,6 +678,8 @@ const ( flagMountAdd = "mount-add" flagName = "name" flagNetwork = "network" + flagNetworkAdd = "network-add" + flagNetworkRemove = "network-rm" flagPublish = "publish" flagPublishRemove = "publish-rm" flagPublishAdd = "publish-add" diff --git a/cli/command/service/update.go b/cli/command/service/update.go index bf428b4ae6..b59f163829 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -74,6 +74,10 @@ func newUpdateCommand(dockerCli *command.DockerCli) *cobra.Command { flags.SetAnnotation(flagPlacementPrefAdd, "version", []string{"1.28"}) flags.Var(&placementPrefOpts{}, flagPlacementPrefRemove, "Remove a placement preference") flags.SetAnnotation(flagPlacementPrefRemove, "version", []string{"1.28"}) + flags.Var(&serviceOpts.networks, flagNetworkAdd, "Add a network") + flags.SetAnnotation(flagNetworkAdd, "version", []string{"1.29"}) + flags.Var(newListOptsVar(), flagNetworkRemove, "Remove a network") + flags.SetAnnotation(flagNetworkRemove, "version", []string{"1.29"}) flags.Var(&serviceOpts.endpoint.publishPorts, flagPublishAdd, "Add or update a published port") flags.Var(&serviceOpts.groups, flagGroupAdd, "Add an additional supplementary user group to the container") flags.SetAnnotation(flagGroupAdd, "version", []string{"1.25"}) @@ -147,7 +151,7 @@ func runUpdate(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *service updateOpts.Rollback = "previous" } - err = updateService(flags, spec) + err = updateService(ctx, apiClient, flags, spec) if err != nil { return err } @@ -207,7 +211,7 @@ func runUpdate(dockerCli *command.DockerCli, flags *pflag.FlagSet, opts *service return waitOnService(ctx, dockerCli, serviceID, opts) } -func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error { +func updateService(ctx context.Context, apiClient client.APIClient, flags *pflag.FlagSet, spec *swarm.ServiceSpec) error { updateString := func(flag string, field *string) { if flags.Changed(flag) { *field, _ = flags.GetString(flag) @@ -316,6 +320,12 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error { updatePlacementPreferences(flags, task.Placement) } + if anyChanged(flags, flagNetworkAdd, flagNetworkRemove) { + if err := updateNetworks(ctx, apiClient, flags, spec); err != nil { + return err + } + } + if err := updateReplicas(flags, &spec.Mode); err != nil { return err } @@ -623,7 +633,6 @@ func (m byMountSource) Less(i, j int) bool { } func updateMounts(flags *pflag.FlagSet, mounts *[]mounttypes.Mount) error { - mountsByTarget := map[string]mounttypes.Mount{} if flags.Changed(flagMountAdd) { @@ -947,3 +956,63 @@ func updateHealthcheck(flags *pflag.FlagSet, containerSpec *swarm.ContainerSpec) } return nil } + +type byNetworkTarget []swarm.NetworkAttachmentConfig + +func (m byNetworkTarget) Len() int { return len(m) } +func (m byNetworkTarget) Swap(i, j int) { m[i], m[j] = m[j], m[i] } +func (m byNetworkTarget) Less(i, j int) bool { + return m[i].Target < m[j].Target +} + +func updateNetworks(ctx context.Context, apiClient client.NetworkAPIClient, flags *pflag.FlagSet, spec *swarm.ServiceSpec) error { + // spec.TaskTemplate.Networks takes precedence over the deprecated + // spec.Networks field. If spec.Network is in use, we'll migrate those + // values to spec.TaskTemplate.Networks. + specNetworks := spec.TaskTemplate.Networks + if len(specNetworks) == 0 { + specNetworks = spec.Networks + } + spec.Networks = nil + + toRemove := buildToRemoveSet(flags, flagNetworkRemove) + idsToRemove := make(map[string]struct{}) + for networkIDOrName := range toRemove { + network, err := apiClient.NetworkInspect(ctx, networkIDOrName, false) + if err != nil { + return err + } + idsToRemove[network.ID] = struct{}{} + } + + existingNetworks := make(map[string]struct{}) + var newNetworks []swarm.NetworkAttachmentConfig + for _, network := range specNetworks { + if _, exists := idsToRemove[network.Target]; exists { + continue + } + + newNetworks = append(newNetworks, network) + existingNetworks[network.Target] = struct{}{} + } + + if flags.Changed(flagNetworkAdd) { + values := flags.Lookup(flagNetworkAdd).Value.(*opts.ListOpts).GetAll() + networks, err := convertNetworks(ctx, apiClient, values) + if err != nil { + return err + } + for _, network := range networks { + if _, exists := existingNetworks[network.Target]; exists { + return errors.Errorf("service is already attached to network %s", network.Target) + } + newNetworks = append(newNetworks, network) + existingNetworks[network.Target] = struct{}{} + } + } + + sort.Sort(byNetworkTarget(newNetworks)) + + spec.TaskTemplate.Networks = newNetworks + return nil +} diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index 7a588d7fef..090372fb78 100644 --- a/cli/command/service/update_test.go +++ b/cli/command/service/update_test.go @@ -22,7 +22,7 @@ func TestUpdateServiceArgs(t *testing.T) { cspec := &spec.TaskTemplate.ContainerSpec cspec.Args = []string{"old", "args"} - updateService(flags, spec) + updateService(nil, nil, flags, spec) assert.EqualStringSlice(t, cspec.Args, []string{"the", "new args"}) } @@ -458,18 +458,18 @@ func TestUpdateReadOnly(t *testing.T) { // Update with --read-only=true, changed to true flags := newUpdateCommand(nil).Flags() flags.Set("read-only", "true") - updateService(flags, spec) + updateService(nil, nil, flags, spec) assert.Equal(t, cspec.ReadOnly, true) // Update without --read-only, no change flags = newUpdateCommand(nil).Flags() - updateService(flags, spec) + updateService(nil, nil, flags, spec) assert.Equal(t, cspec.ReadOnly, true) // Update with --read-only=false, changed to false flags = newUpdateCommand(nil).Flags() flags.Set("read-only", "false") - updateService(flags, spec) + updateService(nil, nil, flags, spec) assert.Equal(t, cspec.ReadOnly, false) } @@ -480,17 +480,17 @@ func TestUpdateStopSignal(t *testing.T) { // Update with --stop-signal=SIGUSR1 flags := newUpdateCommand(nil).Flags() flags.Set("stop-signal", "SIGUSR1") - updateService(flags, spec) + updateService(nil, nil, flags, spec) assert.Equal(t, cspec.StopSignal, "SIGUSR1") // Update without --stop-signal, no change flags = newUpdateCommand(nil).Flags() - updateService(flags, spec) + updateService(nil, nil, flags, spec) assert.Equal(t, cspec.StopSignal, "SIGUSR1") // Update with --stop-signal=SIGWINCH flags = newUpdateCommand(nil).Flags() flags.Set("stop-signal", "SIGWINCH") - updateService(flags, spec) + updateService(nil, nil, flags, spec) assert.Equal(t, cspec.StopSignal, "SIGWINCH") } diff --git a/docs/reference/commandline/service_update.md b/docs/reference/commandline/service_update.md index 559217b504..f79caeb41c 100644 --- a/docs/reference/commandline/service_update.md +++ b/docs/reference/commandline/service_update.md @@ -58,6 +58,8 @@ Options: --log-opt list Logging driver options (default []) --mount-add mount Add or update a mount on a service --mount-rm list Remove a mount by its target path (default []) + --network-add list Add a network + --network-rm list Remove a network --no-healthcheck Disable any container-specified HEALTHCHECK --placement-pref-add pref Add a placement preference --placement-pref-rm pref Remove a placement preference diff --git a/integration-cli/daemon/daemon_swarm.go b/integration-cli/daemon/daemon_swarm.go index 5fb68abb9b..48ceae0470 100644 --- a/integration-cli/daemon/daemon_swarm.go +++ b/integration-cli/daemon/daemon_swarm.go @@ -205,7 +205,30 @@ func (d *Swarm) CheckServiceTasks(service string) func(*check.C) (interface{}, c } } -// CheckRunningTaskImages returns the number of different images attached to a running task +// CheckRunningTaskNetworks returns the number of times each network is referenced from a task. +func (d *Swarm) CheckRunningTaskNetworks(c *check.C) (interface{}, check.CommentInterface) { + var tasks []swarm.Task + + filterArgs := filters.NewArgs() + filterArgs.Add("desired-state", "running") + filters, err := filters.ToParam(filterArgs) + c.Assert(err, checker.IsNil) + + status, out, err := d.SockRequest("GET", "/tasks?filters="+filters, nil) + c.Assert(err, checker.IsNil, check.Commentf(string(out))) + c.Assert(status, checker.Equals, http.StatusOK, check.Commentf("output: %q", string(out))) + c.Assert(json.Unmarshal(out, &tasks), checker.IsNil) + + result := make(map[string]int) + for _, task := range tasks { + for _, network := range task.Spec.Networks { + result[network.Target]++ + } + } + return result, nil +} + +// CheckRunningTaskImages returns the times each image is running as a task. func (d *Swarm) CheckRunningTaskImages(c *check.C) (interface{}, check.CommentInterface) { var tasks []swarm.Task diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go index c079a3ae2f..f419e2bf8b 100644 --- a/integration-cli/docker_cli_swarm_test.go +++ b/integration-cli/docker_cli_swarm_test.go @@ -855,6 +855,45 @@ func (s *DockerSwarmSuite) TestSwarmServiceTTYUpdate(c *check.C) { c.Assert(strings.TrimSpace(out), checker.Equals, "true") } +func (s *DockerSwarmSuite) TestSwarmServiceNetworkUpdate(c *check.C) { + d := s.AddDaemon(c, true, true) + + result := icmd.RunCmd(d.Command("network", "create", "-d", "overlay", "foo")) + result.Assert(c, icmd.Success) + fooNetwork := strings.TrimSpace(string(result.Combined())) + + result = icmd.RunCmd(d.Command("network", "create", "-d", "overlay", "bar")) + result.Assert(c, icmd.Success) + barNetwork := strings.TrimSpace(string(result.Combined())) + + result = icmd.RunCmd(d.Command("network", "create", "-d", "overlay", "baz")) + result.Assert(c, icmd.Success) + bazNetwork := strings.TrimSpace(string(result.Combined())) + + // Create a service + name := "top" + result = icmd.RunCmd(d.Command("service", "create", "--network", "foo", "--network", "bar", "--name", name, "busybox", "top")) + result.Assert(c, icmd.Success) + + // Make sure task has been deployed. + waitAndAssert(c, defaultReconciliationTimeout, d.CheckRunningTaskNetworks, checker.DeepEquals, + map[string]int{fooNetwork: 1, barNetwork: 1}) + + // Remove a network + result = icmd.RunCmd(d.Command("service", "update", "--network-rm", "foo", name)) + result.Assert(c, icmd.Success) + + waitAndAssert(c, defaultReconciliationTimeout, d.CheckRunningTaskNetworks, checker.DeepEquals, + map[string]int{barNetwork: 1}) + + // Add a network + result = icmd.RunCmd(d.Command("service", "update", "--network-add", "baz", name)) + result.Assert(c, icmd.Success) + + waitAndAssert(c, defaultReconciliationTimeout, d.CheckRunningTaskNetworks, checker.DeepEquals, + map[string]int{barNetwork: 1, bazNetwork: 1}) +} + func (s *DockerSwarmSuite) TestDNSConfig(c *check.C) { d := s.AddDaemon(c, true, true)