-
Notifications
You must be signed in to change notification settings - Fork 2.1k
cli/flags: add "hostVar" to handle --host / -H as a single string #6281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
c925a2d to
ecbea9b
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
--host / -H flag--host / -H flag
The ClientOptions struct and related flags were inherited from the Moby repository, where originally the CLI and Daemon used the same implementation and had a "Common" options struct. When the CLI moved to a separate repository, those structs were duplicated, but some daemon-specific logic remained. For example, the daemon can be configured to listen on multiple ports and sockets ([moby@dede158]), but the CLI [can only connect to a single host][1]. The daemon config also had to account for flags conflicting with `daemon.json`, and use special flag-vars for this ([moby@677a6b3]). Unfortunately, the `ClientConfig` struct became part of the public API and is used as argument in various places, but we can remove the use of the special flag var. This patch replaces the use of `NewNamedListOptsRef` for a regular `StringArray`. Unfortunately this changes the flag's type description from `list` to `stringArray`, but we can look at changing that separately. [moby@dede158]: moby/moby@dede158 [1]: https://github.com/moby/moby/blob/0af135e9065562e14a77439e13a29b4f1eb627a0/docker/docker.go#L191-L193 [moby@677a6b3]: moby/moby@677a6b3 Signed-off-by: Sebastiaan van Stijn <[email protected]>
ecbea9b to
0bc52fd
Compare
--host / -H flag0bc52fd to
bbbf93b
Compare
Benehiko
approved these changes
Aug 18, 2025
Member
Author
|
Oh! I need to update a test, because the flag-validation adds |
bbbf93b to
a96a409
Compare
hostVar is used for the '--host' / '-H' flag to set [ClientOptions.Hosts]. The [ClientOptions.Hosts] field is a slice because it was originally shared with the daemon config. However, the CLI only allows for a single host to be specified. hostVar presents itself as a "string", but stores the value in a string slice. It produces an error when trying to set multiple values, matching the check in [getServerHost]. [getServerHost]: https://github.com/docker/cli/blob/7eab668982645def1cd46fe1b60894cba6fd17a4/cli/command/cli.go#L542-L551 Signed-off-by: Sebastiaan van Stijn <[email protected]>
a96a409 to
f14eeeb
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cli/flags: use a regular StringArray for the
--host/-HflagThe ClientOptions struct and related flags were inherited from the Moby repository, where originally the CLI and Daemon used the same implementation and had a "Common" options struct. When the CLI moved to a separate repository, those structs were duplicated, but some daemon-specific logic remained. For example, the daemon can be configured to listen on multiple ports and sockets (moby@dede158), but the CLI can only connect to a single host. The daemon config also had to account for flags conflicting with
daemon.json, and use special flag-vars for this (moby@677a6b3).Unfortunately, the
ClientConfigstruct became part of the public API and is used as argument in various places, but we can remove the use of the special flag var. This patch replaces the use ofNewNamedListOptsReffor a regularStringArray.Unfortunately this changes the flag's type description from
listtostringArray, but we can look at changing that separately.cli/flags: add "hostVar" to handle --host / -H as a single string
hostVar is used for the '--host' / '-H' flag to set [ClientOptions.Hosts].
The [ClientOptions.Hosts] field is a slice because it was originally shared
with the daemon config. However, the CLI only allows for a single host to
be specified.
hostVar presents itself as a "string", but stores the value in a string
slice. It produces an error when trying to set multiple values, matching
the check in getServerHost.
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)