diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index e31ba8eb..e73c61d2 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/weaveworks/weave/api" "github.com/weaveworks/weave/common" "github.com/weaveworks/weave/net/address" "github.com/weaveworks/weave/testing/gossip" @@ -30,6 +31,10 @@ func (alloc *Allocator) SimplyClaim(ident string, cidr address.CIDR) error { return alloc.Claim(ident, cidr, true, true, returnFalse) } +func (alloc *Allocator) AnonymousClaim(cidr address.CIDR) error { + return alloc.Claim(api.NoContainerID, cidr, false, true, returnFalse) +} + func TestAllocFree(t *testing.T) { const ( container1 = "abcdef" @@ -170,6 +175,8 @@ func TestAllocatorClaim(t *testing.T) { universe = "10.0.3.0/24" testAddr1 = "10.0.3.5/24" testAddr2 = "10.0.4.2/24" + testAddr4 = "10.0.2.3/23" + subnet4 = "10.0.2.0/23" testPre = "10.0.3.1/24" ) @@ -203,6 +210,9 @@ func TestAllocatorClaim(t *testing.T) { // claim for a different container should fail err = alloc.SimplyClaim(container1, addr1) require.Error(t, err) + // claim for an anonymous container, will assume it's the same one + err = alloc.AnonymousClaim(addr1) + require.NoError(t, err) // claiming the address allocated on the other peer should fail err = alloc.SimplyClaim(container1, address.MakeCIDR(subnet, addrx)) require.Error(t, err, "claiming address allocated on other peer should fail") @@ -215,6 +225,15 @@ func TestAllocatorClaim(t *testing.T) { addr2, _ := address.ParseCIDR(testAddr2) err = alloc.SimplyClaim(container1, addr2) require.NoError(t, err) + err = alloc.SimplyClaim(container3, addr2) + require.Error(t, err) + // Anonymous claim outside of our universe + addr4, _ := address.ParseCIDR(testAddr4) + subnet4cidr, _ := address.ParseCIDR(subnet4) + require.NoError(t, alloc.AnonymousClaim(addr4)) + addr5, err := alloc.Allocate(api.NoContainerID, subnet4cidr, false, returnFalse) + require.NoError(t, err) + require.NotEqual(t, addr4.Start(), addr5) } func (alloc *Allocator) pause() func() { diff --git a/ipam/claim.go b/ipam/claim.go index 9a827e54..c4a5529e 100644 --- a/ipam/claim.go +++ b/ipam/claim.go @@ -40,10 +40,29 @@ func (c *claim) Try(alloc *Allocator) bool { return true } + addOwned := func() { + if c.ident == api.NoContainerID { + alloc.addOwned(c.cidr.Addr.String(), c.cidr, c.isContainer) + } else { + alloc.addOwned(c.ident, c.cidr, c.isContainer) + } + } + if !alloc.ring.Contains(c.cidr.Addr) { - // Address not within our universe; assume user knows what they are doing alloc.infof("Address %s claimed by %s - not in our range", c.cidr, c.ident) - alloc.addOwned(c.ident, c.cidr, c.isContainer) + previousOwner := alloc.findOwner(c.cidr.Addr) + switch { + case previousOwner == "": + addOwned() + case previousOwner == c.ident: // already owned by this ID + case c.ident == api.NoContainerID: // already owned by anonymous container + // do nothing (no automatic fall-through in Go) + case !alloc.dead[previousOwner].IsZero(): // already owned by dead container + alloc.removeOwned(previousOwner, c.cidr.Addr) + addOwned() + default: + c.sendResult(fmt.Errorf("address %s already in use by %s", c.cidr, previousOwner)) + } c.sendResult(nil) return true } @@ -85,11 +104,7 @@ func (c *claim) Try(alloc *Allocator) bool { // Unused address, we try to claim it: if err := alloc.space.Claim(c.cidr.Addr); err == nil { alloc.debugln("Claimed", c.cidr, "for", c.ident) - if c.ident == api.NoContainerID { - alloc.addOwned(c.cidr.Addr.String(), c.cidr, c.isContainer) - } else { - alloc.addOwned(c.ident, c.cidr, c.isContainer) - } + addOwned() c.sendResult(nil) } else { c.sendResult(err)