From 78c4b06492c0db83aad654aed84f8d81670d5647 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 8 Jul 2015 16:35:21 +0100 Subject: [PATCH] Don't wait for certainty before responding to Claim Conflicts: ipam/claim.go --- ipam/claim.go | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/ipam/claim.go b/ipam/claim.go index 8d96e6e2..750ad950 100644 --- a/ipam/claim.go +++ b/ipam/claim.go @@ -3,6 +3,7 @@ package ipam import ( "fmt" + "github.com/weaveworks/weave/common" "github.com/weaveworks/weave/ipam/address" "github.com/weaveworks/weave/router" ) @@ -14,6 +15,18 @@ type claim struct { hasBeenCancelled func() bool } +func (c *claim) sendResult(result error) { + // Make sure we only send a result once, since listener stops listening after that + if c.resultChan != nil { + c.resultChan <- result + close(c.resultChan) + c.resultChan = nil + } + if result != nil { + common.Error.Println("[allocator] " + result.Error()) + } +} + // Try returns true for success (or failure), false if we need to try again later func (c *claim) Try(alloc *Allocator) bool { if (c.hasBeenCancelled)() { @@ -24,7 +37,7 @@ func (c *claim) Try(alloc *Allocator) bool { if !alloc.ring.Contains(c.addr) { // Address not within our universe; assume user knows what they are doing alloc.infof("Ignored address %s claimed by %s - not in our universe\n", c.addr, c.ident) - c.resultChan <- nil + c.sendResult(nil) return true } @@ -34,6 +47,7 @@ func (c *claim) Try(alloc *Allocator) bool { owner := alloc.ring.Owner(c.addr) if owner == router.UnknownPeerName { alloc.infof("Ring is empty; will try later.\n", c.addr, owner) + c.sendResult(nil) // don't make the caller wait return false } if owner != alloc.ourName { @@ -41,34 +55,34 @@ func (c *claim) Try(alloc *Allocator) bool { if found { name = " (" + name + ")" } - c.resultChan <- fmt.Errorf("address %s is owned by other peer %s%s", c.addr.String(), owner, name) + c.sendResult(fmt.Errorf("address %s is owned by other peer %s%s", c.addr.String(), owner, name)) return true } // We are the owner, check we haven't given it to another container existingIdent := alloc.findOwner(c.addr) if existingIdent == c.ident { // same identifier is claiming same address; that's OK - c.resultChan <- nil + c.sendResult(nil) return true } if existingIdent == "" { err := alloc.space.Claim(c.addr) if err != nil { - c.resultChan <- err + c.sendResult(err) return true } alloc.debugln("Claimed", c.addr, "for", c.ident) alloc.addOwned(c.ident, c.addr) - c.resultChan <- nil + c.sendResult(nil) return true } // Addr already owned by container on this machine - c.resultChan <- fmt.Errorf("address %s is already owned by %s", c.addr.String(), existingIdent) + c.sendResult(fmt.Errorf("address %s is already owned by %s", c.addr.String(), existingIdent)) return true } func (c *claim) Cancel() { - c.resultChan <- fmt.Errorf("Operation cancelled.") + c.sendResult(fmt.Errorf("Operation cancelled.")) } func (c *claim) String() string {