diff --git a/cli/command/service/update.go b/cli/command/service/update.go index a9aa9c9987..34cc9bc3d8 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -181,7 +181,9 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error { updateEnvironment(flags, &cspec.Env) updateString(flagWorkdir, &cspec.Dir) updateString(flagUser, &cspec.User) - updateMounts(flags, &cspec.Mounts) + if err := updateMounts(flags, &cspec.Mounts); err != nil { + return err + } if flags.Changed(flagLimitCPU) || flags.Changed(flagLimitMemory) { taskResources().Limits = &swarm.Resources{} @@ -402,20 +404,53 @@ func removeItems( return newSeq } -func updateMounts(flags *pflag.FlagSet, mounts *[]mounttypes.Mount) { +type byMountSource []mounttypes.Mount + +func (m byMountSource) Len() int { return len(m) } +func (m byMountSource) Swap(i, j int) { m[i], m[j] = m[j], m[i] } +func (m byMountSource) Less(i, j int) bool { + a, b := m[i], m[j] + + if a.Source == b.Source { + return a.Target < b.Target + } + + return a.Source < b.Source +} + +func updateMounts(flags *pflag.FlagSet, mounts *[]mounttypes.Mount) error { + + mountsByTarget := map[string]mounttypes.Mount{} + if flags.Changed(flagMountAdd) { values := flags.Lookup(flagMountAdd).Value.(*opts.MountOpt).Value() - *mounts = append(*mounts, values...) + for _, mount := range values { + if _, ok := mountsByTarget[mount.Target]; ok { + return fmt.Errorf("duplicate mount target") + } + mountsByTarget[mount.Target] = mount + } + } + + // Add old list of mount points minus updated one. + for _, mount := range *mounts { + if _, ok := mountsByTarget[mount.Target]; !ok { + mountsByTarget[mount.Target] = mount + } } - toRemove := buildToRemoveSet(flags, flagMountRemove) newMounts := []mounttypes.Mount{} - for _, mount := range *mounts { + + toRemove := buildToRemoveSet(flags, flagMountRemove) + + for _, mount := range mountsByTarget { if _, exists := toRemove[mount.Target]; !exists { newMounts = append(newMounts, mount) } } + sort.Sort(byMountSource(newMounts)) *mounts = newMounts + return nil } func updateGroups(flags *pflag.FlagSet, groups *[]string) error { diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index 731358753e..2123d1b794 100644 --- a/cli/command/service/update_test.go +++ b/cli/command/service/update_test.go @@ -122,18 +122,36 @@ func TestUpdateGroups(t *testing.T) { func TestUpdateMounts(t *testing.T) { flags := newUpdateCommand(nil).Flags() - flags.Set("mount-add", "type=volume,target=/toadd") + flags.Set("mount-add", "type=volume,source=vol2,target=/toadd") flags.Set("mount-rm", "/toremove") mounts := []mounttypes.Mount{ - {Target: "/toremove", Type: mounttypes.TypeBind}, - {Target: "/tokeep", Type: mounttypes.TypeBind}, + {Target: "/toremove", Source: "vol1", Type: mounttypes.TypeBind}, + {Target: "/tokeep", Source: "vol3", Type: mounttypes.TypeBind}, } updateMounts(flags, &mounts) assert.Equal(t, len(mounts), 2) - assert.Equal(t, mounts[0].Target, "/tokeep") - assert.Equal(t, mounts[1].Target, "/toadd") + assert.Equal(t, mounts[0].Target, "/toadd") + assert.Equal(t, mounts[1].Target, "/tokeep") + +} + +func TestUpdateMountsWithDuplicateMounts(t *testing.T) { + flags := newUpdateCommand(nil).Flags() + flags.Set("mount-add", "type=volume,source=vol4,target=/toadd") + + mounts := []mounttypes.Mount{ + {Target: "/tokeep1", Source: "vol1", Type: mounttypes.TypeBind}, + {Target: "/toadd", Source: "vol2", Type: mounttypes.TypeBind}, + {Target: "/tokeep2", Source: "vol3", Type: mounttypes.TypeBind}, + } + + updateMounts(flags, &mounts) + assert.Equal(t, len(mounts), 3) + assert.Equal(t, mounts[0].Target, "/tokeep1") + assert.Equal(t, mounts[1].Target, "/tokeep2") + assert.Equal(t, mounts[2].Target, "/toadd") } func TestUpdatePorts(t *testing.T) {