Add information for `Manager Addresses` in the output of `docker info` #28042

yongtang · 2016-11-03T18:41:28Z

- What I did
As is specified in #28018, it would be useful to know the manager's addresses even in a worker node. This is especially useful when there are many worker nodes in a big cluster.

The information is available in info.Swarm.RemoteManagers.

This fix add the information of Manager Addresses to the output of docker info, to explicitly show it.

- How I did it

- How to verify it

A test has been added for this fix.

Also the new output:

$ docker info
...
Cgroup Driver: cgroupfs
Plugins: 
 Volume: local
 Network: bridge host macvlan null overlay
Swarm: active
 NodeID: bu6m8b45fxu8ac4w4wx26iw19
 Is Manager: false
 Node Address: 127.0.0.1
 Manager Addresses: 127.0.0.1:2477
Runtimes: runc
Default Runtime: runc
...

- Description for the changelog

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

This fix fixes #28018.

Signed-off-by: Yong Tang yong.tang.github@outlook.com

allencloud · 2016-11-03T23:33:26Z

cli/command/system/info.go

+		}
+		sort.Strings(managers)
+		if len(managers) > 0 {
+			fmt.Fprintf(dockerCli.Out(), " Manager Addresses: %s\n", strings.Join(managers, " "))


I was wondering if there is lots of managers, this line could be some kind of unreadable. Is it OK for separate line for each one? Just my thoughts.

Thanks @allencloud. The PR has been updated with separated lines.

allencloud · 2016-11-03T23:34:43Z

Or just show the address of leader manager?

yongtang · 2016-11-04T00:43:59Z

@allencloud To show the lead manager is slightly tricky because it needs to query other managers to get the leader. If there is an error while querying managers, not sure we should return an error (error for docker info), or we should just ignore.

vdemeester · 2016-11-04T07:12:27Z

/cc @aluzzardi @stevvooe @aaronlehmann

stevvooe · 2016-11-08T23:01:04Z

Or just show the address of leader manager?

Also, the utility of knowing the leader is limited, since it may change at any time. From the perspective of a client, the managers should be treated equally. The current leader should only be of interest the quorum.

allencloud · 2016-11-09T02:34:42Z

Thanks @stevvooe
I think we need to think more about whether to bring this or how to bring this better.
If we have lots of managers, docker info would be not so reasonable than before with a list of manager address. I got your point of leader's meaningless for client.

In addition, a rebase needed, though. @yongtang

yongtang · 2016-11-09T02:38:11Z

Thanks @allencloud, rebased.

thaJeztah · 2016-11-10T15:24:48Z

Sorry @yongtang rebase needed again 😢 (it's busy before code freeze 😄)

yongtang · 2016-11-10T17:19:56Z

Thanks @thaJeztah. The PR has been rebased. 😅

aaronlehmann · 2016-11-10T23:48:52Z

LGTM

tonistiigi · 2016-11-11T02:07:09Z

@yongtang needs rebase

LGTM

As is specified in 28018, it would be useful to know the manager's addresses even in a worker node. This is especially useful when there are many worker nodes in a big cluster. The information is available in `info.Swarm.RemoteManagers`. This fix add the information of `Manager Addresses` to the output of `docker info`, to explicitly show it. A test has been added for this fix. This fix fixes 28018. Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

yongtang · 2016-11-11T15:40:24Z

Thanks @tonistiigi @aaronlehmann for the review. The PR has been rebased.

thaJeztah · 2016-11-11T16:02:48Z

I added this to the 1.13 milestone, @yongtang can you do a follow up PR for docs?

yongtang · 2016-11-11T17:50:32Z

Thanks @thaJeztah . Since this PR only changes the output of docker info and there is no API changes, I only updated the docs/man for docker info in PR #28316.

thaJeztah · 2016-11-11T17:51:58Z

Perfect, thanks! i'll review later

…anagers Add information for `Manager Addresses` in the output of `docker info`

GordonTheTurtle added the status/0-triage label Nov 3, 2016

yongtang mentioned this pull request Nov 3, 2016

[Question] Can worker in Swarm get address of managers? #28018

Closed

allencloud reviewed Nov 3, 2016

View reviewed changes

yongtang force-pushed the 28018-docker-info-swarm-managers branch from a40642a to e8317cc Compare November 4, 2016 00:36

yongtang force-pushed the 28018-docker-info-swarm-managers branch from e8317cc to a7c2881 Compare November 4, 2016 04:21

vdemeester added status/1-design-review and removed status/0-triage labels Nov 4, 2016

yongtang force-pushed the 28018-docker-info-swarm-managers branch from a7c2881 to db93ca7 Compare November 8, 2016 03:36

yongtang force-pushed the 28018-docker-info-swarm-managers branch from db93ca7 to 3434635 Compare November 9, 2016 02:37

yongtang force-pushed the 28018-docker-info-swarm-managers branch from 3434635 to 7083b29 Compare November 10, 2016 17:18

yongtang force-pushed the 28018-docker-info-swarm-managers branch from 7083b29 to 828bd44 Compare November 11, 2016 15:39

thaJeztah added this to the 1.13.0 milestone Nov 11, 2016

thaJeztah added status/4-merge docs/revisit impact/changelog impact/cli impact/documentation and removed status/1-design-review labels Nov 11, 2016

thaJeztah mentioned this pull request Nov 11, 2016

add 1.13.0 CHANGELOG.md #28275

Merged

vieux merged commit 6f3720c into moby:master Nov 11, 2016

yongtang mentioned this pull request Nov 11, 2016

Update docs for docker info for most recent changes. #28316

Merged

yongtang deleted the 28018-docker-info-swarm-managers branch November 11, 2016 17:56

thaJeztah removed the docs/revisit label Nov 24, 2016

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Merge pull request moby#28042 from yongtang/28018-docker-info-swarm-m…

460a7e2

…anagers Add information for `Manager Addresses` in the output of `docker info`

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add information for `Manager Addresses` in the output of `docker info` #28042

Add information for `Manager Addresses` in the output of `docker info` #28042

yongtang commented Nov 3, 2016 •

edited

Loading

allencloud Nov 3, 2016

yongtang Nov 4, 2016

allencloud commented Nov 3, 2016

yongtang commented Nov 4, 2016

vdemeester commented Nov 4, 2016

stevvooe commented Nov 8, 2016

allencloud commented Nov 9, 2016

yongtang commented Nov 9, 2016

thaJeztah commented Nov 10, 2016

yongtang commented Nov 10, 2016

aaronlehmann commented Nov 10, 2016

tonistiigi commented Nov 11, 2016

yongtang commented Nov 11, 2016

thaJeztah commented Nov 11, 2016

yongtang commented Nov 11, 2016

thaJeztah commented Nov 11, 2016

Add information for Manager Addresses in the output of docker info #28042

Add information for Manager Addresses in the output of docker info #28042

Conversation

yongtang commented Nov 3, 2016 • edited Loading

allencloud Nov 3, 2016

Choose a reason for hiding this comment

yongtang Nov 4, 2016

Choose a reason for hiding this comment

allencloud commented Nov 3, 2016

yongtang commented Nov 4, 2016

vdemeester commented Nov 4, 2016

stevvooe commented Nov 8, 2016

allencloud commented Nov 9, 2016

yongtang commented Nov 9, 2016

thaJeztah commented Nov 10, 2016

yongtang commented Nov 10, 2016

aaronlehmann commented Nov 10, 2016

tonistiigi commented Nov 11, 2016

yongtang commented Nov 11, 2016

thaJeztah commented Nov 11, 2016

yongtang commented Nov 11, 2016

thaJeztah commented Nov 11, 2016

Add information for `Manager Addresses` in the output of `docker info` #28042

Add information for `Manager Addresses` in the output of `docker info` #28042

yongtang commented Nov 3, 2016 •

edited

Loading