Do not clear swarm directory on swarm init and swarm join#33341
Do not clear swarm directory on swarm init and swarm join#33341aaronlehmann merged 1 commit intomoby:masterfrom cyli:do-not-clear-state-on-swarm-init-join
swarm init and swarm join#33341Conversation
|
Do we need to also clear the state if a join operation fails ( |
|
@aaronlehmann That seems like a good idea - is there any reason we'd want to keep state from prior to a join, though? Similarly, if init fails, the pre-generated certificates would also be wiped out, which seems ok to me but may be inconvenient for users to have to re-generate (although I'd like to deprecate having to do that anyway). |
I don't think so. I think the reason we need to remove it after a failed join is to avoid ending up with an inconsistent state that prevents a second |
|
@cyli build error in ci |
|
@tonistiigi Oops, sorry, thanks! |
daemon/cluster/swarm.go
Outdated
There was a problem hiding this comment.
I think c.nr should be cleared whether the state clearing fails or not.
…join now. However, do clear the directory if init or join fails, because we don't want to leave it in a half-finished state. Signed-off-by: Ying Li <ying.li@docker.com>
|
LGTM |
1 similar comment
|
LGTM |
|
@cyli: Does this need to be cherry-picked in 17.06? @thaJeztah: @GordonTheTurtle added a 17.06.0 milestone automatically, but I believe this is an error, since 17.06 has already branched. |
|
@aaronlehmann Yes that would be ideal, thank you. |
|
@aaronlehmann 17.06 already branched, but we may have to remove milestones from this repository, because moby milestones != docker milestones |
|
@aaronlehmann 17.06 already branched, but we may have to remove milestones from this repository, because moby milestones != docker milestones
Understood, but let's please make sure the bot does not add misleading milestones.
What do we need to do to make sure this gets cherry-picked downstream?
|
|
I added the cherry-pick label for visibility, but not sure at this moment what to do with the |
However, this continues to clear the directory if init fails, because we don't want to leave it in a half-finished state.
This would fix #33216 to be compatible with the method for pre-generating external CA certs that was in 17.03.
After some discussion with @aaronlehmann and @tonistiigi, this is probably the correct behavior (unless the state dir can get corrupted?) either way.
This would also allow the extra flags as suggested in #33216 to be added after 17.06 if necessary.