Skip to content

Conversation

@dvdksn
Copy link
Contributor

@dvdksn dvdksn commented May 5, 2023

Signed-off-by: David Karlsson david.karlsson@docker.com

- What I did

Added a description for the docker -H flag. Since we don't really have a better place to put this atm, I used the cli.md file.

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@dvdksn dvdksn requested a review from thaJeztah as a code owner May 5, 2023 14:49
@codecov-commenter
Copy link

codecov-commenter commented May 5, 2023

Codecov Report

Merging #4260 (759fa58) into master (b403a49) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4260      +/-   ##
==========================================
- Coverage   58.88%   58.87%   -0.01%     
==========================================
  Files         570      570              
  Lines       49566    49558       -8     
==========================================
- Hits        29186    29178       -8     
  Misses      18616    18616              
  Partials     1764     1764              

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Left some suggestions / comments

daemon with IP address `174.17.0.1`, listening on port `3131`:

```console
$ docker -H tcp://174.17.0.1:3131 ps
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps use :2376 here to encourage using the secure connection.

We have 3 ports registered; https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?search=docker

  • 2375: tcp, non-TLS (insecure)
  • 2376: tcp (TLS)
  • 2377: used for swarm (secure)

So only the first 2 are relevant for this one, but perhaps we need to mention them somewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a small note about it here, but we might want to add a similar description for this somewhere else too. Let's take it in a followup. LMK if the note I added seems OK to you.

@thaJeztah
Copy link
Member

Signed-off-by: David Karlsson <david.karlsson@docker.com>
@dvdksn dvdksn force-pushed the docs/host-flag branch from ffb812f to 759fa58 Compare May 7, 2023 19:46
@dvdksn dvdksn requested a review from thaJeztah May 8, 2023 11:34
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

// opts.ValidateHost is not used here, so as to allow connection helpers
hostOpt := opts.NewNamedListOptsRef("hosts", &o.Hosts, nil)
flags.VarP(hostOpt, "host", "H", "Daemon socket(s) to connect to")
flags.VarP(hostOpt, "host", "H", "Daemon socket to connect to")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a follow up we need to look at better wording for this, as this is not only for "sockets".

Okay for now, because we use this terminology in other places, so something to look at in a wider scope

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants