Merge pull request #907 from bboreham/819-check-its-alive

Free allocated address if container is not running

Fixes #819.

common/docker/client.go

@@ -60,16 +60,21 @@ func (c *Client) AddObserver(ob ContainerObserver) error {
 	return nil
 }
 
-// IsContainerRunning returns true if the container is up and running
-func (c *Client) IsContainerRunning(idStr string) (bool, error) {
-	// check with Docker whether the container is really running
-	container, err := c.InspectContainer(idStr)
-	if err != nil {
-		if _, notThere := err.(*docker.NoSuchContainer); notThere {
-			return false, nil
-		}
-		return false, err
+// IsContainerNotRunning returns true if we have checked with Docker that the ID is not running
+func (c *Client) IsContainerNotRunning(idStr string) bool {
+	if !IsContainerID(idStr) {
+		Debug.Printf("[docker] '%s' does not seem to be a container id", idStr)
+		return false
 	}
 
-	return container.State.Running, nil
+	// check with Docker whether the container is really running
+	container, err := c.InspectContainer(idStr)
+	if err == nil {
+		return !container.State.Running
+	}
+	if _, notThere := err.(*docker.NoSuchContainer); notThere {
+		return true
+	}
+	Error.Printf("[docker] Could not check container status: %s", err)
+	return false
 }

ipam/http.go

@@ -7,6 +7,7 @@ import (
 	"github.com/gorilla/mux"
 
 	"github.com/weaveworks/weave/common"
+	"github.com/weaveworks/weave/common/docker"
 	"github.com/weaveworks/weave/ipam/address"
 )
 
@@ -16,7 +17,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, defaultSubnet address.CIDR) {
+func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CIDR, dockerCli *docker.Client) {
 	router.Methods("PUT").Path("/ip/{id}/{ip}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		closedChan := w.(http.CloseNotifier).CloseNotify()
 		vars := mux.Vars(r)
@@ -77,6 +78,11 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CID
 			badRequest(w, fmt.Errorf("Unable to allocate: %s", err))
 			return
 		}
+		if dockerCli != nil && dockerCli.IsContainerNotRunning(ident) {
+			common.Info.Printf("[allocator] '%s' is not running: freeing %s", ident, addr)
+			alloc.Free(ident, addr)
+			return
+		}
 
 		fmt.Fprintf(w, "%s/%d", addr, cidr.PrefixLen)
 	})
@@ -89,6 +95,11 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CID
 			badRequest(w, err)
 			return
 		}
+		if dockerCli != nil && dockerCli.IsContainerNotRunning(ident) {
+			common.Info.Printf("[allocator] '%s' is not running: freeing %s", ident, newAddr)
+			alloc.Free(ident, newAddr)
+			return
+		}
 
 		fmt.Fprintf(w, "%s/%d", newAddr, defaultSubnet.PrefixLen)
 	})

ipam/http_test.go

@@ -44,7 +44,7 @@ func listenHTTP(alloc *Allocator, subnet address.CIDR) int {
 	router.HandleFunc("/status", func(w http.ResponseWriter, r *http.Request) {
 		io.WriteString(w, fmt.Sprintln(alloc))
 	})
-	alloc.HandleHTTP(router, subnet)
+	alloc.HandleHTTP(router, subnet, nil)
 
 	httpListener, err := net.Listen("tcp", ":0")
 	if err != nil {

nameserver/http.go

@@ -75,18 +75,9 @@ func ServeHTTP(listener net.Listener, version string, server *DNSServer, dockerC
 			} // oh, I already know this. whatever.
 		}
 
-		if dockerCli != nil {
-			if !docker.IsContainerID(idStr) {
-				Debug.Printf("[http] '%s' does not seem to be a container id", idStr)
-				return
-			}
-
-			if running, err := dockerCli.IsContainerRunning(idStr); err != nil {
-				Error.Printf("[http] Could not check container status: %s", err)
-			} else if !running {
-				Info.Printf("[http] '%s' is not running: removing", idStr)
-				server.Zone.DeleteRecords(idStr, name, ip)
-			}
+		if dockerCli != nil && dockerCli.IsContainerNotRunning(idStr) {
+			Info.Printf("[http] '%s' is not running: removing", idStr)
+			server.Zone.DeleteRecords(idStr, name, ip)
 		}
 	})
 

prog/weaver/main.go

@@ -141,8 +141,16 @@ func main() {
 
 	var allocator *ipam.Allocator
 	var defaultSubnet address.CIDR
+	var dockerCli *docker.Client
 	if iprangeCIDR != "" {
-		allocator, defaultSubnet = createAllocator(router, apiPath, iprangeCIDR, ipsubnetCIDR, determineQuorum(peerCount, peers))
+		allocator, defaultSubnet = createAllocator(router, iprangeCIDR, ipsubnetCIDR, determineQuorum(peerCount, peers))
+		dockerCli, err = docker.NewClient(apiPath)
+		if err != nil {
+			Error.Fatal("Unable to start docker client: ", err)
+		}
+		if err = dockerCli.AddObserver(allocator); err != nil {
+			Error.Fatal("Unable to start watcher", err)
+		}
 	} else if peerCount > 0 {
 		log.Fatal("-initpeercount flag specified without -iprange")
 	} else {
@@ -158,7 +166,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, defaultSubnet)
+		go handleHTTP(router, httpAddr, allocator, defaultSubnet, dockerCli)
 	}
 
 	SignalHandlerLoop(router)
@@ -209,7 +217,7 @@ func parseAndCheckCIDR(cidrStr string) address.CIDR {
 	return cidr
 }
 
-func createAllocator(router *weave.Router, apiPath string, ipRangeStr string, defaultSubnetStr string, quorum uint) (*ipam.Allocator, address.CIDR) {
+func createAllocator(router *weave.Router, ipRangeStr string, defaultSubnetStr string, quorum uint) (*ipam.Allocator, address.CIDR) {
 	ipRange := parseAndCheckCIDR(ipRangeStr)
 	defaultSubnet := ipRange
 	if defaultSubnetStr != "" {
@@ -223,14 +231,6 @@ func createAllocator(router *weave.Router, apiPath string, ipRangeStr string, de
 	allocator.SetInterfaces(router.NewGossip("IPallocation", allocator))
 	allocator.Start()
 
-	dockerCli, err := docker.NewClient(apiPath)
-	if err != nil {
-		Error.Fatal("Unable to start docker client: ", err)
-	}
-	if err = dockerCli.AddObserver(allocator); err != nil {
-		Error.Fatal("Unable to start watcher", err)
-	}
-
 	return allocator, defaultSubnet
 }
 
@@ -254,11 +254,11 @@ func determineQuorum(initPeerCountFlag int, peers []string) uint {
 	return quorum
 }
 
-func handleHTTP(router *weave.Router, httpAddr string, allocator *ipam.Allocator, defaultSubnet address.CIDR) {
+func handleHTTP(router *weave.Router, httpAddr string, allocator *ipam.Allocator, defaultSubnet address.CIDR, docker *docker.Client) {
 	muxRouter := mux.NewRouter()
 
 	if allocator != nil {
-		allocator.HandleHTTP(muxRouter, defaultSubnet)
+		allocator.HandleHTTP(muxRouter, defaultSubnet, docker)
 	}
 
 	muxRouter.Methods("GET").Path("/status").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {