8000 Warn on 'weave reset' if we cannot release IP space by bboreham · Pull Request #2332 · weaveworks/weave · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Jun 20, 2024. It is now read-only.

Warn on 'weave reset' if we cannot release IP space #2332

Merged
merged 1 commit into from
Jun 6, 2016
Merged

Warn on 'weave reset' if we cannot release IP space #2332

merged 1 commit into from
Jun 6, 2016

Conversation

bboreham
Copy link
Contributor
@bboreham bboreham commented Jun 2, 2016

Fixes #2326

I did not put anything in about re-launching and re-resetting, because during reset we will have deleted the persistence data so re-resetting would require that we can connect to another peer who can supply us with our old IPAM ring and then do the reset.

@bboreham bboreham added this to the 1.6.0 milestone Jun 2, 2016
@rade
Copy link
Member
rade commented Jun 2, 2016

Hmm. If the user has just run weave stop, this is very much a "you shouldn't have done that; but I'll carry on anyway" behaviour. Or, it can be an annoyance, in the case where there was just one node.

Perhaps a) the warning should more directly highlight that problem, and b) instead of warning this should be an error, which the user can get around with a 8000 --force.

@bboreham
Copy link
Contributor Author
bboreham commented Jun 3, 2016

highlight that problem

what is "that problem"? They shouldn't have run stop ?

If the underlying issue is that the weave router has crashed (maybe is repeatedly crashing), this would be unhelpful.

I guess we can check with Docker that the weave container exists, is not running, Restarting is false and ExitCode is 0, then assume the user stopped it.

@bboreham
Copy link
Contributor Author
bboreham commented Jun 3, 2016

We could have a weaveutil command that examines the persistent IPAM data to check if there were any other peers in the ring.

@rade
Copy link
Member
rade commented Jun 3, 2016

what is "that problem"? They shouldn't have run stop ?

Correct. And I know from my own workflow that I run weave reset habitually, often some time after a weave stop. Getting a warning in this scenario is just annoying, but I can easily retrain my fingers to type weave reset --force to avoid it.

@bboreham bboreham self-assigned this Jun 3, 2016
@bboreham bboreham force-pushed the issues/2326-warn-rmpeer branch from 1978dd6 to 39326f6 Compare June 3, 2016 11:10
@bboreham bboreham removed their assignment Jun 3, 2016
@bboreham
Copy link
Contributor Author
bboreham commented Jun 3, 2016

I added the --force and specific handling for the running, not-running and not-present cases.

I did not handle the case where the container seems to be running but the DELETE fails, and I did not check to see if some other bogus argument was added after weave reset.

awh
awh reviewed Jun 3, 2016
View reviewed changes
weave
@@ -2177,11 +2177,25 @@ EOF
stop_plugin
;;
reset)
[ $# -eq 0 ] || usage
[ $# -lt 2 ] || usage

This comment was marked as abuse.

Sign in to view

@awh
Copy link
Contributor
awh commented Jun 6, 2016

One minor comment + needs rebase

@bboreham bboreham force-pushed the issues/2326-warn-rmpeer branch from 39326f6 to 9a17439 Compare June 6, 2016 14:46
@bboreham
Copy link
Contributor Author
bboreham commented Jun 6, 2016

Updated and rebased

@bboreham bboreham removed their assignment Jun 6, 2016
@awh awh merged commit 843a993 into master Jun 6, 2016
@awh awh deleted the issues/2326-warn-rmpeer branch June 6, 2016 15:56
@bboreham bboreham changed the title Warn on 'weave reset' if DELETE fails Warn on 'weave reset' if we cannot release IP space Jun 14, 2016
@bboreham bboreham mentioned this pull request Jun 14, 2016
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.6.0
Development

Successfully merging this pull request may close these issues.
3 participants
@bboreham @rade @awh
0