Check manually-assigned addresses with IPAM to see if they clash #1200

bboreham · 2015-07-20T17:37:32Z

Fixes #687 and #598

One quiet change in this PR:

avoid printing error messages from 'claim' twice in the logs

rade · 2015-07-20T19:18:17Z

I could add some more lines to remove this if desired.

yes, please.

bboreham · 2015-07-21T09:24:10Z

Done

ipam/allocator.go

 	resultChan := make(chan error)
-	op := &claim{resultChan: resultChan, ident: ident, addr: addr}
+	op := &claim{resultChan: resultChan, ident: ident, addr: addr, launch: launch}


squaremo · 2015-07-21T15:43:59Z

I think these are two different operations, rather than a variation on one operation (and perhaps that is why I was confused by "launch").

The first is to tell weave about an IP that had been claimed previously. The second is to ask for a particular IP that has not been handed out previously. It's actually the latter that deserves the name "Claim"; the former is more like, well, "Reclaim" or "Recover".

How this is reflected in the HTTP API, I'm not sure -- PUT to a URL seems a good match for both. So a query arg might be appropriate for discriminating.

bboreham · 2015-07-21T16:05:56Z

I renamed 'launch' to 'reclaim'; rebased and squashed.
Also changed the behaviour to output a message and carry on when IPAM is waiting for quorum, rather than hang the caller. This is more in line with what #687 asks for.

ipam/claim.go

@@ -12,6 +12,7 @@ type claim struct {
 	resultChan chan<- error
 	ident      string
 	addr       address.Address
+	reclaim    bool
 }

 func (c *claim) sendResult(result error) {


…tter

bboreham · 2015-07-23T19:18:14Z

I decided noErrorOnUnknown described the behaviour best.

ipam/claim.go

+			alloc.infof("Claim %s for %s: address allocator still initializing; will try later.", c.addr, c.ident)
+			c.sendResult(nil) // don't make the caller wait
+		} else {
+			c.sendResult(fmt.Errorf("%s is in the range %s, but the allocator is not initialized yet", c.addr, alloc.universe.AsCIDRString()))


rade · 2015-09-03T09:03:16Z

I struggle to see how this solves #598. Please explain.

bboreham · 2015-09-03T09:13:42Z

Line 866 in the weave script informs IPAM of a manually-selected address. If this can be granted, it is, and hence IPAM will not then allocate that address to something else, which I thought was the main issue of #598.

rade · 2015-09-03T09:14:19Z

ok

bboreham · 2015-09-03T09:15:18Z

I should perhaps have updated the docs, where it says

make sure they don’t conflict with any IP ranges in use on the hosts (including those delegated to weave’s automatic IP address allocator)

rade · 2015-09-03T09:20:08Z

Surely http://docs.weave.works/weave/master/head/ipam.html#manual should be updated.

Update docs for mixed manual and automatic IP allocation Missed from #1200.

squaremo self-assigned this Jul 21, 2015

squaremo reviewed Jul 21, 2015
View reviewed changes

Check manually-assigned addresses with IPAM to see if they clash

647b755

bboreham force-pushed the 687-detect-ipam-clash branch from c406723 to 647b755 Compare July 21, 2015 16:09

squaremo reviewed Jul 22, 2015
View reviewed changes

ipam/claim.go

@@ -12,6 +12,7 @@ type claim struct {

resultChan chan<- error

ident string

addr address.Address

reclaim bool

}

func (c *claim) sendResult(result error) {

This comment was marked as abuse.

Sign in to view

Rename 'reclaim' to 'noErrorUnknown' which describes the behaviour be…

f091522

…tter

squaremo reviewed Jul 27, 2015
View reviewed changes

squaremo merged commit f091522 into master Jul 27, 2015

This was referenced Jul 28, 2015

frequent failure in 150_connect_forget_test #1030

Closed

Don't init the ring if the address is not in range. #1228

Merged

rade deleted the 687-detect-ipam-clash branch July 28, 2015 17:21

rade modified the milestone: 1.1.0 Jul 29, 2015

bboreham mentioned this pull request Aug 24, 2015

[ipam] mixing manual and automatic allocation in the same subnet #598

Closed

bboreham mentioned this pull request Sep 3, 2015

Update docs for mixed manual and automatic IP allocation #1380

Merged

rade added a commit that referenced this pull request Sep 3, 2015

Merge pull request #1380 from weaveworks/598-update-docs

c71d9b1

Update docs for mixed manual and automatic IP allocation Missed from #1200.

bboreham mentioned this pull request Nov 4, 2015

IPAM docs are out of date wrt manual allocation #1633

Closed

bboreham mentioned this pull request Mar 24, 2016

Report errors when weave script talks to weave component via http #2096

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check manually-assigned addresses with IPAM to see if they clash #1200

Check manually-assigned addresses with IPAM to see if they clash #1200

bboreham commented Jul 20, 2015

rade commented Jul 20, 2015

bboreham commented Jul 21, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

squaremo commented Jul 21, 2015

bboreham commented Jul 21, 2015

This comment was marked as abuse.

bboreham commented Jul 23, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

rade commented Sep 3, 2015

bboreham commented Sep 3, 2015

rade commented Sep 3, 2015

bboreham commented Sep 3, 2015

rade commented Sep 3, 2015

Check manually-assigned addresses with IPAM to see if they clash #1200

Check manually-assigned addresses with IPAM to see if they clash #1200

Conversation

bboreham commented Jul 20, 2015

rade commented Jul 20, 2015

bboreham commented Jul 21, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

squaremo commented Jul 21, 2015

bboreham commented Jul 21, 2015

This comment was marked as abuse.

bboreham commented Jul 23, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

rade commented Sep 3, 2015

bboreham commented Sep 3, 2015

rade commented Sep 3, 2015

bboreham commented Sep 3, 2015

rade commented Sep 3, 2015