diff --git a/command/formatter/service.go b/command/formatter/service.go index f5eb1aeaf7..8f57af22d9 100644 --- a/command/formatter/service.go +++ b/command/formatter/service.go @@ -6,6 +6,7 @@ import ( "time" "github.com/docker/distribution/reference" + "github.com/docker/docker/api/types" mounttypes "github.com/docker/docker/api/types/mount" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/cli/command/inspect" @@ -137,8 +138,20 @@ func NewServiceFormat(source string) Format { } } +func resolveNetworks(service swarm.Service, getNetwork inspect.GetRefFunc) map[string]string { + networkNames := make(map[string]string) + for _, network := range service.Spec.TaskTemplate.Networks { + if resolved, _, err := getNetwork(network.Target); err == nil { + if resolvedNetwork, ok := resolved.(types.NetworkResource); ok { + networkNames[resolvedNetwork.ID] = resolvedNetwork.Name + } + } + } + return networkNames +} + // ServiceInspectWrite renders the context for a list of services -func ServiceInspectWrite(ctx Context, refs []string, getRef inspect.GetRefFunc) error { +func ServiceInspectWrite(ctx Context, refs []string, getRef, getNetwork inspect.GetRefFunc) error { if ctx.Format != serviceInspectPrettyTemplate { return inspect.Inspect(ctx.Output, refs, string(ctx.Format), getRef) } @@ -152,7 +165,7 @@ func ServiceInspectWrite(ctx Context, refs []string, getRef inspect.GetRefFunc) if !ok { return errors.Errorf("got wrong object to inspect") } - if err := format(&serviceInspectContext{Service: service}); err != nil { + if err := format(&serviceInspectContext{Service: service, networkNames: resolveNetworks(service, getNetwork)}); err != nil { return err } } @@ -164,6 +177,10 @@ func ServiceInspectWrite(ctx Context, refs []string, getRef inspect.GetRefFunc) type serviceInspectContext struct { swarm.Service subContext + + // networkNames is a map from network IDs (as found in + // Networks[x].Target) to network names. + networkNames map[string]string } func (ctx *serviceInspectContext) MarshalJSON() ([]byte, error) { @@ -383,8 +400,12 @@ func (ctx *serviceInspectContext) ResourceLimitMemory() string { func (ctx *serviceInspectContext) Networks() []string { var out []string - for _, n := range ctx.Service.Spec.Networks { - out = append(out, n.Target) + for _, n := range ctx.Service.Spec.TaskTemplate.Networks { + if name, ok := ctx.networkNames[n.Target]; ok { + out = append(out, name) + } else { + out = append(out, n.Target) + } } return out } diff --git a/command/service/create.go b/command/service/create.go index 76b61f6c2e..0e77f73d32 100644 --- a/command/service/create.go +++ b/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/command/service/inspect.go b/command/service/inspect.go index 8247d45afa..8a8b51cd0e 100644 --- a/command/service/inspect.go +++ b/command/service/inspect.go @@ -58,6 +58,14 @@ func runInspect(dockerCli *command.DockerCli, opts inspectOptions) error { return nil, nil, errors.Errorf("Error: no such service: %s", ref) } + getNetwork := func(ref string) (interface{}, []byte, error) { + network, _, err := client.NetworkInspectWithRaw(ctx, ref, false) + if err == nil || !apiclient.IsErrNetworkNotFound(err) { + return network, nil, err + } + return nil, nil, errors.Errorf("Error: no such network: %s", ref) + } + f := opts.format if len(f) == 0 { f = "raw" @@ -77,7 +85,7 @@ func runInspect(dockerCli *command.DockerCli, opts inspectOptions) error { Format: formatter.NewServiceFormat(f), } - if err := formatter.ServiceInspectWrite(serviceCtx, opts.refs, getRef); err != nil { + if err := formatter.ServiceInspectWrite(serviceCtx, opts.refs, getRef, getNetwork); err != nil { return cli.StatusError{StatusCode: 1, Status: err.Error()} } return nil diff --git a/command/service/inspect_test.go b/command/service/inspect_test.go index 94c96cc164..44d9df9176 100644 --- a/command/service/inspect_test.go +++ b/command/service/inspect_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/swarm" "github.com/docker/docker/cli/command/formatter" "github.com/docker/docker/pkg/testutil/assert" @@ -43,18 +44,18 @@ func formatServiceInspect(t *testing.T, format formatter.Format, now time.Time) ContainerSpec: swarm.ContainerSpec{ Image: "foo/bar@sha256:this_is_a_test", }, + Networks: []swarm.NetworkAttachmentConfig{ + { + Target: "5vpyomhb6ievnk0i0o60gcnei", + Aliases: []string{"web"}, + }, + }, }, Mode: swarm.ServiceMode{ Replicated: &swarm.ReplicatedService{ Replicas: &two, }, }, - Networks: []swarm.NetworkAttachmentConfig{ - { - Target: "5vpyomhb6ievnk0i0o60gcnei", - Aliases: []string{"web"}, - }, - }, EndpointSpec: endpointSpec, }, Endpoint: swarm.Endpoint{ @@ -84,9 +85,17 @@ func formatServiceInspect(t *testing.T, format formatter.Format, now time.Time) Format: format, } - err := formatter.ServiceInspectWrite(ctx, []string{"de179gar9d0o7ltdybungplod"}, func(ref string) (interface{}, []byte, error) { - return s, nil, nil - }) + err := formatter.ServiceInspectWrite(ctx, []string{"de179gar9d0o7ltdybungplod"}, + func(ref string) (interface{}, []byte, error) { + return s, nil, nil + }, + func(ref string) (interface{}, []byte, error) { + return types.NetworkResource{ + ID: "5vpyomhb6ievnk0i0o60gcnei", + Name: "mynetwork", + }, nil, nil + }, + ) if err != nil { t.Fatal(err) } @@ -98,6 +107,9 @@ func TestPrettyPrintWithNoUpdateConfig(t *testing.T) { if strings.Contains(s, "UpdateStatus") { t.Fatal("Pretty print failed before parsing UpdateStatus") } + if !strings.Contains(s, "mynetwork") { + t.Fatal("network name not found in inspect output") + } } func TestJSONFormatWithNoUpdateConfig(t *testing.T) { diff --git a/command/service/opts.go b/command/service/opts.go index 47d417fb25..066436838c 100644 --- a/command/service/opts.go +++ b/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/command/service/update.go b/command/service/update.go index bf428b4ae6..b59f163829 100644 --- a/command/service/update.go +++ b/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/command/service/update_test.go b/command/service/update_test.go index 7a588d7fef..090372fb78 100644 --- a/command/service/update_test.go +++ b/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/compose/convert/service.go b/compose/convert/service.go index 7af24b2ec7..66f7868767 100644 --- a/compose/convert/service.go +++ b/compose/convert/service.go @@ -9,6 +9,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/swarm" + "github.com/docker/docker/api/types/versions" servicecli "github.com/docker/docker/cli/command/service" composetypes "github.com/docker/docker/cli/compose/types" "github.com/docker/docker/client" @@ -20,11 +21,10 @@ import ( const defaultNetwork = "default" // Services from compose-file types to engine API types -// TODO: fix secrets API so that SecretAPIClient is not required here func Services( namespace Namespace, config *composetypes.Config, - client client.SecretAPIClient, + client client.CommonAPIClient, ) (map[string]swarm.ServiceSpec, error) { result := make(map[string]swarm.ServiceSpec) @@ -33,12 +33,11 @@ func Services( networks := config.Networks for _, service := range services { - secrets, err := convertServiceSecrets(client, namespace, service.Secrets, config.Secrets) if err != nil { return nil, errors.Wrapf(err, "service %s", service.Name) } - serviceSpec, err := convertService(namespace, service, networks, volumes, secrets) + serviceSpec, err := convertService(client.ClientVersion(), namespace, service, networks, volumes, secrets) if err != nil { return nil, errors.Wrapf(err, "service %s", service.Name) } @@ -49,6 +48,7 @@ func Services( } func convertService( + apiVersion string, namespace Namespace, service composetypes.ServiceConfig, networkConfigs map[string]composetypes.NetworkConfig, @@ -133,10 +133,21 @@ func convertService( }, EndpointSpec: endpoint, Mode: mode, - Networks: networks, UpdateConfig: convertUpdateConfig(service.Deploy.UpdateConfig), } + // ServiceSpec.Networks is deprecated and should not have been used by + // this package. It is possible to update TaskTemplate.Networks, but it + // is not possible to update ServiceSpec.Networks. Unfortunately, we + // can't unconditionally start using TaskTemplate.Networks, because that + // will break with older daemons that don't support migrating from + // ServiceSpec.Networks to TaskTemplate.Networks. So which field to use + // is conditional on daemon version. + if versions.LessThan(apiVersion, "1.29") { + serviceSpec.Networks = networks + } else { + serviceSpec.TaskTemplate.Networks = networks + } return serviceSpec, nil }