Skip to content

WIP: [cli] Interactive root rotation#2104

Closed
cyli wants to merge 5 commits into
moby:masterfrom
cyli:cli-root-rotation
Closed

WIP: [cli] Interactive root rotation#2104
cyli wants to merge 5 commits into
moby:masterfrom
cyli:cli-root-rotation

Conversation

@cyli
Copy link
Copy Markdown
Contributor

@cyli cyli commented Apr 7, 2017

This is based on moby/moby#31144.

Todo:

  • what should the command line flags for accepting a cert and key, and accepting a cert + external CAs look like?
  • should we be adding the progress bar to cluster inspect or add a new command to resume checking root CA progress?

Basic root rotation: https://asciinema.org/a/ddsnrnzob16v3ampmbqjf3j55
Root rotation when there's another root rotation: https://asciinema.org/a/19ah16sg29it0fyeellflpe68

This is stacked on top of #2100

cc @diogomonica @aaronlehmann

cyli added 5 commits April 6, 2017 17:53
…es all the nodes

to have an IssuanceStateRotate to trigger all the nodes to get new certificates.

When all the nodes have rotated their certificates to be signed by the desired issuer,
complete root rotation.

Signed-off-by: cyli <ying.li@docker.com>
Signed-off-by: cyli <ying.li@docker.com>
…ng the store for

all nodes at intervals, rely on the cluster and node watches to update an in-memory mapping
of the current nodes.  At regular intervals, update the store to tell a throttled number
of the unconverged nodes to rotate their certificates.

Also, remove the leader rotation part of the root rotation integration test, as that
takes a very long time. There are server tests to ensure that multiple CA servers
running reconciliation loops, and starting a CA server from a stopped state, does not
break root reconciliation.

Signed-off-by: cyli <ying.li@docker.com>
Signed-off-by: cyli <ying.li@docker.com>
@aaronlehmann
Copy link
Copy Markdown
Collaborator

This is neat but I'm not sure about putting fancy features into swarmctl. Shouldn't we be focusing our energy on the docker CLI?

I'm a bit confused about the long-term future of swarmctl. I know there are efforts to make it work for "stand alone swarmd", but it seems strange to fragment our efforts across two CLIs. What do we gain by having two CLIs?

cc @aluzzardi

@cyli
Copy link
Copy Markdown
Contributor Author

cyli commented Apr 7, 2017

@aaronlehmann Makes sense - I interpreted swarmctl as more of a 'trial run for the docker CLI'. Happy to just close this and add it to docker instead.

@thaJeztah
Copy link
Copy Markdown
Member

From a UX perspective looking at the example recordings ;

  • I missed feedback why the progressbar "hangs" (node unavailable, other rotation in progress). Without this, we can expect bugreports like "bug: ca rotation hangs"
  • What happens if you cancel the action (^C)? (e.g. in the example where one node was down); does it roll back?

@aaronlehmann
Copy link
Copy Markdown
Collaborator

I think it is worth having a --rotate-ca option in swarmctl, and displaying the TLS info in inspect, but I'd vote against having interactive progress in swarmctl, at least for now. Among other things it looks like it would involve vendoring some Docker CLI stuff that I don't really want to bring in.

IMHO the priority should be getting this working end-to-end in the Docker CLI. Hopefully the progress indicator can be ported over there without too much trouble.

@cyli
Copy link
Copy Markdown
Contributor Author

cyli commented Apr 7, 2017

What happens if you cancel the action (^C)? (e.g. in the example where one node was down); does it roll back?

@thaJeztah No, it detaches from waiting, and the root rotation goes on (or not) as it was.

@cyli
Copy link
Copy Markdown
Contributor Author

cyli commented Apr 7, 2017

If the priority is just the docker CLI, I'll abandon this one and just open the docker one, if that's ok. We can come back and add stuff to swarmctl later.

@cyli cyli mentioned this pull request Apr 10, 2017
10 tasks
@diogomonica
Copy link
Copy Markdown
Contributor

SGTM

@cyli cyli closed this Apr 10, 2017
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.

4 participants