From e6d230042de44361e1856af41ae278da64b49560 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 28 May 2015 11:28:39 +0100 Subject: [PATCH 01/34] Introduce address.CIDR type --- ipam/address/address.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/ipam/address/address.go b/ipam/address/address.go index 692ffaab..0eae12b5 100644 --- a/ipam/address/address.go +++ b/ipam/address/address.go @@ -1,8 +1,10 @@ package address import ( - "github.com/weaveworks/weave/common" + "fmt" "net" + + "github.com/weaveworks/weave/common" ) // Using 32-bit integer to represent IPv4 address @@ -13,6 +15,11 @@ type Range struct { Start, End Address // [Start, End); Start <= End } +type CIDR struct { + Start Address + PrefixLen int +} + func ParseIP(s string) (Address, error) { if ip := net.ParseIP(s); ip != nil { return FromIP4(ip), nil @@ -20,6 +27,25 @@ func ParseIP(s string) (Address, error) { return 0, &net.ParseError{Type: "IP Address", Text: s} } +func ParseCIDR(s string) (Address, CIDR, error) { + if ip, ipnet, err := net.ParseCIDR(s); err != nil { + return 0, CIDR{}, err + } else if ipnet.IP.To4() == nil { + return 0, 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 + } +} + +func (cidr CIDR) Size() Offset { return 1 << uint(32-cidr.PrefixLen) } +func (cidr CIDR) End() Address { return Add(cidr.Start, cidr.Size()) } +func (cidr CIDR) Blank() bool { return cidr.Start == 0 && cidr.PrefixLen == 0 } + +func (cidr CIDR) String() string { + return fmt.Sprintf("%s/%d", cidr.Start.String(), cidr.PrefixLen) +} + // FromIP4 converts an ipv4 address to our integer address type func FromIP4(ip4 net.IP) (r Address) { for _, b := range ip4.To4() { From 2553850ed4a5a409a4f2c78c13c36e6f06ba9122 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 2 Jun 2015 14:15:37 +0100 Subject: [PATCH 02/34] Initial implementation of multi-subnet IPAM --- ipam/allocate.go | 9 ++++-- ipam/allocator.go | 67 +++++++++++++++++----------------------- ipam/http.go | 33 ++++++++++++++++++-- ipam/ring/ring.go | 13 ++++++-- ipam/ring/ring_test.go | 22 ++++++++----- ipam/space/space.go | 26 +++++++++++----- ipam/space/space_test.go | 19 ++++++------ ipam/testutils_test.go | 10 +++--- prog/weaver/main.go | 24 +++++++++++--- router/protocol.go | 2 +- weave | 2 +- 11 files changed, 145 insertions(+), 82 deletions(-) diff --git a/ipam/allocate.go b/ipam/allocate.go index d709784d..9fe2522c 100644 --- a/ipam/allocate.go +++ b/ipam/allocate.go @@ -14,6 +14,7 @@ type allocate struct { resultChan chan<- allocateResult hasBeenCancelled func() bool ident string + subnet address.CIDR } // Try returns true if the request is completed, false if pending @@ -29,15 +30,17 @@ func (g *allocate) Try(alloc *Allocator) bool { return true } - if ok, addr := alloc.space.Allocate(); ok { - alloc.debugln("Allocated", addr, "for", g.ident) + // Respect RFC1122 exclusions of first and last addresses + start, end := g.subnet.Start+1, g.subnet.End()-1 + if ok, addr := alloc.space.Allocate(start, end); ok { + alloc.debugln("Allocated", addr, "for", g.ident, "in", g.subnet) alloc.addOwned(g.ident, addr) g.resultChan <- allocateResult{addr, nil} return true } // out of space - if donor, err := alloc.ring.ChoosePeerToAskForSpace(); err == nil { + if donor, err := alloc.ring.ChoosePeerToAskForSpace(start, end); err == nil { alloc.debugln("Decided to ask peer", donor, "for space") alloc.sendRequest(donor, msgSpaceRequest) } diff --git a/ipam/allocator.go b/ipam/allocator.go index 210eb796..ceea6caf 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -3,9 +3,7 @@ package ipam import ( "bytes" "encoding/gob" - "errors" "fmt" - "net" "sort" "time" @@ -46,12 +44,11 @@ type operation interface { type Allocator struct { actionChan chan<- func() ourName router.PeerName - subnetStart address.Address // start address of space all peers are allocating from - subnetSize address.Offset // length of space all peers are allocating from - prefixLen int // network prefix length, e.g. 24 for a /24 network + defaultSubnet address.CIDR + universe address.CIDR // 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 address, indexed by container-ID + owned map[string]address.Address // who owns what address, indexed by container-ID -- fixme - needs to be multi-subnet nicknames map[router.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 @@ -63,34 +60,16 @@ type Allocator struct { } // NewAllocator creates and initialises a new Allocator -func NewAllocator(ourName router.PeerName, ourUID router.PeerUID, ourNickname string, subnetCIDR string, quorum uint) (*Allocator, error) { - _, subnet, err := net.ParseCIDR(subnetCIDR) - if err != nil { - return nil, err - } - if subnet.IP.To4() == nil { - return nil, errors.New("Non-IPv4 address not supported") - } - // Get the size of the network from the mask - ones, bits := subnet.Mask.Size() - var subnetSize address.Offset = 1 << uint(bits-ones) - if subnetSize < 4 { - return nil, errors.New("Allocation subnet too small") - } - subnetStart := address.FromIP4(subnet.IP) - alloc := &Allocator{ - ourName: ourName, - subnetStart: subnetStart, - subnetSize: subnetSize, - prefixLen: ones, - // per RFC 1122, don't allocate the first and last address in the subnet - ring: ring.New(address.Add(subnetStart, 1), address.Add(subnetStart, subnetSize-1), ourName), +func NewAllocator(ourName router.PeerName, ourUID router.PeerUID, ourNickname string, universe address.CIDR, quorum uint) *Allocator { + return &Allocator{ + ourName: ourName, + universe: universe, + ring: ring.New(universe.Start, address.Add(universe.Start, universe.Size()), ourName), owned: make(map[string]address.Address), paxos: paxos.NewNode(ourName, ourUID, quorum), nicknames: map[router.PeerName]string{ourName: ourNickname}, now: time.Now, } - return alloc, nil } // Start runs the allocator goroutine @@ -197,11 +176,24 @@ func hasBeenCancelled(cancelChan <-chan bool) func() bool { // Actor client API +func (alloc *Allocator) SetDefaultSubnet(subnet address.CIDR) error { + resultChan := make(chan error) + if subnet.Size() < 4 { + return fmt.Errorf("Allocation range smaller than minimum size 4: %s", subnet) + } + alloc.actionChan <- func() { + // todo: check default subnet is in range + alloc.defaultSubnet = subnet + resultChan <- nil + } + return <-resultChan +} + // Allocate (Sync) - get IP address for container with given name // if there isn't any space we block indefinitely -func (alloc *Allocator) Allocate(ident string, cancelChan <-chan bool) (address.Address, error) { +func (alloc *Allocator) Allocate(ident string, subnet address.CIDR, cancelChan <-chan bool) (address.Address, error) { resultChan := make(chan allocateResult) - op := &allocate{resultChan: resultChan, ident: ident, + op := &allocate{subnet: subnet, resultChan: resultChan, ident: ident, hasBeenCancelled: hasBeenCancelled(cancelChan)} alloc.doOperation(op, &alloc.pendingAllocates) result := <-resultChan @@ -470,20 +462,17 @@ func (alloc *Allocator) actorLoop(actionChan <-chan func()) { func (alloc *Allocator) string() string { var buf bytes.Buffer - fmt.Fprintf(&buf, "Allocator subnet %s/%d", alloc.subnetStart.String(), alloc.prefixLen) + fmt.Fprintf(&buf, "Allocator universe %s", alloc.universe) + if !alloc.defaultSubnet.Blank() { + fmt.Fprintf(&buf, " (default subnet %s)", alloc.defaultSubnet) + } if alloc.ring.Empty() { if alloc.paxosTicker != nil { fmt.Fprintf(&buf, " awaiting consensus: %s", alloc.paxos.String()) } } else { - localFreeSpace := alloc.space.NumFreeAddresses() - remoteFreeSpace := alloc.ring.TotalRemoteFree() - percentFree := 100 * float64(localFreeSpace+remoteFreeSpace) / float64(alloc.subnetSize) - fmt.Fprintf(&buf, " Free IPs: ~%.1f%%, %d local, ~%d remote\n", - percentFree, localFreeSpace, remoteFreeSpace) - - fmt.Fprint(&buf, "Owned Ranges:") + fmt.Fprint(&buf, "\nOwned Ranges:") alloc.ring.FprintWithNicknames(&buf, alloc.nicknames) } if len(alloc.pendingAllocates)+len(alloc.pendingClaims) > 0 { diff --git a/ipam/http.go b/ipam/http.go index 4e7cd907..925fe5fd 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -34,24 +34,51 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) { }) router.Methods("GET").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if alloc.defaultSubnet.Blank() { + badRequest(w, fmt.Errorf("No subnet specified")) + return + } addr, err := alloc.Lookup(mux.Vars(r)["id"]) if err != nil { http.NotFound(w, r) return } - fmt.Fprintf(w, "%s/%d", addr.String(), alloc.prefixLen) + fmt.Fprintf(w, "%s/%d", addr.String(), alloc.defaultSubnet.PrefixLen) + }) + + router.Methods("POST").Path("/ip/{id}/{ip}/{prefixlen}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + closedChan := w.(http.CloseNotifier).CloseNotify() + vars := mux.Vars(r) + ident := vars["id"] + cidrStr := vars["ip"] + "/" + vars["prefixlen"] + _, cidr, err := address.ParseCIDR(cidrStr) + if err != nil { + badRequest(w, err) + return + } + addr, err := alloc.Allocate(ident, cidr, closedChan) + if err != nil { + badRequest(w, fmt.Errorf("Unable to allocate: %s", err)) + return + } + + fmt.Fprintf(w, "%s/%d", addr.String(), cidr.PrefixLen) }) router.Methods("POST").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if alloc.defaultSubnet.Blank() { + badRequest(w, fmt.Errorf("No subnet specified")) + return + } closedChan := w.(http.CloseNotifier).CloseNotify() ident := mux.Vars(r)["id"] - newAddr, err := alloc.Allocate(ident, closedChan) + newAddr, err := alloc.Allocate(ident, alloc.defaultSubnet, closedChan) if err != nil { badRequest(w, err) return } - fmt.Fprintf(w, "%s/%d", newAddr.String(), alloc.prefixLen) + fmt.Fprintf(w, "%s/%d", newAddr.String(), alloc.defaultSubnet.PrefixLen) }) router.Methods("DELETE").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/ipam/ring/ring.go b/ipam/ring/ring.go index ddb2f631..910c87c6 100644 --- a/ipam/ring/ring.go +++ b/ipam/ring/ring.go @@ -409,15 +409,22 @@ func (r *Ring) ReportFree(freespace map[address.Address]address.Offset) { } // ChoosePeerToAskForSpace chooses a weighted-random peer to ask -// for space. -func (r *Ring) ChoosePeerToAskForSpace() (result router.PeerName, err error) { +// for space in the range [start, end). Assumes start= end { + break + } // Ignore ranges with no free space if entry.Free <= 0 { continue diff --git a/ipam/ring/ring_test.go b/ipam/ring/ring_test.go index f3ec1e0f..11aa47f9 100644 --- a/ipam/ring/ring_test.go +++ b/ipam/ring/ring_test.go @@ -345,34 +345,40 @@ func TestGossip(t *testing.T) { func TestFindFree(t *testing.T) { ring1 := New(start, end, peer1name) - _, err := ring1.ChoosePeerToAskForSpace() + _, err := ring1.ChoosePeerToAskForSpace(start, end) wt.AssertTrue(t, err == ErrNoFreeSpace, "Expected ErrNoFreeSpace") ring1.Entries = []*entry{{Token: start, Peer: peer1name}} - _, err = ring1.ChoosePeerToAskForSpace() + _, err = ring1.ChoosePeerToAskForSpace(start, end) wt.AssertTrue(t, err == ErrNoFreeSpace, "Expected ErrNoFreeSpace") // We shouldn't return outselves ring1.ReportFree(map[address.Address]address.Offset{start: 10}) - _, err = ring1.ChoosePeerToAskForSpace() + _, err = ring1.ChoosePeerToAskForSpace(start, end) wt.AssertTrue(t, err == ErrNoFreeSpace, "Expected ErrNoFreeSpace") ring1.Entries = []*entry{{Token: start, Peer: peer1name, Free: 1}, - {Token: start, Peer: peer1name, Free: 1}} - _, err = ring1.ChoosePeerToAskForSpace() + {Token: middle, Peer: peer1name, Free: 1}} + _, err = ring1.ChoosePeerToAskForSpace(start, end) wt.AssertTrue(t, err == ErrNoFreeSpace, "Expected ErrNoFreeSpace") + ring1.assertInvariants() // We should return others ring1.Entries = []*entry{{Token: start, Peer: peer2name, Free: 1}} - peer, err := ring1.ChoosePeerToAskForSpace() + peer, err := ring1.ChoosePeerToAskForSpace(start, end) wt.AssertSuccess(t, err) wt.AssertEquals(t, peer, peer2name) ring1.Entries = []*entry{{Token: start, Peer: peer2name, Free: 1}, - {Token: start, Peer: peer2name, Free: 1}} - peer, err = ring1.ChoosePeerToAskForSpace() + {Token: middle, Peer: peer3name, Free: 1}} + peer, err = ring1.ChoosePeerToAskForSpace(start, middle) wt.AssertSuccess(t, err) wt.AssertEquals(t, peer, peer2name) + + peer, err = ring1.ChoosePeerToAskForSpace(middle, end) + wt.AssertSuccess(t, err) + wt.AssertEquals(t, peer, peer3name) + ring1.assertInvariants() } func TestReportFree(t *testing.T) { diff --git a/ipam/space/space.go b/ipam/space/space.go index 548d9e55..c4d8ea5f 100644 --- a/ipam/space/space.go +++ b/ipam/space/space.go @@ -37,14 +37,26 @@ func (s *Space) Clear() { s.ours = s.ours[:0] } -func (s *Space) Allocate() (bool, address.Address) { - if len(s.free) == 0 { - return false, 0 +func (s *Space) Allocate(rangeStart, rangeEnd address.Address) (bool, address.Address) { + // Need to walk forward to the range, then check free space, then pick an address + // Walk down the free spaces, looking for one in range + for i := 0; i < len(s.free); i += 2 { + start, end := s.free[i], s.free[i+1] + if end-1 < rangeStart { + continue + } + if start > rangeEnd { + break + } + res := start + if res < rangeStart { + res = rangeStart + } + s.ours = add(s.ours, res, res+1) + s.free = subtract(s.free, res, res+1) + return true, res } - res := s.free[0] - s.ours = add(s.ours, res, res+1) - s.free = subtract(s.free, res, res+1) - return true, res + return false, 0 } func (s *Space) Claim(addr address.Address) error { diff --git a/ipam/space/space_test.go b/ipam/space/space_test.go index d49324b7..11df7b58 100644 --- a/ipam/space/space_test.go +++ b/ipam/space/space_test.go @@ -52,12 +52,12 @@ func TestLowlevel(t *testing.T) { s := New() wt.AssertEquals(t, s.NumFreeAddresses(), address.Offset(0)) - ok, got := s.Allocate() + ok, got := s.Allocate(0, 1000) wt.AssertFalse(t, ok, "allocate in empty space should fail") s.Add(100, 100) wt.AssertEquals(t, s.NumFreeAddresses(), address.Offset(100)) - ok, got = s.Allocate() + ok, got = s.Allocate(0, 1000) wt.AssertTrue(t, ok && got == 100, "allocate") wt.AssertEquals(t, s.NumFreeAddresses(), address.Offset(99)) wt.AssertNoErr(t, s.Claim(150)) @@ -86,6 +86,7 @@ func TestLowlevel(t *testing.T) { func TestSpaceAllocate(t *testing.T) { const ( testAddr1 = "10.0.3.4" + testAddr2 = "10.0.3.5" testAddrx = "10.0.3.19" testAddry = "10.0.9.19" containerID = "deadbeef" @@ -95,13 +96,13 @@ func TestSpaceAllocate(t *testing.T) { wt.AssertEquals(t, space1.NumFreeAddresses(), address.Offset(20)) space1.assertInvariants() - _, addr1 := space1.Allocate() - wt.AssertEqualString(t, addr1.String(), testAddr1, "address") + _, addr1 := space1.Allocate(ip(testAddr2), ip(testAddrx)) + wt.AssertEqualString(t, addr1.String(), testAddr2, "address") wt.AssertEquals(t, space1.NumFreeAddresses(), address.Offset(19)) space1.assertInvariants() - _, addr2 := space1.Allocate() - wt.AssertFalse(t, addr2.String() == testAddr1, "address") + _, addr2 := space1.Allocate(ip(testAddr2), ip(testAddrx)) + wt.AssertFalse(t, addr2.String() == testAddr2, "address") wt.AssertEquals(t, space1.NumFreeAddresses(), address.Offset(18)) wt.AssertEquals(t, space1.NumFreeAddressesInRange(ip(testAddr1), ip(testAddrx)), address.Offset(13)) wt.AssertEquals(t, space1.NumFreeAddressesInRange(ip(testAddr1), ip(testAddry)), address.Offset(18)) @@ -132,12 +133,12 @@ func TestSpaceFree(t *testing.T) { wt.AssertTrue(t, space.free[pos] == ip(testAddr1) && size == 20, "Wrong space") for i := 0; i < 20; i++ { - ok, _ := space.Allocate() + ok, _ := space.Allocate(ip(testAddr1), ip(testAddry)) wt.AssertTrue(t, ok, "Failed to get address") } // Check we are full - ok, _ := space.Allocate() + ok, _ := space.Allocate(ip(testAddr1), ip(testAddry)) wt.AssertTrue(t, !ok, "Should have failed to get address") _, size, ok = space.Donate() wt.AssertTrue(t, size == 0, "Wrong space") @@ -222,7 +223,7 @@ func TestDonateHard(t *testing.T) { // Fill a fresh space spaceset := makeSpace(start, size) for i := address.Offset(0); i < size; i++ { - ok, _ := spaceset.Allocate() + ok, _ := spaceset.Allocate(start, address.Add(start, size)) wt.AssertTrue(t, ok, "Failed to get IP!") } diff --git a/ipam/testutils_test.go b/ipam/testutils_test.go index c47273cb..f4a79f18 100644 --- a/ipam/testutils_test.go +++ b/ipam/testutils_test.go @@ -128,17 +128,19 @@ func CheckAllExpectedMessagesSent(allocs ...*Allocator) { } } -func makeAllocator(name string, cidr string, quorum uint) *Allocator { +func makeAllocator(name string, cidrStr string, quorum uint) *Allocator { peername, err := router.PeerNameFromString(name) if err != nil { panic(err) } - alloc, err := NewAllocator(peername, router.PeerUID(rand.Int63()), - "nick-"+name, cidr, quorum) + _, cidr, err := address.ParseCIDR(cidrStr) if err != nil { panic(err) } + alloc := NewAllocator(peername, router.PeerUID(rand.Int63()), + "nick-"+name, cidr, quorum) + alloc.defaultSubnet = cidr return alloc } @@ -163,7 +165,7 @@ func (alloc *Allocator) claimRingForTesting(allocs ...*Allocator) { func (alloc *Allocator) NumFreeAddresses() address.Offset { resultChan := make(chan address.Offset) alloc.actionChan <- func() { - resultChan <- alloc.space.NumFreeAddresses() + resultChan <- alloc.space.NumFreeAddressesInRange(alloc.defaultSubnet.Start+1, alloc.defaultSubnet.End()-1) } return <-resultChan } diff --git a/prog/weaver/main.go b/prog/weaver/main.go index e59b4202..809ffd49 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -17,6 +17,7 @@ import ( . "github.com/weaveworks/weave/common" "github.com/weaveworks/weave/common/updater" "github.com/weaveworks/weave/ipam" + "github.com/weaveworks/weave/ipam/address" weavenet "github.com/weaveworks/weave/net" weave "github.com/weaveworks/weave/router" ) @@ -51,6 +52,7 @@ func main() { noDiscovery bool httpAddr string iprangeCIDR string + defltSubnet string peerCount int apiPath string peers []string @@ -71,6 +73,7 @@ func main() { flag.IntVar(&bufSzMB, "bufsz", 8, "capture buffer size in MB") flag.StringVar(&httpAddr, "httpaddr", fmt.Sprintf(":%d", weave.HTTPPort), "address to bind HTTP interface to (disabled if blank, absolute path indicates unix domain socket)") flag.StringVar(&iprangeCIDR, "iprange", "", "IP address range to allocate within, in CIDR notation") + flag.StringVar(&defltSubnet, "defaultsubnet", "", "default subnet for IP address allocation, in CIDR notation") flag.IntVar(&peerCount, "initpeercount", 0, "number of peers in network (for IP address allocation)") flag.StringVar(&apiPath, "api", "unix:///var/run/docker.sock", "Path to Docker API socket") flag.Parse() @@ -138,9 +141,9 @@ func main() { var allocator *ipam.Allocator if iprangeCIDR != "" { - allocator = createAllocator(router, apiPath, iprangeCIDR, determineQuorum(peerCount, peers)) + allocator = createAllocator(router, apiPath, iprangeCIDR, defltSubnet, determineQuorum(peerCount, peers)) } else if peerCount > 0 { - log.Fatal("-initpeercount flag specified without -iprange") + log.Fatal("-initpeercount flag specified without -iprange or -ipuniverse") } else { router.NewGossip("IPallocation", &ipam.DummyAllocator{}) } @@ -194,13 +197,26 @@ func logFrameFunc(debug bool) weave.LogFrameFunc { } } -func createAllocator(router *weave.Router, apiPath string, iprangeCIDR string, quorum uint) *ipam.Allocator { - allocator, err := ipam.NewAllocator(router.Ourself.Peer.Name, router.Ourself.Peer.UID, router.Ourself.Peer.NickName, iprangeCIDR, quorum) +func createAllocator(router *weave.Router, apiPath string, ipUniverse string, defaultSubnet string, quorum uint) *ipam.Allocator { + _, universe, err := address.ParseCIDR(ipUniverse) if err != nil { log.Fatal(err) } + if universe.Size() < 4 { + log.Fatal("IP allocation range too small") + } + allocator := ipam.NewAllocator(router.Ourself.Peer.Name, router.Ourself.Peer.UID, router.Ourself.Peer.NickName, universe, quorum) allocator.SetInterfaces(router.NewGossip("IPallocation", allocator)) allocator.Start() + if defaultSubnet != "" { + _, subnet, err := address.ParseCIDR(defaultSubnet) + if err != nil { + log.Fatal(err) + } + if err = allocator.SetDefaultSubnet(subnet); err != nil { + log.Fatal(err) + } + } err = updater.Start(apiPath, allocator) if err != nil { log.Fatal("Unable to start watcher", err) diff --git a/router/protocol.go b/router/protocol.go index 5e9c7036..60482621 100644 --- a/router/protocol.go +++ b/router/protocol.go @@ -2,7 +2,7 @@ package router const ( Protocol = "weave" - ProtocolVersion = 18 + ProtocolVersion = 19 ) type ProtocolTag byte diff --git a/weave b/weave index 40f8e2a9..f96e25e4 100755 --- a/weave +++ b/weave @@ -28,7 +28,7 @@ usage() { echo "Usage:" echo "weave setup" echo "weave launch [-password ] [-nickname ] [-nodiscovery]" - echo " [-iprange ] ..." + echo " [-iprange [-ipsubnet ]] ..." echo "weave launch-dns " echo "weave launch-proxy [-D ] [-H
] [--with-dns] [--with-ipam]" echo "weave connect [--replace] [ ...]" From c7a9dfbb3f3e6181c07fb1bce3e1124de944ad9a Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 2 Jun 2015 15:27:31 +0100 Subject: [PATCH 03/34] Remember multiple addresses owned by each container, for multi-subnet use --- ipam/address/address.go | 9 +++++++++ ipam/allocate.go | 15 ++++++++++++--- ipam/allocator.go | 34 ++++++++++++++++++---------------- 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/ipam/address/address.go b/ipam/address/address.go index 0eae12b5..b5dbc2cb 100644 --- a/ipam/address/address.go +++ b/ipam/address/address.go @@ -46,6 +46,15 @@ func (cidr CIDR) String() string { return fmt.Sprintf("%s/%d", cidr.Start.String(), cidr.PrefixLen) } +func (cidr1 CIDR) Overlaps(cidr2 CIDR) bool { + return cidr1.Start >= cidr2.Start && cidr1.Start < cidr2.End() || + cidr2.Start >= cidr1.Start && cidr2.Start < cidr1.End() +} + +func (cidr CIDR) Contains(addr Address) bool { + return addr >= cidr.Start && addr < cidr.End() +} + // FromIP4 converts an ipv4 address to our integer address type func FromIP4(ip4 net.IP) (r Address) { for _, b := range ip4.To4() { diff --git a/ipam/allocate.go b/ipam/allocate.go index 9fe2522c..8fe28ad0 100644 --- a/ipam/allocate.go +++ b/ipam/allocate.go @@ -24,9 +24,18 @@ func (g *allocate) Try(alloc *Allocator) bool { return true } - // If we have previously stored an address for this container, return it. - if addr, found := alloc.owned[g.ident]; found { - g.resultChan <- allocateResult{addr, nil} + // If we have previously stored an address for this container in this subnet, return it. + if addrs, found := alloc.owned[g.ident]; found { + for _, addr := range addrs { + if g.subnet.Contains(addr) { + g.resultChan <- allocateResult{addr, nil} + return true + } + } + } + + if !alloc.universe.Overlaps(g.subnet) { + g.resultChan <- allocateResult{0, fmt.Errorf("Subnet %s out of bounds: %s", g.subnet, alloc.universe)} return true } diff --git a/ipam/allocator.go b/ipam/allocator.go index ceea6caf..cd714f3e 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -45,14 +45,14 @@ type Allocator struct { actionChan chan<- func() ourName router.PeerName defaultSubnet address.CIDR - universe address.CIDR // 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 address, indexed by container-ID -- fixme - needs to be multi-subnet - nicknames map[router.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 - gossip router.Gossip // our link to the outside world for sending messages + universe address.CIDR // 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 address, indexed by container-ID + nicknames map[router.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 + gossip router.Gossip // our link to the outside world for sending messages paxos *paxos.Node paxosTicker *time.Ticker shuttingDown bool // to avoid doing any requests while trying to shut down @@ -65,7 +65,7 @@ func NewAllocator(ourName router.PeerName, ourUID router.PeerUID, ourNickname st ourName: ourName, universe: universe, ring: ring.New(universe.Start, address.Add(universe.Start, universe.Size()), ourName), - owned: make(map[string]address.Address), + owned: make(map[string][]address.Address), paxos: paxos.NewNode(ourName, ourUID, quorum), nicknames: map[router.PeerName]string{ourName: ourNickname}, now: time.Now, @@ -205,7 +205,7 @@ func (alloc *Allocator) Lookup(ident string) (address.Address, error) { alloc.actionChan <- func() { addr, found := alloc.owned[ident] if found { - resultChan <- allocateResult{addr: addr} + resultChan <- allocateResult{addr: addr[0]} return } resultChan <- allocateResult{err: fmt.Errorf("lookup: no address found")} @@ -237,8 +237,8 @@ func (alloc *Allocator) ContainerDied(ident string) error { func (alloc *Allocator) free(ident string) error { errChan := make(chan error) alloc.actionChan <- func() { - addr, found := alloc.owned[ident] - if found { + addrs, found := alloc.owned[ident] + for _, addr := range addrs { alloc.space.Free(addr) } delete(alloc.owned, ident) @@ -669,13 +669,15 @@ func (alloc *Allocator) reportFreeSpace() { // Owned addresses func (alloc *Allocator) addOwned(ident string, addr address.Address) { - alloc.owned[ident] = addr + alloc.owned[ident] = append(alloc.owned[ident], addr) } func (alloc *Allocator) findOwner(addr address.Address) string { - for ident, candidate := range alloc.owned { - if candidate == addr { - return ident + for ident, addrs := range alloc.owned { + for _, candidate := range addrs { + if candidate == addr { + return ident + } } } return "" From ff545feae39d8769d10bfda7f806208b6ecf881c Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 1 Jun 2015 11:38:23 +0100 Subject: [PATCH 04/34] Add some unit tests for multi-subnet --- ipam/allocator_test.go | 70 +++++++++++++++++++++++++----------------- ipam/http_test.go | 56 +++++++++++++++++---------------- 2 files changed, 72 insertions(+), 54 deletions(-) diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index cfca9867..05531b08 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -24,36 +24,49 @@ func TestAllocFree(t *testing.T) { container1 = "abcdef" container2 = "baddf00d" container3 = "b01df00d" - universe = "10.0.3.0/28" - testAddr1 = "10.0.3.1" // first address allocated should be .1 because .0 is network addr - spaceSize = 14 // 16 IP addresses in /28, minus .0 and .15 + universe = "10.0.3.0/26" + subnet1 = "10.0.3.0/28" + subnet2 = "10.0.3.32/28" + testAddr1 = "10.0.3.1" + testAddr2 = "10.0.3.33" + spaceSize = 62 // 64 IP addresses in /26, minus .0 and .63 ) alloc := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", universe, 1) defer alloc.Stop() + _, cidr1, _ := address.ParseCIDR(subnet1) + _, cidr2, _ := address.ParseCIDR(subnet2) alloc.claimRingForTesting() - addr1, _ := alloc.Allocate(container1, nil) + addr1, _ := alloc.Allocate(container1, cidr1, nil) wt.AssertEqualString(t, addr1.String(), testAddr1, "address") + addr2, err := alloc.Allocate(container1, cidr2, nil) + wt.AssertNoErr(t, err) + wt.AssertEqualString(t, addr2.String(), testAddr2, "address") + CheckAllExpectedMessagesSent(alloc) + // Ask for another address for a different container and check it's different - addr2, _ := alloc.Allocate(container2, nil) - if addr2.String() == testAddr1 { - t.Fatalf("Expected different address but got %s", addr2.String()) + addr1b, _ := alloc.Allocate(container2, cidr1, nil) + 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 address again - addr1a, _ := alloc.Allocate(container1, nil) + // Ask for the first container again and we should get the same addresses again + addr1a, _ := alloc.Allocate(container1, cidr1, nil) wt.AssertEqualString(t, addr1a.String(), testAddr1, "address") + addr2a, _ := alloc.Allocate(container1, cidr2, nil) + wt.AssertEqualString(t, addr2a.String(), testAddr2, "address") - // Now free the first one, and we should get it back when we ask + // Now free the first container, and we should get its addresses back wt.AssertSuccess(t, alloc.Free(container1)) - addr3, _ := alloc.Allocate(container3, nil) + addr3, _ := alloc.Allocate(container3, cidr1, nil) wt.AssertEqualString(t, addr3.String(), testAddr1, "address") + addr4, _ := alloc.Allocate(container3, cidr2, nil) + wt.AssertEqualString(t, addr4.String(), testAddr2, "address") alloc.ContainerDied(container2) alloc.ContainerDied(container3) - alloc.String() // force sync-up after async call wt.AssertEquals(t, alloc.NumFreeAddresses(), address.Offset(spaceSize)) } @@ -80,7 +93,7 @@ func TestBootstrap(t *testing.T) { ExpectBroadcastMessage(alloc1, nil) // alloc1 will try to form consensus done := make(chan bool) go func() { - alloc1.Allocate("somecontainer", nil) + alloc1.Allocate("somecontainer", alloc1.defaultSubnet, nil) done <- true }() time.Sleep(100 * time.Millisecond) @@ -128,15 +141,16 @@ func TestAllocatorClaim(t *testing.T) { defer alloc.Stop() alloc.claimRingForTesting() - addr1, _ := alloc.Allocate(container1, nil) - alloc.Allocate(container2, nil) + addr1, _ := alloc.Allocate(container1, alloc.defaultSubnet, nil) + alloc.Allocate(container2, alloc.defaultSubnet, nil) // Now free the first one, and try to claim it wt.AssertSuccess(t, alloc.Free(container1)) t.Log(alloc) err := alloc.Claim(container3, addr1, nil) wt.AssertNoErr(t, err) - addr3, _ := alloc.Allocate(container3, nil) + // Check we get this address back if we try an allocate + addr3, _ := alloc.Allocate(container3, alloc.defaultSubnet, nil) wt.AssertEqualString(t, addr3.String(), testAddr1, "address") } @@ -175,9 +189,9 @@ func TestCancel(t *testing.T) { alloc1.OnGossipBroadcast(alloc2.Encode()) // Get some IPs, so each allocator has some space - res1, _ := alloc1.Allocate("foo", nil) + res1, _ := alloc1.Allocate("foo", alloc1.defaultSubnet, nil) common.Debug.Printf("res1 = %s", res1.String()) - res2, _ := alloc2.Allocate("bar", nil) + res2, _ := alloc2.Allocate("bar", alloc2.defaultSubnet, nil) common.Debug.Printf("res2 = %s", res2.String()) if res1 == res2 { wt.Fatalf(t, "Error: got same ips!") @@ -189,12 +203,12 @@ func TestCancel(t *testing.T) { // Use up all the IPs that alloc1 owns, so the allocation after this will prompt a request to alloc2 for i := 0; alloc1.NumFreeAddresses() > 0; i++ { - alloc1.Allocate(fmt.Sprintf("tmp%d", i), nil) + alloc1.Allocate(fmt.Sprintf("tmp%d", i), alloc1.defaultSubnet, nil) } cancelChan := make(chan bool, 1) doneChan := make(chan bool) go func() { - _, ok := alloc1.Allocate("baz", cancelChan) + _, ok := alloc1.Allocate("baz", alloc1.defaultSubnet, cancelChan) doneChan <- ok == nil }() @@ -221,12 +235,12 @@ func TestGossipShutdown(t *testing.T) { defer alloc.Stop() alloc.claimRingForTesting() - addr1, _ := alloc.Allocate(container1, nil) + addr1, _ := alloc.Allocate(container1, alloc.defaultSubnet, nil) wt.AssertEqualString(t, addr1.String(), testAddr1, "address") alloc.Shutdown() - _, err := alloc.Allocate(container2, nil) // trying to allocate after shutdown should fail + _, err := alloc.Allocate(container2, alloc.defaultSubnet, nil) // trying to allocate after shutdown should fail wt.AssertFalse(t, err == nil, "no address") CheckAllExpectedMessagesSent(alloc) @@ -241,10 +255,10 @@ func TestTransfer(t *testing.T) { alloc2 := allocs[1] alloc3 := allocs[2] // This will be 'master' and get the first range - _, err := alloc2.Allocate("foo", nil) + _, err := alloc2.Allocate("foo", alloc2.defaultSubnet, nil) wt.AssertTrue(t, err == nil, "Failed to get address") - _, err = alloc3.Allocate("bar", nil) + _, err = alloc3.Allocate("bar", alloc3.defaultSubnet, nil) wt.AssertTrue(t, err == nil, "Failed to get address") router.GossipBroadcast(alloc2.Gossip()) @@ -258,7 +272,7 @@ func TestTransfer(t *testing.T) { wt.AssertEquals(t, alloc1.NumFreeAddresses(), address.Offset(1022)) - _, err = alloc1.Allocate("foo", nil) + _, err = alloc1.Allocate("foo", alloc1.defaultSubnet, nil) wt.AssertTrue(t, err == nil, "Failed to get address") alloc1.Stop() } @@ -274,7 +288,7 @@ func TestFakeRouterSimple(t *testing.T) { alloc1 := allocs[0] //alloc2 := allocs[1] - addr, _ := alloc1.Allocate("foo", nil) + addr, _ := alloc1.Allocate("foo", alloc1.defaultSubnet, nil) println("Got addr", addr) } @@ -332,7 +346,7 @@ func TestAllocatorFuzz(t *testing.T) { allocIndex := rand.Int31n(nodes) alloc := allocs[allocIndex] //common.Info.Printf("Allocate: asking allocator %d", allocIndex) - addr, err := alloc.Allocate(name, nil) + addr, err := alloc.Allocate(name, alloc.defaultSubnet, nil) if err != nil { panic(fmt.Sprintf("Could not allocate addr")) @@ -394,7 +408,7 @@ func TestAllocatorFuzz(t *testing.T) { //common.Info.Printf("Asking for %s on allocator %d again", addr, res.alloc) - newAddr, _ := alloc.Allocate(res.name, nil) + newAddr, _ := alloc.Allocate(res.name, alloc.defaultSubnet, nil) oldAddr, _ := address.ParseIP(addr) if newAddr != oldAddr { panic(fmt.Sprintf("Got different address for repeat request for %s: %s != %s", res.name, newAddr, oldAddr)) diff --git a/ipam/http_test.go b/ipam/http_test.go index f1a828d1..c66b2d85 100644 --- a/ipam/http_test.go +++ b/ipam/http_test.go @@ -59,45 +59,49 @@ func listenHTTP(alloc *Allocator) int { return httpListener.Addr().(*net.TCPAddr).Port } -func allocURL(port int, containerID string) string { +func identURL(port int, containerID string) string { return fmt.Sprintf("http://localhost:%d/ip/%s", port, containerID) } +func allocURL(port int, cidr string, containerID string) string { + return fmt.Sprintf("http://localhost:%d/ip/%s/%s", port, containerID, cidr) +} + func TestHttp(t *testing.T) { var ( containerID = "deadbeef" container2 = "baddf00d" container3 = "b01df00d" - testAddr1 = "10.0.3.9" - netSuffix = "/29" - testCIDR1 = "10.0.3.8" + netSuffix + universe = "10.0.0.0/8" + testAddr1 = "10.0.3.9/29" + testCIDR1 = "10.0.3.8/29" + testCIDR2 = "10.2.0.0/16" + testAddr2 = "10.2.0.1/16" ) - alloc := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", testCIDR1, 1) + alloc := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", universe, 1) port := listenHTTP(alloc) alloc.claimRingForTesting() - // Ask the http server for a new address - cidr1 := HTTPPost(t, allocURL(port, containerID)) - wt.AssertEqualString(t, cidr1, testAddr1+netSuffix, "address") - check := HTTPGet(t, allocURL(port, containerID)) - wt.AssertEqualString(t, check, cidr1, "address") + cidr1a := HTTPPost(t, allocURL(port, testCIDR1, containerID)) + wt.AssertEqualString(t, cidr1a, testAddr1, "address") - // Ask the http server for another address and check it's different - cidr2 := HTTPPost(t, allocURL(port, container2)) - wt.AssertFalse(t, cidr2 == testAddr1+netSuffix, "address") + cidr2a := HTTPPost(t, allocURL(port, testCIDR2, containerID)) + wt.AssertEqualString(t, cidr2a, testAddr2, "address") - // Ask for the first container again and we should get the same address again - cidr1a := HTTPPost(t, allocURL(port, containerID)) - wt.AssertEqualString(t, cidr1a, testAddr1+netSuffix, "address") - // Now free the first one, and we should get it back when we ask - doHTTP("DELETE", allocURL(port, containerID)) - resp, err := doHTTP("GET", allocURL(port, containerID)) - wt.AssertNoErr(t, err) - wt.AssertStatus(t, resp.StatusCode, http.StatusNotFound, "http response") + // Ask the http server for a pair of addresses for another container and check they're different + cidr1b := HTTPPost(t, allocURL(port, testCIDR1, container2)) + wt.AssertFalse(t, cidr1b == testAddr1, "address") + cidr2b := HTTPPost(t, allocURL(port, testCIDR2, container2)) + wt.AssertFalse(t, cidr2b == testAddr2, "address") - cidr3 := HTTPPost(t, allocURL(port, container3)) - wt.AssertEqualString(t, cidr3, testAddr1+netSuffix, "address") + // Now free the first container, and we should get it back when we ask + doHTTP("DELETE", identURL(port, containerID)) + + cidr1c := HTTPPost(t, allocURL(port, testCIDR1, container3)) + wt.AssertEqualString(t, cidr1c, testAddr1, "address") + cidr2c := HTTPPost(t, allocURL(port, testCIDR2, container3)) + wt.AssertEqualString(t, cidr2c, testAddr2, "address") // Would like to shut down the http server at the end of this test // but it's complicated. @@ -115,7 +119,7 @@ func TestBadHttp(t *testing.T) { port := listenHTTP(alloc) alloc.claimRingForTesting() - cidr1 := HTTPPost(t, allocURL(port, containerID)) + cidr1 := HTTPPost(t, allocURL(port, testCIDR1, containerID)) parts := strings.Split(cidr1, "/") testAddr1 := parts[0] // Verb that's not handled @@ -142,7 +146,7 @@ func impTestHTTPCancel(t *testing.T) { common.InitDefaultLogging(true) var ( containerID = "deadbeef" - testCIDR1 = "10.0.3.5/29" + testCIDR1 = "10.0.3.0/29" ) alloc := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", testCIDR1, 1) @@ -155,7 +159,7 @@ func impTestHTTPCancel(t *testing.T) { // Ask the http server for a new address done := make(chan *http.Response) - req, _ := http.NewRequest("POST", allocURL(port, containerID), nil) + req, _ := http.NewRequest("POST", allocURL(port, testCIDR1, containerID), nil) go func() { res, _ := http.DefaultClient.Do(req) done <- res From 0613a066539b9ad9edce8ff4921428967562cd39 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 5 Jun 2015 17:11:28 +0100 Subject: [PATCH 05/34] IPAM: handle multiple CIDRs in weave script --- weave | 57 ++++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 40 insertions(+), 17 deletions(-) diff --git a/weave b/weave index f96e25e4..af4fcfe1 100755 --- a/weave +++ b/weave @@ -33,14 +33,14 @@ usage() { echo "weave launch-proxy [-D ] [-H
] [--with-dns] [--with-ipam]" echo "weave connect [--replace] [ ...]" echo "weave forget ..." - echo "weave run [--with-dns] [ ...] ..." - echo "weave start [ ...] " - echo "weave attach [ ...] " - echo "weave detach [ ...] " + echo "weave run [--with-dns] [[--subnet] ...] ..." + echo "weave start [[--subnet] ...] " + echo "weave attach [[--subnet] ...] " + echo "weave detach [[--subnet] ...] " echo "weave dns-add [ ...] [-h ]" echo "weave dns-remove [ ...] [-h ]" - echo "weave expose [ ...] [-h ]" - echo "weave hide [ ...]" + echo "weave expose [[--subnet] ...] [-h ]" + echo "weave hide [[--subnet] ...]" echo "weave ps [ ...]" echo "weave status" echo "weave proxy-env" @@ -105,7 +105,7 @@ validate_cidr() { collect_cidr_args() { CIDR_ARGS="" CIDR_COUNT=0 - while is_cidr "$1" ; do + while is_cidr "$1" || [ "$1" = "--subnet" ] ; do CIDR_ARGS="$CIDR_ARGS $1" CIDR_COUNT=$((CIDR_COUNT + 1)) shift 1 @@ -751,13 +751,35 @@ ipam_claim() { done } -# If we don't already have any CIDRs passed from the command-line, get -# one from IPAM. $1 is the full container id. -get_ipam_cidr() { - if [ -n "$CIDR_ARGS" ] ; then +# Pass all CIDRs to IPAM so that network addresses can be interpreted +# as subnets to allocate within. $1 is the full container id; following +# args are previously parsed CIDR_ARGS. +ipam_cidrs() { + CONTAINER_ID="$1" + shift 1 + if [ $# -gt 0 ] ; then + CIDR_ARGS="" + while [ $# -gt 0 ] ; do + if [ $# -gt 1 -a "$1" = "--subnet" ] ; then + shift 1 + # (assuming weave is now listening to /ip URLs all the time) + CIDR=$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID/$1) + CIDR_COUNT=$((CIDR_COUNT - 1)) + else + CIDR=$1 + fi + CIDR_ARGS="$CIDR_ARGS $CIDR" + shift 1 + done return 0 fi - CIDR_ARGS="$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$1)" + # If no addresses passed in, get one in the default subnet + CIDR_ARGS="$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID)" + if [ "$CIDR_ARGS" = "No subnet specified" ] ; then + echo "No IP address supplied (use the -iprange and -defaultsubnet options on 'weave launch' to enable IP address allocation)" >&2 + return 1 + fi + # todo: do we still have to handle this error? Considering above assumption? if [ "$CIDR_ARGS" = "404 page not found" ] ; then echo "No IP address supplied (use the -iprange option on 'weave launch' to enable IP address allocation)" >&2 return 1 @@ -1031,7 +1053,7 @@ case "$COMMAND" in shift $CIDR_COUNT CONTAINER=$(docker run $DNS_ARGS -d "$@") create_bridge - get_ipam_cidr $CONTAINER + ipam_cidrs $CONTAINER $CIDR_ARGS with_container_netns_or_die $CONTAINER attach $CIDR_ARGS when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS echo $CONTAINER @@ -1051,7 +1073,7 @@ case "$COMMAND" in RES=$(docker start $1) CONTAINER=$(container_id $1) create_bridge - get_ipam_cidr $CONTAINER + ipam_cidrs $CONTAINER $CIDR_ARGS with_container_netns_or_die $CONTAINER attach $CIDR_ARGS when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS echo $RES @@ -1062,7 +1084,7 @@ case "$COMMAND" in [ $# -eq 1 ] || usage CONTAINER=$(container_id $1) create_bridge - get_ipam_cidr $CONTAINER + ipam_cidrs $CONTAINER $CIDR_ARGS with_container_netns $CONTAINER attach $CIDR_ARGS >/dev/null when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS ;; @@ -1071,6 +1093,7 @@ case "$COMMAND" in shift $CIDR_COUNT [ $# -eq 1 ] || usage CONTAINER=$(container_id $1) + ipam_cidrs $CONTAINER $CIDR_ARGS with_container_netns $CONTAINER detach $CIDR_ARGS >/dev/null when_dns_running with_container_fqdn $CONTAINER delete_dns_fqdn $CIDR_ARGS ;; @@ -1101,7 +1124,7 @@ case "$COMMAND" in expose) collect_cidr_args "$@" shift $CIDR_COUNT - get_ipam_cidr weave:expose + ipam_cidrs weave:expose $CIDR_ARGS if [ $# -eq 0 ] ; then FQDN="" else @@ -1124,7 +1147,7 @@ case "$COMMAND" in hide) collect_cidr_args "$@" shift $CIDR_COUNT - get_ipam_cidr weave:expose + ipam_cidrs weave:expose $CIDR_ARGS create_bridge --without-ethtool for CIDR in $CIDR_ARGS ; do if ip addr show dev $BRIDGE | grep -qF $CIDR ; then From 551394a831ad0ab5edfa4eb69395c7174aea66b0 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 4 Jun 2015 10:29:37 +0100 Subject: [PATCH 06/34] Add subnet info to space requests from one peer to another --- ipam/allocate.go | 4 ++-- ipam/allocator.go | 30 +++++++++++++++++------ ipam/space/space.go | 15 ++++++++---- ipam/space/space_test.go | 51 +++++++++++++++++++++++----------------- 4 files changed, 65 insertions(+), 35 deletions(-) diff --git a/ipam/allocate.go b/ipam/allocate.go index 8fe28ad0..71eb8e82 100644 --- a/ipam/allocate.go +++ b/ipam/allocate.go @@ -50,8 +50,8 @@ func (g *allocate) Try(alloc *Allocator) bool { // out of space if donor, err := alloc.ring.ChoosePeerToAskForSpace(start, end); err == nil { - alloc.debugln("Decided to ask peer", donor, "for space") - alloc.sendRequest(donor, msgSpaceRequest) + alloc.debugln("Decided to ask peer", donor, "for space in subnet", g.subnet) + alloc.sendSpaceRequest(donor, g.subnet) } return false diff --git a/ipam/allocator.go b/ipam/allocator.go index cd714f3e..f30d3949 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -341,7 +341,13 @@ func (alloc *Allocator) OnGossipUnicast(sender router.PeerName, msg []byte) erro switch msg[0] { case msgSpaceRequest: // some other peer asked us for space - alloc.donateSpace(sender) + decoder := gob.NewDecoder(bytes.NewReader(msg[1:])) + var cidr address.CIDR + if err := decoder.Decode(&cidr); err != nil { + resultChan <- err + return + } + alloc.donateSpace(cidr, sender) resultChan <- nil case msgRingUpdate: resultChan <- alloc.update(msg[1:]) @@ -562,8 +568,18 @@ func (alloc *Allocator) propose() { alloc.gossip.GossipBroadcast(alloc.Gossip()) } -func (alloc *Allocator) sendRequest(dest router.PeerName, kind byte) { - msg := router.Concat([]byte{kind}, alloc.encode()) +func (alloc *Allocator) sendSpaceRequest(dest router.PeerName, subnet address.CIDR) { + buf := new(bytes.Buffer) + enc := gob.NewEncoder(buf) + if err := enc.Encode(subnet); err != nil { + panic(err) + } + msg := router.Concat([]byte{msgSpaceRequest}, buf.Bytes()) + alloc.gossip.GossipUnicast(dest, msg) +} + +func (alloc *Allocator) sendRingUpdate(dest router.PeerName) { + msg := router.Concat([]byte{msgRingUpdate}, alloc.encode()) alloc.gossip.GossipUnicast(dest, msg) } @@ -615,18 +631,18 @@ func (alloc *Allocator) update(msg []byte) error { return nil } -func (alloc *Allocator) donateSpace(to router.PeerName) { +func (alloc *Allocator) donateSpace(subnet address.CIDR, to router.PeerName) { // No matter what we do, we'll send a unicast gossip // of our ring back to tha chap who asked for space. // This serves to both tell him of any space we might // have given him, or tell him where he might find some // more. - defer alloc.sendRequest(to, msgRingUpdate) + defer alloc.sendRingUpdate(to) alloc.debugln("Peer", to, "asked me for space") - start, size, ok := alloc.space.Donate() + start, size, ok := alloc.space.Donate(subnet.Start, subnet.End()) if !ok { - free := alloc.space.NumFreeAddresses() + free := alloc.space.NumFreeAddressesInRange(subnet.Start, subnet.End()) common.Assert(free == 0) alloc.debugln("No space to give to peer", to) return diff --git a/ipam/space/space.go b/ipam/space/space.go index c4d8ea5f..2e2bd8d5 100644 --- a/ipam/space/space.go +++ b/ipam/space/space.go @@ -108,12 +108,19 @@ func (s *Space) Free(addr address.Address) error { return nil } -func (s *Space) biggestFreeRange() (int, address.Offset) { +func (s *Space) biggestFreeRange(rangeStart, rangeEnd address.Address) (int, address.Offset) { pos := -1 biggest := address.Offset(0) for i := 0; i < len(s.free); i += 2 { - size := address.Subtract(s.free[i+1], s.free[i]) + start, end := s.free[i], s.free[i+1] + if start < rangeStart { + start = rangeStart + } + if end > rangeEnd { + end = rangeEnd + } + size := address.Subtract(end, start) if size >= biggest { pos = i biggest = size @@ -122,12 +129,12 @@ func (s *Space) biggestFreeRange() (int, address.Offset) { return pos, biggest } -func (s *Space) Donate() (address.Address, address.Offset, bool) { +func (s *Space) Donate(rangeStart, rangeEnd address.Address) (address.Address, address.Offset, bool) { if len(s.free) == 0 { return 0, 0, false } - pos, biggest := s.biggestFreeRange() + pos, biggest := s.biggestFreeRange(rangeStart, rangeEnd) // Donate half of that biggest free range, rounding up so // that the donation can't be empty diff --git a/ipam/space/space_test.go b/ipam/space/space_test.go index 11df7b58..1585a2c5 100644 --- a/ipam/space/space_test.go +++ b/ipam/space/space_test.go @@ -67,19 +67,19 @@ func TestLowlevel(t *testing.T) { wt.AssertErrorInterface(t, s.Free(0), (*error)(nil), "free not allocated") wt.AssertErrorInterface(t, s.Free(100), (*error)(nil), "double free") - start, size, ok := s.Donate() + start, size, ok := s.Donate(0, 1000) wt.AssertTrue(t, ok && start == 125 && size == 25, "donate") // test Donate when addresses are scarce s = New() - start, size, ok = s.Donate() + start, size, ok = s.Donate(0, 1000) wt.AssertTrue(t, !ok, "donate on empty space should fail") s.Add(0, 3) wt.AssertNoErr(t, s.Claim(0)) wt.AssertNoErr(t, s.Claim(2)) - start, size, ok = s.Donate() + start, size, ok = s.Donate(0, 1000) wt.AssertTrue(t, ok && start == 1 && size == 1, "donate") - start, size, ok = s.Donate() + start, size, ok = s.Donate(0, 1000) wt.AssertTrue(t, !ok, "donate should fail") } @@ -90,19 +90,24 @@ func TestSpaceAllocate(t *testing.T) { testAddrx = "10.0.3.19" testAddry = "10.0.9.19" containerID = "deadbeef" + size = 20 + ) + var ( + start = ip(testAddr1) + end = address.Add(ip(testAddr1), 20) ) - space1 := makeSpace(ip(testAddr1), 20) + space1 := makeSpace(start, size) wt.AssertEquals(t, space1.NumFreeAddresses(), address.Offset(20)) space1.assertInvariants() - _, addr1 := space1.Allocate(ip(testAddr2), ip(testAddrx)) - wt.AssertEqualString(t, addr1.String(), testAddr2, "address") + _, addr1 := space1.Allocate(start, end) + wt.AssertEqualString(t, addr1.String(), testAddr1, "address") wt.AssertEquals(t, space1.NumFreeAddresses(), address.Offset(19)) space1.assertInvariants() - _, addr2 := space1.Allocate(ip(testAddr2), ip(testAddrx)) - wt.AssertFalse(t, addr2.String() == testAddr2, "address") + _, addr2 := space1.Allocate(start, end) + wt.AssertFalse(t, addr2.String() == testAddr1, "address") wt.AssertEquals(t, space1.NumFreeAddresses(), address.Offset(18)) wt.AssertEquals(t, space1.NumFreeAddressesInRange(ip(testAddr1), ip(testAddrx)), address.Offset(13)) wt.AssertEquals(t, space1.NumFreeAddressesInRange(ip(testAddr1), ip(testAddry)), address.Offset(18)) @@ -129,7 +134,7 @@ func TestSpaceFree(t *testing.T) { space := makeSpace(ip(testAddr1), 20) // Check we are prepared to give up the entire space - pos, size := space.biggestFreeRange() + pos, size := space.biggestFreeRange(ip(testAddr1), ip(testAddry)) wt.AssertTrue(t, space.free[pos] == ip(testAddr1) && size == 20, "Wrong space") for i := 0; i < 20; i++ { @@ -140,17 +145,17 @@ func TestSpaceFree(t *testing.T) { // Check we are full ok, _ := space.Allocate(ip(testAddr1), ip(testAddry)) wt.AssertTrue(t, !ok, "Should have failed to get address") - _, size, ok = space.Donate() + _, size, ok = space.Donate(ip(testAddr1), ip(testAddry)) wt.AssertTrue(t, size == 0, "Wrong space") // Free in the middle wt.AssertSuccess(t, space.Free(ip("10.0.3.13"))) - pos, size = space.biggestFreeRange() + pos, size = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) wt.AssertTrue(t, space.free[pos] == ip("10.0.3.13") && size == 1, "Wrong space") // Free one at the end wt.AssertSuccess(t, space.Free(ip("10.0.3.23"))) - pos, size = space.biggestFreeRange() + pos, size = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) wt.AssertTrue(t, space.free[pos] == ip("10.0.3.23") && size == 1, "Wrong space") // Now free a few at the end @@ -160,7 +165,7 @@ func TestSpaceFree(t *testing.T) { wt.AssertEquals(t, space.NumFreeAddresses(), address.Offset(4)) // Now get the biggest free space; should be 3.21 - pos, size = space.biggestFreeRange() + pos, size = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) wt.AssertTrue(t, space.free[pos] == ip("10.0.3.21") && size == 3, "Wrong space") // Now free a few in the middle @@ -171,7 +176,7 @@ func TestSpaceFree(t *testing.T) { wt.AssertEquals(t, space.NumFreeAddresses(), address.Offset(7)) // Now get the biggest free space; should be 3.21 - pos, size = space.biggestFreeRange() + pos, size = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) wt.AssertTrue(t, space.free[pos] == ip("10.0.3.10") && size == 4, "Wrong space") wt.AssertEquals(t, space.OwnedRanges(), []address.Range{address.Range{Start: ip("10.0.3.4"), End: ip("10.0.3.24")}}) @@ -185,32 +190,33 @@ func TestDonateSimple(t *testing.T) { const ( testAddr1 = "10.0.1.0" testAddr2 = "10.0.1.32" + size = 48 ) var ( ipAddr1 = ip(testAddr1) ) - ps1 := makeSpace(ipAddr1, 48) + ps1 := makeSpace(ipAddr1, size) // Empty space set should split in two and give me the second half - start, numGivenUp, ok := ps1.Donate() + start, numGivenUp, ok := ps1.Donate(ip(testAddr1), ip(testAddr1)+size) wt.AssertTrue(t, ok, "Donate result") wt.AssertEqualString(t, start.String(), "10.0.1.24", "Invalid start") - wt.AssertEquals(t, numGivenUp, address.Offset(24)) - wt.AssertEquals(t, ps1.NumFreeAddresses(), address.Offset(24)) + wt.AssertEquals(t, numGivenUp, address.Offset(size/2)) + wt.AssertEquals(t, ps1.NumFreeAddresses(), address.Offset(size/2)) // Now check we can give the rest up. count := 0 // count to avoid infinite loop for ; count < 1000; count++ { - _, size, ok := ps1.Donate() + _, size, ok := ps1.Donate(ip(testAddr1), ip(testAddr1)+size) if !ok { break } numGivenUp += size } wt.AssertEquals(t, ps1.NumFreeAddresses(), address.Offset(0)) - wt.AssertEquals(t, numGivenUp, address.Offset(48)) + wt.AssertEquals(t, numGivenUp, address.Offset(size)) } func TestDonateHard(t *testing.T) { @@ -218,6 +224,7 @@ func TestDonateHard(t *testing.T) { var ( start = ip("10.0.1.0") size address.Offset = 48 + end = address.Add(start, size) ) // Fill a fresh space @@ -236,7 +243,7 @@ func TestDonateHard(t *testing.T) { } // Now split - newRange, numGivenUp, ok := spaceset.Donate() + newRange, numGivenUp, ok := spaceset.Donate(start, end) wt.AssertTrue(t, ok, "GiveUpSpace result") wt.AssertEquals(t, newRange, ip("10.0.1.23")) wt.AssertEquals(t, numGivenUp, address.Offset(24)) From d20d2e91428c734694b14f7e83c5de89f868dd26 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 2 Jun 2015 16:57:25 +0100 Subject: [PATCH 07/34] Update smoke tests that use IPAM with default subnet --- test/100_cross_hosts_test.sh | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/test/100_cross_hosts_test.sh b/test/100_cross_hosts_test.sh index 869be0ec..6d2e4875 100755 --- a/test/100_cross_hosts_test.sh +++ b/test/100_cross_hosts_test.sh @@ -4,12 +4,14 @@ C1=10.2.1.4 C2=10.2.1.7 -UNIVERSE=10.2.2.0/24 +UNIVERSE=10.2.0.0/16 +SUBNET_1=10.2.2.0/24 +SUBNET_2=10.2.3.0/24 start_suite "Ping over cross-host weave network (with and without IPAM)" -weave_on $HOST1 launch -iprange $UNIVERSE -weave_on $HOST2 launch -iprange $UNIVERSE $HOST1 +weave_on $HOST1 launch -iprange $UNIVERSE -ipsubnet $SUBNET_1 +weave_on $HOST2 launch -iprange $UNIVERSE -ipsubnet $SUBNET_1 $HOST1 start_container $HOST1 $C1/24 --name=c1 start_container $HOST2 $C2/24 --name=c2 @@ -20,4 +22,9 @@ start_container $HOST2 --name=c4 C4=$(container_ip $HOST2 c4) assert_raises "exec_on $HOST1 c3 $PING $C4" +start_container $HOST1 --subnet $SUBNET_2 --name=c5 +start_container $HOST2 --subnet $SUBNET_2 --name=c6 +C6=$(container_ip $HOST2 c6) +assert_raises "exec_on $HOST1 c5 $PING $C6" + end_suite From a617e7918c9964f321df7c402ec6e344b2fc35b9 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 3 Jun 2015 11:25:53 +0100 Subject: [PATCH 08/34] Introduce single-address Free() and rename the previous one to Delete() --- ipam/allocator.go | 43 +++++++++++++++++++++++++++++------------- ipam/allocator_test.go | 13 +++++-------- ipam/claim.go | 1 + ipam/http.go | 17 ++++++++++++++++- 4 files changed, 52 insertions(+), 22 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index f30d3949..aeef0f73 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -223,18 +223,8 @@ func (alloc *Allocator) Claim(ident string, addr address.Address, cancelChan <-c return <-resultChan } -// Free (Sync) - release IP address for container with given name -func (alloc *Allocator) Free(ident string) error { - return alloc.free(ident) -} - -// ContainerDied is provided to satisfy the updater interface; does a free underneath. Async. -func (alloc *Allocator) ContainerDied(ident string) error { - alloc.debugln("Container", ident, "died; releasing addresses") - return alloc.free(ident) -} - -func (alloc *Allocator) free(ident string) error { +// Delete (Sync) - release all IP addresses for container with given name +func (alloc *Allocator) Delete(ident string) error { errChan := make(chan error) alloc.actionChan <- func() { addrs, found := alloc.owned[ident] @@ -248,7 +238,7 @@ func (alloc *Allocator) free(ident string) error { found = alloc.cancelOpsFor(&alloc.pendingClaims, ident) || found if !found { - errChan <- fmt.Errorf("Free: no addresses for %s", ident) + errChan <- fmt.Errorf("Delete: no addresses for %s", ident) return } errChan <- nil @@ -256,6 +246,33 @@ func (alloc *Allocator) free(ident string) error { return <-errChan } +// Free (Sync) - release single IP address for container +func (alloc *Allocator) Free(ident string, addrToFree address.Address) error { + errChan := make(chan error) + alloc.actionChan <- func() { + addrs := alloc.owned[ident] + for i, ownedAddr := range addrs { + if ownedAddr == addrToFree { + alloc.debugln("Freed", addrToFree, "for", ident) + addrs = append(addrs[:i], addrs[i+1:]...) + alloc.owned[ident] = addrs + alloc.space.Free(addrToFree) + errChan <- nil + return + } + } + + errChan <- fmt.Errorf("Free: address %s not found for %s", addrToFree, ident) + } + return <-errChan +} + +// ContainerDied is provided to satisfy the updater interface; does a 'Delete' underneath. Sync. +func (alloc *Allocator) ContainerDied(ident string) error { + alloc.debugln("Container", ident, "died; releasing addresses") + return alloc.Delete(ident) +} + // Sync. func (alloc *Allocator) String() string { resultChan := make(chan string) diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index 05531b08..ad30d675 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -58,8 +58,8 @@ func TestAllocFree(t *testing.T) { addr2a, _ := alloc.Allocate(container1, cidr2, nil) wt.AssertEqualString(t, addr2a.String(), testAddr2, "address") - // Now free the first container, and we should get its addresses back - wt.AssertSuccess(t, alloc.Free(container1)) + // Now delete the first container, and we should get its addresses back + wt.AssertSuccess(t, alloc.Delete(container1)) addr3, _ := alloc.Allocate(container3, cidr1, nil) wt.AssertEqualString(t, addr3.String(), testAddr1, "address") addr4, _ := alloc.Allocate(container3, cidr2, nil) @@ -141,12 +141,8 @@ func TestAllocatorClaim(t *testing.T) { defer alloc.Stop() alloc.claimRingForTesting() - addr1, _ := alloc.Allocate(container1, alloc.defaultSubnet, nil) - alloc.Allocate(container2, alloc.defaultSubnet, nil) + addr1, _ := address.ParseIP(testAddr1) - // Now free the first one, and try to claim it - wt.AssertSuccess(t, alloc.Free(container1)) - t.Log(alloc) err := alloc.Claim(container3, addr1, nil) wt.AssertNoErr(t, err) // Check we get this address back if we try an allocate @@ -391,7 +387,8 @@ func TestAllocatorFuzz(t *testing.T) { alloc := allocs[res.alloc] //common.Info.Printf("Freeing %s (%s) on allocator %d", res.name, addr, res.alloc) - wt.AssertSuccess(t, alloc.Free(res.name)) + oldAddr, _ := address.ParseIP(addr) + wt.AssertSuccess(t, alloc.Free(res.name, oldAddr)) } // Do a Allocate on an existing container & allocator diff --git a/ipam/claim.go b/ipam/claim.go index 18e84016..f17bd477 100644 --- a/ipam/claim.go +++ b/ipam/claim.go @@ -57,6 +57,7 @@ func (c *claim) Try(alloc *Allocator) bool { c.resultChan <- err return true } + alloc.debugln("Claimed", c.addr, "for", c.ident) alloc.addOwned(c.ident, c.addr) c.resultChan <- nil return true diff --git a/ipam/http.go b/ipam/http.go index 925fe5fd..a851f35d 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -81,9 +81,24 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) { fmt.Fprintf(w, "%s/%d", newAddr.String(), alloc.defaultSubnet.PrefixLen) }) + router.Methods("DELETE").Path("/ip/{id}/{ip}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + vars := mux.Vars(r) + ident := vars["id"] + ipStr := vars["ip"] + if ip, err := address.ParseIP(ipStr); err != nil { + badRequest(w, err) + return + } else if err := alloc.Free(ident, ip); err != nil { + badRequest(w, fmt.Errorf("Unable to free: %s", err)) + return + } + + w.WriteHeader(204) + }) + router.Methods("DELETE").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ident := mux.Vars(r)["id"] - if err := alloc.Free(ident); err != nil { + if err := alloc.Delete(ident); err != nil { badRequest(w, err) return } From d18ccac43bd19977f918c8eae55413aa88d4e1ef Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 3 Jun 2015 11:35:23 +0100 Subject: [PATCH 09/34] Free IPAM addresses individually in 'weave hide', because the user now has the ability to free some subnets and not others --- weave | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/weave b/weave index af4fcfe1..52451017 100755 --- a/weave +++ b/weave @@ -1156,8 +1156,10 @@ case "$COMMAND" in delete_iptables_rule nat WEAVE -s $CIDR ! -d $CIDR -j MASQUERADE when_dns_running delete_dns 'weave:expose' $CIDR fi + # Tell IPAM we don't need this address, but ignore errors because + # the address might not have come from IPAM + http_call $CONTAINER_NAME $HTTP_PORT DELETE /ip/weave:expose/${CIDR%/*} >/dev/null 2>&1 || true done - [ $CIDR_COUNT -eq 0 ] && http_call $CONTAINER_NAME $HTTP_PORT DELETE /ip/weave:expose 2>/dev/null || true ;; stop) [ $# -eq 0 ] || usage From 2a00b0092d826c1a96dc6afdc138b1589a24a267 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 3 Jun 2015 12:10:28 +0100 Subject: [PATCH 10/34] Extend 'Lookup' to multi-subnet --- ipam/allocator.go | 11 ++++++----- ipam/http.go | 18 +++++++++++++++++- ipam/http_test.go | 5 +++++ 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index aeef0f73..0444ff69 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -200,13 +200,14 @@ func (alloc *Allocator) Allocate(ident string, subnet address.CIDR, cancelChan < return result.addr, result.err } -func (alloc *Allocator) Lookup(ident string) (address.Address, error) { +func (alloc *Allocator) Lookup(ident string, subnet address.CIDR) (address.Address, error) { resultChan := make(chan allocateResult) alloc.actionChan <- func() { - addr, found := alloc.owned[ident] - if found { - resultChan <- allocateResult{addr: addr[0]} - return + for _, addr := range alloc.owned[ident] { + if subnet.Contains(addr) { + resultChan <- allocateResult{addr: addr} + return + } } resultChan <- allocateResult{err: fmt.Errorf("lookup: no address found")} } diff --git a/ipam/http.go b/ipam/http.go index a851f35d..35cdb437 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -33,12 +33,28 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) { w.WriteHeader(204) }) + router.Methods("GET").Path("/ip/{id}/{ip}/{prefixlen}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + vars := mux.Vars(r) + cidr := vars["ip"] + "/" + vars["prefixlen"] + _, subnet, err := address.ParseCIDR(cidr) + if err != nil { + badRequest(w, err) + return + } + addr, err := alloc.Lookup(vars["id"], subnet) + if err != nil { + http.NotFound(w, r) + return + } + fmt.Fprintf(w, "%s/%d", addr.String(), subnet.PrefixLen) + }) + router.Methods("GET").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if alloc.defaultSubnet.Blank() { badRequest(w, fmt.Errorf("No subnet specified")) return } - addr, err := alloc.Lookup(mux.Vars(r)["id"]) + addr, err := alloc.Lookup(mux.Vars(r)["id"], alloc.defaultSubnet) if err != nil { http.NotFound(w, r) return diff --git a/ipam/http_test.go b/ipam/http_test.go index c66b2d85..19efaa95 100644 --- a/ipam/http_test.go +++ b/ipam/http_test.go @@ -89,6 +89,11 @@ func TestHttp(t *testing.T) { cidr2a := HTTPPost(t, allocURL(port, testCIDR2, containerID)) wt.AssertEqualString(t, cidr2a, testAddr2, "address") + check := HTTPGet(t, allocURL(port, testCIDR1, containerID)) + wt.AssertEqualString(t, check, cidr1a, "address") + check = HTTPGet(t, allocURL(port, testCIDR2, containerID)) + wt.AssertEqualString(t, check, cidr2a, "address") + // Ask the http server for a pair of addresses for another container and check they're different cidr1b := HTTPPost(t, allocURL(port, testCIDR1, container2)) wt.AssertFalse(t, cidr1b == testAddr1, "address") From 50eaccc60c94d7a59c598da7ff4c9e3d7a211583 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 3 Jun 2015 13:58:17 +0100 Subject: [PATCH 11/34] Allocate() and Lookup() now return a CIDR, to simplify output --- ipam/allocator.go | 8 ++++---- ipam/allocator_test.go | 22 +++++++++++----------- ipam/http.go | 8 ++++---- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index 0444ff69..71fad920 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -191,16 +191,16 @@ func (alloc *Allocator) SetDefaultSubnet(subnet address.CIDR) error { // Allocate (Sync) - get IP address for container with given name // if there isn't any space we block indefinitely -func (alloc *Allocator) Allocate(ident string, subnet address.CIDR, cancelChan <-chan bool) (address.Address, error) { +func (alloc *Allocator) Allocate(ident string, subnet address.CIDR, cancelChan <-chan bool) (address.CIDR, error) { resultChan := make(chan allocateResult) op := &allocate{subnet: subnet, resultChan: resultChan, ident: ident, hasBeenCancelled: hasBeenCancelled(cancelChan)} alloc.doOperation(op, &alloc.pendingAllocates) result := <-resultChan - return result.addr, result.err + return address.CIDR{Start: result.addr, PrefixLen: subnet.PrefixLen}, result.err } -func (alloc *Allocator) Lookup(ident string, subnet address.CIDR) (address.Address, error) { +func (alloc *Allocator) Lookup(ident string, subnet address.CIDR) (address.CIDR, error) { resultChan := make(chan allocateResult) alloc.actionChan <- func() { for _, addr := range alloc.owned[ident] { @@ -212,7 +212,7 @@ func (alloc *Allocator) Lookup(ident string, subnet address.CIDR) (address.Addre resultChan <- allocateResult{err: fmt.Errorf("lookup: no address found")} } result := <-resultChan - return result.addr, result.err + return address.CIDR{Start: result.addr, PrefixLen: subnet.PrefixLen}, result.err } // Claim an address that we think we should own (Sync) diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index ad30d675..bf378ad0 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -27,8 +27,8 @@ func TestAllocFree(t *testing.T) { universe = "10.0.3.0/26" subnet1 = "10.0.3.0/28" subnet2 = "10.0.3.32/28" - testAddr1 = "10.0.3.1" - testAddr2 = "10.0.3.33" + testAddr1 = "10.0.3.1/28" + testAddr2 = "10.0.3.33/28" spaceSize = 62 // 64 IP addresses in /26, minus .0 and .63 ) @@ -134,7 +134,7 @@ func TestAllocatorClaim(t *testing.T) { container2 = "baddf00d" container3 = "b01df00d" universe = "10.0.3.0/30" - testAddr1 = "10.0.3.1" // first address allocated should be .1 because .0 is network addr + testAddr1 = "10.0.3.1/30" // first address allocated should be .1 because .0 is network addr ) alloc := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", universe, 1) @@ -224,15 +224,13 @@ func TestGossipShutdown(t *testing.T) { container1 = "abcdef" container2 = "baddf00d" universe = "10.0.3.0/30" - testAddr1 = "10.0.3.1" // first address allocated should be .1 because .0 is network addr ) alloc := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", universe, 1) defer alloc.Stop() alloc.claimRingForTesting() - addr1, _ := alloc.Allocate(container1, alloc.defaultSubnet, nil) - wt.AssertEqualString(t, addr1.String(), testAddr1, "address") + alloc.Allocate(container1, alloc.defaultSubnet, nil) alloc.Shutdown() @@ -284,8 +282,7 @@ func TestFakeRouterSimple(t *testing.T) { alloc1 := allocs[0] //alloc2 := allocs[1] - addr, _ := alloc1.Allocate("foo", alloc1.defaultSubnet, nil) - println("Got addr", addr) + alloc1.Allocate("foo", alloc1.defaultSubnet, nil) } func TestAllocatorFuzz(t *testing.T) { @@ -387,7 +384,10 @@ func TestAllocatorFuzz(t *testing.T) { alloc := allocs[res.alloc] //common.Info.Printf("Freeing %s (%s) on allocator %d", res.name, addr, res.alloc) - oldAddr, _ := address.ParseIP(addr) + oldAddr, _, err := address.ParseCIDR(addr) + if err != nil { + panic(err) + } wt.AssertSuccess(t, alloc.Free(res.name, oldAddr)) } @@ -406,8 +406,8 @@ func TestAllocatorFuzz(t *testing.T) { //common.Info.Printf("Asking for %s on allocator %d again", addr, res.alloc) newAddr, _ := alloc.Allocate(res.name, alloc.defaultSubnet, nil) - oldAddr, _ := address.ParseIP(addr) - if newAddr != oldAddr { + oldAddr, _, _ := address.ParseCIDR(addr) + if newAddr.Start != oldAddr { panic(fmt.Sprintf("Got different address for repeat request for %s: %s != %s", res.name, newAddr, oldAddr)) } diff --git a/ipam/http.go b/ipam/http.go index 35cdb437..13889cf1 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -46,7 +46,7 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) { http.NotFound(w, r) return } - fmt.Fprintf(w, "%s/%d", addr.String(), subnet.PrefixLen) + fmt.Fprint(w, addr) }) router.Methods("GET").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -59,7 +59,7 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) { http.NotFound(w, r) return } - fmt.Fprintf(w, "%s/%d", addr.String(), alloc.defaultSubnet.PrefixLen) + fmt.Fprint(w, addr) }) router.Methods("POST").Path("/ip/{id}/{ip}/{prefixlen}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -78,7 +78,7 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) { return } - fmt.Fprintf(w, "%s/%d", addr.String(), cidr.PrefixLen) + fmt.Fprint(w, addr) }) router.Methods("POST").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -94,7 +94,7 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) { return } - fmt.Fprintf(w, "%s/%d", newAddr.String(), alloc.defaultSubnet.PrefixLen) + fmt.Fprint(w, newAddr) }) router.Methods("DELETE").Path("/ip/{id}/{ip}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From ecbd514a8372aeff73db594e024a379afe272aed Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 10 Jun 2015 13:34:43 +0100 Subject: [PATCH 12/34] Tighten error checking --- ipam/allocator.go | 5 ++++- ipam/http.go | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index 71fad920..0eef318d 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -182,7 +182,10 @@ func (alloc *Allocator) SetDefaultSubnet(subnet address.CIDR) error { return fmt.Errorf("Allocation range smaller than minimum size 4: %s", subnet) } alloc.actionChan <- func() { - // todo: check default subnet is in range + if !alloc.universe.Overlaps(subnet) { + resultChan <- fmt.Errorf("Default subnet %s out of bounds: %s", subnet, alloc.universe) + return + } alloc.defaultSubnet = subnet resultChan <- nil } diff --git a/ipam/http.go b/ipam/http.go index 13889cf1..e174505e 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -67,11 +67,15 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) { vars := mux.Vars(r) ident := vars["id"] cidrStr := vars["ip"] + "/" + vars["prefixlen"] - _, cidr, err := address.ParseCIDR(cidrStr) + subnetAddr, cidr, err := address.ParseCIDR(cidrStr) if err != nil { badRequest(w, err) return } + if cidr.Start != subnetAddr { + badRequest(w, fmt.Errorf("Invalid subnet %s - bits after network prefix are not all zero", cidrStr)) + return + } addr, err := alloc.Allocate(ident, cidr, closedChan) if err != nil { badRequest(w, fmt.Errorf("Unable to allocate: %s", err)) From f956a76455c05b3b4e2220282aa97dc7d645852b Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 10 Jun 2015 13:34:55 +0100 Subject: [PATCH 13/34] Tighten error checking in weave script - --subnet is used but IPAM is off - arg that comes after --subnet is not a cidr --- weave | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/weave b/weave index 52451017..c137da9c 100755 --- a/weave +++ b/weave @@ -762,8 +762,15 @@ ipam_cidrs() { while [ $# -gt 0 ] ; do if [ $# -gt 1 -a "$1" = "--subnet" ] ; then shift 1 - # (assuming weave is now listening to /ip URLs all the time) CIDR=$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID/$1) + if [ "$CIDR" = "404 page not found" ] ; then + echo "IP address allocation must be enabled to use --subnet (use the -iprange option on 'weave launch')" >&2 + return 1 + fi + if ! is_cidr "$CIDR" ; then + echo "$CIDR" >&2 + return 1 + fi CIDR_COUNT=$((CIDR_COUNT - 1)) else CIDR=$1 @@ -779,7 +786,6 @@ ipam_cidrs() { echo "No IP address supplied (use the -iprange and -defaultsubnet options on 'weave launch' to enable IP address allocation)" >&2 return 1 fi - # todo: do we still have to handle this error? Considering above assumption? if [ "$CIDR_ARGS" = "404 page not found" ] ; then echo "No IP address supplied (use the -iprange option on 'weave launch' to enable IP address allocation)" >&2 return 1 From 3a9af4413d8dbcd2c9a22e7eb7ab42c800f8697a Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 5 Jun 2015 18:23:46 +0100 Subject: [PATCH 14/34] Rationalize ipam arguments - rename -defaultsubnet to -ipsubnet - default subnet defaults to whole -iprange - parse and check inside ipam package - removes some error cases --- ipam/address/address.go | 4 +-- ipam/allocator.go | 61 +++++++++++++++++++++++++---------------- ipam/http.go | 8 ------ ipam/testutils_test.go | 6 ++-- prog/weaver/main.go | 61 ++++++++++++++++++----------------------- weave | 4 --- 6 files changed, 67 insertions(+), 77 deletions(-) diff --git a/ipam/address/address.go b/ipam/address/address.go index b5dbc2cb..67915345 100644 --- a/ipam/address/address.go +++ b/ipam/address/address.go @@ -40,15 +40,13 @@ func ParseCIDR(s string) (Address, CIDR, error) { func (cidr CIDR) Size() Offset { return 1 << uint(32-cidr.PrefixLen) } func (cidr CIDR) End() Address { return Add(cidr.Start, cidr.Size()) } -func (cidr CIDR) Blank() bool { return cidr.Start == 0 && cidr.PrefixLen == 0 } func (cidr CIDR) String() string { return fmt.Sprintf("%s/%d", cidr.Start.String(), cidr.PrefixLen) } func (cidr1 CIDR) Overlaps(cidr2 CIDR) bool { - return cidr1.Start >= cidr2.Start && cidr1.Start < cidr2.End() || - cidr2.Start >= cidr1.Start && cidr2.Start < cidr1.End() + return !(cidr1.Start >= cidr2.End() || cidr1.End() <= cidr2.Start) } func (cidr CIDR) Contains(addr Address) bool { diff --git a/ipam/allocator.go b/ipam/allocator.go index 0eef318d..f54d5a0c 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -21,6 +21,7 @@ const ( msgRingUpdate paxosInterval = time.Second * 5 + minSubnetSize = 4 // first and last addresses are excluded, so 2 would be too small ) // operation represents something which Allocator wants to do, but @@ -60,16 +61,21 @@ type Allocator struct { } // NewAllocator creates and initialises a new Allocator -func NewAllocator(ourName router.PeerName, ourUID router.PeerUID, ourNickname string, universe address.CIDR, quorum uint) *Allocator { - return &Allocator{ - ourName: ourName, - universe: universe, - ring: ring.New(universe.Start, address.Add(universe.Start, universe.Size()), ourName), - owned: make(map[string][]address.Address), - paxos: paxos.NewNode(ourName, ourUID, quorum), - nicknames: map[router.PeerName]string{ourName: ourNickname}, - now: time.Now, +func NewAllocator(ourName router.PeerName, ourUID router.PeerUID, ourNickname string, universeStr string, quorum uint) (*Allocator, error) { + universe, err := parseAndCheckCIDR(universeStr) + if err != nil { + return nil, err } + return &Allocator{ + ourName: ourName, + universe: universe, + defaultSubnet: universe, + ring: ring.New(universe.Start, address.Add(universe.Start, universe.Size()), ourName), + owned: make(map[string][]address.Address), + paxos: paxos.NewNode(ourName, ourUID, quorum), + nicknames: map[router.PeerName]string{ourName: ourNickname}, + now: time.Now, + }, nil } // Start runs the allocator goroutine @@ -176,11 +182,12 @@ func hasBeenCancelled(cancelChan <-chan bool) func() bool { // Actor client API -func (alloc *Allocator) SetDefaultSubnet(subnet address.CIDR) error { - resultChan := make(chan error) - if subnet.Size() < 4 { - return fmt.Errorf("Allocation range smaller than minimum size 4: %s", subnet) +func (alloc *Allocator) SetDefaultSubnet(subnetStr string) error { + subnet, err := parseAndCheckCIDR(subnetStr) + if err != nil { + return err } + resultChan := make(chan error) alloc.actionChan <- func() { if !alloc.universe.Overlaps(subnet) { resultChan <- fmt.Errorf("Default subnet %s out of bounds: %s", subnet, alloc.universe) @@ -227,6 +234,12 @@ func (alloc *Allocator) Claim(ident string, addr address.Address, cancelChan <-c return <-resultChan } +// ContainerDied is provided to satisfy the updater interface; does a 'Delete' underneath. Sync. +func (alloc *Allocator) ContainerDied(ident string) error { + alloc.debugln("Container", ident, "died; releasing addresses") + return alloc.Delete(ident) +} + // Delete (Sync) - release all IP addresses for container with given name func (alloc *Allocator) Delete(ident string) error { errChan := make(chan error) @@ -271,12 +284,6 @@ func (alloc *Allocator) Free(ident string, addrToFree address.Address) error { return <-errChan } -// ContainerDied is provided to satisfy the updater interface; does a 'Delete' underneath. Sync. -func (alloc *Allocator) ContainerDied(ident string) error { - alloc.debugln("Container", ident, "died; releasing addresses") - return alloc.Delete(ident) -} - // Sync. func (alloc *Allocator) String() string { resultChan := make(chan string) @@ -487,12 +494,20 @@ func (alloc *Allocator) actorLoop(actionChan <-chan func()) { // Helper functions +func parseAndCheckCIDR(cidrStr string) (address.CIDR, error) { + _, cidr, err := address.ParseCIDR(cidrStr) + if err != nil { + return address.CIDR{}, err + } + if cidr.Size() < minSubnetSize { + return address.CIDR{}, fmt.Errorf("Allocation range smaller than minimum size %d: %s", minSubnetSize, cidrStr) + } + return cidr, nil +} + func (alloc *Allocator) string() string { var buf bytes.Buffer - fmt.Fprintf(&buf, "Allocator universe %s", alloc.universe) - if !alloc.defaultSubnet.Blank() { - fmt.Fprintf(&buf, " (default subnet %s)", alloc.defaultSubnet) - } + fmt.Fprintf(&buf, "Allocator range %s (default subnet %s)", alloc.universe, alloc.defaultSubnet) if alloc.ring.Empty() { if alloc.paxosTicker != nil { diff --git a/ipam/http.go b/ipam/http.go index e174505e..2026ba1b 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -50,10 +50,6 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) { }) router.Methods("GET").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if alloc.defaultSubnet.Blank() { - badRequest(w, fmt.Errorf("No subnet specified")) - return - } addr, err := alloc.Lookup(mux.Vars(r)["id"], alloc.defaultSubnet) if err != nil { http.NotFound(w, r) @@ -86,10 +82,6 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) { }) router.Methods("POST").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if alloc.defaultSubnet.Blank() { - badRequest(w, fmt.Errorf("No subnet specified")) - return - } closedChan := w.(http.CloseNotifier).CloseNotify() ident := mux.Vars(r)["id"] newAddr, err := alloc.Allocate(ident, alloc.defaultSubnet, closedChan) diff --git a/ipam/testutils_test.go b/ipam/testutils_test.go index f4a79f18..69569b7e 100644 --- a/ipam/testutils_test.go +++ b/ipam/testutils_test.go @@ -134,13 +134,11 @@ func makeAllocator(name string, cidrStr string, quorum uint) *Allocator { panic(err) } - _, cidr, err := address.ParseCIDR(cidrStr) + alloc, err := NewAllocator(peername, router.PeerUID(rand.Int63()), + "nick-"+name, cidrStr, quorum) if err != nil { panic(err) } - alloc := NewAllocator(peername, router.PeerUID(rand.Int63()), - "nick-"+name, cidr, quorum) - alloc.defaultSubnet = cidr return alloc } diff --git a/prog/weaver/main.go b/prog/weaver/main.go index 809ffd49..ee4c403b 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -17,7 +17,6 @@ import ( . "github.com/weaveworks/weave/common" "github.com/weaveworks/weave/common/updater" "github.com/weaveworks/weave/ipam" - "github.com/weaveworks/weave/ipam/address" weavenet "github.com/weaveworks/weave/net" weave "github.com/weaveworks/weave/router" ) @@ -38,24 +37,24 @@ func main() { runtime.GOMAXPROCS(procs) var ( - config weave.Config - justVersion bool - ifaceName string - routerName string - nickName string - password string - wait int - debug bool - pktdebug bool - prof string - bufSzMB int - noDiscovery bool - httpAddr string - iprangeCIDR string - defltSubnet string - peerCount int - apiPath string - peers []string + config weave.Config + justVersion bool + ifaceName string + routerName string + nickName string + password string + wait int + debug bool + pktdebug bool + prof string + bufSzMB int + noDiscovery bool + httpAddr string + iprangeCIDR string + ipsubnetCIDR string + peerCount int + apiPath string + peers []string ) flag.BoolVar(&justVersion, "version", false, "print version and exit") @@ -72,8 +71,8 @@ func main() { flag.BoolVar(&noDiscovery, "nodiscovery", false, "disable peer discovery") flag.IntVar(&bufSzMB, "bufsz", 8, "capture buffer size in MB") flag.StringVar(&httpAddr, "httpaddr", fmt.Sprintf(":%d", weave.HTTPPort), "address to bind HTTP interface to (disabled if blank, absolute path indicates unix domain socket)") - flag.StringVar(&iprangeCIDR, "iprange", "", "IP address range to allocate within, in CIDR notation") - flag.StringVar(&defltSubnet, "defaultsubnet", "", "default subnet for IP address allocation, in CIDR notation") + flag.StringVar(&iprangeCIDR, "iprange", "", "IP address range reserved for automatic allocation, in CIDR notation") + flag.StringVar(&ipsubnetCIDR, "ipsubnet", "", "subnet to allocate within by default, in CIDR notation") flag.IntVar(&peerCount, "initpeercount", 0, "number of peers in network (for IP address allocation)") flag.StringVar(&apiPath, "api", "unix:///var/run/docker.sock", "Path to Docker API socket") flag.Parse() @@ -141,9 +140,9 @@ func main() { var allocator *ipam.Allocator if iprangeCIDR != "" { - allocator = createAllocator(router, apiPath, iprangeCIDR, defltSubnet, determineQuorum(peerCount, peers)) + allocator = createAllocator(router, apiPath, iprangeCIDR, ipsubnetCIDR, determineQuorum(peerCount, peers)) } else if peerCount > 0 { - log.Fatal("-initpeercount flag specified without -iprange or -ipuniverse") + log.Fatal("-initpeercount flag specified without -iprange") } else { router.NewGossip("IPallocation", &ipam.DummyAllocator{}) } @@ -197,23 +196,15 @@ func logFrameFunc(debug bool) weave.LogFrameFunc { } } -func createAllocator(router *weave.Router, apiPath string, ipUniverse string, defaultSubnet string, quorum uint) *ipam.Allocator { - _, universe, err := address.ParseCIDR(ipUniverse) +func createAllocator(router *weave.Router, apiPath string, ipRangeStr string, defaultSubnetStr string, quorum uint) *ipam.Allocator { + allocator, err := ipam.NewAllocator(router.Ourself.Peer.Name, router.Ourself.Peer.UID, router.Ourself.Peer.NickName, ipRangeStr, quorum) if err != nil { log.Fatal(err) } - if universe.Size() < 4 { - log.Fatal("IP allocation range too small") - } - allocator := ipam.NewAllocator(router.Ourself.Peer.Name, router.Ourself.Peer.UID, router.Ourself.Peer.NickName, universe, quorum) allocator.SetInterfaces(router.NewGossip("IPallocation", allocator)) allocator.Start() - if defaultSubnet != "" { - _, subnet, err := address.ParseCIDR(defaultSubnet) - if err != nil { - log.Fatal(err) - } - if err = allocator.SetDefaultSubnet(subnet); err != nil { + if defaultSubnetStr != "" { + if err = allocator.SetDefaultSubnet(defaultSubnetStr); err != nil { log.Fatal(err) } } diff --git a/weave b/weave index c137da9c..9bbd98cc 100755 --- a/weave +++ b/weave @@ -782,10 +782,6 @@ ipam_cidrs() { fi # If no addresses passed in, get one in the default subnet CIDR_ARGS="$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID)" - if [ "$CIDR_ARGS" = "No subnet specified" ] ; then - echo "No IP address supplied (use the -iprange and -defaultsubnet options on 'weave launch' to enable IP address allocation)" >&2 - return 1 - fi if [ "$CIDR_ARGS" = "404 page not found" ] ; then echo "No IP address supplied (use the -iprange option on 'weave launch' to enable IP address allocation)" >&2 return 1 From 2b4eec39cf2213f3522b76493b63b11cb2553264 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 5 Jun 2015 14:46:19 +0100 Subject: [PATCH 15/34] Support '--subnet default' --- test/100_cross_hosts_test.sh | 2 +- weave | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/test/100_cross_hosts_test.sh b/test/100_cross_hosts_test.sh index 6d2e4875..425749e9 100755 --- a/test/100_cross_hosts_test.sh +++ b/test/100_cross_hosts_test.sh @@ -18,7 +18,7 @@ start_container $HOST2 $C2/24 --name=c2 assert_raises "exec_on $HOST1 c1 $PING $C2" start_container $HOST1 --name=c3 -start_container $HOST2 --name=c4 +start_container $HOST2 --subnet default --name=c4 C4=$(container_ip $HOST2 c4) assert_raises "exec_on $HOST1 c3 $PING $C4" diff --git a/weave b/weave index 9bbd98cc..ec7b34f8 100755 --- a/weave +++ b/weave @@ -108,6 +108,12 @@ collect_cidr_args() { while is_cidr "$1" || [ "$1" = "--subnet" ] ; do CIDR_ARGS="$CIDR_ARGS $1" CIDR_COUNT=$((CIDR_COUNT + 1)) + # only accept 'default' after '--subnet', in case someone has a container called 'default' + if [ "$1" = "--subnet" -a "$2" = "default" ] ; then + CIDR_ARGS="$CIDR_ARGS $2" + CIDR_COUNT=$((CIDR_ARG_COUNT + 1)) + shift 1 + fi shift 1 done } @@ -762,7 +768,11 @@ ipam_cidrs() { while [ $# -gt 0 ] ; do if [ $# -gt 1 -a "$1" = "--subnet" ] ; then shift 1 - CIDR=$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID/$1) + if [ "$1" = "default" ] ; then + CIDR=$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID) + else + CIDR=$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID/$1) + fi if [ "$CIDR" = "404 page not found" ] ; then echo "IP address allocation must be enabled to use --subnet (use the -iprange option on 'weave launch')" >&2 return 1 From 3cdc3522be500a4f658134c8479e011b0d494623 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 5 Jun 2015 14:07:30 +0100 Subject: [PATCH 16/34] Don't overwrite CIDR_ARGS; write new variables ALL_CIDRS and IPAM_CIDRS Handle manual CIDRs and IPAM CIDRs separately, to avoid inconsistencies and misleading messages --- weave | 120 ++++++++++++++++++++++++++++++---------------------------- 1 file changed, 62 insertions(+), 58 deletions(-) diff --git a/weave b/weave index ec7b34f8..8aa8c713 100755 --- a/weave +++ b/weave @@ -104,14 +104,14 @@ validate_cidr() { collect_cidr_args() { CIDR_ARGS="" - CIDR_COUNT=0 + CIDR_ARG_COUNT=0 while is_cidr "$1" || [ "$1" = "--subnet" ] ; do CIDR_ARGS="$CIDR_ARGS $1" - CIDR_COUNT=$((CIDR_COUNT + 1)) + CIDR_ARG_COUNT=$((CIDR_ARG_COUNT + 1)) # only accept 'default' after '--subnet', in case someone has a container called 'default' if [ "$1" = "--subnet" -a "$2" = "default" ] ; then CIDR_ARGS="$CIDR_ARGS $2" - CIDR_COUNT=$((CIDR_ARG_COUNT + 1)) + CIDR_ARG_COUNT=$((CIDR_ARG_COUNT + 1)) shift 1 fi shift 1 @@ -155,7 +155,7 @@ if [ "$1" = "run" ] ; then DNS_ARGS=$(exec_remote dns-args "$@") fi collect_cidr_args "$@" - shift $CIDR_COUNT + shift $CIDR_ARG_COUNT CONTAINER=$(docker $DOCKER_CLIENT_ARGS run $DNS_ARGS -d "$@") exec_remote attach $CIDR_ARGS $CONTAINER echo $CONTAINER @@ -757,45 +757,46 @@ ipam_claim() { done } -# Pass all CIDRs to IPAM so that network addresses can be interpreted -# as subnets to allocate within. $1 is the full container id; following -# args are previously parsed CIDR_ARGS. +# Call IPAM as necessary to allocate addresses +# $1 is the full container id and following args are previously parsed CIDR_ARGS. +# Returns ALL_CIDRS and IPAM_CIDRS ipam_cidrs() { CONTAINER_ID="$1" shift 1 - if [ $# -gt 0 ] ; then - CIDR_ARGS="" - while [ $# -gt 0 ] ; do - if [ $# -gt 1 -a "$1" = "--subnet" ] ; then - shift 1 - if [ "$1" = "default" ] ; then - CIDR=$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID) - else - CIDR=$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID/$1) - fi - if [ "$CIDR" = "404 page not found" ] ; then - echo "IP address allocation must be enabled to use --subnet (use the -iprange option on 'weave launch')" >&2 - return 1 - fi - if ! is_cidr "$CIDR" ; then - echo "$CIDR" >&2 - return 1 - fi - CIDR_COUNT=$((CIDR_COUNT - 1)) - else - CIDR=$1 - fi - CIDR_ARGS="$CIDR_ARGS $CIDR" + ALL_CIDRS="" + IPAM_CIDRs="" + if [ $# -eq 0 ] ; then + # If no addresses passed in, get one in the default subnet + IPAM_CIDRS="$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID)" + if [ "$IPAM_CIDRS" = "404 page not found" ] ; then + echo "No IP address supplied (use the -iprange option on 'weave launch' to enable IP address allocation)" >&2 + return 1 + fi + ALL_CIDRS="$IPAM_CIDRS" + fi + while [ $# -gt 0 ] ; do + if [ $# -gt 1 -a "$1" = "--subnet" ] ; then shift 1 - done - return 0 - fi - # If no addresses passed in, get one in the default subnet - CIDR_ARGS="$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID)" - if [ "$CIDR_ARGS" = "404 page not found" ] ; then - echo "No IP address supplied (use the -iprange option on 'weave launch' to enable IP address allocation)" >&2 - return 1 - fi + if [ "$1" = "default" ] ; then + CIDR=$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID) + else + CIDR=$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID/$1) + fi + if [ "$CIDR" = "404 page not found" ] ; then + echo "IP address allocation must be enabled to use --subnet (use the -iprange option on 'weave launch')" >&2 + return 1 + fi + if ! is_cidr "$CIDR" ; then + echo "$CIDR" >&2 + return 1 + fi + IPAM_CIDRS="$IPAM_CIDRS $CIDR" + ALL_CIDRS="$ALL_CIDRS $CIDR" + else + ALL_CIDRS="$ALL_CIDRS $1" + fi + shift 1 + done } ###################################################################### @@ -1062,12 +1063,12 @@ case "$COMMAND" in dns_args "$@" fi collect_cidr_args "$@" - shift $CIDR_COUNT + shift $CIDR_ARG_COUNT CONTAINER=$(docker run $DNS_ARGS -d "$@") create_bridge ipam_cidrs $CONTAINER $CIDR_ARGS - with_container_netns_or_die $CONTAINER attach $CIDR_ARGS - when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS + with_container_netns_or_die $CONTAINER attach $ALL_CIDRS + when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $ALL_CIDRS echo $CONTAINER ;; dns-args) @@ -1080,34 +1081,37 @@ case "$COMMAND" in ;; start) collect_cidr_args "$@" - shift $CIDR_COUNT + shift $CIDR_ARG_COUNT [ $# -eq 1 ] || usage RES=$(docker start $1) CONTAINER=$(container_id $1) create_bridge ipam_cidrs $CONTAINER $CIDR_ARGS - with_container_netns_or_die $CONTAINER attach $CIDR_ARGS - when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS + with_container_netns_or_die $CONTAINER attach $ALL_CIDRS + when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $ALL_CIDRS echo $RES ;; attach) collect_cidr_args "$@" - shift $CIDR_COUNT + shift $CIDR_ARG_COUNT [ $# -eq 1 ] || usage CONTAINER=$(container_id $1) create_bridge ipam_cidrs $CONTAINER $CIDR_ARGS - with_container_netns $CONTAINER attach $CIDR_ARGS >/dev/null - when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS + with_container_netns $CONTAINER attach $ALL_CIDRS >/dev/null + when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $ALL_CIDRS ;; detach) collect_cidr_args "$@" - shift $CIDR_COUNT + shift $CIDR_ARG_COUNT [ $# -eq 1 ] || usage CONTAINER=$(container_id $1) ipam_cidrs $CONTAINER $CIDR_ARGS - with_container_netns $CONTAINER detach $CIDR_ARGS >/dev/null - when_dns_running with_container_fqdn $CONTAINER delete_dns_fqdn $CIDR_ARGS + with_container_netns $CONTAINER detach $ALL_CIDRS >/dev/null + when_dns_running with_container_fqdn $CONTAINER delete_dns_fqdn $ALL_CIDRS + for CIDR in $IPAM_CIDRS ; do + http_call $CONTAINER_NAME $HTTP_PORT DELETE /ip/$CONTAINER/${CIDR%/*} + done ;; dns-add) collect_ip_args "$@" @@ -1135,7 +1139,7 @@ case "$COMMAND" in ;; expose) collect_cidr_args "$@" - shift $CIDR_COUNT + shift $CIDR_ARG_COUNT ipam_cidrs weave:expose $CIDR_ARGS if [ $# -eq 0 ] ; then FQDN="" @@ -1144,7 +1148,7 @@ case "$COMMAND" in FQDN="$2" fi create_bridge --without-ethtool - for CIDR in $CIDR_ARGS ; do + for CIDR in $ALL_CIDRS ; do if ! ip addr show dev $BRIDGE | grep -qF $CIDR ; then ip addr add dev $BRIDGE $CIDR arp_update $BRIDGE $CIDR @@ -1158,19 +1162,19 @@ case "$COMMAND" in ;; hide) collect_cidr_args "$@" - shift $CIDR_COUNT + shift $CIDR_ARG_COUNT ipam_cidrs weave:expose $CIDR_ARGS create_bridge --without-ethtool - for CIDR in $CIDR_ARGS ; do + for CIDR in $ALL_CIDRS ; do if ip addr show dev $BRIDGE | grep -qF $CIDR ; then ip addr del dev $BRIDGE $CIDR delete_iptables_rule nat WEAVE -d $CIDR ! -s $CIDR -j MASQUERADE delete_iptables_rule nat WEAVE -s $CIDR ! -d $CIDR -j MASQUERADE when_dns_running delete_dns 'weave:expose' $CIDR fi - # Tell IPAM we don't need this address, but ignore errors because - # the address might not have come from IPAM - http_call $CONTAINER_NAME $HTTP_PORT DELETE /ip/weave:expose/${CIDR%/*} >/dev/null 2>&1 || true + done + for CIDR in $IPAM_CIDRS ; do + http_call $CONTAINER_NAME $HTTP_PORT DELETE /ip/weave:expose/${CIDR%/*} done ;; stop) From e5113889f44580d16437d2c03c9702302687bfe8 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Fri, 5 Jun 2015 15:23:01 +0100 Subject: [PATCH 17/34] Move NumFreeAddresses() into test --- ipam/space/space.go | 8 -------- ipam/space/space_test.go | 10 ++++++++++ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/ipam/space/space.go b/ipam/space/space.go index 2e2bd8d5..549424fd 100644 --- a/ipam/space/space.go +++ b/ipam/space/space.go @@ -69,14 +69,6 @@ func (s *Space) Claim(addr address.Address) error { return nil } -func (s *Space) NumFreeAddresses() address.Offset { - res := address.Offset(0) - for i := 0; i < len(s.free); i += 2 { - res += address.Subtract(s.free[i+1], s.free[i]) - } - return res -} - func (s *Space) NumFreeAddressesInRange(start, end address.Address) address.Offset { res := address.Offset(0) for i := 0; i < len(s.free); i += 2 { diff --git a/ipam/space/space_test.go b/ipam/space/space_test.go index 1585a2c5..12ede1e4 100644 --- a/ipam/space/space_test.go +++ b/ipam/space/space_test.go @@ -19,6 +19,16 @@ func ip(s string) address.Address { return addr } +// Helper function to avoid 'NumFreeAddressesInRange(start, end)' +// dozens of times in tests +func (s *Space) NumFreeAddresses() address.Offset { + res := address.Offset(0) + for i := 0; i < len(s.free); i += 2 { + res += address.Subtract(s.free[i+1], s.free[i]) + } + return res +} + func TestLowlevel(t *testing.T) { a := []address.Address{} a = add(a, 100, 200) From 7e7a2227a0d28155a9248bd26f1bb027094b0d37 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Thu, 4 Jun 2015 15:47:17 +0100 Subject: [PATCH 18/34] Document multi-subnet IPAM --- docs/ipam.md | 13 +++++++++---- site/ipam.md | 39 +++++++++++++++++++++++++++++++++------ 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/docs/ipam.md b/docs/ipam.md index 706ad842..35bad67e 100644 --- a/docs/ipam.md +++ b/docs/ipam.md @@ -20,7 +20,7 @@ plugin) uses to request IP addresses. The commands supported by the allocator via the http interface are: -- Allocate: request one IP address for a container +- Allocate: request one IP address for a container in a subnet - Free: return an IP address that is currently allocated - Claim: request a specific IP address for a container (e.g. because it is already using that address) @@ -46,18 +46,23 @@ freed. ### The Allocation Process +Each allocation request either specifies a subnet to allocate within, +or if no subnet is specified this is taken as a request to allocate in +a pre-defined default subnet. + When a peer owns some range(s), it can allocate freely from within those ranges to containers on the same machine. If it runs out of -space (all owned ranges are full), it will ask another peer for space: +space in a subnet (all owned ranges are full), it will ask another +peer for space: - it picks a peer to ask at random, weighted by the amount of space - owned by each peer + owned by each peer in the subnet - if the target peer decides to give up space, it unicasts a message back to the asker with the newly-updated ring. - if the target peer has no space, it unicasts a message back to the asker with its current copy of the ring, on the basis that the requestor must have acted on out-of-date information. - it will continue to ask for space until it receives some, or its - copy of the ring tells it all peers are full. + copy of the ring tells it all peers are full in that subnet. ### Claiming an address diff --git a/site/ipam.md b/site/ipam.md index 836e980f..b29cdca4 100644 --- a/site/ipam.md +++ b/site/ipam.md @@ -9,10 +9,10 @@ Weave can automatically assign unique IP addresses to each container across the network. To make this work, weave must be told on startup what range of addresses to allocate from, for example: - host1# weave launch -iprange 10.2.3.0/24 + host1# weave launch -iprange 10.2.0.0/16 -The `run`, `start`, `attach`, `expose` and `hide` commands will then -fetch an address automatically if none is specified, i.e.: +The `run`, `start`, `attach`, `detach`, `expose` and `hide` commands +will then fetch an address automatically if none is specified, i.e.: host1# C=$(weave run -ti ubuntu) @@ -22,7 +22,35 @@ You can see which address was allocated with host1# weave ps $C a7aff7249393 7a:51:d1:09:21:78 10.2.3.1/24 -The `-iprange` parameter is given in [CIDR +You may wish to [run containers on different +subnets](features.html#application-isolation). To set a default +subnet, launch with `-ipsubnet`: + + host1# weave launch -iprange 10.2.0.0/16 -ipsubnet 10.2.3.0/24 + +`-iprange` should cover the entire range that you will ever use for +allocation, and `-ipsubnet` is the subnet that will be used when +you don't explicitly specify one. + +Then, to request the allocation of an address from a subnet other than +the default, use `--subnet`: + + host1# C=$(weave run --subnet 10.2.7.0/24 -ti ubuntu) + +To explicitly request the default subnet, use `--subnet default`. + +Note that `-iprange` can be smaller than `-ipsubnet`, if you want to +use a mixture of automatically-allocated addresses and manually-chosen +addresses, and have the containers communicate with each other. For +example, if you say: + + host1# weave launch -iprange 10.9.0.0/17 -ipsubnet 10.9.0.0/16 + +then you can run all containers in the 10.9.0.0/16 subnet, with IPAM +using the lower half and leaving the upper half free for manual +allocation. + +The subnet parameters are written in [CIDR notation](http://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing) format - in this example "/24" means the first 24 bits of the address form the network address and the allocator is to allocate container @@ -117,8 +145,7 @@ reports on the current status of the weave router and IP allocator: ```` weave router git-8f675f15c0b5 ... -Allocator subnet 10.2.1.0/24 - Free IPs: ~98.0%, 62 local, ~189 remote +Allocator universe 10.2.0.0/16 (default subnet 10.2.1.0/24) Owned Ranges: 10.2.1.1 -> 96:e9:e2:2e:2d:bc (host1) (v3) 10.2.1.128 -> ea:84:25:9b:31:2e (host2) (v3) From 6536034ab6983cfa4e20414af01664a13da0f9e5 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 8 Jun 2015 11:36:00 +0100 Subject: [PATCH 19/34] Move default subnet outside of Allocator --- docs/ipam.md | 9 ++++--- ipam/allocator.go | 56 ++++++++-------------------------------- ipam/allocator_test.go | 58 +++++++++++++++++++++--------------------- ipam/http.go | 6 ++--- ipam/http_test.go | 17 +++++++------ ipam/testutils_test.go | 28 +++++++++++--------- prog/weaver/main.go | 39 +++++++++++++++++++--------- site/ipam.md | 3 ++- 8 files changed, 102 insertions(+), 114 deletions(-) diff --git a/docs/ipam.md b/docs/ipam.md index 35bad67e..d8fcb1e3 100644 --- a/docs/ipam.md +++ b/docs/ipam.md @@ -21,10 +21,15 @@ plugin) uses to request IP addresses. The commands supported by the allocator via the http interface are: - Allocate: request one IP address for a container in a subnet +- Lookup: fetch the previously-allocated IP address for a container in a subnet - Free: return an IP address that is currently allocated - Claim: request a specific IP address for a container (e.g. because it is already using that address) +Each http request either specifies a subnet, or if no subnet is +specified this is taken as a request to allocate in a pre-defined +default subnet. + The allocator also watches via the Docker event mechanism: if a container dies then all IP addresses allocated to that container are freed. @@ -46,10 +51,6 @@ freed. ### The Allocation Process -Each allocation request either specifies a subnet to allocate within, -or if no subnet is specified this is taken as a request to allocate in -a pre-defined default subnet. - When a peer owns some range(s), it can allocate freely from within those ranges to containers on the same machine. If it runs out of space in a subnet (all owned ranges are full), it will ask another diff --git a/ipam/allocator.go b/ipam/allocator.go index f54d5a0c..0949658a 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -21,7 +21,7 @@ const ( msgRingUpdate paxosInterval = time.Second * 5 - minSubnetSize = 4 // first and last addresses are excluded, so 2 would be too small + MinSubnetSize = 4 // first and last addresses are excluded, so 2 would be too small ) // operation represents something which Allocator wants to do, but @@ -45,7 +45,6 @@ type operation interface { type Allocator struct { actionChan chan<- func() ourName router.PeerName - defaultSubnet address.CIDR universe address.CIDR // superset of all ranges ring *ring.Ring // information on ranges owned by all peers space space.Space // more detail on ranges owned by us @@ -61,21 +60,16 @@ type Allocator struct { } // NewAllocator creates and initialises a new Allocator -func NewAllocator(ourName router.PeerName, ourUID router.PeerUID, ourNickname string, universeStr string, quorum uint) (*Allocator, error) { - universe, err := parseAndCheckCIDR(universeStr) - if err != nil { - return nil, err - } +func NewAllocator(ourName router.PeerName, ourUID router.PeerUID, ourNickname string, universe address.CIDR, quorum uint) *Allocator { return &Allocator{ - ourName: ourName, - universe: universe, - defaultSubnet: universe, - ring: ring.New(universe.Start, address.Add(universe.Start, universe.Size()), ourName), - owned: make(map[string][]address.Address), - paxos: paxos.NewNode(ourName, ourUID, quorum), - nicknames: map[router.PeerName]string{ourName: ourNickname}, - now: time.Now, - }, nil + ourName: ourName, + universe: universe, + ring: ring.New(universe.Start, address.Add(universe.Start, universe.Size()), ourName), + owned: make(map[string][]address.Address), + paxos: paxos.NewNode(ourName, ourUID, quorum), + nicknames: map[router.PeerName]string{ourName: ourNickname}, + now: time.Now, + } } // Start runs the allocator goroutine @@ -182,23 +176,6 @@ func hasBeenCancelled(cancelChan <-chan bool) func() bool { // Actor client API -func (alloc *Allocator) SetDefaultSubnet(subnetStr string) error { - subnet, err := parseAndCheckCIDR(subnetStr) - if err != nil { - return err - } - resultChan := make(chan error) - alloc.actionChan <- func() { - if !alloc.universe.Overlaps(subnet) { - resultChan <- fmt.Errorf("Default subnet %s out of bounds: %s", subnet, alloc.universe) - return - } - alloc.defaultSubnet = subnet - resultChan <- nil - } - return <-resultChan -} - // Allocate (Sync) - get IP address for container with given name // if there isn't any space we block indefinitely func (alloc *Allocator) Allocate(ident string, subnet address.CIDR, cancelChan <-chan bool) (address.CIDR, error) { @@ -494,20 +471,9 @@ func (alloc *Allocator) actorLoop(actionChan <-chan func()) { // Helper functions -func parseAndCheckCIDR(cidrStr string) (address.CIDR, error) { - _, cidr, err := address.ParseCIDR(cidrStr) - if err != nil { - return address.CIDR{}, err - } - if cidr.Size() < minSubnetSize { - return address.CIDR{}, fmt.Errorf("Allocation range smaller than minimum size %d: %s", minSubnetSize, cidrStr) - } - return cidr, nil -} - func (alloc *Allocator) string() string { var buf bytes.Buffer - fmt.Fprintf(&buf, "Allocator range %s (default subnet %s)", alloc.universe, alloc.defaultSubnet) + fmt.Fprintf(&buf, "Allocator range %s", alloc.universe) if alloc.ring.Empty() { if alloc.paxosTicker != nil { diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index bf378ad0..d8473c0d 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -32,7 +32,7 @@ func TestAllocFree(t *testing.T) { spaceSize = 62 // 64 IP addresses in /26, minus .0 and .63 ) - alloc := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", universe, 1) + alloc, subnet := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", universe, 1) defer alloc.Stop() _, cidr1, _ := address.ParseCIDR(subnet1) _, cidr2, _ := address.ParseCIDR(subnet2) @@ -67,7 +67,7 @@ func TestAllocFree(t *testing.T) { alloc.ContainerDied(container2) alloc.ContainerDied(container3) - wt.AssertEquals(t, alloc.NumFreeAddresses(), address.Offset(spaceSize)) + wt.AssertEquals(t, alloc.NumFreeAddresses(subnet), address.Offset(spaceSize)) } func TestBootstrap(t *testing.T) { @@ -79,11 +79,11 @@ func TestBootstrap(t *testing.T) { peerNameString = "02:00:00:02:00:00" ) - alloc1 := makeAllocatorWithMockGossip(t, ourNameString, testStart1+"/22", 2) + alloc1, subnet := makeAllocatorWithMockGossip(t, ourNameString, testStart1+"/22", 2) defer alloc1.Stop() // Simulate another peer on the gossip network - alloc2 := makeAllocatorWithMockGossip(t, peerNameString, testStart1+"/22", 2) + alloc2, _ := makeAllocatorWithMockGossip(t, peerNameString, testStart1+"/22", 2) defer alloc2.Stop() alloc1.OnGossipBroadcast(alloc2.Encode()) @@ -93,7 +93,7 @@ func TestBootstrap(t *testing.T) { ExpectBroadcastMessage(alloc1, nil) // alloc1 will try to form consensus done := make(chan bool) go func() { - alloc1.Allocate("somecontainer", alloc1.defaultSubnet, nil) + alloc1.Allocate("somecontainer", subnet, nil) done <- true }() time.Sleep(100 * time.Millisecond) @@ -137,7 +137,7 @@ func TestAllocatorClaim(t *testing.T) { testAddr1 = "10.0.3.1/30" // first address allocated should be .1 because .0 is network addr ) - alloc := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", universe, 1) + alloc, subnet := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", universe, 1) defer alloc.Stop() alloc.claimRingForTesting() @@ -146,7 +146,7 @@ func TestAllocatorClaim(t *testing.T) { err := alloc.Claim(container3, addr1, nil) wt.AssertNoErr(t, err) // Check we get this address back if we try an allocate - addr3, _ := alloc.Allocate(container3, alloc.defaultSubnet, nil) + addr3, _ := alloc.Allocate(container3, subnet, nil) wt.AssertEqualString(t, addr3.String(), testAddr1, "address") } @@ -170,10 +170,10 @@ func TestCancel(t *testing.T) { router := TestGossipRouter{make(map[router.PeerName]chan gossipMessage), 0.0} - alloc1 := makeAllocator("01:00:00:02:00:00", CIDR, 2) + alloc1, subnet := makeAllocator("01:00:00:02:00:00", CIDR, 2) alloc1.SetInterfaces(router.connect(alloc1.ourName, alloc1)) - alloc2 := makeAllocator("02:00:00:02:00:00", CIDR, 2) + alloc2, subnet := makeAllocator("02:00:00:02:00:00", CIDR, 2) alloc2.SetInterfaces(router.connect(alloc2.ourName, alloc2)) alloc1.claimRingForTesting(alloc1, alloc2) alloc2.claimRingForTesting(alloc1, alloc2) @@ -185,9 +185,9 @@ func TestCancel(t *testing.T) { alloc1.OnGossipBroadcast(alloc2.Encode()) // Get some IPs, so each allocator has some space - res1, _ := alloc1.Allocate("foo", alloc1.defaultSubnet, nil) + res1, _ := alloc1.Allocate("foo", subnet, nil) common.Debug.Printf("res1 = %s", res1.String()) - res2, _ := alloc2.Allocate("bar", alloc2.defaultSubnet, nil) + res2, _ := alloc2.Allocate("bar", subnet, nil) common.Debug.Printf("res2 = %s", res2.String()) if res1 == res2 { wt.Fatalf(t, "Error: got same ips!") @@ -198,13 +198,13 @@ 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() > 0; i++ { - alloc1.Allocate(fmt.Sprintf("tmp%d", i), alloc1.defaultSubnet, nil) + for i := 0; alloc1.NumFreeAddresses(subnet) > 0; i++ { + alloc1.Allocate(fmt.Sprintf("tmp%d", i), subnet, nil) } cancelChan := make(chan bool, 1) doneChan := make(chan bool) go func() { - _, ok := alloc1.Allocate("baz", alloc1.defaultSubnet, cancelChan) + _, ok := alloc1.Allocate("baz", subnet, cancelChan) doneChan <- ok == nil }() @@ -226,15 +226,15 @@ func TestGossipShutdown(t *testing.T) { universe = "10.0.3.0/30" ) - alloc := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", universe, 1) + alloc, subnet := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", universe, 1) defer alloc.Stop() alloc.claimRingForTesting() - alloc.Allocate(container1, alloc.defaultSubnet, nil) + alloc.Allocate(container1, subnet, nil) alloc.Shutdown() - _, err := alloc.Allocate(container2, alloc.defaultSubnet, nil) // trying to allocate after shutdown should fail + _, err := alloc.Allocate(container2, subnet, nil) // trying to allocate after shutdown should fail wt.AssertFalse(t, err == nil, "no address") CheckAllExpectedMessagesSent(alloc) @@ -244,15 +244,15 @@ func TestTransfer(t *testing.T) { const ( cidr = "10.0.1.7/22" ) - allocs, router := makeNetworkOfAllocators(3, cidr) + allocs, router, subnet := makeNetworkOfAllocators(3, cidr) alloc1 := allocs[0] alloc2 := allocs[1] alloc3 := allocs[2] // This will be 'master' and get the first range - _, err := alloc2.Allocate("foo", alloc2.defaultSubnet, nil) + _, err := alloc2.Allocate("foo", subnet, nil) wt.AssertTrue(t, err == nil, "Failed to get address") - _, err = alloc3.Allocate("bar", alloc3.defaultSubnet, nil) + _, err = alloc3.Allocate("bar", subnet, nil) wt.AssertTrue(t, err == nil, "Failed to get address") router.GossipBroadcast(alloc2.Gossip()) @@ -264,9 +264,9 @@ func TestTransfer(t *testing.T) { wt.AssertSuccess(t, alloc1.AdminTakeoverRanges(alloc2.ourName.String())) wt.AssertSuccess(t, alloc1.AdminTakeoverRanges(alloc3.ourName.String())) - wt.AssertEquals(t, alloc1.NumFreeAddresses(), address.Offset(1022)) + wt.AssertEquals(t, alloc1.NumFreeAddresses(subnet), address.Offset(1022)) - _, err = alloc1.Allocate("foo", alloc1.defaultSubnet, nil) + _, err = alloc1.Allocate("foo", subnet, nil) wt.AssertTrue(t, err == nil, "Failed to get address") alloc1.Stop() } @@ -276,13 +276,13 @@ func TestFakeRouterSimple(t *testing.T) { const ( cidr = "10.0.1.7/22" ) - allocs, _ := makeNetworkOfAllocators(2, cidr) + allocs, _, subnet := makeNetworkOfAllocators(2, cidr) defer stopNetworkOfAllocators(allocs) alloc1 := allocs[0] //alloc2 := allocs[1] - alloc1.Allocate("foo", alloc1.defaultSubnet, nil) + alloc1.Allocate("foo", subnet, nil) } func TestAllocatorFuzz(t *testing.T) { @@ -295,7 +295,7 @@ func TestAllocatorFuzz(t *testing.T) { concurrency = 10 cidr = "10.0.1.7/22" ) - allocs, _ := makeNetworkOfAllocators(nodes, cidr) + allocs, _, subnet := makeNetworkOfAllocators(nodes, cidr) defer stopNetworkOfAllocators(allocs) // Test state @@ -339,7 +339,7 @@ func TestAllocatorFuzz(t *testing.T) { allocIndex := rand.Int31n(nodes) alloc := allocs[allocIndex] //common.Info.Printf("Allocate: asking allocator %d", allocIndex) - addr, err := alloc.Allocate(name, alloc.defaultSubnet, nil) + addr, err := alloc.Allocate(name, subnet, nil) if err != nil { panic(fmt.Sprintf("Could not allocate addr")) @@ -405,7 +405,7 @@ func TestAllocatorFuzz(t *testing.T) { //common.Info.Printf("Asking for %s on allocator %d again", addr, res.alloc) - newAddr, _ := alloc.Allocate(res.name, alloc.defaultSubnet, nil) + newAddr, _ := alloc.Allocate(res.name, subnet, nil) oldAddr, _, _ := address.ParseCIDR(addr) if newAddr.Start != oldAddr { panic(fmt.Sprintf("Got different address for repeat request for %s: %s != %s", res.name, newAddr, oldAddr)) @@ -464,9 +464,9 @@ func TestAllocatorFuzz(t *testing.T) { } func TestGossipSkew(t *testing.T) { - alloc1 := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", "10.0.1.0/22", 2) + alloc1, _ := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", "10.0.1.0/22", 2) defer alloc1.Stop() - alloc2 := makeAllocatorWithMockGossip(t, "02:00:00:02:00:00", "10.0.1.0/22", 2) + alloc2, _ := makeAllocatorWithMockGossip(t, "02:00:00:02:00:00", "10.0.1.0/22", 2) alloc2.now = func() time.Time { return time.Now().Add(time.Hour * 2) } defer alloc2.Stop() diff --git a/ipam/http.go b/ipam/http.go index 2026ba1b..cdb858a0 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -16,7 +16,7 @@ func badRequest(w http.ResponseWriter, err error) { } // HandleHTTP wires up ipams HTTP endpoints to the provided mux. -func (alloc *Allocator) HandleHTTP(router *mux.Router) { +func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CIDR) { router.Methods("PUT").Path("/ip/{id}/{ip}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { closedChan := w.(http.CloseNotifier).CloseNotify() vars := mux.Vars(r) @@ -50,7 +50,7 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) { }) router.Methods("GET").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - addr, err := alloc.Lookup(mux.Vars(r)["id"], alloc.defaultSubnet) + addr, err := alloc.Lookup(mux.Vars(r)["id"], defaultSubnet) if err != nil { http.NotFound(w, r) return @@ -84,7 +84,7 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router) { router.Methods("POST").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { closedChan := w.(http.CloseNotifier).CloseNotify() ident := mux.Vars(r)["id"] - newAddr, err := alloc.Allocate(ident, alloc.defaultSubnet, closedChan) + newAddr, err := alloc.Allocate(ident, defaultSubnet, closedChan) if err != nil { badRequest(w, err) return diff --git a/ipam/http_test.go b/ipam/http_test.go index 19efaa95..68578bf9 100644 --- a/ipam/http_test.go +++ b/ipam/http_test.go @@ -13,6 +13,7 @@ import ( "github.com/gorilla/mux" "github.com/weaveworks/weave/common" + "github.com/weaveworks/weave/ipam/address" wt "github.com/weaveworks/weave/testing" ) @@ -38,12 +39,12 @@ func doHTTP(method string, url string) (resp *http.Response, err error) { return http.DefaultClient.Do(req) } -func listenHTTP(alloc *Allocator) int { +func listenHTTP(alloc *Allocator, subnet address.CIDR) int { router := mux.NewRouter() router.HandleFunc("/status", func(w http.ResponseWriter, r *http.Request) { io.WriteString(w, fmt.Sprintln(alloc)) }) - alloc.HandleHTTP(router) + alloc.HandleHTTP(router, subnet) httpListener, err := net.Listen("tcp", ":0") if err != nil { @@ -79,8 +80,8 @@ func TestHttp(t *testing.T) { testAddr2 = "10.2.0.1/16" ) - alloc := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", universe, 1) - port := listenHTTP(alloc) + alloc, subnet := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", universe, 1) + port := listenHTTP(alloc, subnet) alloc.claimRingForTesting() cidr1a := HTTPPost(t, allocURL(port, testCIDR1, containerID)) @@ -119,9 +120,9 @@ func TestBadHttp(t *testing.T) { testCIDR1 = "10.0.0.0/8" ) - alloc := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", testCIDR1, 1) + alloc, subnet := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", testCIDR1, 1) defer alloc.Stop() - port := listenHTTP(alloc) + port := listenHTTP(alloc, subnet) alloc.claimRingForTesting() cidr1 := HTTPPost(t, allocURL(port, testCIDR1, containerID)) @@ -154,10 +155,10 @@ func impTestHTTPCancel(t *testing.T) { testCIDR1 = "10.0.3.0/29" ) - alloc := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", testCIDR1, 1) + alloc, subnet := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", testCIDR1, 1) defer alloc.Stop() alloc.claimRingForTesting() - port := listenHTTP(alloc) + port := listenHTTP(alloc, subnet) // Stop the alloc so nothing actually works unpause := alloc.pause() diff --git a/ipam/testutils_test.go b/ipam/testutils_test.go index 69569b7e..6d1b892e 100644 --- a/ipam/testutils_test.go +++ b/ipam/testutils_test.go @@ -128,27 +128,29 @@ func CheckAllExpectedMessagesSent(allocs ...*Allocator) { } } -func makeAllocator(name string, cidrStr string, quorum uint) *Allocator { +func makeAllocator(name string, cidrStr string, quorum uint) (*Allocator, address.CIDR) { peername, err := router.PeerNameFromString(name) if err != nil { panic(err) } - alloc, err := NewAllocator(peername, router.PeerUID(rand.Int63()), - "nick-"+name, cidrStr, quorum) + _, cidr, err := address.ParseCIDR(cidrStr) if err != nil { panic(err) } - return alloc + alloc := NewAllocator(peername, router.PeerUID(rand.Int63()), + "nick-"+name, cidr, quorum) + + return alloc, cidr } -func makeAllocatorWithMockGossip(t *testing.T, name string, universeCIDR string, quorum uint) *Allocator { - alloc := makeAllocator(name, universeCIDR, quorum) +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) alloc.Start() - return alloc + return alloc, subnet } func (alloc *Allocator) claimRingForTesting(allocs ...*Allocator) { @@ -160,10 +162,10 @@ func (alloc *Allocator) claimRingForTesting(allocs ...*Allocator) { alloc.space.AddRanges(alloc.ring.OwnedRanges()) } -func (alloc *Allocator) NumFreeAddresses() address.Offset { +func (alloc *Allocator) NumFreeAddresses(subnet address.CIDR) address.Offset { resultChan := make(chan address.Offset) alloc.actionChan <- func() { - resultChan <- alloc.space.NumFreeAddressesInRange(alloc.defaultSubnet.Start+1, alloc.defaultSubnet.End()-1) + resultChan <- alloc.space.NumFreeAddressesInRange(subnet.Start+1, subnet.End()-1) } return <-resultChan } @@ -285,13 +287,15 @@ func (client TestGossipRouterClient) GossipBroadcast(update router.GossipData) e return client.router.GossipBroadcast(update) } -func makeNetworkOfAllocators(size int, cidr string) ([]*Allocator, TestGossipRouter) { +func makeNetworkOfAllocators(size int, cidr string) ([]*Allocator, TestGossipRouter, address.CIDR) { gossipRouter := TestGossipRouter{make(map[router.PeerName]chan gossipMessage), 0.0} allocs := make([]*Allocator, size) + var subnet address.CIDR for i := 0; i < size; i++ { - alloc := makeAllocator(fmt.Sprintf("%02d:00:00:02:00:00", i), + var alloc *Allocator + alloc, subnet = makeAllocator(fmt.Sprintf("%02d:00:00:02:00:00", i), cidr, uint(size/2+1)) alloc.SetInterfaces(gossipRouter.connect(alloc.ourName, alloc)) alloc.Start() @@ -300,7 +304,7 @@ func makeNetworkOfAllocators(size int, cidr string) ([]*Allocator, TestGossipRou gossipRouter.GossipBroadcast(allocs[size-1].Gossip()) time.Sleep(1000 * time.Millisecond) - return allocs, gossipRouter + return allocs, gossipRouter, subnet } func stopNetworkOfAllocators(allocs []*Allocator) { diff --git a/prog/weaver/main.go b/prog/weaver/main.go index ee4c403b..9608c4b7 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -17,6 +17,7 @@ import ( . "github.com/weaveworks/weave/common" "github.com/weaveworks/weave/common/updater" "github.com/weaveworks/weave/ipam" + "github.com/weaveworks/weave/ipam/address" weavenet "github.com/weaveworks/weave/net" weave "github.com/weaveworks/weave/router" ) @@ -139,8 +140,9 @@ func main() { log.Println("Our name is", router.Ourself) var allocator *ipam.Allocator + var defaultSubnet address.CIDR if iprangeCIDR != "" { - allocator = createAllocator(router, apiPath, iprangeCIDR, ipsubnetCIDR, determineQuorum(peerCount, peers)) + allocator, defaultSubnet = createAllocator(router, apiPath, iprangeCIDR, ipsubnetCIDR, determineQuorum(peerCount, peers)) } else if peerCount > 0 { log.Fatal("-initpeercount flag specified without -iprange") } else { @@ -156,7 +158,7 @@ func main() { // so there is no point in doing "weave launch -httpaddr ''". // This is here to support stand-alone use of weaver. if httpAddr != "" { - go handleHTTP(router, httpAddr, allocator) + go handleHTTP(router, httpAddr, allocator, defaultSubnet) } SignalHandlerLoop(router) @@ -196,23 +198,35 @@ func logFrameFunc(debug bool) weave.LogFrameFunc { } } -func createAllocator(router *weave.Router, apiPath string, ipRangeStr string, defaultSubnetStr string, quorum uint) *ipam.Allocator { - allocator, err := ipam.NewAllocator(router.Ourself.Peer.Name, router.Ourself.Peer.UID, router.Ourself.Peer.NickName, ipRangeStr, quorum) +func parseAndCheckCIDR(cidrStr string) address.CIDR { + _, cidr, err := address.ParseCIDR(cidrStr) if err != nil { log.Fatal(err) } - allocator.SetInterfaces(router.NewGossip("IPallocation", allocator)) - allocator.Start() + if cidr.Size() < ipam.MinSubnetSize { + log.Fatalf("Allocation range smaller than minimum size %d: %s", ipam.MinSubnetSize, cidrStr) + } + return cidr +} + +func createAllocator(router *weave.Router, apiPath string, ipRangeStr string, defaultSubnetStr string, quorum uint) (*ipam.Allocator, address.CIDR) { + ipRange := parseAndCheckCIDR(ipRangeStr) + defaultSubnet := ipRange if defaultSubnetStr != "" { - if err = allocator.SetDefaultSubnet(defaultSubnetStr); err != nil { - log.Fatal(err) + defaultSubnet = parseAndCheckCIDR(defaultSubnetStr) + if !ipRange.Overlaps(defaultSubnet) { + log.Fatalf("Default subnet %s out of bounds: %s", defaultSubnet, ipRange) } } - err = updater.Start(apiPath, allocator) + allocator := ipam.NewAllocator(router.Ourself.Peer.Name, router.Ourself.Peer.UID, router.Ourself.Peer.NickName, ipRange, quorum) + + allocator.SetInterfaces(router.NewGossip("IPallocation", allocator)) + allocator.Start() + err := updater.Start(apiPath, allocator) if err != nil { log.Fatal("Unable to start watcher", err) } - return allocator + return allocator, defaultSubnet } // Pick a quorum size heuristically based on the number of peer @@ -235,11 +249,11 @@ func determineQuorum(initPeerCountFlag int, peers []string) uint { return quorum } -func handleHTTP(router *weave.Router, httpAddr string, allocator *ipam.Allocator) { +func handleHTTP(router *weave.Router, httpAddr string, allocator *ipam.Allocator, defaultSubnet address.CIDR) { muxRouter := mux.NewRouter() if allocator != nil { - allocator.HandleHTTP(muxRouter) + allocator.HandleHTTP(muxRouter, defaultSubnet) } muxRouter.Methods("GET").Path("/status").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -247,6 +261,7 @@ func handleHTTP(router *weave.Router, httpAddr string, allocator *ipam.Allocator fmt.Fprintln(w, router.Status()) if allocator != nil { fmt.Fprintln(w, allocator.String()) + fmt.Fprintln(w, "Allocator default subnet:", defaultSubnet) } }) diff --git a/site/ipam.md b/site/ipam.md index b29cdca4..92a0142e 100644 --- a/site/ipam.md +++ b/site/ipam.md @@ -145,11 +145,12 @@ reports on the current status of the weave router and IP allocator: ```` weave router git-8f675f15c0b5 ... -Allocator universe 10.2.0.0/16 (default subnet 10.2.1.0/24) +Allocator universe 10.2.0.0/16 Owned Ranges: 10.2.1.1 -> 96:e9:e2:2e:2d:bc (host1) (v3) 10.2.1.128 -> ea:84:25:9b:31:2e (host2) (v3) 10.2.1.192 -> ea:6c:21:09:cf:f0 (host3) (v9) +Allocator default subnet: 10.2.1.0/24 ```` The first section covers the router; see the [troubleshooting From 147962d10191ab2c54b1f96b0c300694d57dbcad Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 8 Jun 2015 12:03:06 +0100 Subject: [PATCH 20/34] Clarify some unit test operations --- ipam/allocator_test.go | 4 ++-- ipam/http_test.go | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index d8473c0d..ccf1043a 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -38,13 +38,13 @@ func TestAllocFree(t *testing.T) { _, cidr2, _ := address.ParseCIDR(subnet2) alloc.claimRingForTesting() - addr1, _ := alloc.Allocate(container1, cidr1, nil) + addr1, err := alloc.Allocate(container1, cidr1, nil) + wt.AssertNoErr(t, err) wt.AssertEqualString(t, addr1.String(), testAddr1, "address") addr2, err := alloc.Allocate(container1, cidr2, nil) wt.AssertNoErr(t, err) wt.AssertEqualString(t, addr2.String(), testAddr2, "address") - CheckAllExpectedMessagesSent(alloc) // Ask for another address for a different container and check it's different addr1b, _ := alloc.Allocate(container2, cidr1, nil) diff --git a/ipam/http_test.go b/ipam/http_test.go index 68578bf9..bdf48277 100644 --- a/ipam/http_test.go +++ b/ipam/http_test.go @@ -74,22 +74,22 @@ func TestHttp(t *testing.T) { container2 = "baddf00d" container3 = "b01df00d" universe = "10.0.0.0/8" - testAddr1 = "10.0.3.9/29" testCIDR1 = "10.0.3.8/29" testCIDR2 = "10.2.0.0/16" + testAddr1 = "10.0.3.9/29" testAddr2 = "10.2.0.1/16" ) alloc, subnet := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", universe, 1) port := listenHTTP(alloc, subnet) - alloc.claimRingForTesting() + + // Allocate an address in each subnet, and check we got what we expected cidr1a := HTTPPost(t, allocURL(port, testCIDR1, containerID)) wt.AssertEqualString(t, cidr1a, testAddr1, "address") - cidr2a := HTTPPost(t, allocURL(port, testCIDR2, containerID)) wt.AssertEqualString(t, cidr2a, testAddr2, "address") - + // Now, make the same requests again to check the operation is idempotent check := HTTPGet(t, allocURL(port, testCIDR1, containerID)) wt.AssertEqualString(t, check, cidr1a, "address") check = HTTPGet(t, allocURL(port, testCIDR2, containerID)) @@ -101,7 +101,7 @@ func TestHttp(t *testing.T) { cidr2b := HTTPPost(t, allocURL(port, testCIDR2, container2)) wt.AssertFalse(t, cidr2b == testAddr2, "address") - // Now free the first container, and we should get it back when we ask + // Now free the first container, and we should get its addresses back when we ask doHTTP("DELETE", identURL(port, containerID)) cidr1c := HTTPPost(t, allocURL(port, testCIDR1, container3)) From 40c9adc8d3a9e02c7fc5e6e3a27df2be7c89b21d Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 8 Jun 2015 12:31:41 +0100 Subject: [PATCH 21/34] Extract common freelist-walking code --- ipam/space/space.go | 75 ++++++++++++++++++++++++--------------------- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/ipam/space/space.go b/ipam/space/space.go index 549424fd..c0ddcb57 100644 --- a/ipam/space/space.go +++ b/ipam/space/space.go @@ -37,26 +37,46 @@ func (s *Space) Clear() { s.ours = s.ours[:0] } -func (s *Space) Allocate(rangeStart, rangeEnd address.Address) (bool, address.Address) { - // Need to walk forward to the range, then check free space, then pick an address - // Walk down the free spaces, looking for one in range +// Walk down the free list calling f() on the in-range portions, until +// f() returns true or we run out of free space. Return true iff f() returned true +func (s *Space) walkFree(rangeStart, rangeEnd address.Address, f func(pos int, start, end address.Address) bool) bool { + if rangeStart >= rangeEnd { // degenerate case + return false + } for i := 0; i < len(s.free); i += 2 { start, end := s.free[i], s.free[i+1] - if end-1 < rangeStart { + if end <= rangeStart { // this chunk comes before the range continue } - if start > rangeEnd { + if start >= rangeEnd { + // all remaining free space is completely after range break } - res := start - if res < rangeStart { - res = rangeStart + // at this point we know end>start && end>rangeStart && rangeEnd>start && rangeEnd>rangeStart + // therefore max(start, rangeStart) < min(end, rangeEnd) + // Restrict this block of free space to be in range + if start < rangeStart { + start = rangeStart + } + if end > rangeEnd { + end = rangeEnd + } + // at this point we know start end { - e = end - } - if s >= e { - continue - } - res += address.Subtract(e, s) - } + s.walkFree(rangeStart, rangeEnd, func(_ int, start, end address.Address) bool { + res += address.Subtract(end, start) + return false + }) return res } @@ -104,20 +115,14 @@ func (s *Space) biggestFreeRange(rangeStart, rangeEnd address.Address) (int, add pos := -1 biggest := address.Offset(0) - for i := 0; i < len(s.free); i += 2 { - start, end := s.free[i], s.free[i+1] - if start < rangeStart { - start = rangeStart - } - if end > rangeEnd { - end = rangeEnd - } + s.walkFree(rangeStart, rangeEnd, func(i int, start, end address.Address) bool { size := address.Subtract(end, start) if size >= biggest { pos = i biggest = size } - } + return false + }) return pos, biggest } From a80e5cb028b6f55bdaad70be40221ba3f198e1dd Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 8 Jun 2015 13:42:55 +0100 Subject: [PATCH 22/34] Corrected space.Donate() to restrict the space donated to be in-range, and associated unit test changes --- ipam/allocator.go | 5 ++--- ipam/space/space.go | 42 +++++++++++++++++------------------ ipam/space/space_test.go | 47 ++++++++++++++++++++-------------------- 3 files changed, 46 insertions(+), 48 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index 0949658a..809605e0 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -642,15 +642,14 @@ func (alloc *Allocator) donateSpace(subnet address.CIDR, to router.PeerName) { defer alloc.sendRingUpdate(to) alloc.debugln("Peer", to, "asked me for space") - start, size, ok := alloc.space.Donate(subnet.Start, subnet.End()) + start, end, ok := alloc.space.Donate(subnet.Start, subnet.End()) if !ok { free := alloc.space.NumFreeAddressesInRange(subnet.Start, subnet.End()) common.Assert(free == 0) alloc.debugln("No space to give to peer", to) return } - end := address.Add(start, size) - alloc.debugln("Giving range", start, end, size, "to", to) + alloc.debugln("Giving range", start, end, "to", to) alloc.ring.GrantRangeToHost(start, end, to) } diff --git a/ipam/space/space.go b/ipam/space/space.go index c0ddcb57..b456e0df 100644 --- a/ipam/space/space.go +++ b/ipam/space/space.go @@ -39,7 +39,7 @@ func (s *Space) Clear() { // Walk down the free list calling f() on the in-range portions, until // f() returns true or we run out of free space. Return true iff f() returned true -func (s *Space) walkFree(rangeStart, rangeEnd address.Address, f func(pos int, start, end address.Address) bool) bool { +func (s *Space) walkFree(rangeStart, rangeEnd address.Address, f func(start, end address.Address) bool) bool { if rangeStart >= rangeEnd { // degenerate case return false } @@ -62,7 +62,7 @@ func (s *Space) walkFree(rangeStart, rangeEnd address.Address, f func(pos int, s end = rangeEnd } // at this point we know start= biggest { - pos = i - biggest = size + if size >= biggestSize { + biggestStart, biggestEnd = start, end + biggestSize = size } return false }) - return pos, biggest + return } -func (s *Space) Donate(rangeStart, rangeEnd address.Address) (address.Address, address.Offset, bool) { - if len(s.free) == 0 { +func (s *Space) Donate(rangeStart, rangeEnd address.Address) (address.Address, address.Address, bool) { + start, end := s.biggestFreeRange(rangeStart, rangeEnd) + size := address.Subtract(end, start) + + if size == 0 { return 0, 0, false } - pos, biggest := s.biggestFreeRange(rangeStart, rangeEnd) - - // Donate half of that biggest free range, rounding up so - // that the donation can't be empty - end := s.free[pos+1] - start := end - address.Address((biggest+1)/2) + // Donate half of that biggest free range. Note size/2 rounds down, so + // the resulting donation size rounds up, and in particular can't be empty. + start = address.Add(start, size/2) s.ours = subtract(s.ours, start, end) s.free = subtract(s.free, start, end) - return start, address.Subtract(end, start), true + return start, end, true } func firstGreater(a []address.Address, x address.Address) int { diff --git a/ipam/space/space_test.go b/ipam/space/space_test.go index 12ede1e4..785b17f5 100644 --- a/ipam/space/space_test.go +++ b/ipam/space/space_test.go @@ -77,19 +77,19 @@ func TestLowlevel(t *testing.T) { wt.AssertErrorInterface(t, s.Free(0), (*error)(nil), "free not allocated") wt.AssertErrorInterface(t, s.Free(100), (*error)(nil), "double free") - start, size, ok := s.Donate(0, 1000) - wt.AssertTrue(t, ok && start == 125 && size == 25, "donate") + start, end, ok := s.Donate(0, 1000) + wt.AssertTrue(t, ok && start == 125 && address.Subtract(end, start) == 25, "donate") // test Donate when addresses are scarce s = New() - start, size, ok = s.Donate(0, 1000) + start, end, ok = s.Donate(0, 1000) wt.AssertTrue(t, !ok, "donate on empty space should fail") s.Add(0, 3) wt.AssertNoErr(t, s.Claim(0)) wt.AssertNoErr(t, s.Claim(2)) - start, size, ok = s.Donate(0, 1000) - wt.AssertTrue(t, ok && start == 1 && size == 1, "donate") - start, size, ok = s.Donate(0, 1000) + start, end, ok = s.Donate(0, 1000) + wt.AssertTrue(t, ok && start == 1 && end == 2, "donate") + start, end, ok = s.Donate(0, 1000) wt.AssertTrue(t, !ok, "donate should fail") } @@ -144,8 +144,8 @@ func TestSpaceFree(t *testing.T) { space := makeSpace(ip(testAddr1), 20) // Check we are prepared to give up the entire space - pos, size := space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, space.free[pos] == ip(testAddr1) && size == 20, "Wrong space") + start, end := space.biggestFreeRange(ip(testAddr1), ip(testAddry)) + wt.AssertTrue(t, start == ip(testAddr1) && address.Subtract(end, start) == 20, "Wrong space") for i := 0; i < 20; i++ { ok, _ := space.Allocate(ip(testAddr1), ip(testAddry)) @@ -155,18 +155,18 @@ func TestSpaceFree(t *testing.T) { // Check we are full ok, _ := space.Allocate(ip(testAddr1), ip(testAddry)) wt.AssertTrue(t, !ok, "Should have failed to get address") - _, size, ok = space.Donate(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, size == 0, "Wrong space") + start, end, ok = space.Donate(ip(testAddr1), ip(testAddry)) + wt.AssertTrue(t, address.Subtract(end, start) == 0, "Wrong space") // Free in the middle wt.AssertSuccess(t, space.Free(ip("10.0.3.13"))) - pos, size = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, space.free[pos] == ip("10.0.3.13") && size == 1, "Wrong space") + start, end = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) + wt.AssertTrue(t, start == ip("10.0.3.13") && address.Subtract(end, start) == 1, "Wrong space") // Free one at the end wt.AssertSuccess(t, space.Free(ip("10.0.3.23"))) - pos, size = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, space.free[pos] == ip("10.0.3.23") && size == 1, "Wrong space") + start, end = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) + wt.AssertTrue(t, start == ip("10.0.3.23") && address.Subtract(end, start) == 1, "Wrong space") // Now free a few at the end wt.AssertSuccess(t, space.Free(ip("10.0.3.22"))) @@ -175,8 +175,8 @@ func TestSpaceFree(t *testing.T) { wt.AssertEquals(t, space.NumFreeAddresses(), address.Offset(4)) // Now get the biggest free space; should be 3.21 - pos, size = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, space.free[pos] == ip("10.0.3.21") && size == 3, "Wrong space") + start, end = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) + wt.AssertTrue(t, start == ip("10.0.3.21") && address.Subtract(end, start) == 3, "Wrong space") // Now free a few in the middle wt.AssertSuccess(t, space.Free(ip("10.0.3.12"))) @@ -186,8 +186,8 @@ func TestSpaceFree(t *testing.T) { wt.AssertEquals(t, space.NumFreeAddresses(), address.Offset(7)) // Now get the biggest free space; should be 3.21 - pos, size = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, space.free[pos] == ip("10.0.3.10") && size == 4, "Wrong space") + start, end = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) + wt.AssertTrue(t, start == ip("10.0.3.10") && address.Subtract(end, start) == 4, "Wrong space") wt.AssertEquals(t, space.OwnedRanges(), []address.Range{address.Range{Start: ip("10.0.3.4"), End: ip("10.0.3.24")}}) } @@ -210,7 +210,8 @@ func TestDonateSimple(t *testing.T) { ps1 := makeSpace(ipAddr1, size) // Empty space set should split in two and give me the second half - start, numGivenUp, ok := ps1.Donate(ip(testAddr1), ip(testAddr1)+size) + start, end, ok := ps1.Donate(ip(testAddr1), ip(testAddr1)+size) + numGivenUp := address.Subtract(end, start) wt.AssertTrue(t, ok, "Donate result") wt.AssertEqualString(t, start.String(), "10.0.1.24", "Invalid start") wt.AssertEquals(t, numGivenUp, address.Offset(size/2)) @@ -219,11 +220,11 @@ func TestDonateSimple(t *testing.T) { // Now check we can give the rest up. count := 0 // count to avoid infinite loop for ; count < 1000; count++ { - _, size, ok := ps1.Donate(ip(testAddr1), ip(testAddr1)+size) + start, end, ok := ps1.Donate(ip(testAddr1), ip(testAddr1)+size) if !ok { break } - numGivenUp += size + numGivenUp += address.Subtract(end, start) } wt.AssertEquals(t, ps1.NumFreeAddresses(), address.Offset(0)) wt.AssertEquals(t, numGivenUp, address.Offset(size)) @@ -253,10 +254,10 @@ func TestDonateHard(t *testing.T) { } // Now split - newRange, numGivenUp, ok := spaceset.Donate(start, end) + newRange, end, ok := spaceset.Donate(start, end) wt.AssertTrue(t, ok, "GiveUpSpace result") wt.AssertEquals(t, newRange, ip("10.0.1.23")) - wt.AssertEquals(t, numGivenUp, address.Offset(24)) + wt.AssertEquals(t, address.Subtract(end, newRange), address.Offset(24)) wt.AssertEquals(t, spaceset.NumFreeAddresses(), address.Offset(23)) //Space set should now have 2 spaces From 1e71d2dacb767c04469aadd7c024a6cc7d35230d Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 8 Jun 2015 15:13:03 +0100 Subject: [PATCH 23/34] Introduce new '' type in usage text --- weave | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/weave b/weave index 8aa8c713..5bcf1b29 100755 --- a/weave +++ b/weave @@ -33,14 +33,14 @@ usage() { echo "weave launch-proxy [-D ] [-H
] [--with-dns] [--with-ipam]" echo "weave connect [--replace] [ ...]" echo "weave forget ..." - echo "weave run [--with-dns] [[--subnet] ...] ..." - echo "weave start [[--subnet] ...] " - echo "weave attach [[--subnet] ...] " - echo "weave detach [[--subnet] ...] " + echo "weave run [--with-dns] [ ...] ..." + echo "weave start [ ...] " + echo "weave attach [ ...] " + echo "weave detach [ ...] " echo "weave dns-add [ ...] [-h ]" echo "weave dns-remove [ ...] [-h ]" - echo "weave expose [[--subnet] ...] [-h ]" - echo "weave hide [[--subnet] ...]" + echo "weave expose [ ...] [-h ]" + echo "weave hide [ ...]" echo "weave ps [ ...]" echo "weave status" echo "weave proxy-env" @@ -52,9 +52,10 @@ usage() { echo "weave reset" echo "weave rmpeer " echo - echo "where = [:]" - echo " = /" - echo " = or weave internal peer ID" + echo "where = [:]" + echo " = /" + echo " = or weave internal peer ID" + echo " = | --subnet | --subnet default" echo "
= [tcp://][]:" exit 1 } From e21f66db0a0d128a8190cc3d1acbb7b8133ca9d8 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 8 Jun 2015 17:46:58 +0100 Subject: [PATCH 24/34] Use address.Range instead of (start,end) pairs --- ipam/address/address.go | 9 +++++ ipam/allocate.go | 20 +++++------ ipam/allocator.go | 30 +++++++++-------- ipam/allocator_test.go | 2 +- ipam/space/space.go | 65 ++++++++++++++++++----------------- ipam/space/space_test.go | 73 ++++++++++++++++++++-------------------- ipam/testutils_test.go | 4 +-- prog/weaver/main.go | 2 +- router/protocol.go | 2 +- 9 files changed, 107 insertions(+), 100 deletions(-) diff --git a/ipam/address/address.go b/ipam/address/address.go index 67915345..377e283f 100644 --- a/ipam/address/address.go +++ b/ipam/address/address.go @@ -15,6 +15,12 @@ type Range struct { Start, End Address // [Start, End); Start <= End } +func MakeRange(start Address, size Offset) Range { return Range{Start: start, End: Add(start, size)} } +func (r Range) Size() Offset { return Subtract(r.End, r.Start) } +func (r Range) String() string { return fmt.Sprintf("[%s-%s)", r.Start, r.End) } +func (r1 Range) Overlaps(r2 Range) bool { return !(r1.Start >= r2.End || r1.End <= r2.Start) } +func (r Range) Contains(addr Address) bool { return addr >= r.Start && addr < r.End } + type CIDR struct { Start Address PrefixLen int @@ -39,7 +45,10 @@ func ParseCIDR(s string) (Address, CIDR, error) { } func (cidr CIDR) Size() Offset { return 1 << uint(32-cidr.PrefixLen) } + +// End is exclusive, same as in Range func (cidr CIDR) End() Address { return Add(cidr.Start, cidr.Size()) } +func (cidr CIDR) Range() Range { return Range{Start: cidr.Start, End: Add(cidr.Start, cidr.Size())} } func (cidr CIDR) String() string { return fmt.Sprintf("%s/%d", cidr.Start.String(), cidr.PrefixLen) diff --git a/ipam/allocate.go b/ipam/allocate.go index 71eb8e82..59239b80 100644 --- a/ipam/allocate.go +++ b/ipam/allocate.go @@ -14,7 +14,7 @@ type allocate struct { resultChan chan<- allocateResult hasBeenCancelled func() bool ident string - subnet address.CIDR + r address.Range // Range we are trying to allocate within } // Try returns true if the request is completed, false if pending @@ -27,31 +27,29 @@ func (g *allocate) Try(alloc *Allocator) bool { // If we have previously stored an address for this container in this subnet, return it. if addrs, found := alloc.owned[g.ident]; found { for _, addr := range addrs { - if g.subnet.Contains(addr) { + if g.r.Contains(addr) { g.resultChan <- allocateResult{addr, nil} return true } } } - if !alloc.universe.Overlaps(g.subnet) { - g.resultChan <- allocateResult{0, fmt.Errorf("Subnet %s out of bounds: %s", g.subnet, alloc.universe)} + if !alloc.universe.Overlaps(g.r) { + g.resultChan <- allocateResult{0, fmt.Errorf("range %s out of bounds: %s", g.r, alloc.universe)} return true } - // Respect RFC1122 exclusions of first and last addresses - start, end := g.subnet.Start+1, g.subnet.End()-1 - if ok, addr := alloc.space.Allocate(start, end); ok { - alloc.debugln("Allocated", addr, "for", g.ident, "in", g.subnet) + if ok, addr := alloc.space.Allocate(g.r); ok { + alloc.debugln("Allocated", addr, "for", g.ident, "in", g.r) alloc.addOwned(g.ident, addr) g.resultChan <- allocateResult{addr, nil} return true } // out of space - if donor, err := alloc.ring.ChoosePeerToAskForSpace(start, end); err == nil { - alloc.debugln("Decided to ask peer", donor, "for space in subnet", g.subnet) - alloc.sendSpaceRequest(donor, g.subnet) + if donor, err := alloc.ring.ChoosePeerToAskForSpace(g.r.Start, g.r.End); err == nil { + alloc.debugln("Decided to ask peer", donor, "for space in range", g.r) + alloc.sendSpaceRequest(donor, g.r) } return false diff --git a/ipam/allocator.go b/ipam/allocator.go index 809605e0..398b05bc 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -45,7 +45,7 @@ type operation interface { type Allocator struct { actionChan chan<- func() ourName router.PeerName - universe address.CIDR // superset of all ranges + 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 address, indexed by container-ID @@ -60,7 +60,7 @@ type Allocator struct { } // NewAllocator creates and initialises a new Allocator -func NewAllocator(ourName router.PeerName, ourUID router.PeerUID, ourNickname string, universe address.CIDR, quorum uint) *Allocator { +func NewAllocator(ourName router.PeerName, ourUID router.PeerUID, ourNickname string, universe address.Range, quorum uint) *Allocator { return &Allocator{ ourName: ourName, universe: universe, @@ -180,7 +180,9 @@ func hasBeenCancelled(cancelChan <-chan bool) func() bool { // if there isn't any space we block indefinitely func (alloc *Allocator) Allocate(ident string, subnet address.CIDR, cancelChan <-chan bool) (address.CIDR, error) { resultChan := make(chan allocateResult) - op := &allocate{subnet: subnet, resultChan: resultChan, ident: ident, + // Respect RFC1122 exclusions of first and last addresses + r := address.Range{Start: subnet.Start + 1, End: subnet.End() - 1} + op := &allocate{r: r, resultChan: resultChan, ident: ident, hasBeenCancelled: hasBeenCancelled(cancelChan)} alloc.doOperation(op, &alloc.pendingAllocates) result := <-resultChan @@ -347,12 +349,12 @@ func (alloc *Allocator) OnGossipUnicast(sender router.PeerName, msg []byte) erro case msgSpaceRequest: // some other peer asked us for space decoder := gob.NewDecoder(bytes.NewReader(msg[1:])) - var cidr address.CIDR - if err := decoder.Decode(&cidr); err != nil { + var r address.Range + if err := decoder.Decode(&r); err != nil { resultChan <- err return } - alloc.donateSpace(cidr, sender) + alloc.donateSpace(r, sender) resultChan <- nil case msgRingUpdate: resultChan <- alloc.update(msg[1:]) @@ -570,10 +572,10 @@ func (alloc *Allocator) propose() { alloc.gossip.GossipBroadcast(alloc.Gossip()) } -func (alloc *Allocator) sendSpaceRequest(dest router.PeerName, subnet address.CIDR) { +func (alloc *Allocator) sendSpaceRequest(dest router.PeerName, r address.Range) { buf := new(bytes.Buffer) enc := gob.NewEncoder(buf) - if err := enc.Encode(subnet); err != nil { + if err := enc.Encode(r); err != nil { panic(err) } msg := router.Concat([]byte{msgSpaceRequest}, buf.Bytes()) @@ -633,7 +635,7 @@ func (alloc *Allocator) update(msg []byte) error { return nil } -func (alloc *Allocator) donateSpace(subnet address.CIDR, to router.PeerName) { +func (alloc *Allocator) donateSpace(r address.Range, to router.PeerName) { // No matter what we do, we'll send a unicast gossip // of our ring back to tha chap who asked for space. // This serves to both tell him of any space we might @@ -642,15 +644,15 @@ func (alloc *Allocator) donateSpace(subnet address.CIDR, to router.PeerName) { defer alloc.sendRingUpdate(to) alloc.debugln("Peer", to, "asked me for space") - start, end, ok := alloc.space.Donate(subnet.Start, subnet.End()) + chunk, ok := alloc.space.Donate(r) if !ok { - free := alloc.space.NumFreeAddressesInRange(subnet.Start, subnet.End()) + free := alloc.space.NumFreeAddressesInRange(r) common.Assert(free == 0) alloc.debugln("No space to give to peer", to) return } - alloc.debugln("Giving range", start, end, "to", to) - alloc.ring.GrantRangeToHost(start, end, to) + alloc.debugln("Giving range", chunk, "to", to) + alloc.ring.GrantRangeToHost(chunk.Start, chunk.End, to) } func (alloc *Allocator) assertInvariants() { @@ -678,7 +680,7 @@ func (alloc *Allocator) reportFreeSpace() { freespace := make(map[address.Address]address.Offset) for _, r := range ranges { - freespace[r.Start] = alloc.space.NumFreeAddressesInRange(r.Start, r.End) + freespace[r.Start] = alloc.space.NumFreeAddressesInRange(r) } alloc.ring.ReportFree(freespace) } diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index ccf1043a..03b30f83 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -173,7 +173,7 @@ func TestCancel(t *testing.T) { alloc1, subnet := makeAllocator("01:00:00:02:00:00", CIDR, 2) alloc1.SetInterfaces(router.connect(alloc1.ourName, alloc1)) - alloc2, subnet := makeAllocator("02:00:00:02:00:00", CIDR, 2) + alloc2, _ := makeAllocator("02:00:00:02:00:00", CIDR, 2) alloc2.SetInterfaces(router.connect(alloc2.ourName, alloc2)) alloc1.claimRingForTesting(alloc1, alloc2) alloc2.claimRingForTesting(alloc1, alloc2) diff --git a/ipam/space/space.go b/ipam/space/space.go index b456e0df..72b8b6ec 100644 --- a/ipam/space/space.go +++ b/ipam/space/space.go @@ -39,40 +39,41 @@ func (s *Space) Clear() { // Walk down the free list calling f() on the in-range portions, until // f() returns true or we run out of free space. Return true iff f() returned true -func (s *Space) walkFree(rangeStart, rangeEnd address.Address, f func(start, end address.Address) bool) bool { - if rangeStart >= rangeEnd { // degenerate case +func (s *Space) walkFree(r address.Range, f func(address.Range) bool) bool { + if r.Start >= r.End { // degenerate case return false } for i := 0; i < len(s.free); i += 2 { - start, end := s.free[i], s.free[i+1] - if end <= rangeStart { // this chunk comes before the range + chunk := address.Range{Start: s.free[i], End: s.free[i+1]} + if chunk.End <= r.Start { // this chunk comes before the range continue } - if start >= rangeEnd { + if chunk.Start >= r.End { // all remaining free space is completely after range break } - // at this point we know end>start && end>rangeStart && rangeEnd>start && rangeEnd>rangeStart - // therefore max(start, rangeStart) < min(end, rangeEnd) + // at this point we know chunk.End>chunk.Start && + // chunk.End>r.Start && r.End>chunk.Start && r.End>r.Start + // therefore max(start, r.Start) < min(end, r.End) // Restrict this block of free space to be in range - if start < rangeStart { - start = rangeStart + if chunk.Start < r.Start { + chunk.Start = r.Start } - if end > rangeEnd { - end = rangeEnd + if chunk.End > r.End { + chunk.End = r.End } // at this point we know start= biggestSize { - biggestStart, biggestEnd = start, end +func (s *Space) biggestFreeRange(r address.Range) (biggest address.Range) { + biggestSize := address.Offset(0) + s.walkFree(r, func(chunk address.Range) bool { + if size := chunk.Size(); size >= biggestSize { + biggest = chunk biggestSize = size } return false @@ -124,21 +124,20 @@ func (s *Space) biggestFreeRange(rangeStart, rangeEnd address.Address) (biggestS return } -func (s *Space) Donate(rangeStart, rangeEnd address.Address) (address.Address, address.Address, bool) { - start, end := s.biggestFreeRange(rangeStart, rangeEnd) - size := address.Subtract(end, start) +func (s *Space) Donate(r address.Range) (address.Range, bool) { + biggest := s.biggestFreeRange(r) - if size == 0 { - return 0, 0, false + if biggest.Size() == 0 { + return address.Range{}, false } // Donate half of that biggest free range. Note size/2 rounds down, so // the resulting donation size rounds up, and in particular can't be empty. - start = address.Add(start, size/2) + biggest.Start = address.Add(biggest.Start, biggest.Size()/2) - s.ours = subtract(s.ours, start, end) - s.free = subtract(s.free, start, end) - return start, end, true + s.ours = subtract(s.ours, biggest.Start, biggest.End) + s.free = subtract(s.free, biggest.Start, biggest.End) + return biggest, true } func firstGreater(a []address.Address, x address.Address) int { diff --git a/ipam/space/space_test.go b/ipam/space/space_test.go index 785b17f5..8ab0f369 100644 --- a/ipam/space/space_test.go +++ b/ipam/space/space_test.go @@ -62,12 +62,12 @@ func TestLowlevel(t *testing.T) { s := New() wt.AssertEquals(t, s.NumFreeAddresses(), address.Offset(0)) - ok, got := s.Allocate(0, 1000) + ok, got := s.Allocate(address.MakeRange(0, 1000)) wt.AssertFalse(t, ok, "allocate in empty space should fail") s.Add(100, 100) wt.AssertEquals(t, s.NumFreeAddresses(), address.Offset(100)) - ok, got = s.Allocate(0, 1000) + ok, got = s.Allocate(address.MakeRange(0, 1000)) wt.AssertTrue(t, ok && got == 100, "allocate") wt.AssertEquals(t, s.NumFreeAddresses(), address.Offset(99)) wt.AssertNoErr(t, s.Claim(150)) @@ -77,19 +77,19 @@ func TestLowlevel(t *testing.T) { wt.AssertErrorInterface(t, s.Free(0), (*error)(nil), "free not allocated") wt.AssertErrorInterface(t, s.Free(100), (*error)(nil), "double free") - start, end, ok := s.Donate(0, 1000) - wt.AssertTrue(t, ok && start == 125 && address.Subtract(end, start) == 25, "donate") + r, ok := s.Donate(address.MakeRange(0, 1000)) + wt.AssertTrue(t, ok && r.Start == 125 && r.Size() == 25, "donate") // test Donate when addresses are scarce s = New() - start, end, ok = s.Donate(0, 1000) + r, ok = s.Donate(address.MakeRange(0, 1000)) wt.AssertTrue(t, !ok, "donate on empty space should fail") s.Add(0, 3) wt.AssertNoErr(t, s.Claim(0)) wt.AssertNoErr(t, s.Claim(2)) - start, end, ok = s.Donate(0, 1000) - wt.AssertTrue(t, ok && start == 1 && end == 2, "donate") - start, end, ok = s.Donate(0, 1000) + r, ok = s.Donate(address.MakeRange(0, 1000)) + wt.AssertTrue(t, ok && r.Start == 1 && r.End == 2, "donate") + r, ok = s.Donate(address.MakeRange(0, 1000)) wt.AssertTrue(t, !ok, "donate should fail") } @@ -104,23 +104,22 @@ func TestSpaceAllocate(t *testing.T) { ) var ( start = ip(testAddr1) - end = address.Add(ip(testAddr1), 20) ) space1 := makeSpace(start, size) wt.AssertEquals(t, space1.NumFreeAddresses(), address.Offset(20)) space1.assertInvariants() - _, addr1 := space1.Allocate(start, end) + _, addr1 := space1.Allocate(address.MakeRange(start, size)) wt.AssertEqualString(t, addr1.String(), testAddr1, "address") wt.AssertEquals(t, space1.NumFreeAddresses(), address.Offset(19)) space1.assertInvariants() - _, addr2 := space1.Allocate(start, end) + _, addr2 := space1.Allocate(address.MakeRange(start, size)) wt.AssertFalse(t, addr2.String() == testAddr1, "address") wt.AssertEquals(t, space1.NumFreeAddresses(), address.Offset(18)) - wt.AssertEquals(t, space1.NumFreeAddressesInRange(ip(testAddr1), ip(testAddrx)), address.Offset(13)) - wt.AssertEquals(t, space1.NumFreeAddressesInRange(ip(testAddr1), ip(testAddry)), address.Offset(18)) + wt.AssertEquals(t, space1.NumFreeAddressesInRange(address.Range{Start: ip(testAddr1), End: ip(testAddrx)}), address.Offset(13)) + wt.AssertEquals(t, space1.NumFreeAddressesInRange(address.Range{Start: ip(testAddr1), End: ip(testAddry)}), address.Offset(18)) space1.assertInvariants() space1.Free(addr2) @@ -141,32 +140,33 @@ func TestSpaceFree(t *testing.T) { containerID = "deadbeef" ) + entireRange := address.MakeRange(ip(testAddr1), 20) space := makeSpace(ip(testAddr1), 20) // Check we are prepared to give up the entire space - start, end := space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, start == ip(testAddr1) && address.Subtract(end, start) == 20, "Wrong space") + r := space.biggestFreeRange(entireRange) + wt.AssertTrue(t, r.Start == ip(testAddr1) && r.Size() == 20, "Wrong space") for i := 0; i < 20; i++ { - ok, _ := space.Allocate(ip(testAddr1), ip(testAddry)) + ok, _ := space.Allocate(entireRange) wt.AssertTrue(t, ok, "Failed to get address") } // Check we are full - ok, _ := space.Allocate(ip(testAddr1), ip(testAddry)) + ok, _ := space.Allocate(entireRange) wt.AssertTrue(t, !ok, "Should have failed to get address") - start, end, ok = space.Donate(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, address.Subtract(end, start) == 0, "Wrong space") + r, ok = space.Donate(entireRange) + wt.AssertTrue(t, r.Size() == 0, "Wrong space") // Free in the middle wt.AssertSuccess(t, space.Free(ip("10.0.3.13"))) - start, end = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, start == ip("10.0.3.13") && address.Subtract(end, start) == 1, "Wrong space") + r = space.biggestFreeRange(entireRange) + wt.AssertTrue(t, r.Start == ip("10.0.3.13") && r.Size() == 1, "Wrong space") // Free one at the end wt.AssertSuccess(t, space.Free(ip("10.0.3.23"))) - start, end = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, start == ip("10.0.3.23") && address.Subtract(end, start) == 1, "Wrong space") + r = space.biggestFreeRange(entireRange) + wt.AssertTrue(t, r.Start == ip("10.0.3.23") && r.Size() == 1, "Wrong space") // Now free a few at the end wt.AssertSuccess(t, space.Free(ip("10.0.3.22"))) @@ -175,8 +175,8 @@ func TestSpaceFree(t *testing.T) { wt.AssertEquals(t, space.NumFreeAddresses(), address.Offset(4)) // Now get the biggest free space; should be 3.21 - start, end = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, start == ip("10.0.3.21") && address.Subtract(end, start) == 3, "Wrong space") + r = space.biggestFreeRange(entireRange) + wt.AssertTrue(t, r.Start == ip("10.0.3.21") && r.Size() == 3, "Wrong space") // Now free a few in the middle wt.AssertSuccess(t, space.Free(ip("10.0.3.12"))) @@ -186,8 +186,8 @@ func TestSpaceFree(t *testing.T) { wt.AssertEquals(t, space.NumFreeAddresses(), address.Offset(7)) // Now get the biggest free space; should be 3.21 - start, end = space.biggestFreeRange(ip(testAddr1), ip(testAddry)) - wt.AssertTrue(t, start == ip("10.0.3.10") && address.Subtract(end, start) == 4, "Wrong space") + r = space.biggestFreeRange(entireRange) + wt.AssertTrue(t, r.Start == ip("10.0.3.10") && r.Size() == 4, "Wrong space") wt.AssertEquals(t, space.OwnedRanges(), []address.Range{address.Range{Start: ip("10.0.3.4"), End: ip("10.0.3.24")}}) } @@ -210,21 +210,21 @@ func TestDonateSimple(t *testing.T) { ps1 := makeSpace(ipAddr1, size) // Empty space set should split in two and give me the second half - start, end, ok := ps1.Donate(ip(testAddr1), ip(testAddr1)+size) - numGivenUp := address.Subtract(end, start) + r, ok := ps1.Donate(address.MakeRange(ip(testAddr1), size)) + numGivenUp := r.Size() wt.AssertTrue(t, ok, "Donate result") - wt.AssertEqualString(t, start.String(), "10.0.1.24", "Invalid start") + wt.AssertEqualString(t, r.Start.String(), "10.0.1.24", "Invalid start") wt.AssertEquals(t, numGivenUp, address.Offset(size/2)) wt.AssertEquals(t, ps1.NumFreeAddresses(), address.Offset(size/2)) // Now check we can give the rest up. count := 0 // count to avoid infinite loop for ; count < 1000; count++ { - start, end, ok := ps1.Donate(ip(testAddr1), ip(testAddr1)+size) + r, ok := ps1.Donate(address.MakeRange(ip(testAddr1), size)) if !ok { break } - numGivenUp += address.Subtract(end, start) + numGivenUp += r.Size() } wt.AssertEquals(t, ps1.NumFreeAddresses(), address.Offset(0)) wt.AssertEquals(t, numGivenUp, address.Offset(size)) @@ -235,13 +235,12 @@ func TestDonateHard(t *testing.T) { var ( start = ip("10.0.1.0") size address.Offset = 48 - end = address.Add(start, size) ) // Fill a fresh space spaceset := makeSpace(start, size) for i := address.Offset(0); i < size; i++ { - ok, _ := spaceset.Allocate(start, address.Add(start, size)) + ok, _ := spaceset.Allocate(address.MakeRange(start, size)) wt.AssertTrue(t, ok, "Failed to get IP!") } @@ -254,10 +253,10 @@ func TestDonateHard(t *testing.T) { } // Now split - newRange, end, ok := spaceset.Donate(start, end) + newRange, ok := spaceset.Donate(address.MakeRange(start, size)) wt.AssertTrue(t, ok, "GiveUpSpace result") - wt.AssertEquals(t, newRange, ip("10.0.1.23")) - wt.AssertEquals(t, address.Subtract(end, newRange), address.Offset(24)) + wt.AssertEquals(t, newRange.Start, ip("10.0.1.23")) + wt.AssertEquals(t, newRange.Size(), address.Offset(24)) wt.AssertEquals(t, spaceset.NumFreeAddresses(), address.Offset(23)) //Space set should now have 2 spaces diff --git a/ipam/testutils_test.go b/ipam/testutils_test.go index 6d1b892e..8d7da37a 100644 --- a/ipam/testutils_test.go +++ b/ipam/testutils_test.go @@ -140,7 +140,7 @@ func makeAllocator(name string, cidrStr string, quorum uint) (*Allocator, addres } alloc := NewAllocator(peername, router.PeerUID(rand.Int63()), - "nick-"+name, cidr, quorum) + "nick-"+name, cidr.Range(), quorum) return alloc, cidr } @@ -165,7 +165,7 @@ func (alloc *Allocator) claimRingForTesting(allocs ...*Allocator) { func (alloc *Allocator) NumFreeAddresses(subnet address.CIDR) address.Offset { resultChan := make(chan address.Offset) alloc.actionChan <- func() { - resultChan <- alloc.space.NumFreeAddressesInRange(subnet.Start+1, subnet.End()-1) + resultChan <- alloc.space.NumFreeAddressesInRange(address.MakeRange(subnet.Start+1, subnet.Size()-2)) } return <-resultChan } diff --git a/prog/weaver/main.go b/prog/weaver/main.go index 9608c4b7..0ab87b66 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -218,7 +218,7 @@ func createAllocator(router *weave.Router, apiPath string, ipRangeStr string, de log.Fatalf("Default subnet %s out of bounds: %s", defaultSubnet, ipRange) } } - allocator := ipam.NewAllocator(router.Ourself.Peer.Name, router.Ourself.Peer.UID, router.Ourself.Peer.NickName, ipRange, quorum) + allocator := ipam.NewAllocator(router.Ourself.Peer.Name, router.Ourself.Peer.UID, router.Ourself.Peer.NickName, ipRange.Range(), quorum) allocator.SetInterfaces(router.NewGossip("IPallocation", allocator)) allocator.Start() diff --git a/router/protocol.go b/router/protocol.go index 60482621..1ffe5f90 100644 --- a/router/protocol.go +++ b/router/protocol.go @@ -2,7 +2,7 @@ package router const ( Protocol = "weave" - ProtocolVersion = 19 + ProtocolVersion = 20 ) type ProtocolTag byte From fa0b0042b2d7b77a2123ab5115de83430d4c5aca Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 8 Jun 2015 18:12:47 +0100 Subject: [PATCH 25/34] Eradicate the last vestiges of CIDR in Allocator --- ipam/address/address.go | 12 ++++-------- ipam/allocator.go | 12 +++++------- ipam/allocator_test.go | 26 +++++++++++++------------- ipam/http.go | 16 ++++++++-------- ipam/http_test.go | 15 +++++++++------ ipam/testutils_test.go | 14 +++++++------- prog/weaver/main.go | 2 +- 7 files changed, 47 insertions(+), 50 deletions(-) diff --git a/ipam/address/address.go b/ipam/address/address.go index 377e283f..194a09db 100644 --- a/ipam/address/address.go +++ b/ipam/address/address.go @@ -49,19 +49,15 @@ func (cidr CIDR) Size() Offset { return 1 << uint(32-cidr.PrefixLen) } // End is exclusive, same as in Range func (cidr CIDR) End() Address { return Add(cidr.Start, cidr.Size()) } func (cidr CIDR) Range() Range { return Range{Start: cidr.Start, End: Add(cidr.Start, cidr.Size())} } +func (cidr CIDR) HostRange() Range { + // Respect RFC1122 exclusions of first and last addresses + return Range{Start: cidr.Start + 1, End: cidr.End() - 1} +} func (cidr CIDR) String() string { return fmt.Sprintf("%s/%d", cidr.Start.String(), cidr.PrefixLen) } -func (cidr1 CIDR) Overlaps(cidr2 CIDR) bool { - return !(cidr1.Start >= cidr2.End() || cidr1.End() <= cidr2.Start) -} - -func (cidr CIDR) Contains(addr Address) bool { - return addr >= cidr.Start && addr < cidr.End() -} - // FromIP4 converts an ipv4 address to our integer address type func FromIP4(ip4 net.IP) (r Address) { for _, b := range ip4.To4() { diff --git a/ipam/allocator.go b/ipam/allocator.go index 398b05bc..036b8e31 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -178,22 +178,20 @@ func hasBeenCancelled(cancelChan <-chan bool) func() bool { // Allocate (Sync) - get IP address for container with given name // if there isn't any space we block indefinitely -func (alloc *Allocator) Allocate(ident string, subnet address.CIDR, cancelChan <-chan bool) (address.CIDR, error) { +func (alloc *Allocator) Allocate(ident string, r address.Range, cancelChan <-chan bool) (address.Address, error) { resultChan := make(chan allocateResult) - // Respect RFC1122 exclusions of first and last addresses - r := address.Range{Start: subnet.Start + 1, End: subnet.End() - 1} op := &allocate{r: r, resultChan: resultChan, ident: ident, hasBeenCancelled: hasBeenCancelled(cancelChan)} alloc.doOperation(op, &alloc.pendingAllocates) result := <-resultChan - return address.CIDR{Start: result.addr, PrefixLen: subnet.PrefixLen}, result.err + return result.addr, result.err } -func (alloc *Allocator) Lookup(ident string, subnet address.CIDR) (address.CIDR, error) { +func (alloc *Allocator) Lookup(ident string, r address.Range) (address.Address, error) { resultChan := make(chan allocateResult) alloc.actionChan <- func() { for _, addr := range alloc.owned[ident] { - if subnet.Contains(addr) { + if r.Contains(addr) { resultChan <- allocateResult{addr: addr} return } @@ -201,7 +199,7 @@ func (alloc *Allocator) Lookup(ident string, subnet address.CIDR) (address.CIDR, resultChan <- allocateResult{err: fmt.Errorf("lookup: no address found")} } result := <-resultChan - return address.CIDR{Start: result.addr, PrefixLen: subnet.PrefixLen}, result.err + return result.addr, result.err } // Claim an address that we think we should own (Sync) diff --git a/ipam/allocator_test.go b/ipam/allocator_test.go index 03b30f83..57a66b2f 100644 --- a/ipam/allocator_test.go +++ b/ipam/allocator_test.go @@ -27,8 +27,8 @@ func TestAllocFree(t *testing.T) { universe = "10.0.3.0/26" subnet1 = "10.0.3.0/28" subnet2 = "10.0.3.32/28" - testAddr1 = "10.0.3.1/28" - testAddr2 = "10.0.3.33/28" + testAddr1 = "10.0.3.1" + testAddr2 = "10.0.3.33" spaceSize = 62 // 64 IP addresses in /26, minus .0 and .63 ) @@ -38,31 +38,31 @@ func TestAllocFree(t *testing.T) { _, cidr2, _ := address.ParseCIDR(subnet2) alloc.claimRingForTesting() - addr1, err := alloc.Allocate(container1, cidr1, nil) + addr1, err := alloc.Allocate(container1, cidr1.HostRange(), nil) wt.AssertNoErr(t, err) wt.AssertEqualString(t, addr1.String(), testAddr1, "address") - addr2, err := alloc.Allocate(container1, cidr2, nil) + addr2, err := alloc.Allocate(container1, cidr2.HostRange(), nil) wt.AssertNoErr(t, err) wt.AssertEqualString(t, addr2.String(), testAddr2, "address") // Ask for another address for a different container and check it's different - addr1b, _ := alloc.Allocate(container2, cidr1, nil) + addr1b, _ := alloc.Allocate(container2, cidr1.HostRange(), nil) 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, nil) + addr1a, _ := alloc.Allocate(container1, cidr1.HostRange(), nil) wt.AssertEqualString(t, addr1a.String(), testAddr1, "address") - addr2a, _ := alloc.Allocate(container1, cidr2, nil) + addr2a, _ := alloc.Allocate(container1, cidr2.HostRange(), nil) wt.AssertEqualString(t, addr2a.String(), testAddr2, "address") // Now delete the first container, and we should get its addresses back wt.AssertSuccess(t, alloc.Delete(container1)) - addr3, _ := alloc.Allocate(container3, cidr1, nil) + addr3, _ := alloc.Allocate(container3, cidr1.HostRange(), nil) wt.AssertEqualString(t, addr3.String(), testAddr1, "address") - addr4, _ := alloc.Allocate(container3, cidr2, nil) + addr4, _ := alloc.Allocate(container3, cidr2.HostRange(), nil) wt.AssertEqualString(t, addr4.String(), testAddr2, "address") alloc.ContainerDied(container2) @@ -134,7 +134,7 @@ func TestAllocatorClaim(t *testing.T) { container2 = "baddf00d" container3 = "b01df00d" universe = "10.0.3.0/30" - testAddr1 = "10.0.3.1/30" // first address allocated should be .1 because .0 is network addr + testAddr1 = "10.0.3.1" // first address allocated should be .1 because .0 is network addr ) alloc, subnet := makeAllocatorWithMockGossip(t, "01:00:00:01:00:00", universe, 1) @@ -384,7 +384,7 @@ func TestAllocatorFuzz(t *testing.T) { alloc := allocs[res.alloc] //common.Info.Printf("Freeing %s (%s) on allocator %d", res.name, addr, res.alloc) - oldAddr, _, err := address.ParseCIDR(addr) + oldAddr, err := address.ParseIP(addr) if err != nil { panic(err) } @@ -406,8 +406,8 @@ func TestAllocatorFuzz(t *testing.T) { //common.Info.Printf("Asking for %s on allocator %d again", addr, res.alloc) newAddr, _ := alloc.Allocate(res.name, subnet, nil) - oldAddr, _, _ := address.ParseCIDR(addr) - if newAddr.Start != oldAddr { + oldAddr, _ := address.ParseIP(addr) + if newAddr != oldAddr { panic(fmt.Sprintf("Got different address for repeat request for %s: %s != %s", res.name, newAddr, oldAddr)) } diff --git a/ipam/http.go b/ipam/http.go index cdb858a0..a9de0988 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -41,21 +41,21 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CID badRequest(w, err) return } - addr, err := alloc.Lookup(vars["id"], subnet) + addr, err := alloc.Lookup(vars["id"], subnet.HostRange()) if err != nil { http.NotFound(w, r) return } - fmt.Fprint(w, addr) + fmt.Fprintf(w, "%s/%d", addr, subnet.PrefixLen) }) router.Methods("GET").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - addr, err := alloc.Lookup(mux.Vars(r)["id"], defaultSubnet) + addr, err := alloc.Lookup(mux.Vars(r)["id"], defaultSubnet.HostRange()) if err != nil { http.NotFound(w, r) return } - fmt.Fprint(w, addr) + fmt.Fprintf(w, "%s/%d", addr, defaultSubnet.PrefixLen) }) router.Methods("POST").Path("/ip/{id}/{ip}/{prefixlen}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -72,25 +72,25 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CID badRequest(w, fmt.Errorf("Invalid subnet %s - bits after network prefix are not all zero", cidrStr)) return } - addr, err := alloc.Allocate(ident, cidr, closedChan) + addr, err := alloc.Allocate(ident, cidr.HostRange(), closedChan) if err != nil { badRequest(w, fmt.Errorf("Unable to allocate: %s", err)) return } - fmt.Fprint(w, addr) + fmt.Fprintf(w, "%s/%d", addr, cidr.PrefixLen) }) router.Methods("POST").Path("/ip/{id}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { closedChan := w.(http.CloseNotifier).CloseNotify() ident := mux.Vars(r)["id"] - newAddr, err := alloc.Allocate(ident, defaultSubnet, closedChan) + newAddr, err := alloc.Allocate(ident, defaultSubnet.HostRange(), closedChan) if err != nil { badRequest(w, err) return } - fmt.Fprint(w, newAddr) + fmt.Fprintf(w, "%s/%d", newAddr, defaultSubnet.PrefixLen) }) router.Methods("DELETE").Path("/ip/{id}/{ip}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/ipam/http_test.go b/ipam/http_test.go index bdf48277..b93360c6 100644 --- a/ipam/http_test.go +++ b/ipam/http_test.go @@ -80,8 +80,9 @@ func TestHttp(t *testing.T) { testAddr2 = "10.2.0.1/16" ) - alloc, subnet := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", universe, 1) - port := listenHTTP(alloc, subnet) + alloc, _ := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", universe, 1) + _, cidr, _ := address.ParseCIDR(universe) + port := listenHTTP(alloc, cidr) alloc.claimRingForTesting() // Allocate an address in each subnet, and check we got what we expected @@ -120,9 +121,10 @@ func TestBadHttp(t *testing.T) { testCIDR1 = "10.0.0.0/8" ) - alloc, subnet := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", testCIDR1, 1) + alloc, _ := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", testCIDR1, 1) defer alloc.Stop() - port := listenHTTP(alloc, subnet) + _, cidr, _ := address.ParseCIDR(testCIDR1) + port := listenHTTP(alloc, cidr) alloc.claimRingForTesting() cidr1 := HTTPPost(t, allocURL(port, testCIDR1, containerID)) @@ -155,10 +157,11 @@ func impTestHTTPCancel(t *testing.T) { testCIDR1 = "10.0.3.0/29" ) - alloc, subnet := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", testCIDR1, 1) + alloc, _ := makeAllocatorWithMockGossip(t, "08:00:27:01:c3:9a", testCIDR1, 1) defer alloc.Stop() alloc.claimRingForTesting() - port := listenHTTP(alloc, subnet) + _, cidr, _ := address.ParseCIDR(testCIDR1) + port := listenHTTP(alloc, cidr) // Stop the alloc so nothing actually works unpause := alloc.pause() diff --git a/ipam/testutils_test.go b/ipam/testutils_test.go index 8d7da37a..6fd369c7 100644 --- a/ipam/testutils_test.go +++ b/ipam/testutils_test.go @@ -128,7 +128,7 @@ func CheckAllExpectedMessagesSent(allocs ...*Allocator) { } } -func makeAllocator(name string, cidrStr string, quorum uint) (*Allocator, address.CIDR) { +func makeAllocator(name string, cidrStr string, quorum uint) (*Allocator, address.Range) { peername, err := router.PeerNameFromString(name) if err != nil { panic(err) @@ -142,10 +142,10 @@ func makeAllocator(name string, cidrStr string, quorum uint) (*Allocator, addres alloc := NewAllocator(peername, router.PeerUID(rand.Int63()), "nick-"+name, cidr.Range(), quorum) - return alloc, cidr + return alloc, cidr.HostRange() } -func makeAllocatorWithMockGossip(t *testing.T, name string, universeCIDR string, quorum uint) (*Allocator, address.CIDR) { +func makeAllocatorWithMockGossip(t *testing.T, name string, universeCIDR string, quorum uint) (*Allocator, address.Range) { alloc, subnet := makeAllocator(name, universeCIDR, quorum) gossip := &mockGossipComms{t: t, name: name} alloc.SetInterfaces(gossip) @@ -162,10 +162,10 @@ func (alloc *Allocator) claimRingForTesting(allocs ...*Allocator) { alloc.space.AddRanges(alloc.ring.OwnedRanges()) } -func (alloc *Allocator) NumFreeAddresses(subnet address.CIDR) address.Offset { +func (alloc *Allocator) NumFreeAddresses(r address.Range) address.Offset { resultChan := make(chan address.Offset) alloc.actionChan <- func() { - resultChan <- alloc.space.NumFreeAddressesInRange(address.MakeRange(subnet.Start+1, subnet.Size()-2)) + resultChan <- alloc.space.NumFreeAddressesInRange(r) } return <-resultChan } @@ -287,11 +287,11 @@ func (client TestGossipRouterClient) GossipBroadcast(update router.GossipData) e return client.router.GossipBroadcast(update) } -func makeNetworkOfAllocators(size int, cidr string) ([]*Allocator, TestGossipRouter, address.CIDR) { +func makeNetworkOfAllocators(size int, cidr string) ([]*Allocator, TestGossipRouter, address.Range) { gossipRouter := TestGossipRouter{make(map[router.PeerName]chan gossipMessage), 0.0} allocs := make([]*Allocator, size) - var subnet address.CIDR + var subnet address.Range for i := 0; i < size; i++ { var alloc *Allocator diff --git a/prog/weaver/main.go b/prog/weaver/main.go index 0ab87b66..0ce26cd3 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -214,7 +214,7 @@ func createAllocator(router *weave.Router, apiPath string, ipRangeStr string, de defaultSubnet := ipRange if defaultSubnetStr != "" { defaultSubnet = parseAndCheckCIDR(defaultSubnetStr) - if !ipRange.Overlaps(defaultSubnet) { + if !ipRange.Range().Overlaps(defaultSubnet.Range()) { log.Fatalf("Default subnet %s out of bounds: %s", defaultSubnet, ipRange) } } From a3d73be8b8431cdf363c2dcad0331dee61372037 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 9 Jun 2015 11:57:22 +0100 Subject: [PATCH 26/34] General tidying tidy Address helper functions, make ordering of members consistent in allocate and claim, make linter happier, remove some unused items, and update some comments --- ipam/address/address.go | 20 +++++++++++--------- ipam/allocate.go | 2 +- ipam/allocator.go | 7 ++++--- ipam/claim.go | 2 +- ipam/ring/ring.go | 16 ---------------- ipam/ring/ring_test.go | 4 ++++ ipam/space/space.go | 2 -- ipam/space/space_test.go | 31 +++++++++++++------------------ 8 files changed, 34 insertions(+), 50 deletions(-) diff --git a/ipam/address/address.go b/ipam/address/address.go index 194a09db..f97d44c8 100644 --- a/ipam/address/address.go +++ b/ipam/address/address.go @@ -15,11 +15,13 @@ type Range struct { Start, End Address // [Start, End); Start <= End } -func MakeRange(start Address, size Offset) Range { return Range{Start: start, End: Add(start, size)} } -func (r Range) Size() Offset { return Subtract(r.End, r.Start) } -func (r Range) String() string { return fmt.Sprintf("[%s-%s)", r.Start, r.End) } -func (r1 Range) Overlaps(r2 Range) bool { return !(r1.Start >= r2.End || r1.End <= r2.Start) } -func (r Range) Contains(addr Address) bool { return addr >= r.Start && addr < r.End } +func NewRange(start Address, size Offset) Range { + return Range{Start: start, End: Add(start, size)} +} +func (r Range) Size() Offset { return Subtract(r.End, r.Start) } +func (r Range) String() string { return fmt.Sprintf("[%s-%s)", r.Start, r.End) } +func (r Range) Overlaps(or Range) bool { return !(r.Start >= or.End || r.End <= or.Start) } +func (r Range) Contains(addr Address) bool { return addr >= r.Start && addr < r.End } type CIDR struct { Start Address @@ -46,12 +48,12 @@ func ParseCIDR(s string) (Address, CIDR, error) { func (cidr CIDR) Size() Offset { return 1 << uint(32-cidr.PrefixLen) } -// End is exclusive, same as in Range -func (cidr CIDR) End() Address { return Add(cidr.Start, cidr.Size()) } -func (cidr CIDR) Range() Range { return Range{Start: cidr.Start, End: Add(cidr.Start, cidr.Size())} } +func (cidr CIDR) Range() Range { + return NewRange(cidr.Start, cidr.Size()) +} func (cidr CIDR) HostRange() Range { // Respect RFC1122 exclusions of first and last addresses - return Range{Start: cidr.Start + 1, End: cidr.End() - 1} + return NewRange(cidr.Start+1, cidr.Size()-2) } func (cidr CIDR) String() string { diff --git a/ipam/allocate.go b/ipam/allocate.go index 59239b80..f4ffa2e9 100644 --- a/ipam/allocate.go +++ b/ipam/allocate.go @@ -12,9 +12,9 @@ type allocateResult struct { type allocate struct { resultChan chan<- allocateResult - hasBeenCancelled func() bool ident string r address.Range // Range we are trying to allocate within + hasBeenCancelled func() bool } // Try returns true if the request is completed, false if pending diff --git a/ipam/allocator.go b/ipam/allocator.go index 036b8e31..80a3ae73 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -176,17 +176,18 @@ func hasBeenCancelled(cancelChan <-chan bool) func() bool { // Actor client API -// Allocate (Sync) - get IP address for container with given name -// if there isn't any space we block indefinitely +// 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, cancelChan <-chan bool) (address.Address, error) { resultChan := make(chan allocateResult) - op := &allocate{r: r, resultChan: resultChan, ident: ident, + op := &allocate{resultChan: resultChan, ident: ident, r: r, hasBeenCancelled: hasBeenCancelled(cancelChan)} alloc.doOperation(op, &alloc.pendingAllocates) result := <-resultChan 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) alloc.actionChan <- func() { diff --git a/ipam/claim.go b/ipam/claim.go index f17bd477..8d96e6e2 100644 --- a/ipam/claim.go +++ b/ipam/claim.go @@ -9,9 +9,9 @@ import ( type claim struct { resultChan chan<- error - hasBeenCancelled func() bool ident string addr address.Address + hasBeenCancelled func() bool } // Try returns true for success (or failure), false if we need to try again later diff --git a/ipam/ring/ring.go b/ipam/ring/ring.go index 910c87c6..fcdb362e 100644 --- a/ipam/ring/ring.go +++ b/ipam/ring/ring.go @@ -91,18 +91,6 @@ func New(start, end address.Address, peer router.PeerName) *Ring { return ring } -// TotalRemoteFree returns the approximate number of free IPs -// on other hosts. -func (r *Ring) TotalRemoteFree() address.Offset { - result := address.Offset(0) - for _, entry := range r.Entries { - if entry.Peer != r.Peer { - result += entry.Free - } - } - return result -} - // Returns the distance between two tokens on this ring, dealing // with ranges which cross the origin func (r *Ring) distance(start, end address.Address) address.Offset { @@ -341,10 +329,6 @@ func (r *Ring) ClaimForPeers(peers []router.PeerName) { common.Assert(pos == r.End) } -func (r *Ring) ClaimItAll() { - r.ClaimForPeers([]router.PeerName{r.Peer}) -} - func (r *Ring) FprintWithNicknames(w io.Writer, m map[router.PeerName]string) { for _, entry := range r.Entries { nickname, found := m[entry.Peer] diff --git a/ipam/ring/ring_test.go b/ipam/ring/ring_test.go index 11aa47f9..ce63a38e 100644 --- a/ipam/ring/ring_test.go +++ b/ipam/ring/ring_test.go @@ -716,6 +716,10 @@ func TestFuzzRingHard(t *testing.T) { } } +func (r *Ring) ClaimItAll() { + r.ClaimForPeers([]router.PeerName{r.Peer}) +} + func (es entries) String() string { var buffer bytes.Buffer fmt.Fprintf(&buffer, "[") diff --git a/ipam/space/space.go b/ipam/space/space.go index 72b8b6ec..1d912370 100644 --- a/ipam/space/space.go +++ b/ipam/space/space.go @@ -9,8 +9,6 @@ import ( "github.com/weaveworks/weave/ipam/address" ) -type Addr address.Address - type Space struct { // ours and free represent a set of addresses as a sorted // sequences of ranges. Even elements give the inclusive diff --git a/ipam/space/space_test.go b/ipam/space/space_test.go index 8ab0f369..46a135c5 100644 --- a/ipam/space/space_test.go +++ b/ipam/space/space_test.go @@ -1,7 +1,6 @@ package space import ( - "reflect" "testing" "github.com/weaveworks/weave/ipam/address" @@ -62,12 +61,12 @@ func TestLowlevel(t *testing.T) { s := New() wt.AssertEquals(t, s.NumFreeAddresses(), address.Offset(0)) - ok, got := s.Allocate(address.MakeRange(0, 1000)) + ok, got := s.Allocate(address.NewRange(0, 1000)) wt.AssertFalse(t, ok, "allocate in empty space should fail") s.Add(100, 100) wt.AssertEquals(t, s.NumFreeAddresses(), address.Offset(100)) - ok, got = s.Allocate(address.MakeRange(0, 1000)) + ok, got = s.Allocate(address.NewRange(0, 1000)) wt.AssertTrue(t, ok && got == 100, "allocate") wt.AssertEquals(t, s.NumFreeAddresses(), address.Offset(99)) wt.AssertNoErr(t, s.Claim(150)) @@ -77,19 +76,19 @@ func TestLowlevel(t *testing.T) { wt.AssertErrorInterface(t, s.Free(0), (*error)(nil), "free not allocated") wt.AssertErrorInterface(t, s.Free(100), (*error)(nil), "double free") - r, ok := s.Donate(address.MakeRange(0, 1000)) + r, ok := s.Donate(address.NewRange(0, 1000)) wt.AssertTrue(t, ok && r.Start == 125 && r.Size() == 25, "donate") // test Donate when addresses are scarce s = New() - r, ok = s.Donate(address.MakeRange(0, 1000)) + r, ok = s.Donate(address.NewRange(0, 1000)) wt.AssertTrue(t, !ok, "donate on empty space should fail") s.Add(0, 3) wt.AssertNoErr(t, s.Claim(0)) wt.AssertNoErr(t, s.Claim(2)) - r, ok = s.Donate(address.MakeRange(0, 1000)) + r, ok = s.Donate(address.NewRange(0, 1000)) wt.AssertTrue(t, ok && r.Start == 1 && r.End == 2, "donate") - r, ok = s.Donate(address.MakeRange(0, 1000)) + r, ok = s.Donate(address.NewRange(0, 1000)) wt.AssertTrue(t, !ok, "donate should fail") } @@ -110,12 +109,12 @@ func TestSpaceAllocate(t *testing.T) { wt.AssertEquals(t, space1.NumFreeAddresses(), address.Offset(20)) space1.assertInvariants() - _, addr1 := space1.Allocate(address.MakeRange(start, size)) + _, addr1 := space1.Allocate(address.NewRange(start, size)) wt.AssertEqualString(t, addr1.String(), testAddr1, "address") wt.AssertEquals(t, space1.NumFreeAddresses(), address.Offset(19)) space1.assertInvariants() - _, addr2 := space1.Allocate(address.MakeRange(start, size)) + _, addr2 := space1.Allocate(address.NewRange(start, size)) wt.AssertFalse(t, addr2.String() == testAddr1, "address") wt.AssertEquals(t, space1.NumFreeAddresses(), address.Offset(18)) wt.AssertEquals(t, space1.NumFreeAddressesInRange(address.Range{Start: ip(testAddr1), End: ip(testAddrx)}), address.Offset(13)) @@ -140,7 +139,7 @@ func TestSpaceFree(t *testing.T) { containerID = "deadbeef" ) - entireRange := address.MakeRange(ip(testAddr1), 20) + entireRange := address.NewRange(ip(testAddr1), 20) space := makeSpace(ip(testAddr1), 20) // Check we are prepared to give up the entire space @@ -192,10 +191,6 @@ func TestSpaceFree(t *testing.T) { wt.AssertEquals(t, space.OwnedRanges(), []address.Range{address.Range{Start: ip("10.0.3.4"), End: ip("10.0.3.24")}}) } -func (s1 *Space) Equal(s2 *Space) bool { - return reflect.DeepEqual(s1.ours, s2.ours) && reflect.DeepEqual(s1.free, s2.free) -} - func TestDonateSimple(t *testing.T) { const ( testAddr1 = "10.0.1.0" @@ -210,7 +205,7 @@ func TestDonateSimple(t *testing.T) { ps1 := makeSpace(ipAddr1, size) // Empty space set should split in two and give me the second half - r, ok := ps1.Donate(address.MakeRange(ip(testAddr1), size)) + r, ok := ps1.Donate(address.NewRange(ip(testAddr1), size)) numGivenUp := r.Size() wt.AssertTrue(t, ok, "Donate result") wt.AssertEqualString(t, r.Start.String(), "10.0.1.24", "Invalid start") @@ -220,7 +215,7 @@ func TestDonateSimple(t *testing.T) { // Now check we can give the rest up. count := 0 // count to avoid infinite loop for ; count < 1000; count++ { - r, ok := ps1.Donate(address.MakeRange(ip(testAddr1), size)) + r, ok := ps1.Donate(address.NewRange(ip(testAddr1), size)) if !ok { break } @@ -240,7 +235,7 @@ func TestDonateHard(t *testing.T) { // Fill a fresh space spaceset := makeSpace(start, size) for i := address.Offset(0); i < size; i++ { - ok, _ := spaceset.Allocate(address.MakeRange(start, size)) + ok, _ := spaceset.Allocate(address.NewRange(start, size)) wt.AssertTrue(t, ok, "Failed to get IP!") } @@ -253,7 +248,7 @@ func TestDonateHard(t *testing.T) { } // Now split - newRange, ok := spaceset.Donate(address.MakeRange(start, size)) + newRange, ok := spaceset.Donate(address.NewRange(start, size)) wt.AssertTrue(t, ok, "GiveUpSpace result") wt.AssertEquals(t, newRange.Start, ip("10.0.1.23")) wt.AssertEquals(t, newRange.Size(), address.Offset(24)) From c301e6a38041abda04e524caf07bab3b7557b2ab Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 9 Jun 2015 15:19:31 +0100 Subject: [PATCH 27/34] Expand test to use IPAM and check addresses are freed --- test/500_weave_multi_cidr_test.sh | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/test/500_weave_multi_cidr_test.sh b/test/500_weave_multi_cidr_test.sh index 1e8a63a5..1ea8215c 100755 --- a/test/500_weave_multi_cidr_test.sh +++ b/test/500_weave_multi_cidr_test.sh @@ -11,7 +11,7 @@ assert_container_cidrs() { CIDRS="$@" # Assert container has attached CIDRs - assert_raises "weave_on $HOST ps | grep '^$CID [^ ]* $CIDRS$'" + assert_raises "weave_on $HOST ps $CID | grep '$CIDRS$'" } # assert_zone_records [ ...] @@ -42,36 +42,41 @@ assert_bridge_cidrs() { start_suite "Weave run/start/attach/detach with multiple cidr arguments" -weave_on $HOST1 launch +weave_on $HOST1 launch -debug -iprange 10.2.3.0/24 weave_on $HOST1 launch-dns 10.254.254.254/24 # Run container with three cidrs -CID=$(start_container $HOST1 10.2.1.1/24 10.2.2.1/24 10.2.3.1/24 --name=multicidr -h $NAME | cut -b 1-12) +CID=$(start_container $HOST1 10.2.1.1/24 10.2.2.1/24 --subnet 10.2.3.0/24 --name=multicidr -h $NAME | cut -b 1-12) assert_container_cidrs $HOST1 $CID 10.2.1.1/24 10.2.2.1/24 10.2.3.1/24 assert_zone_records $HOST1 $CID $NAME. 10.2.1.1 10.2.2.1 10.2.3.1 # Remove two of them -weave_on $HOST1 detach 10.2.1.1/24 10.2.3.1/24 $CID +weave_on $HOST1 detach 10.2.1.1/24 --subnet 10.2.3.0/24 $CID assert_container_cidrs $HOST1 $CID 10.2.2.1/24 assert_zone_records $HOST1 $CID $NAME. 10.2.2.1 # Put them both back -weave_on $HOST1 attach 10.2.1.1/24 10.2.3.1/24 $CID +weave_on $HOST1 attach 10.2.1.1/24 --subnet 10.2.3.0/24 $CID assert_container_cidrs $HOST1 $CID 10.2.2.1/24 10.2.1.1/24 10.2.3.1/24 assert_zone_records $HOST1 $CID $NAME. 10.2.2.1 10.2.1.1 10.2.3.1 # Stop the container, restart with three IPs docker_on $HOST1 stop $CID -weave_on $HOST1 start 10.2.1.1/24 10.2.2.1/24 10.2.3.1/24 $CID +weave_on $HOST1 start 10.2.1.1/24 10.2.2.1/24 --subnet 10.2.3.0/24 $CID assert_container_cidrs $HOST1 $CID 10.2.1.1/24 10.2.2.1/24 10.2.3.1/24 assert_zone_records $HOST1 $CID $NAME. 10.2.1.1 10.2.2.1 10.2.3.1 # Expose some cidrs -weave_on $HOST1 expose 10.2.1.2/24 10.2.2.2/24 10.2.3.2/24 +weave_on $HOST1 expose 10.2.1.2/24 10.2.2.2/24 --subnet 10.2.3.0/24 assert_bridge_cidrs $HOST1 weave 10.2.1.2/24 10.2.2.2/24 10.2.3.2/24 # Hide some cidrs -weave_on $HOST1 hide 10.2.1.2/24 10.2.3.2/24 +weave_on $HOST1 hide 10.2.1.2/24 --subnet 10.2.3.0/24 assert_bridge_cidrs $HOST1 weave 10.2.2.2/24 +# Now detach and run another container to check we have released IPs in IPAM +weave_on $HOST1 detach --subnet 10.2.3.0/24 $CID +CID2=$(start_container $HOST1 --subnet 10.2.3.0/24) +assert_container_cidrs $HOST1 $CID2 10.2.3.1/24 + end_suite From e9d194327addfa699d24d14d31c87b288647facb Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 9 Jun 2015 15:19:55 +0100 Subject: [PATCH 28/34] Reduce level of noise in debug logs from containers dying --- ipam/allocator.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index 80a3ae73..7ab81b31 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -214,8 +214,11 @@ func (alloc *Allocator) Claim(ident string, addr address.Address, cancelChan <-c // ContainerDied is provided to satisfy the updater interface; does a 'Delete' underneath. Sync. func (alloc *Allocator) ContainerDied(ident string) error { - alloc.debugln("Container", ident, "died; releasing addresses") - return alloc.Delete(ident) + err := alloc.Delete(ident) + if err == nil { + alloc.debugln("Container", ident, "died; released addresses") + } + return err } // Delete (Sync) - release all IP addresses for container with given name From 487bd2eaf745e0077cb4acc3a471019e2ad4783c Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 9 Jun 2015 15:22:04 +0100 Subject: [PATCH 29/34] Use zero-cidr-args form of 'weave detach' to test #862 --- test/500_weave_multi_cidr_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/500_weave_multi_cidr_test.sh b/test/500_weave_multi_cidr_test.sh index 1ea8215c..1f455bd6 100755 --- a/test/500_weave_multi_cidr_test.sh +++ b/test/500_weave_multi_cidr_test.sh @@ -75,7 +75,7 @@ weave_on $HOST1 hide 10.2.1.2/24 --subnet 10.2.3.0/24 assert_bridge_cidrs $HOST1 weave 10.2.2.2/24 # Now detach and run another container to check we have released IPs in IPAM -weave_on $HOST1 detach --subnet 10.2.3.0/24 $CID +weave_on $HOST1 detach $CID CID2=$(start_container $HOST1 --subnet 10.2.3.0/24) assert_container_cidrs $HOST1 $CID2 10.2.3.1/24 From 7d881a0f0c18a154ffa10f99dc31510426ffeca3 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 10 Jun 2015 14:03:48 +0100 Subject: [PATCH 30/34] Check that weave ps returns exactly the CIDRs we are looking for, and comment assumption --- test/500_weave_multi_cidr_test.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/500_weave_multi_cidr_test.sh b/test/500_weave_multi_cidr_test.sh index 1f455bd6..b400ad1d 100755 --- a/test/500_weave_multi_cidr_test.sh +++ b/test/500_weave_multi_cidr_test.sh @@ -11,7 +11,7 @@ assert_container_cidrs() { CIDRS="$@" # Assert container has attached CIDRs - assert_raises "weave_on $HOST ps $CID | grep '$CIDRS$'" + assert_raises "weave_on $HOST ps $CID | grep -E '^$CID [0-9a-f:]{17} $CIDRS$'" } # assert_zone_records [ ...] @@ -42,6 +42,10 @@ assert_bridge_cidrs() { start_suite "Weave run/start/attach/detach with multiple cidr arguments" +# NOTE: in these tests, --subnet arguments are checked against a +# specific address, i.e. we are assuming that IPAM always returns the +# lowest available address in the subnet + weave_on $HOST1 launch -debug -iprange 10.2.3.0/24 weave_on $HOST1 launch-dns 10.254.254.254/24 From c27cd5b4abaf6fb4cf6fa8709add74dc4399d5a4 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 10 Jun 2015 14:20:58 +0100 Subject: [PATCH 31/34] Extract lookup function for owned addresses --- ipam/allocate.go | 11 +++-------- ipam/allocator.go | 22 +++++++++++++++------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/ipam/allocate.go b/ipam/allocate.go index f4ffa2e9..130380e9 100644 --- a/ipam/allocate.go +++ b/ipam/allocate.go @@ -24,14 +24,9 @@ func (g *allocate) Try(alloc *Allocator) bool { return true } - // If we have previously stored an address for this container in this subnet, return it. - if addrs, found := alloc.owned[g.ident]; found { - for _, addr := range addrs { - if g.r.Contains(addr) { - g.resultChan <- allocateResult{addr, nil} - return true - } - } + if addr, found := alloc.lookupOwned(g.ident, g.r); found { + g.resultChan <- allocateResult{addr, nil} + return true } if !alloc.universe.Overlaps(g.r) { diff --git a/ipam/allocator.go b/ipam/allocator.go index 7ab81b31..e3edfcc7 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -48,7 +48,7 @@ type Allocator struct { 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 address, indexed by container-ID + owned map[string][]address.Address // who owns what addresses, indexed by container-ID nicknames map[router.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 @@ -191,13 +191,11 @@ func (alloc *Allocator) Allocate(ident string, r address.Range, cancelChan <-cha func (alloc *Allocator) Lookup(ident string, r address.Range) (address.Address, error) { resultChan := make(chan allocateResult) alloc.actionChan <- func() { - for _, addr := range alloc.owned[ident] { - if r.Contains(addr) { - resultChan <- allocateResult{addr: addr} - return - } + if addr, found := alloc.lookupOwned(ident, r); found { + resultChan <- allocateResult{addr: addr} + return } - resultChan <- allocateResult{err: fmt.Errorf("lookup: no address found")} + resultChan <- allocateResult{err: fmt.Errorf("lookup: no address found for %s in range %s", ident, r)} } result := <-resultChan return result.addr, result.err @@ -689,10 +687,20 @@ func (alloc *Allocator) reportFreeSpace() { // Owned addresses +// 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) lookupOwned(ident string, r address.Range) (address.Address, bool) { + for _, addr := range alloc.owned[ident] { + if r.Contains(addr) { + return addr, true + } + } + return 0, false +} + func (alloc *Allocator) findOwner(addr address.Address) string { for ident, addrs := range alloc.owned { for _, candidate := range addrs { From 36f793c1f44cd3f0c229d679a30b7420121f7e09 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 10 Jun 2015 14:25:42 +0100 Subject: [PATCH 32/34] Delete entry from addresses map when slice is empty --- ipam/allocator.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ipam/allocator.go b/ipam/allocator.go index e3edfcc7..72ef3072 100644 --- a/ipam/allocator.go +++ b/ipam/allocator.go @@ -250,8 +250,11 @@ func (alloc *Allocator) Free(ident string, addrToFree address.Address) error { for i, ownedAddr := range addrs { if ownedAddr == addrToFree { alloc.debugln("Freed", addrToFree, "for", ident) - addrs = append(addrs[:i], addrs[i+1:]...) - alloc.owned[ident] = addrs + if len(addrs) == 1 { + delete(alloc.owned, ident) + } else { + alloc.owned[ident] = append(addrs[:i], addrs[i+1:]...) + } alloc.space.Free(addrToFree) errChan <- nil return From 91ad3ef6843fa79276a4b621ba6fc30910bd29f9 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 10 Jun 2015 14:41:16 +0100 Subject: [PATCH 33/34] Illustrate mix-and-match of ipam and non-ipam CIDRs on command-line --- site/ipam.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/site/ipam.md b/site/ipam.md index 92a0142e..f97086e7 100644 --- a/site/ipam.md +++ b/site/ipam.md @@ -39,6 +39,11 @@ the default, use `--subnet`: To explicitly request the default subnet, use `--subnet default`. +And, you can ask for multiple addresses in different subnets and add +in manually-assigned addresses on the same line, for instance: + + host1# C=$(weave run --subnet 10.2.7.0/24 --subnet 10.2.8.0/24 10.3.9.1/24 -ti ubuntu) + Note that `-iprange` can be smaller than `-ipsubnet`, if you want to use a mixture of automatically-allocated addresses and manually-chosen addresses, and have the containers communicate with each other. For From a89b549c40dd3e3e51b791596c702e08dc8b1d85 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 10 Jun 2015 15:44:43 +0100 Subject: [PATCH 34/34] Change command-line syntax to net: prefix instead of --subnet --- site/ipam.md | 8 ++++---- test/100_cross_hosts_test.sh | 8 ++++---- test/500_weave_multi_cidr_test.sh | 16 ++++++++-------- weave | 19 ++++++------------- 4 files changed, 22 insertions(+), 29 deletions(-) diff --git a/site/ipam.md b/site/ipam.md index f97086e7..c2b4856f 100644 --- a/site/ipam.md +++ b/site/ipam.md @@ -33,16 +33,16 @@ allocation, and `-ipsubnet` is the subnet that will be used when you don't explicitly specify one. Then, to request the allocation of an address from a subnet other than -the default, use `--subnet`: +the default, use the `net:` prefix: - host1# C=$(weave run --subnet 10.2.7.0/24 -ti ubuntu) + host1# C=$(weave run net:10.2.7.0/24 -ti ubuntu) -To explicitly request the default subnet, use `--subnet default`. +To explicitly request the default subnet, use `net:default`. And, you can ask for multiple addresses in different subnets and add in manually-assigned addresses on the same line, for instance: - host1# C=$(weave run --subnet 10.2.7.0/24 --subnet 10.2.8.0/24 10.3.9.1/24 -ti ubuntu) + host1# C=$(weave run net:10.2.7.0/24 net:10.2.8.0/24 ip:10.3.9.1/24 -ti ubuntu) Note that `-iprange` can be smaller than `-ipsubnet`, if you want to use a mixture of automatically-allocated addresses and manually-chosen diff --git a/test/100_cross_hosts_test.sh b/test/100_cross_hosts_test.sh index 425749e9..2266b9dd 100755 --- a/test/100_cross_hosts_test.sh +++ b/test/100_cross_hosts_test.sh @@ -14,16 +14,16 @@ weave_on $HOST1 launch -iprange $UNIVERSE -ipsubnet $SUBNET_1 weave_on $HOST2 launch -iprange $UNIVERSE -ipsubnet $SUBNET_1 $HOST1 start_container $HOST1 $C1/24 --name=c1 -start_container $HOST2 $C2/24 --name=c2 +start_container $HOST2 ip:$C2/24 --name=c2 assert_raises "exec_on $HOST1 c1 $PING $C2" start_container $HOST1 --name=c3 -start_container $HOST2 --subnet default --name=c4 +start_container $HOST2 net:default --name=c4 C4=$(container_ip $HOST2 c4) assert_raises "exec_on $HOST1 c3 $PING $C4" -start_container $HOST1 --subnet $SUBNET_2 --name=c5 -start_container $HOST2 --subnet $SUBNET_2 --name=c6 +start_container $HOST1 net:$SUBNET_2 --name=c5 +start_container $HOST2 net:$SUBNET_2 --name=c6 C6=$(container_ip $HOST2 c6) assert_raises "exec_on $HOST1 c5 $PING $C6" diff --git a/test/500_weave_multi_cidr_test.sh b/test/500_weave_multi_cidr_test.sh index b400ad1d..2d392715 100755 --- a/test/500_weave_multi_cidr_test.sh +++ b/test/500_weave_multi_cidr_test.sh @@ -42,7 +42,7 @@ assert_bridge_cidrs() { start_suite "Weave run/start/attach/detach with multiple cidr arguments" -# NOTE: in these tests, --subnet arguments are checked against a +# NOTE: in these tests, net: arguments are checked against a # specific address, i.e. we are assuming that IPAM always returns the # lowest available address in the subnet @@ -50,37 +50,37 @@ weave_on $HOST1 launch -debug -iprange 10.2.3.0/24 weave_on $HOST1 launch-dns 10.254.254.254/24 # Run container with three cidrs -CID=$(start_container $HOST1 10.2.1.1/24 10.2.2.1/24 --subnet 10.2.3.0/24 --name=multicidr -h $NAME | cut -b 1-12) +CID=$(start_container $HOST1 ip:10.2.1.1/24 10.2.2.1/24 net:10.2.3.0/24 --name=multicidr -h $NAME | cut -b 1-12) assert_container_cidrs $HOST1 $CID 10.2.1.1/24 10.2.2.1/24 10.2.3.1/24 assert_zone_records $HOST1 $CID $NAME. 10.2.1.1 10.2.2.1 10.2.3.1 # Remove two of them -weave_on $HOST1 detach 10.2.1.1/24 --subnet 10.2.3.0/24 $CID +weave_on $HOST1 detach 10.2.1.1/24 net:10.2.3.0/24 $CID assert_container_cidrs $HOST1 $CID 10.2.2.1/24 assert_zone_records $HOST1 $CID $NAME. 10.2.2.1 # Put them both back -weave_on $HOST1 attach 10.2.1.1/24 --subnet 10.2.3.0/24 $CID +weave_on $HOST1 attach ip:10.2.1.1/24 net:10.2.3.0/24 $CID assert_container_cidrs $HOST1 $CID 10.2.2.1/24 10.2.1.1/24 10.2.3.1/24 assert_zone_records $HOST1 $CID $NAME. 10.2.2.1 10.2.1.1 10.2.3.1 # Stop the container, restart with three IPs docker_on $HOST1 stop $CID -weave_on $HOST1 start 10.2.1.1/24 10.2.2.1/24 --subnet 10.2.3.0/24 $CID +weave_on $HOST1 start 10.2.1.1/24 ip:10.2.2.1/24 net:10.2.3.0/24 $CID assert_container_cidrs $HOST1 $CID 10.2.1.1/24 10.2.2.1/24 10.2.3.1/24 assert_zone_records $HOST1 $CID $NAME. 10.2.1.1 10.2.2.1 10.2.3.1 # Expose some cidrs -weave_on $HOST1 expose 10.2.1.2/24 10.2.2.2/24 --subnet 10.2.3.0/24 +weave_on $HOST1 expose 10.2.1.2/24 10.2.2.2/24 net:10.2.3.0/24 assert_bridge_cidrs $HOST1 weave 10.2.1.2/24 10.2.2.2/24 10.2.3.2/24 # Hide some cidrs -weave_on $HOST1 hide 10.2.1.2/24 --subnet 10.2.3.0/24 +weave_on $HOST1 hide 10.2.1.2/24 net:10.2.3.0/24 assert_bridge_cidrs $HOST1 weave 10.2.2.2/24 # Now detach and run another container to check we have released IPs in IPAM weave_on $HOST1 detach $CID -CID2=$(start_container $HOST1 --subnet 10.2.3.0/24) +CID2=$(start_container $HOST1 net:10.2.3.0/24) assert_container_cidrs $HOST1 $CID2 10.2.3.1/24 end_suite diff --git a/weave b/weave index 5bcf1b29..8f3153fb 100755 --- a/weave +++ b/weave @@ -55,7 +55,7 @@ usage() { echo "where = [:]" echo " = /" echo " = or weave internal peer ID" - echo " = | --subnet | --subnet default" + echo " = [ip:] | net: | net:default" echo "
= [tcp://][]:" exit 1 } @@ -106,15 +106,9 @@ validate_cidr() { collect_cidr_args() { CIDR_ARGS="" CIDR_ARG_COUNT=0 - while is_cidr "$1" || [ "$1" = "--subnet" ] ; do - CIDR_ARGS="$CIDR_ARGS $1" + while [ "$1" = "net:default" ] || is_cidr "$1" || is_cidr "${1#ip:}" || is_cidr "${1#net:}" ; do + CIDR_ARGS="$CIDR_ARGS ${1#ip:}" CIDR_ARG_COUNT=$((CIDR_ARG_COUNT + 1)) - # only accept 'default' after '--subnet', in case someone has a container called 'default' - if [ "$1" = "--subnet" -a "$2" = "default" ] ; then - CIDR_ARGS="$CIDR_ARGS $2" - CIDR_ARG_COUNT=$((CIDR_ARG_COUNT + 1)) - shift 1 - fi shift 1 done } @@ -776,12 +770,11 @@ ipam_cidrs() { ALL_CIDRS="$IPAM_CIDRS" fi while [ $# -gt 0 ] ; do - if [ $# -gt 1 -a "$1" = "--subnet" ] ; then - shift 1 - if [ "$1" = "default" ] ; then + if [ "${1%:*}" = "net" ] ; then + if [ "$1" = "net:default" ] ; then CIDR=$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID) else - CIDR=$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID/$1) + CIDR=$(http_call $CONTAINER_NAME $HTTP_PORT POST /ip/$CONTAINER_ID/"${1#net:}") fi if [ "$CIDR" = "404 page not found" ] ; then echo "IP address allocation must be enabled to use --subnet (use the -iprange option on 'weave launch')" >&2