Merge branch '1.5'

ipam/allocator.go

@@ -278,7 +278,7 @@ func (alloc *Allocator) Lookup(ident string, r address.Range) ([]address.CIDR, e
 }
 
 // Claim an address that we think we should own (Sync)
-func (alloc *Allocator) Claim(ident string, cidr address.CIDR, isContainer, noErrorOnUnknown bool) error {
+func (alloc *Allocator) Claim(ident string, cidr address.CIDR, isContainer, noErrorOnUnknown bool, hasBeenCancelled func() bool) error {
 	resultChan := make(chan error)
 	op := &claim{
 		resultChan:       resultChan,
@@ -286,6 +286,7 @@ func (alloc *Allocator) Claim(ident string, cidr address.CIDR, isContainer, noEr
 		cidr:             cidr,
 		isContainer:      isContainer,
 		noErrorOnUnknown: noErrorOnUnknown,
+		hasBeenCancelled: hasBeenCancelled,
 	}
 	alloc.doOperation(op, &alloc.pendingClaims)
 	return <-resultChan

ipam/allocator_test.go

@@ -27,7 +27,7 @@ func (alloc *Allocator) SimplyAllocate(ident string, cidr address.CIDR) (address
 }
 
 func (alloc *Allocator) SimplyClaim(ident string, cidr address.CIDR) error {
-	return alloc.Claim(ident, cidr, true, true)
+	return alloc.Claim(ident, cidr, true, true, returnFalse)
 }
 
 func TestAllocFree(t *testing.T) {

ipam/claim.go

@@ -15,6 +15,7 @@ type claim struct {
 	cidr             address.CIDR
 	isContainer      bool
 	noErrorOnUnknown bool
+	hasBeenCancelled func() bool
 }
 
 // Send an error (or nil for success) back to caller listening on resultChan
@@ -33,6 +34,11 @@ func (c *claim) sendResult(result error) {
 
 // Try returns true for success (or failure), false if we need to try again later
 func (c *claim) Try(alloc *Allocator) bool {
+	if c.hasBeenCancelled() {
+		c.Cancel()
+		return true
+	}
+
 	if !alloc.ring.Contains(c.cidr.Addr) {
 		// Address not within our universe; assume user knows what they are doing
 		alloc.infof("Address %s claimed by %s - not in our range", c.cidr, c.ident)
@@ -51,11 +57,10 @@ func (c *claim) Try(alloc *Allocator) bool {
 		// success
 	case mesh.UnknownPeerName:
 		// If our ring doesn't know, it must be empty.
+		alloc.infof("Claim %s for %s: is in the range %s, but the allocator is not initialized yet; will try later.",
+			c.cidr, c.ident, alloc.universe.AsCIDRString())
 		if c.noErrorOnUnknown {
-			alloc.infof("Claim %s for %s: address allocator still initializing; will try later.", c.cidr, c.ident)
 			c.sendResult(nil)
-		} else {
-			c.sendResult(fmt.Errorf("%s is in the range %s, but the allocator is not initialized yet", c.cidr, alloc.universe.AsCIDRString()))
 		}
 		return false
 	default:

ipam/http.go

@@ -38,32 +38,52 @@ func writeAddresses(w http.ResponseWriter, cidrs []address.CIDR) {
 	}
 }
 
+func hasBeenCanceled(dockerCli *docker.Client, closedChan <-chan bool, ident string, checkAlive bool) func() bool {
+	return func() bool {
+		select {
+		case <-closedChan:
+			return true
+		default:
+			res := checkAlive && dockerCli != nil && dockerCli.IsContainerNotRunning(ident)
+			checkAlive = false // we check only once; if the container dies later we learn about that through events
+			return res
+		}
+	}
+}
+
+func cancellationErr(w http.ResponseWriter, err error) bool {
+	if _, ok := err.(*errorCancelled); ok {
+		common.Log.Infoln("[allocator]:", err.Error())
+		fmt.Fprint(w, "cancelled")
+		return true
+	}
+	return false
+}
+
 func (alloc *Allocator) handleHTTPAllocate(dockerCli *docker.Client, w http.ResponseWriter, ident string, checkAlive bool, subnet address.CIDR) {
-	closedChan := w.(http.CloseNotifier).CloseNotify()
 	addr, err := alloc.Allocate(ident, subnet, checkAlive,
-		func() bool {
-			select {
-			case <-closedChan:
-				return true
-			default:
-				res := checkAlive && dockerCli != nil && dockerCli.IsContainerNotRunning(ident)
-				checkAlive = false // we check only once; if the container dies later we learn about that through events
-				return res
-			}
-		})
+		hasBeenCanceled(dockerCli, w.(http.CloseNotifier).CloseNotify(), ident, checkAlive))
 	if err != nil {
-		if _, ok := err.(*errorCancelled); ok { // cancellation is not really an error
-			common.Log.Infoln("[allocator]:", err.Error())
-			fmt.Fprint(w, "cancelled")
-			return
+		if !cancellationErr(w, err) {
+			badRequest(w, err)
 		}
-		badRequest(w, err)
 		return
 	}
-
 	fmt.Fprintf(w, "%s/%d", addr, subnet.PrefixLen)
 }
 
+func (alloc *Allocator) handleHTTPClaim(dockerCli *docker.Client, w http.ResponseWriter, ident string, cidr address.CIDR, checkAlive, noErrorOnUnknown bool) {
+	err := alloc.Claim(ident, cidr, checkAlive, noErrorOnUnknown,
+		hasBeenCanceled(dockerCli, w.(http.CloseNotifier).CloseNotify(), ident, checkAlive))
+	if err != nil {
+		if !cancellationErr(w, err) {
+			badRequest(w, fmt.Errorf("Unable to claim: %s", err))
+		}
+		return
+	}
+	w.WriteHeader(204)
+}
+
 // HandleHTTP wires up ipams HTTP endpoints to the provided mux.
 func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CIDR, dockerCli *docker.Client) {
 	router.Methods("GET").Path("/ipinfo/defaultsubnet").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -73,13 +93,10 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CID
 	router.Methods("PUT").Path("/ip/{id}/{ip}/{prefixlen}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		vars := mux.Vars(r)
 		if cidr, ok := parseCIDR(w, vars["ip"]+"/"+vars["prefixlen"], false); ok {
-			noErrorOnUnknown := r.FormValue("noErrorOnUnknown") == "true"
+			ident := vars["id"]
 			checkAlive := r.FormValue("check-alive") == "true"
-			if err := alloc.Claim(vars["id"], cidr, checkAlive, noErrorOnUnknown); err != nil {
-				badRequest(w, fmt.Errorf("Unable to claim: %s", err))
-				return
-			}
-			w.WriteHeader(204)
+			noErrorOnUnknown := r.FormValue("noErrorOnUnknown") == "true"
+			alloc.handleHTTPClaim(dockerCli, w, ident, cidr, checkAlive, noErrorOnUnknown)
 		}
 	})
 

test/320_claim_3_test.sh

@@ -2,6 +2,8 @@
 
 . ./config.sh
 
+UNIVERSE=10.32.0.0/12
+
 delete_persistence() {
     for host in "$@" ; do
         docker_on $host rm -v weavedb >/dev/null
@@ -52,4 +54,22 @@ weave_on $HOST3 prime
 stop_weave_on $HOST2
 weave_on $HOST1 launch-router $HOST3
 
+stop_weave_on $HOST1
+stop_weave_on $HOST2
+stop_weave_on $HOST3
+delete_persistence $HOST1 $HOST2 $HOST3
+
+weave_on $HOST1 launch --ipalloc-range $UNIVERSE $HOST2
+# Start another container on host1. The starting should block, because host1 is
+# not able to establish the ring due to host2 being offline.
+CMD="proxy_start_container $HOST1 --name c4 -e WEAVE_CIDR=10.32.0.4/12"
+assert_raises "timeout 5 cat <( $CMD )" 124
+# However, allocation for an external subnet should not block.
+assert_raises "proxy_start_container $HOST1 -e WEAVE_CIDR=10.48.0.1/12"
+
+# Launch host2, so that host1 can establish the ring.
+weave_on $HOST2 launch --ipalloc-range $UNIVERSE $HOST1
+weave_on $HOST1 prime
+assert_raises "[ $(container_ip $HOST1 c4) == 10.32.0.4 ]"
+
 end_suite