feat(service): support comma-separated values for host-to-IP mappings
PR Title: support comma-separated values for --host-add in docker service update
Closes https://github.com/moby/moby/issues/50769
- What I did
- Updated
--host-addflag inservice updateto support comma-separated host mappings (e.g.,--host-add a:1.1.1.1,b:2.2.2.2). - Introduced
ListOptsCSV, a wrapper aroundListOptsthat implementspflag.SliceValuefor parsing comma-separated lists. - Adjusted
updateHoststo usepflag.SliceValueinstead ofListOpts. - Updated unit tests in
update_test.goto cover multiple host mappings provided in a single flag instance. - Updated CLI documentation to reflect support for comma-separated host mappings.
- How I did it
- Implemented a new
ListOptsCSVtype inopts/opts.gothat handles splitting and validating comma-separated values. - Switched
flags.Var(&options.hosts, flagHostAdd, ...)to useopts.NewListOptsCSV(&options.hosts)inupdate.go. - Updated
updateHoststo retrieve values viapflag.SliceValue. - Modified the
service updatereference docs to note that multiple entries can be separated by commas.
- How to verify it
- Run
docker service update --host-add a:1.1.1.1,b:2.2.2.2 <service>. - Inspect the service to confirm that both mappings (
a → 1.1.1.1andb → 2.2.2.2) are applied. - Check that existing behavior (multiple
--host-addflags) still works as expected. - Run updated unit tests:
go test ./cli/command/service/....
- Human readable description for the release notes
Added support for specifying multiple `--host-add` entries in a single flag instance using comma-separated values in `docker service update`.
- A picture of a cute animal (not mandatory but encouraged) 🦦
I'm a bit on the fence on adding this; I think the problem is not (not directly) in the --host-add flag, but the --host-rm flag not validating.
As far as I'm aware, none of the service create/update options were designed to take a comma-separated list, but they can be specified multiple times.
Reading the related ticket https://github.com/moby/moby/issues/50769
It should probably behave the same way as --host-rm since "docker service update --host-rm host1:192.168.1.111,host2:192.168.1.222" my_service does work.
I don't think that's correct; it doesn't produce an error, but it doesn't work (so the problem here may be more along the lines of --host-rm missing validation;
Taking this example; a service with 4 "extra hosts" added;
docker service create --name foo --host one:127.0.0.1 --host two:127.0.0.1 --host three:127.0.0.1 --host four:127.0.0.1 nginx:alpine
docker service inspect --format '{{ json .Spec.TaskTemplate.ContainerSpec.Hosts }}' foo
["127.0.0.1 one","127.0.0.1 two","127.0.0.1 three","127.0.0.1 four"]
Using the --host-rm flag with comma-separated values (either full host:IP or just host gets silently ignored. This was likely to allow using --host-rm to specify hosts that were not present (i.e., making it idempotent), but in this case it resulted in the invalid value being removed;
docker service update --host-rm one:127.0.0.1,two:127.0.0.1 foo
docker service inspect --format '{{ json .Spec.TaskTemplate.ContainerSpec.Hosts }}' foo
["127.0.0.1 one","127.0.0.1 two","127.0.0.1 three","127.0.0.1 four"]
docker service update --host-rm one,two foo
docker service inspect --format '{{ json .Spec.TaskTemplate.ContainerSpec.Hosts }}' foo
["127.0.0.1 one","127.0.0.1 two","127.0.0.1 three","127.0.0.1 four"]
Specifying as separate options works;
docker service update --host-rm one:127.0.0.1 --host-rm two:127.0.0.1 foo
docker service inspect --format '{{ json .Spec.TaskTemplate.ContainerSpec.Hosts }}' foo
["127.0.0.1 three","127.0.0.1 four"]
docker service update --host-rm three --host-rm four foo
docker service inspect --format '{{ json .Spec.TaskTemplate.ContainerSpec.Hosts }}' foo
null
Possibly changing both flagHostAdd and flagHostRm to have a validator would fix the issue;
diff --git a/cli/command/service/update.go b/cli/command/service/update.go
index 413d1a125b..f55d5df794 100644
--- a/cli/command/service/update.go
+++ b/cli/command/service/update.go
@@ -61,7 +61,7 @@ func newUpdateCommand(dockerCLI command.Cli) *cobra.Command {
flags.SetAnnotation(flagDNSOptionRemove, "version", []string{"1.25"})
flags.Var(newListOptsVar(), flagDNSSearchRemove, "Remove a DNS search domain")
flags.SetAnnotation(flagDNSSearchRemove, "version", []string{"1.25"})
- flags.Var(newListOptsVar(), flagHostRemove, `Remove a custom host-to-IP mapping ("host:ip")`)
+ flags.Var(newListOptsVarWithValidator(opts.ValidateExtraHost), flagHostRemove, `Remove a custom host-to-IP mapping ("host:ip")`)
flags.SetAnnotation(flagHostRemove, "version", []string{"1.25"})
flags.Var(&options.labels, flagLabelAdd, "Add or update a service label")
flags.Var(&options.containerLabels, flagContainerLabelAdd, "Add or update a container label")
@@ -95,7 +95,7 @@ func newUpdateCommand(dockerCLI command.Cli) *cobra.Command {
flags.SetAnnotation(flagDNSOptionAdd, "version", []string{"1.25"})
flags.Var(&options.dnsSearch, flagDNSSearchAdd, "Add or update a custom DNS search domain")
flags.SetAnnotation(flagDNSSearchAdd, "version", []string{"1.25"})
- flags.Var(&options.hosts, flagHostAdd, `Add a custom host-to-IP mapping ("host:ip")`)
+ flags.Var(newListOptsVarWithValidator(opts.ValidateExtraHost), flagHostAdd, `Add a custom host-to-IP mapping ("host:ip")`)
flags.SetAnnotation(flagHostAdd, "version", []string{"1.25"})
flags.BoolVar(&options.init, flagInit, false, "Use an init inside each service container to forward signals and reap processes")
flags.SetAnnotation(flagInit, "version", []string{"1.37"})
hi @thaJeztah
Do you mean I should open and fix this new issue
Issue: --host-rm does not validate comma-separated values and silently ignores them ?