Remove swarm inspect and use info instead #24492

vdemeester · 2016-07-10T16:26:40Z

Remove the docker swarm inspect command and use docker info instead to display swarm information if the current node is a manager 🐮.

This only change the cli side and the return of /info but not the /swarm/inspect endpoint. The attribute used in the info endpoint is Swarm.Cluster and is populated only when the node is a manager.

$ docker info
# […]
Swarm: active
 NodeID: b1n3yqwdmvgpbhgmiv4xh48xy
 Is Manager: true
 ClusterID: d3aen0t7fkdw6kq1y4e3w4vv6
 Managers: 1
 Nodes: 1
 Name: default
 Orchestration:
  Task History Retention: 10
 Raft:
  Snapshot interval: 10000
  Heartbeat tick: 1
  Election tick: 3
 Dispatcher:
  Heartbeat period: 5 seconds
 CA configuration:
  Expiry duration: 3 months
# […]

I did not add all the thing displayed by swarm inspect but they are available through the API.

Complete attribute displayed ?
~~Remove /swarm/inspect endpoint ?~~

I'll create a engine-api if this pass design-review 😉

Closes #24148

/cc @aluzzardi @tiborvass @stevvooe @tonistiigi

Signed-off-by: Vincent Demeester vincent@sbr.pm

AkihiroSuda · 2016-07-11T06:46:41Z

api/client/system/info.go

