Skip to content

Conversation

kosiew
Copy link

@kosiew kosiew commented Aug 25, 2025

PR Title: support comma-separated values for --host-add in docker service update

Closes moby/moby#50769

- What I did

  • Updated --host-add flag in service update to support comma-separated host mappings (e.g., --host-add a:1.1.1.1,b:2.2.2.2).
  • Introduced ListOptsCSV, a wrapper around ListOpts that implements pflag.SliceValue for parsing comma-separated lists.
  • Adjusted updateHosts to use pflag.SliceValue instead of ListOpts.
  • Updated unit tests in update_test.go to 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 ListOptsCSV type in opts/opts.go that handles splitting and validating comma-separated values.
  • Switched flags.Var(&options.hosts, flagHostAdd, ...) to use opts.NewListOptsCSV(&options.hosts) in update.go.
  • Updated updateHosts to retrieve values via pflag.SliceValue.
  • Modified the service update reference docs to note that multiple entries can be separated by commas.

- How to verify it

  1. Run docker service update --host-add a:1.1.1.1,b:2.2.2.2 <service>.
  2. Inspect the service to confirm that both mappings (a → 1.1.1.1 and b → 2.2.2.2) are applied.
  3. Check that existing behavior (multiple --host-add flags) still works as expected.
  4. 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)
🦦

@kosiew kosiew requested review from thaJeztah and a team as code owners August 25, 2025 07:02
@thaJeztah
Copy link
Member

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 moby/moby#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

@thaJeztah
Copy link
Member

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"})

@kosiew
Copy link
Author

kosiew commented Aug 26, 2025

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
?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docker service update --host-add does not support list of hosts, though documentation specifies it does
2 participants