From 6596c8b1a7fa540439562625203878e2b87b5da3 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Sat, 4 Jul 2015 19:13:13 +0100 Subject: [PATCH] make container aliveness check part of the ipam & dns API ...rather than conducting it implicitly when an ID "looks like" a container ID. Closes #971 --- common/docker/client.go | 16 ---------------- ipam/http.go | 4 ++-- nameserver/http.go | 2 +- weave | 22 ++++++++++++++++------ 4 files changed, 19 insertions(+), 25 deletions(-) diff --git a/common/docker/client.go b/common/docker/client.go index 71cdf316..6774c213 100644 --- a/common/docker/client.go +++ b/common/docker/client.go @@ -1,20 +1,10 @@ package docker import ( - "regexp" - "github.com/fsouza/go-dockerclient" . "github.com/weaveworks/weave/common" ) -// Regexp that matches container IDs -var containerIDregexp = regexp.MustCompile("^[a-f0-9]+$") - -// IsContainerID returns True if the string provided is a valid container id -func IsContainerID(idStr string) bool { - return containerIDregexp.MatchString(idStr) -} - // An observer for container events type ContainerObserver interface { ContainerDied(ident string) error @@ -62,12 +52,6 @@ func (c *Client) AddObserver(ob ContainerObserver) error { // 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) { - Log.Debugf("[docker] '%s' does not seem to be a container id", idStr) - return false - } - - // check with Docker whether the container is really running container, err := c.InspectContainer(idStr) if err == nil { return !container.State.Running diff --git a/ipam/http.go b/ipam/http.go index 0b665385..911196aa 100644 --- a/ipam/http.go +++ b/ipam/http.go @@ -78,7 +78,7 @@ 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) { + if r.FormValue("check-alive") == "true" && dockerCli != nil && dockerCli.IsContainerNotRunning(ident) { common.Log.Infof("[allocator] '%s' is not running: freeing %s", ident, addr) alloc.Free(ident, addr) return @@ -95,7 +95,7 @@ func (alloc *Allocator) HandleHTTP(router *mux.Router, defaultSubnet address.CID badRequest(w, err) return } - if dockerCli != nil && dockerCli.IsContainerNotRunning(ident) { + if r.FormValue("check-alive") == "true" && dockerCli != nil && dockerCli.IsContainerNotRunning(ident) { common.Log.Infof("[allocator] '%s' is not running: freeing %s", ident, newAddr) alloc.Free(ident, newAddr) return diff --git a/nameserver/http.go b/nameserver/http.go index 42dd3231..02e1d380 100644 --- a/nameserver/http.go +++ b/nameserver/http.go @@ -74,7 +74,7 @@ func ServeHTTP(listener net.Listener, version string, server *DNSServer, dockerC } // oh, I already know this. whatever. } - if dockerCli != nil && dockerCli.IsContainerNotRunning(idStr) { + if r.FormValue("check-alive") == "true" && dockerCli != nil && dockerCli.IsContainerNotRunning(idStr) { Log.Infof("[http] '%s' is not running: removing", idStr) server.Zone.DeleteRecords(idStr, name, ip) } diff --git a/weave b/weave index adb97c8d..20564452 100755 --- a/weave +++ b/weave @@ -767,12 +767,17 @@ with_container_fqdn() { # Register FQDN in $2 as names for addresses $3.. under full container ID $1 put_dns_fqdn() { + CHECK_ALIVE="-d check-alive=true" + if [ "$1" = "--no-check-alive" ] ; then + CHECK_ALIVE= + shift 1 + fi CONTAINER_ID="$1" FQDN="$2" shift 2 for ADDR in "$@" ; do - call_dns PUT /name/$CONTAINER_ID/${ADDR%/*} --data-urlencode fqdn=$FQDN || true + call_dns PUT /name/$CONTAINER_ID/${ADDR%/*} --data-urlencode fqdn=$FQDN $CHECK_ALIVE || true done } @@ -827,13 +832,18 @@ ipam_claim() { # $1 is the full container id and following args are previously parsed CIDR_ARGS. # Returns ALL_CIDRS and IPAM_CIDRS ipam_cidrs() { + CHECK_ALIVE="?check-alive=true" + if [ "$1" = "--no-check-alive" ] ; then + CHECK_ALIVE= + shift 1 + fi CONTAINER_ID="$1" shift 1 ALL_CIDRS="" IPAM_CIDRs="" if [ $# -eq 0 ] ; then # If no addresses passed in, get one in the default subnet - IPAM_CIDRS=$(call_weave POST /ip/$CONTAINER_ID) || return 1 + IPAM_CIDRS=$(call_weave POST /ip/$CONTAINER_ID$CHECK_ALIVE) || return 1 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 @@ -847,7 +857,7 @@ ipam_cidrs() { else IPAM_URL=/ip/$CONTAINER_ID/"${1#net:}" fi - CIDR=$(call_weave POST $IPAM_URL) || return 1 + CIDR=$(call_weave POST $IPAM_URL$CHECK_ALIVE) || return 1 if [ "$CIDR" = "404 page not found" ] ; then echo "IP address allocation must be enabled to use 'net:'" >&2 return 1 @@ -1264,7 +1274,7 @@ case "$COMMAND" in expose) collect_cidr_args "$@" shift $CIDR_ARG_COUNT - ipam_cidrs weave:expose $CIDR_ARGS + ipam_cidrs --no-check-alive weave:expose $CIDR_ARGS if [ $# -eq 0 ] ; then FQDN="" else @@ -1279,7 +1289,7 @@ case "$COMMAND" in add_iptables_rule nat WEAVE -d $CIDR ! -s $CIDR -j MASQUERADE add_iptables_rule nat WEAVE -s $CIDR ! -d $CIDR -j MASQUERADE if [ -n "$FQDN" ] ; then - when_dns_running put_dns_fqdn weave:expose $FQDN $CIDR + when_dns_running put_dns_fqdn --no-check-alive weave:expose $FQDN $CIDR fi fi done @@ -1288,7 +1298,7 @@ case "$COMMAND" in hide) collect_cidr_args "$@" shift $CIDR_ARG_COUNT - ipam_cidrs weave:expose $CIDR_ARGS + ipam_cidrs --no-check-alive weave:expose $CIDR_ARGS create_bridge --without-ethtool for CIDR in $ALL_CIDRS ; do if ip addr show dev $BRIDGE | grep -qF $CIDR ; then