+			fmt.Fprintf(dockerCli.Out(), " Acceptance Policies:\n")
+			for _, policy := range info.Swarm.Cluster.Spec.AcceptancePolicy.Policies {
+				fmt.Fprintf(dockerCli.Out(), "  %s:\n", policy.Role)
+				fmt.Fprintf(dockerCli.Out(), "   auto-accept: %t\n", policy.Autoaccept)


I feel Auto-acceptance and Secret are better for consistency

@AkihiroSuda you're right 👍

icecrime · 2016-07-12T22:18:16Z

A few remarks about acceptance policies:

I don't think we should display the secret: people copy/paste this in GitHub issues 😇
We can probably condense the output (e.g., Auto-accept: (manager|worker|none)).

vdemeester · 2016-07-13T13:59:40Z

Rebased and hopefully, tests fixed 👼

@icecrime

I don't think we should display the secret: people copy/paste this in GitHub issues 😇

docker swarm inspect did display it but I'm ok with not displaying in docker info (it will still be present in the json)

We can probably condense the output (e.g., Auto-accept: (manager|worker|none)).

That's an idea, I'll look into it.

vdemeester · 2016-07-13T19:47:07Z

Hum, removing /swarm/inspectmeans removing client.SwarmInspect (from engine-api) and means making swarm update call the /info endpoint instead.

I'm not sure we should remove it, just adding Cluster information in /info.

@thaJeztah @icecrime wdyt ?

thaJeztah · 2016-07-13T19:58:55Z

Perhaps we can keep the inspect endpoint for now, if that simplifies things

tiborvass · 2016-07-13T20:13:54Z

@vdemeester sorry but would you mind linking where swarm update calls /swarm (inspect) ?

vdemeester · 2016-07-13T20:16:54Z

@tiborvass yes, here https://github.com/docker/docker/blob/master/api/client/swarm/update.go#L35 👼

icecrime · 2016-07-13T20:38:33Z

I think it's ok for swarm update to call the /info endpoint as long as the SwarmSpec is a nested element of the result, and not something that I need to construct myself for the purpose of calling /swarm/update.

icecrime · 2016-07-13T20:38:54Z

Also 😉

11:22:56 FAIL: docker_cli_swarm_test.go:15: DockerSwarmSuite.TestSwarmUpdate
11:22:56 
11:22:56 [d66998809] waiting for daemon to start
11:22:56 [d66998809] daemon started
11:22:56 docker_cli_swarm_test.go:24:
11:22:56     c.Assert(err, checker.IsNil, check.Commentf("out: %v", out))
11:22:56 ... value *exec.ExitError = &exec.ExitError{ProcessState:(*os.ProcessState)(0xc822f77080), Stderr:[]uint8(nil)} ("exit status 1")
11:22:56 ... out: Error response from daemon: page not found

vdemeester · 2016-07-13T20:46:46Z

@icecrime ok, I'll update and prepare a PR for engine-api. The test failure is related to that 😉

vdemeester · 2016-07-14T12:11:48Z

Updated, fixed test failures and created a PR in engine-api (docker/engine-api#318) — vendor is expected to fail 👼.

tiborvass · 2016-07-15T22:22:26Z

@vdemeester let's keep the API unchanged and have inspect make multiple calls for now. Not sure if you would still need docker/engine-api#318 then.

vdemeester · 2016-07-16T07:25:32Z

@tiborvass you mean info 👼

vdemeester · 2016-07-19T13:44:01Z

Updated 👼

vieux · 2016-07-19T22:51:52Z

@vdemeester docker_cli_swarm_test.go uses docker swarm inspect

vdemeester · 2016-07-20T06:48:37Z

@vieux ah damn.. revert the wrong file… I'll fix 👼

tiborvass · 2016-07-23T00:49:36Z

@vdemeester some updates we need on this one:

Rebase :)
Update description to show the new information, since the result from the swarm API has changed (no more acceptance policy, CAhash, etc.)
Make sure we DO NOT display join tokens (because they are returned by the API)
Make only 1 API call to /info, and gather all the swarm information server side.
Having Cluster: and Swarm: makes no sense, they are synonyms, they should be merged, and ID be renamed to ClusterID.
Remove Version from Cluster:
Heartbeat period should use human formatting for duration

kencochrane · 2016-07-23T16:06:27Z

Can we add an IsLeader field to show if this node is the lead manager. I'm currently using swarm inspect for this, but if swarm inspect is going away, then I need another way to get that value.

Another question. Will Cluster:Id be available on the worker nodes as well as the manager nodes? It isn't today and that would be very helpful. It is an easy way to see what swarm a node is in for logging and analytics/metrics gathering for nodes by cluster.

vdemeester · 2016-07-23T17:28:09Z

Make only 1 API call to /info, and gather all the swarm information server side.

Ok this mean I need to make a PR in engine-api 👼

Make sure we DO NOT display join tokens (because they are returned by the API)

Removing only in cli right ? (will stay in the /info api response)

aluzzardi · 2016-07-23T21:02:32Z

Hey Ken, regarding IsLeader, I think you can grab that from docker node
inspect self -f "{{ something something }}"

I do agree it'd be useful in docker info regardless so +1 for that request

On Saturday, July 23, 2016, Vincent Demeester notifications@github.com
wrote:

Make only 1 API call to /info, and gather all the swarm information server
side.

Ok this mean I need to make a PR in engine-api 👼

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#24492 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAK9Lw_GdCh3gr-72PmrXCud3YXN8VH7ks5qYk-CgaJpZM4JI3dy
.

kencochrane · 2016-07-23T21:26:21Z

@aluzzardi thanks for the work around. The one issue with docker node is that it only works for manager nodes, where as docker info works for all mode types.

aluzzardi · 2016-07-23T23:27:08Z

Indeed - but for your use case, you can assume that if it's not a manager
node then it's probably not the leader :)

However I do still agree with you that this information should be available
on docker info - just pointing out a workaround

On Saturday, July 23, 2016, Ken Cochrane notifications@github.com wrote:

@aluzzardi https://github.com/aluzzardi thanks for the work around. The
one issue with docker node is that it only works for manager nodes, where
as docker info works for all mode types.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#24492 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAK9L3EO3jswn-gSR1qJqmr0kjvx2HJpks5qYodVgaJpZM4JI3dy
.

Remove the swarm inspect command and use docker info instead to display swarm information if the current node is a manager. Signed-off-by: Vincent Demeester <vincent@sbr.pm>

tiborvass · 2016-07-25T16:29:18Z

@vdemeester I was thinking about not returning join tokens via the /info API at all not just CLI

tiborvass · 2016-07-26T03:30:02Z

Carried in #25042

GordonTheTurtle added the status/0-triage label Jul 10, 2016

vdemeester added this to the 1.12.0 milestone Jul 10, 2016

vdemeester added status/1-design-review process/cherry-pick and removed status/0-triage labels Jul 10, 2016

AkihiroSuda reviewed Jul 11, 2016
View reviewed changes

vdemeester force-pushed the 24148-swarm-inspect-info branch from 61f43f5 to 978d220 Compare July 11, 2016 19:42

icecrime added the status/failing-ci Indicates that the PR in its current state fails the test suite label Jul 12, 2016

vdemeester force-pushed the 24148-swarm-inspect-info branch from 978d220 to 9fe76d1 Compare July 13, 2016 13:57

vdemeester force-pushed the 24148-swarm-inspect-info branch from 9fe76d1 to 6b47a0c Compare July 13, 2016 17:38

vdemeester force-pushed the 24148-swarm-inspect-info branch from 6b47a0c to a344dd6 Compare July 14, 2016 10:18

vdemeester mentioned this pull request Jul 14, 2016

Remove SwarmInspect method. docker/engine-api#318

Closed

vdemeester removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Jul 14, 2016

mlaventure added the status/needs-vendoring label Jul 14, 2016

aaronlehmann mentioned this pull request Jul 16, 2016

Proposal: Replace secrets with "join tokens" #24430

Closed

vdemeester force-pushed the 24148-swarm-inspect-info branch from a344dd6 to 92c7aa5 Compare July 16, 2016 07:31

vdemeester removed the status/needs-vendoring label Jul 16, 2016

vdemeester force-pushed the 24148-swarm-inspect-info branch from 92c7aa5 to ec2f656 Compare July 16, 2016 08:26

vdemeester force-pushed the 24148-swarm-inspect-info branch 5 times, most recently from 56e8729 to 8eb3f97 Compare July 22, 2016 10:37

tiborvass self-assigned this Jul 23, 2016

vdemeester force-pushed the 24148-swarm-inspect-info branch from 8eb3f97 to 9a134db Compare July 24, 2016 08:32

vdemeester mentioned this pull request Jul 24, 2016

Add swarm.Swarm to swarm.Info… docker/engine-api#334

Merged

Remove swarm inspect and use info instead

3cf0b15

Remove the swarm inspect command and use docker info instead to display swarm information if the current node is a manager. Signed-off-by: Vincent Demeester <vincent@sbr.pm>

vdemeester force-pushed the 24148-swarm-inspect-info branch from 9a134db to 3cf0b15 Compare July 24, 2016 08:54

vdemeester added the status/needs-vendoring label Jul 25, 2016

tiborvass mentioned this pull request Jul 26, 2016

Carry 24492: Remove swarm inspect and use info instead #25042

Merged

tiborvass closed this Jul 26, 2016

tiborvass removed the process/cherry-pick label Jul 26, 2016

vdemeester deleted the 24148-swarm-inspect-info branch July 26, 2016 07:27

kencochrane mentioned this pull request Jul 27, 2016

Add swarm Is Leader field to docker info #25065

Open

thaJeztah mentioned this pull request Jul 27, 2016

Remove join-tokens from /info API #25134

Closed

This was referenced Jul 27, 2016

Carry #25134: Remove join-tokens from the /info API #25145

Merged

Remove join-tokens from the /info API #25154

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove swarm inspect and use info instead #24492

Remove swarm inspect and use info instead #24492

vdemeester commented Jul 10, 2016 •

edited

AkihiroSuda Jul 11, 2016

vdemeester Jul 11, 2016

icecrime commented Jul 12, 2016

vdemeester commented Jul 13, 2016

vdemeester commented Jul 13, 2016

thaJeztah commented Jul 13, 2016

tiborvass commented Jul 13, 2016

vdemeester commented Jul 13, 2016

icecrime commented Jul 13, 2016

icecrime commented Jul 13, 2016

vdemeester commented Jul 13, 2016

vdemeester commented Jul 14, 2016

tiborvass commented Jul 15, 2016 •

edited

vdemeester commented Jul 16, 2016

vdemeester commented Jul 19, 2016

vieux commented Jul 19, 2016

vdemeester commented Jul 20, 2016

tiborvass commented Jul 23, 2016

kencochrane commented Jul 23, 2016

vdemeester commented Jul 23, 2016 •

edited

aluzzardi commented Jul 23, 2016

kencochrane commented Jul 23, 2016

aluzzardi commented Jul 23, 2016

tiborvass commented Jul 25, 2016

tiborvass commented Jul 26, 2016

Remove swarm inspect and use info instead #24492

Remove swarm inspect and use info instead #24492

Conversation

vdemeester commented Jul 10, 2016 • edited

AkihiroSuda Jul 11, 2016

Choose a reason for hiding this comment

vdemeester Jul 11, 2016

Choose a reason for hiding this comment

icecrime commented Jul 12, 2016

vdemeester commented Jul 13, 2016

vdemeester commented Jul 13, 2016

thaJeztah commented Jul 13, 2016

tiborvass commented Jul 13, 2016

vdemeester commented Jul 13, 2016

icecrime commented Jul 13, 2016

icecrime commented Jul 13, 2016

vdemeester commented Jul 13, 2016

vdemeester commented Jul 14, 2016

tiborvass commented Jul 15, 2016 • edited

vdemeester commented Jul 16, 2016

vdemeester commented Jul 19, 2016

vieux commented Jul 19, 2016

vdemeester commented Jul 20, 2016

tiborvass commented Jul 23, 2016

kencochrane commented Jul 23, 2016

vdemeester commented Jul 23, 2016 • edited

aluzzardi commented Jul 23, 2016

kencochrane commented Jul 23, 2016

aluzzardi commented Jul 23, 2016

tiborvass commented Jul 25, 2016

tiborvass commented Jul 26, 2016

vdemeester commented Jul 10, 2016 •

edited

tiborvass commented Jul 15, 2016 •

edited

vdemeester commented Jul 23, 2016 •

edited