From a617e7918c9964f321df7c402ec6e344b2fc35b9 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 3 Jun 2015 11:25:53 +0100 Subject: [PATCH] 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 }