diff --git a/ipam/allocate.go b/ipam/allocate.go index c11f75ed..8146e0c9 100644 --- a/ipam/allocate.go +++ b/ipam/allocate.go @@ -14,7 +14,7 @@ type allocateResult struct { type allocate struct { resultChan chan<- allocateResult ident string - r address.Range // Range we are trying to allocate within + r address.CIDR // Subnet we are trying to allocate within hasBeenCancelled func() bool } @@ -25,36 +25,36 @@ func (g *allocate) Try(alloc *Allocator) bool { return true } - if addr, found := alloc.ownedInRange(g.ident, g.r); found { + if addrs := alloc.ownedInRange(g.ident, g.r.Range()); len(addrs) > 0 { // If we had heard that this container died, resurrect it delete(alloc.dead, g.ident) // delete is no-op if key not in map - g.resultChan <- allocateResult{addr, nil} + g.resultChan <- allocateResult{addrs[0].Addr, nil} return true } - if !alloc.universe.Overlaps(g.r) { - g.resultChan <- allocateResult{0, fmt.Errorf("range %s out of bounds: %s", g.r, alloc.universe)} + if !alloc.universe.Overlaps(g.r.Range()) { + g.resultChan <- allocateResult{err: fmt.Errorf("range %s out of bounds: %s", g.r, alloc.universe)} return true } alloc.establishRing() - if ok, addr := alloc.space.Allocate(g.r); ok { + if ok, addr := alloc.space.Allocate(g.r.HostRange()); ok { // If caller hasn't supplied a unique ID, file it under the IP address // which lets the caller then release the address using DELETE /ip/address if g.ident == "_" { g.ident = addr.String() } alloc.debugln("Allocated", addr, "for", g.ident, "in", g.r) - alloc.addOwned(g.ident, addr) + alloc.addOwned(g.ident, address.MakeCIDR(g.r, addr)) g.resultChan <- allocateResult{addr, nil} return true } // out of space - donors := alloc.ring.ChoosePeersToAskForSpace(g.r.Start, g.r.End) + donors := alloc.ring.ChoosePeersToAskForSpace(g.r.Addr, g.r.Range().End) for _, donor := range donors { - if err := alloc.sendSpaceRequest(donor, g.r); err != nil { + if err := alloc.sendSpaceRequest(donor, g.r.Range()); err != nil { alloc.debugln("Problem asking peer", donor, "for space:", err) } else { alloc.debugln("Decided to ask peer", donor, "for space in range", g.r) @@ -66,7 +66,7 @@ func (g *allocate) Try(alloc *Allocator) bool { } func (g *allocate) Cancel() { - g.resultChan <- allocateResult{0, &errorCancelled{"Allocate", g.ident}} + g.resultChan <- allocateResult{err: &errorCancelled{"Allocate", g.ident}} } func (g *allocate) ForContainer(ident string) bool { diff --git a/ipam/allocator.go b/ipam/allocator.go index 4ea0eb81..f9d67fe0 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -47,16 +47,16 @@ type operation interface { type Allocator struct { actionChan chan<- func() ourName mesh.PeerName - universe address.Range // superset of all ranges - ring *ring.Ring // information on ranges owned by all peers - space space.Space // more detail on ranges owned by us - owned map[string][]address.Address // who owns what addresses, indexed by container-ID - nicknames map[mesh.PeerName]string // so we can map nicknames for rmpeer - pendingAllocates []operation // held until we get some free space - pendingClaims []operation // held until we know who owns the space - dead map[string]time.Time // containers we heard were dead, and when - db db.DB // persistence - gossip mesh.Gossip // our link to the outside world for sending messages + universe address.Range // superset of all ranges + ring *ring.Ring // information on ranges owned by all peers + space space.Space // more detail on ranges owned by us + owned map[string][]address.CIDR // who owns what addresses, indexed by container-ID + nicknames map[mesh.PeerName]string // so we can map nicknames for rmpeer + pendingAllocates []operation // held until we get some free space + pendingClaims []operation // held until we know who owns the space + dead map[string]time.Time // containers we heard were dead, and when + db db.DB // persistence + gossip mesh.Gossip // our link to the outside world for sending messages paxos *paxos.Node paxosActive bool ticker *time.Ticker @@ -71,7 +71,7 @@ func NewAllocator(ourName mesh.PeerName, ourUID mesh.PeerUID, ourNickname string ourName: ourName, universe: universe, ring: ring.New(universe.Start, universe.End, ourName), - owned: make(map[string][]address.Address), + owned: make(map[string][]address.CIDR), db: db, paxos: paxos.NewNode(ourName, ourUID, quorum), nicknames: map[mesh.PeerName]string{ourName: ourNickname}, @@ -173,7 +173,7 @@ func (alloc *Allocator) tryPendingOps() { func (alloc *Allocator) spaceRequestDenied(sender mesh.PeerName, r address.Range) { for i := 0; i < len(alloc.pendingClaims); { claim := alloc.pendingClaims[i].(*claim) - if r.Contains(claim.addr) { + if r.Contains(claim.cidr.Addr) { claim.deniedBy(alloc, sender) alloc.pendingClaims = append(alloc.pendingClaims[:i], alloc.pendingClaims[i+1:]...) continue @@ -195,7 +195,7 @@ func (e *errorCancelled) Error() string { // Allocate (Sync) - get new IP address for container with given name in range // if there isn't any space in that range we block indefinitely -func (alloc *Allocator) Allocate(ident string, r address.Range, hasBeenCancelled func() bool) (address.Address, error) { +func (alloc *Allocator) Allocate(ident string, r address.CIDR, hasBeenCancelled func() bool) (address.Address, error) { resultChan := make(chan allocateResult) op := &allocate{resultChan: resultChan, ident: ident, r: r, hasBeenCancelled: hasBeenCancelled} alloc.doOperation(op, &alloc.pendingAllocates) @@ -203,24 +203,19 @@ func (alloc *Allocator) Allocate(ident string, r address.Range, hasBeenCancelled return result.addr, result.err } -// Lookup (Sync) - get existing IP address for container with given name in range -func (alloc *Allocator) Lookup(ident string, r address.Range) (address.Address, error) { - resultChan := make(chan allocateResult) +// Lookup (Sync) - get existing IP addresses for container with given name in range +func (alloc *Allocator) Lookup(ident string, r address.Range) ([]address.CIDR, error) { + resultChan := make(chan []address.CIDR) alloc.actionChan <- func() { - if addr, found := alloc.ownedInRange(ident, r); found { - resultChan <- allocateResult{addr: addr} - return - } - resultChan <- allocateResult{err: fmt.Errorf("lookup: no address found for %s in range %s", ident, r)} + resultChan <- alloc.ownedInRange(ident, r) } - result := <-resultChan - return result.addr, result.err + return <-resultChan, nil } // Claim an address that we think we should own (Sync) -func (alloc *Allocator) Claim(ident string, addr address.Address, noErrorOnUnknown bool) error { +func (alloc *Allocator) Claim(ident string, cidr address.CIDR, noErrorOnUnknown bool) error { resultChan := make(chan error) - op := &claim{resultChan: resultChan, ident: ident, addr: addr, noErrorOnUnknown: noErrorOnUnknown} + op := &claim{resultChan: resultChan, ident: ident, cidr: cidr, noErrorOnUnknown: noErrorOnUnknown} alloc.doOperation(op, &alloc.pendingClaims) return <-resultChan } @@ -287,12 +282,12 @@ func (alloc *Allocator) Delete(ident string) error { } func (alloc *Allocator) delete(ident string) error { - addrs := alloc.removeAllOwned(ident) - if len(addrs) == 0 { + cidrs := alloc.removeAllOwned(ident) + if len(cidrs) == 0 { return fmt.Errorf("Delete: no addresses for %s", ident) } - for _, addr := range addrs { - alloc.space.Free(addr) + for _, cidr := range cidrs { + alloc.space.Free(cidr.Addr) } return nil } @@ -808,9 +803,9 @@ func (alloc *Allocator) loadPersistedData() { if err := alloc.db.Load(ownedIdent, &alloc.owned); err != nil { alloc.fatalf("Error loading persisted address data: %s", err) } - for _, addrs := range alloc.owned { - for _, addr := range addrs { - alloc.space.Claim(addr) + for _, cidrs := range alloc.owned { + for _, cidr := range cidrs { + alloc.space.Claim(cidr.Addr) } } } @@ -823,22 +818,18 @@ func (alloc *Allocator) persistOwned() { // Owned addresses -func (alloc *Allocator) allOwned(ident string) []address.Address { - return alloc.owned[ident] -} - func (alloc *Allocator) hasOwned(ident string) bool { _, b := alloc.owned[ident] return b } // NB: addr must not be owned by ident already -func (alloc *Allocator) addOwned(ident string, addr address.Address) { - alloc.owned[ident] = append(alloc.owned[ident], addr) +func (alloc *Allocator) addOwned(ident string, cidr address.CIDR) { + alloc.owned[ident] = append(alloc.owned[ident], cidr) alloc.persistOwned() } -func (alloc *Allocator) removeAllOwned(ident string) []address.Address { +func (alloc *Allocator) removeAllOwned(ident string) []address.CIDR { a := alloc.owned[ident] delete(alloc.owned, ident) alloc.persistOwned() @@ -846,13 +837,13 @@ func (alloc *Allocator) removeAllOwned(ident string) []address.Address { } func (alloc *Allocator) removeOwned(ident string, addrToFree address.Address) bool { - addrs, _ := alloc.owned[ident] - for i, ownedAddr := range addrs { - if ownedAddr == addrToFree { - if len(addrs) == 1 { + cidrs, _ := alloc.owned[ident] + for i, ownedCidr := range cidrs { + if ownedCidr.Addr == addrToFree { + if len(cidrs) == 1 { delete(alloc.owned, ident) } else { - alloc.owned[ident] = append(addrs[:i], addrs[i+1:]...) + alloc.owned[ident] = append(cidrs[:i], cidrs[i+1:]...) } alloc.persistOwned() return true @@ -861,19 +852,20 @@ func (alloc *Allocator) removeOwned(ident string, addrToFree address.Address) bo return false } -func (alloc *Allocator) ownedInRange(ident string, r address.Range) (address.Address, bool) { - for _, addr := range alloc.owned[ident] { - if r.Contains(addr) { - return addr, true +func (alloc *Allocator) ownedInRange(ident string, r address.Range) []address.CIDR { + var c []address.CIDR + for _, cidr := range alloc.owned[ident] { + if r.Contains(cidr.Addr) { + c = append(c, cidr) } } - return 0, false + return c } func (alloc *Allocator) findOwner(addr address.Address) string { - for ident, addrs := range alloc.owned { - for _, candidate := range addrs { - if candidate == addr { + for ident, cidrs := range alloc.owned { + for _, candidate := range cidrs { + if candidate.Addr == addr { return ident } } @@ -884,10 +876,10 @@ func (alloc *Allocator) findOwner(addr address.Address) string { // For each ID in the 'owned' map, remove the entry if it isn't in the map func (alloc *Allocator) syncOwned(ids map[string]struct{}) { changed := false - for ident, addrs := range alloc.owned { + for ident, cidrs := range alloc.owned { if _, found := ids[ident]; !found { - for _, addr := range addrs { - alloc.space.Free(addr) + for _, cidr := range cidrs { + alloc.space.Free(cidr.Addr) } delete(alloc.owned, ident) changed = true diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index c9eef59c..5fae2dfb 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -37,41 +37,45 @@ func TestAllocFree(t *testing.T) { alloc, subnet := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", universe, 1) defer alloc.Stop() - _, cidr1, _ := address.ParseCIDR(subnet1) - _, cidr2, _ := address.ParseCIDR(subnet2) + cidr1, _ := address.ParseCIDR(subnet1) + cidr2, _ := address.ParseCIDR(subnet2) alloc.claimRingForTesting() - addr1, err := alloc.Allocate(container1, cidr1.HostRange(), returnFalse) + addr1, err := alloc.Allocate(container1, cidr1, returnFalse) require.NoError(t, err) require.Equal(t, testAddr1, addr1.String(), "address") - addr2, err := alloc.Allocate(container1, cidr2.HostRange(), returnFalse) + addr2, err := alloc.Allocate(container1, cidr2, returnFalse) require.NoError(t, err) require.Equal(t, testAddr2, addr2.String(), "address") + addrs, err := alloc.Lookup(container1, subnet.Range()) + require.NoError(t, err) + require.Equal(t, []address.CIDR{address.MakeCIDR(cidr1, addr1), address.MakeCIDR(cidr2, addr2)}, addrs) + // Ask for another address for a different container and check it's different - addr1b, _ := alloc.Allocate(container2, cidr1.HostRange(), returnFalse) + addr1b, _ := alloc.Allocate(container2, cidr1, returnFalse) if addr1b.String() == testAddr1 { t.Fatalf("Expected different address but got %s", addr1b.String()) } // Ask for the first container again and we should get the same addresses again - addr1a, _ := alloc.Allocate(container1, cidr1.HostRange(), returnFalse) + addr1a, _ := alloc.Allocate(container1, cidr1, returnFalse) require.Equal(t, testAddr1, addr1a.String(), "address") - addr2a, _ := alloc.Allocate(container1, cidr2.HostRange(), returnFalse) + addr2a, _ := alloc.Allocate(container1, cidr2, returnFalse) require.Equal(t, testAddr2, addr2a.String(), "address") // Now delete the first container, and we should get its addresses back require.NoError(t, alloc.Delete(container1)) - addr3, _ := alloc.Allocate(container3, cidr1.HostRange(), returnFalse) + addr3, _ := alloc.Allocate(container3, cidr1, returnFalse) require.Equal(t, testAddr1, addr3.String(), "address") - addr4, _ := alloc.Allocate(container3, cidr2.HostRange(), returnFalse) + addr4, _ := alloc.Allocate(container3, cidr2, returnFalse) require.Equal(t, testAddr2, addr4.String(), "address") alloc.ContainerDied(container2) // Resurrect - addr1c, err := alloc.Allocate(container2, cidr1.HostRange(), returnFalse) + addr1c, err := alloc.Allocate(container2, cidr1, returnFalse) require.NoError(t, err) require.Equal(t, addr1b, addr1c, "address") @@ -80,7 +84,7 @@ func TestAllocFree(t *testing.T) { // Move the clock forward and clear out the dead container alloc.actionChan <- func() { alloc.now = func() time.Time { return time.Now().Add(containerDiedTimeout * 2) } } alloc.actionChan <- func() { alloc.removeDeadContainers() } - require.Equal(t, address.Offset(spaceSize-1), alloc.NumFreeAddresses(subnet)) + require.Equal(t, address.Offset(spaceSize+1), alloc.NumFreeAddresses(subnet.Range())) } func TestBootstrap(t *testing.T) { @@ -145,14 +149,14 @@ func TestAllocatorClaim(t *testing.T) { container1 = "abcdef" container3 = "b01df00d" universe = "10.0.3.0/24" - testAddr1 = "10.0.3.2" - testAddr2 = "10.0.4.2" + testAddr1 = "10.0.3.2/24" + testAddr2 = "10.0.4.2/24" ) allocs, _, subnet := makeNetworkOfAllocators(2, universe) alloc := allocs[1] defer alloc.Stop() - addr1, _ := address.ParseIP(testAddr1) + addr1, _ := address.ParseCIDR(testAddr1) // First claim should trigger "dunno, I'm going to wait" err := alloc.Claim(container3, addr1, true) @@ -168,7 +172,7 @@ func TestAllocatorClaim(t *testing.T) { require.NoError(t, err) // Check we get this address back if we try an allocate addr3, _ := alloc.Allocate(container3, subnet, returnFalse) - require.Equal(t, testAddr1, addr3.String(), "address") + require.Equal(t, testAddr1, address.MakeCIDR(subnet, addr3).String(), "address") // one more claim should still work err = alloc.Claim(container3, addr1, true) require.NoError(t, err) @@ -176,10 +180,10 @@ func TestAllocatorClaim(t *testing.T) { err = alloc.Claim(container1, addr1, true) require.Error(t, err) // claiming the address allocated on the other peer should fail - err = alloc.Claim(container1, addrx, true) + err = alloc.Claim(container1, address.MakeCIDR(subnet, addrx), true) require.Error(t, err, "claiming address allocated on other peer should fail") // Check an address outside of our universe - addr2, _ := address.ParseIP(testAddr2) + addr2, _ := address.ParseCIDR(testAddr2) err = alloc.Claim(container1, addr2, true) require.NoError(t, err) } @@ -231,7 +235,7 @@ func TestCancel(t *testing.T) { unpause := alloc2.pause() // Use up all the IPs that alloc1 owns, so the allocation after this will prompt a request to alloc2 - for i := 0; alloc1.NumFreeAddresses(subnet) > 0; i++ { + for i := 0; alloc1.NumFreeAddresses(subnet.HostRange()) > 0; i++ { alloc1.Allocate(fmt.Sprintf("tmp%d", i), subnet, returnFalse) } cancelChan := make(chan bool, 1) @@ -341,7 +345,7 @@ func TestTransfer(t *testing.T) { require.NoError(t, alloc1.AdminTakeoverRanges(alloc3.ourName.String())) router.Flush() - require.Equal(t, address.Offset(1022), alloc1.NumFreeAddresses(subnet)) + require.Equal(t, address.Offset(1024), alloc1.NumFreeAddresses(subnet.Range())) _, err = alloc1.Allocate("foo", subnet, returnFalse) require.True(t, err == nil, "Failed to get address") @@ -516,8 +520,8 @@ func TestAllocatorFuzz(t *testing.T) { allocIndex := rand.Int31n(nodes) addressIndex := rand.Int31n(int32(subnet.Size())) alloc := allocs[allocIndex] - addr := address.Add(subnet.Start, address.Offset(addressIndex)) - err := alloc.Claim(name, addr, true) + addr := address.Add(subnet.Addr, address.Offset(addressIndex)) + err := alloc.Claim(name, address.MakeCIDR(subnet, addr), true) if err == nil { noteAllocation(allocIndex, name, addr) } @@ -584,3 +588,37 @@ func TestGossipSkew(t *testing.T) { t.Fail() } } + +func TestSpaceRequest(t *testing.T) { + const ( + container1 = "cont-1" + container2 = "cont-2" + universe = "10.32.0.0/12" + ) + allocs, gossipRouter, subnet := makeNetworkOfAllocators(1, universe) + alloc1 := allocs[0] + defer alloc1.Stop() + + addr, err := alloc1.Allocate(container1, subnet, returnFalse) + require.Nil(t, err, "") + // free it again so the donation splits the range neatly + err = alloc1.Free(container1, addr) + require.Nil(t, err, "") + require.Equal(t, cidrRanges(universe), alloc1.ring.OwnedRanges(), "") + + // Start a new peer + alloc2, _ := makeAllocator("02:00:00:02:00:00", universe, 2) + alloc2.SetInterfaces(gossipRouter.Connect(alloc2.ourName, alloc2)) + alloc2.Start() + defer alloc2.Stop() + alloc2.Allocate(container2, subnet, returnFalse) + + // Test whether the universe has been split into two equal halves (GH #2009) + require.Equal(t, cidrRanges("10.32.0.0/13"), alloc1.ring.OwnedRanges(), "") + require.Equal(t, cidrRanges("10.40.0.0/13"), alloc2.ring.OwnedRanges(), "") +} + +func cidrRanges(s string) []address.Range { + c, _ := address.ParseCIDR(s) + return []address.Range{c.Range()} +} diff --git a/ipam/claim.go b/ipam/claim.go index 2b33b145..8dc208f8 100644 --- a/ipam/claim.go +++ b/ipam/claim.go @@ -12,7 +12,7 @@ import ( type claim struct { resultChan chan<- error ident string - addr address.Address + cidr address.CIDR noErrorOnUnknown bool } @@ -32,9 +32,10 @@ func (c *claim) sendResult(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 !alloc.ring.Contains(c.addr) { + if !alloc.ring.Contains(c.cidr.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", c.addr, c.ident) + alloc.infof("Address %s claimed by %s - not in our range", c.cidr, c.ident) + alloc.addOwned(c.ident, c.cidr) c.sendResult(nil) return true } @@ -44,24 +45,24 @@ func (c *claim) Try(alloc *Allocator) bool { // If we had heard that this container died, resurrect it delete(alloc.dead, c.ident) // (delete is no-op if key not in map) - switch owner := alloc.ring.Owner(c.addr); owner { + switch owner := alloc.ring.Owner(c.cidr.Addr); owner { case alloc.ourName: // success case mesh.UnknownPeerName: // If our ring doesn't know, it must be empty. if c.noErrorOnUnknown { - alloc.infof("Claim %s for %s: address allocator still initializing; will try later.", c.addr, c.ident) + alloc.infof("Claim %s for %s: address allocator still initializing; will try later.", c.cidr, c.ident) c.sendResult(nil) } else { - c.sendResult(fmt.Errorf("%s is in the range %s, but the allocator is not initialized yet", c.addr, alloc.universe.AsCIDRString())) + c.sendResult(fmt.Errorf("%s is in the range %s, but the allocator is not initialized yet", c.cidr, alloc.universe.AsCIDRString())) } return false default: - alloc.debugf("requesting address %s from other peer %s", c.addr, owner) - err := alloc.sendSpaceRequest(owner, address.NewRange(c.addr, 1)) + alloc.debugf("requesting address %s from other peer %s", c.cidr, owner) + err := alloc.sendSpaceRequest(owner, address.NewRange(c.cidr.Addr, 1)) if err != nil { // can't speak to owner right now if c.noErrorOnUnknown { - alloc.infof("Claim %s for %s: %s; will try later.", c.addr, c.ident, err) + alloc.infof("Claim %s for %s: %s; will try later.", c.cidr, c.ident, err) c.sendResult(nil) } else { // just tell the user they can't do this. c.deniedBy(alloc, owner) @@ -71,28 +72,28 @@ func (c *claim) Try(alloc *Allocator) bool { } // We are the owner, check we haven't given it to another container - switch existingIdent := alloc.findOwner(c.addr); existingIdent { + switch existingIdent := alloc.findOwner(c.cidr.Addr); existingIdent { case "": - if err := alloc.space.Claim(c.addr); err == nil { - alloc.debugln("Claimed", c.addr, "for", c.ident) + if err := alloc.space.Claim(c.cidr.Addr); err == nil { + alloc.debugln("Claimed", c.cidr, "for", c.ident) if c.ident == "_" { // Special "I don't have a unique ID" identifier - c.ident = c.addr.String() + c.ident = c.cidr.String() } - alloc.addOwned(c.ident, c.addr) + alloc.addOwned(c.ident, c.cidr) c.sendResult(nil) } else { c.sendResult(err) } case c.ident: // same identifier is claiming same address; that's OK - alloc.debugln("Re-Claimed", c.addr, "for", c.ident) + alloc.debugln("Re-Claimed", c.cidr, "for", c.ident) c.sendResult(nil) - case c.addr.String(): + case c.cidr.String(): // Address already allocated via "_" name - c.sendResult(fmt.Errorf("address %s already in use", c.addr)) + c.sendResult(fmt.Errorf("address %s already in use", c.cidr)) default: // Addr already owned by container on this machine - c.sendResult(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.cidr.String(), existingIdent)) } return true } @@ -102,7 +103,7 @@ func (c *claim) deniedBy(alloc *Allocator, owner mesh.PeerName) { if found { name = " (" + name + ")" } - c.sendResult(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.cidr.String(), owner, name)) } func (c *claim) Cancel() { diff --git a/ipam/http.go b/ipam/http.go index eb51b87a..47a5b566 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -16,22 +16,31 @@ func badRequest(w http.ResponseWriter, err error) { common.Log.Warningln("[allocator]:", err.Error()) } -func parseCIDR(w http.ResponseWriter, cidrStr string) (address.CIDR, bool) { - subnetAddr, cidr, err := address.ParseCIDR(cidrStr) +func parseCIDR(w http.ResponseWriter, cidrStr string, net bool) (address.CIDR, bool) { + cidr, err := address.ParseCIDR(cidrStr) if err != nil { badRequest(w, err) return address.CIDR{}, false } - if cidr.Start != subnetAddr { + if net && !cidr.IsSubnet() { badRequest(w, fmt.Errorf("Invalid subnet %s - bits after network prefix are not all zero", cidrStr)) return address.CIDR{}, false } return cidr, true } +func writeAddresses(w http.ResponseWriter, cidrs []address.CIDR) { + for i, cidr := range cidrs { + fmt.Fprint(w, cidr) + if i < len(cidrs)-1 { + w.Write([]byte{' '}) + } + } +} + func (alloc *Allocator) handleHTTPAllocate(dockerCli *docker.Client, w http.ResponseWriter, ident string, checkAlive bool, subnet address.CIDR) { closedChan := w.(http.CloseNotifier).CloseNotify() - addr, err := alloc.Allocate(ident, subnet.HostRange(), + addr, err := alloc.Allocate(ident, subnet, func() bool { select { case <-closedChan: @@ -61,46 +70,42 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CID fmt.Fprintf(w, "%s", defaultSubnet) }) - router.Methods("PUT").Path("/ip/{id}/{ip}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + router.Methods("PUT").Path("/ip/{id}/{ip}/{prefixlen}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) - ident := vars["id"] - ipStr := vars["ip"] - noErrorOnUnknown := r.FormValue("noErrorOnUnknown") == "true" - if ip, err := address.ParseIP(ipStr); err != nil { - badRequest(w, err) - return - } else if err := alloc.Claim(ident, ip, noErrorOnUnknown); err != nil { - badRequest(w, fmt.Errorf("Unable to claim: %s", err)) - return + if cidr, ok := parseCIDR(w, vars["ip"]+"/"+vars["prefixlen"], false); ok { + noErrorOnUnknown := r.FormValue("noErrorOnUnknown") == "true" + if err := alloc.Claim(vars["id"], cidr, noErrorOnUnknown); err != nil { + badRequest(w, fmt.Errorf("Unable to claim: %s", err)) + return + } + w.WriteHeader(204) } - - w.WriteHeader(204) }) router.Methods("GET").Path("/ip/{id}/{ip}/{prefixlen}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) - if subnet, ok := parseCIDR(w, vars["ip"]+"/"+vars["prefixlen"]); ok { - addr, err := alloc.Lookup(vars["id"], subnet.HostRange()) + if subnet, ok := parseCIDR(w, vars["ip"]+"/"+vars["prefixlen"], true); ok { + cidrs, err := alloc.Lookup(vars["id"], subnet.HostRange()) if err != nil { http.NotFound(w, r) return } - fmt.Fprintf(w, "%s/%d", addr, subnet.PrefixLen) + writeAddresses(w, cidrs) } }) router.Methods("GET").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - addr, err := alloc.Lookup(mux.Vars(r)["id"], defaultSubnet.HostRange()) + addrs, err := alloc.Lookup(mux.Vars(r)["id"], defaultSubnet.HostRange()) if err != nil { http.NotFound(w, r) return } - fmt.Fprintf(w, "%s/%d", addr, defaultSubnet.PrefixLen) + writeAddresses(w, addrs) }) router.Methods("POST").Path("/ip/{id}/{ip}/{prefixlen}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { vars := mux.Vars(r) - if subnet, ok := parseCIDR(w, vars["ip"]+"/"+vars["prefixlen"]); ok { + if subnet, ok := parseCIDR(w, vars["ip"]+"/"+vars["prefixlen"], true); ok { alloc.handleHTTPAllocate(dockerCli, w, vars["id"], r.FormValue("check-alive") == "true", subnet) } }) diff --git a/ipam/http_test.go b/ipam/http_test.go index 3d490758..176da222 100644 --- a/ipam/http_test.go +++ b/ipam/http_test.go @@ -80,7 +80,7 @@ func TestHttp(t *testing.T) { ) alloc, _ := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", universe, 1) - _, cidr, _ := address.ParseCIDR(universe) + cidr, _ := address.ParseCIDR(universe) port := listenHTTP(alloc, cidr) alloc.claimRingForTesting() @@ -122,7 +122,7 @@ func TestBadHttp(t *testing.T) { alloc, _ := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", testCIDR1, 1) defer alloc.Stop() - _, cidr, _ := address.ParseCIDR(testCIDR1) + cidr, _ := address.ParseCIDR(testCIDR1) port := listenHTTP(alloc, cidr) alloc.claimRingForTesting() @@ -153,7 +153,7 @@ func TestHTTPCancel(t *testing.T) { alloc, _ := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", testCIDR1, 2) defer alloc.Stop() ExpectBroadcastMessage(alloc, nil) // trying to form consensus - _, cidr, _ := address.ParseCIDR(testCIDR1) + cidr, _ := address.ParseCIDR(testCIDR1) port := listenHTTP(alloc, cidr) // Ask the http server for a new address diff --git a/ipam/status.go b/ipam/status.go index 3c9aeb12..f205ecee 100644 --- a/ipam/status.go +++ b/ipam/status.go @@ -26,8 +26,8 @@ type EntryStatus struct { } type ClaimStatus struct { - Ident string - Address address.Address + Ident string + CIDR address.CIDR } func NewStatus(allocator *Allocator, defaultSubnet address.CIDR) *Status { @@ -80,7 +80,7 @@ func newClaimStatusSlice(allocator *Allocator) []ClaimStatus { var slice []ClaimStatus for _, op := range allocator.pendingClaims { claim := op.(*claim) - slice = append(slice, ClaimStatus{claim.ident, claim.addr}) + slice = append(slice, ClaimStatus{claim.ident, claim.cidr}) } return slice } diff --git a/ipam/testutils_test.go b/ipam/testutils_test.go index 76fdd7f4..fbb4df45 100644 --- a/ipam/testutils_test.go +++ b/ipam/testutils_test.go @@ -128,13 +128,13 @@ type mockDB struct{} func (d *mockDB) Load(_ string, _ interface{}) error { return nil } func (d *mockDB) Save(_ string, _ interface{}) error { return nil } -func makeAllocator(name string, cidrStr string, quorum uint) (*Allocator, address.Range) { +func makeAllocator(name string, cidrStr string, quorum uint) (*Allocator, address.CIDR) { peername, err := mesh.PeerNameFromString(name) if err != nil { panic(err) } - _, cidr, err := address.ParseCIDR(cidrStr) + cidr, err := address.ParseCIDR(cidrStr) if err != nil { panic(err) } @@ -142,10 +142,10 @@ func makeAllocator(name string, cidrStr string, quorum uint) (*Allocator, addres alloc := NewAllocator(peername, mesh.PeerUID(rand.Int63()), "nick-"+name, cidr.Range(), quorum, new(mockDB), func(mesh.PeerName) bool { return true }) - return alloc, cidr.HostRange() + return alloc, cidr } -func makeAllocatorWithMockGossip(t *testing.T, name string, universeCIDR string, quorum uint) (*Allocator, address.Range) { +func makeAllocatorWithMockGossip(t *testing.T, name string, universeCIDR string, quorum uint) (*Allocator, address.CIDR) { alloc, subnet := makeAllocator(name, universeCIDR, quorum) gossip := &mockGossipComms{T: t, name: name} alloc.SetInterfaces(gossip) @@ -199,10 +199,10 @@ func AssertNothingSentErr(t *testing.T, ch <-chan error) { } } -func makeNetworkOfAllocators(size int, cidr string) ([]*Allocator, *gossip.TestRouter, address.Range) { +func makeNetworkOfAllocators(size int, cidr string) ([]*Allocator, *gossip.TestRouter, address.CIDR) { gossipRouter := gossip.NewTestRouter(0.0) allocs := make([]*Allocator, size) - var subnet address.Range + var subnet address.CIDR for i := 0; i < size; i++ { var alloc *Allocator diff --git a/net/address/address.go b/net/address/address.go index 71e00716..67cfee45 100644 --- a/net/address/address.go +++ b/net/address/address.go @@ -31,11 +31,15 @@ func (r Range) AsCIDRString() string { } prefixLen-- } - return CIDR{Start: r.Start, PrefixLen: prefixLen}.String() + return CIDR{Addr: r.Start, PrefixLen: prefixLen}.String() +} + +func MakeCIDR(subnet CIDR, addr Address) CIDR { + return CIDR{Addr: addr, PrefixLen: subnet.PrefixLen} } type CIDR struct { - Start Address + Addr Address PrefixLen int } @@ -46,29 +50,34 @@ func ParseIP(s string) (Address, error) { return 0, &net.ParseError{Type: "IP Address", Text: s} } -func ParseCIDR(s string) (Address, CIDR, error) { +func ParseCIDR(s string) (CIDR, error) { if ip, ipnet, err := net.ParseCIDR(s); err != nil { - return 0, CIDR{}, err + return CIDR{}, err } else if ipnet.IP.To4() == nil { - return 0, CIDR{}, &net.ParseError{Type: "Non-IPv4 address not supported", Text: s} + return CIDR{}, &net.ParseError{Type: "Non-IPv4 address not supported", Text: s} } else { prefixLen, _ := ipnet.Mask.Size() - return FromIP4(ip), CIDR{Start: FromIP4(ipnet.IP), PrefixLen: prefixLen}, nil + return CIDR{Addr: FromIP4(ip), PrefixLen: prefixLen}, nil } } +func (cidr CIDR) IsSubnet() bool { + mask := cidr.Size() - 1 + return Offset(cidr.Addr)&mask == 0 +} + func (cidr CIDR) Size() Offset { return 1 << uint(32-cidr.PrefixLen) } func (cidr CIDR) Range() Range { - return NewRange(cidr.Start, cidr.Size()) + return NewRange(cidr.Addr, cidr.Size()) } func (cidr CIDR) HostRange() Range { // Respect RFC1122 exclusions of first and last addresses - return NewRange(cidr.Start+1, cidr.Size()-2) + return NewRange(cidr.Addr+1, cidr.Size()-2) } func (cidr CIDR) String() string { - return fmt.Sprintf("%s/%d", cidr.Start.String(), cidr.PrefixLen) + return fmt.Sprintf("%s/%d", cidr.Addr.String(), cidr.PrefixLen) } // FromIP4 converts an ipv4 address to our integer address type diff --git a/prog/weaver/main.go b/prog/weaver/main.go index 09ee7f48..70a3aba5 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -340,9 +340,12 @@ func createOverlay(datapathName string, ifaceName string, host string, port int, } func parseAndCheckCIDR(cidrStr string) address.CIDR { - _, cidr, err := address.ParseCIDR(cidrStr) + cidr, err := address.ParseCIDR(cidrStr) checkFatal(err) + if !cidr.IsSubnet() { + Log.Fatalf("Invalid allocation range %s - bits after network prefix are not all zero", cidrStr) + } if cidr.Size() < ipam.MinSubnetSize { Log.Fatalf("Allocation range smaller than minimum size %d: %s", ipam.MinSubnetSize, cidrStr) } diff --git a/weave b/weave index 6364bcfd..a05ff71c 100755 --- a/weave +++ b/weave @@ -1298,7 +1298,7 @@ check_overlap() { # with_container_addresses. ipam_reclaim() { for CIDR in $3 ; do - http_call $HTTP_ADDR PUT /ip/$1/${CIDR%/*}?noErrorOnUnknown=true + http_call $HTTP_ADDR PUT /ip/$1/$CIDR?noErrorOnUnknown=true done } @@ -1343,7 +1343,7 @@ ipam_cidrs() { echo "IP address allocation must be enabled to use 'net:'" >&2 return 1 fi - elif ! is_cidr "$CIDR" ; then + elif [ -n "$CIDR" ] && ! is_cidr "$CIDR" ; then echo "$CIDR" >&2 return 1 else @@ -1354,7 +1354,7 @@ ipam_cidrs() { if [ "$METHOD" = "POST" ] ; then # Assignment of a plain IP address; warn if it clashes but carry on check_overlap $arg || true - when_weave_running http_call $HTTP_ADDR PUT /ip/$CONTAINER_ID/${arg%/*} >&2 + when_weave_running http_call $HTTP_ADDR PUT /ip/$CONTAINER_ID/$arg >&2 fi ALL_CIDRS="$ALL_CIDRS $arg" fi @@ -2064,7 +2064,7 @@ EOF RES=$(docker restart $1) CONTAINER=$(container_id $1) for CIDR in $ALL_CIDRS ; do - call_weave PUT /ip/$CONTAINER/${CIDR%/*} + call_weave PUT /ip/$CONTAINER/$CIDR done with_container_netns_or_die $1 attach $ALL_CIDRS when_weave_running with_container_fqdn $1 put_dns_fqdn $ALL_CIDRS