Merge pull request #706 from awh/dns_delete_individual_records

delete individual dns records

Closes #631. Closes #654.

nameserver/http.go

@@ -17,6 +17,13 @@ func httpErrorAndLog(level *log.Logger, w http.ResponseWriter, msg string,
 	level.Printf("[http] "+logmsg, logargs...)
 }
 
+func extractFQDN(r *http.Request) (string, string) {
+	if fqdnStr := r.FormValue("fqdn"); fqdnStr != "" {
+		return fqdnStr, fqdnStr
+	}
+	return "*", ""
+}
+
 func ServeHTTP(listener net.Listener, version string, server *DNSServer, domain string, db Zone) {
 
 	muxRouter := mux.NewRouter()
@@ -64,28 +71,39 @@ func ServeHTTP(listener net.Listener, version string, server *DNSServer, domain
 	})
 
 	muxRouter.Methods("DELETE").Path("/name/{id:.+}/{ip:.+}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		reqError := func(msg string, logmsg string, logargs ...interface{}) {
-			httpErrorAndLog(Warning, w, msg, http.StatusBadRequest, logmsg, logargs...)
-		}
-
 		vars := mux.Vars(r)
 		idStr := vars["id"]
 		ipStr := vars["ip"]
 
 		ip := net.ParseIP(ipStr)
 		if ip == nil {
-			reqError("Invalid IP in request", "Invalid IP in request: %s", ipStr)
+			httpErrorAndLog(
+				Warning, w, "Invalid IP in request", http.StatusBadRequest,
+				"Invalid IP in request: %s", ipStr)
 			return
 		}
-		Info.Printf("[http] Deleting %s (%s)", idStr, ipStr)
-		if err := db.DeleteRecord(idStr, ip); err != nil {
-			if _, ok := err.(LookupError); !ok {
-				httpErrorAndLog(
-					Error, w, "Internal error", http.StatusInternalServerError,
-					"Unexpected error from DB: %s", err)
-				return
-			}
-		}
+
+		fqdnStr, fqdn := extractFQDN(r)
+
+		Info.Printf("[http] Deleting ID %s, IP %s, FQDN %s", idStr, ipStr, fqdnStr)
+		db.DeleteRecords(idStr, fqdn, ip)
+	})
+
+	muxRouter.Methods("DELETE").Path("/name/{id:.+}").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		vars := mux.Vars(r)
+		idStr := vars["id"]
+
+		fqdnStr, fqdn := extractFQDN(r)
+
+		Info.Printf("[http] Deleting ID %s, IP *, FQDN %s", idStr, fqdnStr)
+		db.DeleteRecords(idStr, fqdn, net.IP{})
+	})
+
+	muxRouter.Methods("DELETE").Path("/name").HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fqdnStr, fqdn := extractFQDN(r)
+
+		Info.Printf("[http] Deleting ID *, IP *, FQDN %s", fqdnStr)
+		db.DeleteRecords("", fqdn, net.IP{})
 	})
 
 	http.Handle("/", muxRouter)

nameserver/mocks_test.go

@@ -83,12 +83,11 @@ func (mz *mockedZoneWithRecords) AddRecord(ident string, name string, ip net.IP)
 	notImplWarn()
 	return nil
 }
-func (mz *mockedZoneWithRecords) DeleteRecord(ident string, ip net.IP) error {
+func (mz *mockedZoneWithRecords) DeleteRecords(ident string, name string, ip net.IP) int {
 	notImplWarn()
-	return nil
+	return 0
 }
-func (mz *mockedZoneWithRecords) DeleteRecordsFor(ident string) error { notImplWarn(); return nil }
-func (mz *mockedZoneWithRecords) Status() string                      { notImplWarn(); return "nothing" }
+func (mz *mockedZoneWithRecords) Status() string { notImplWarn(); return "nothing" }
 func (mz *mockedZoneWithRecords) ObserveName(name string, observer ZoneRecordObserver) error {
 	notImplWarn()
 	return nil

nameserver/server_cache_test.go

@@ -103,7 +103,7 @@ func TestServerDbCacheInvalidation(t *testing.T) {
 	q, _ = assertExchange(t, testName1, dns.TypeA, testPort, 3, 4, 0)
 	assertInCache(t, cache, q, "after asking about the name")
 	Debug.Printf("... and removing one of the IP addresses")
-	zone.DeleteRecord(containerID, net.ParseIP("10.2.2.2"))
+	zone.DeleteRecords(containerID, "", net.ParseIP("10.2.2.2"))
 	assertNotInCache(t, cache, q, "after deleting IP for 10.2.2.2")
 
 	// Zone database at this point:
@@ -125,7 +125,7 @@ func TestServerDbCacheInvalidation(t *testing.T) {
 
 	// now we will check if a removal affects all the responses
 	Debug.Printf("... and removing an IP should invalidate both the cached responses for name and raddr")
-	zone.DeleteRecord(containerID, net.ParseIP("10.2.2.1"))
+	zone.DeleteRecords(containerID, "", net.ParseIP("10.2.2.1"))
 	assertNotInCache(t, cache, qptr, "after deleting record")
 	assertNotInCache(t, cache, qname, "after deleting record")
 	assertInCache(t, cache, qotherName, "after deleting record")
@@ -161,7 +161,7 @@ func TestServerDbCacheInvalidation(t *testing.T) {
 	//   first.weave.local  = 10.2.2.3 10.2.2.7
 	//   second.weave.local = 10.9.9.1
 
-	zone.DeleteRecordsFor(containerID)
+	zone.DeleteRecords(containerID, "", net.IP{})
 	assertNotInCache(t, cache, qotherName, "after removing container")
 	assertNotInCache(t, cache, qotherPtr, "after removing container")
 }
@@ -238,7 +238,7 @@ func TestServerCacheExpiration(t *testing.T) {
 
 	// We delete the IP in the second peer right after
 	Debug.Printf("Removing the IPs from %s in database #2", testName1)
-	dbs[1].Zone.DeleteRecord(containerID, net.ParseIP("10.2.2.1"))
+	dbs[1].Zone.DeleteRecords(containerID, "", net.ParseIP("10.2.2.1"))
 
 	// Check that we return to the initial state: a neg-local entry in the cache
 	qName1, _ = assertExchange(t, testName1, dns.TypeA, testPort, 0, 0, dns.RcodeNameError)
@@ -329,8 +329,8 @@ func TestServerCacheRefresh(t *testing.T) {
 	assertNotLocalInCache(t, cache, qName2, "after asking for a unknown name")
 
 	// delete the IPs, and some time passes by so the cache should be purged...
-	dbs[1].Zone.DeleteRecord(containerID, net.ParseIP("10.2.2.1"))
-	dbs[1].Zone.DeleteRecord(containerID, net.ParseIP("10.2.2.2"))
+	dbs[1].Zone.DeleteRecords(containerID, "", net.ParseIP("10.2.2.1"))
+	dbs[1].Zone.DeleteRecords(containerID, "", net.ParseIP("10.2.2.2"))
 	clk.Forward(refreshInterval + 1)
 
 	qName1, _ = assertExchange(t, testName1, dns.TypeA, testPort, 0, 0, dns.RcodeNameError)

nameserver/zone.go

@@ -49,10 +49,8 @@ type Zone interface {
 	Domain() string
 	// Add a record in the local database
 	AddRecord(ident string, name string, ip net.IP) error
-	// Delete a record in the local database
-	DeleteRecord(ident string, ip net.IP) error
-	// Delete all records for an ident in the local database
-	DeleteRecordsFor(ident string) error
+	// Delete matching records (uninitialised values act as wildcards)
+	DeleteRecords(ident string, name string, ip net.IP) (count int)
 	// Lookup for a name in the whole domain
 	DomainLookupName(name string) ([]ZoneRecord, error)
 	// Lookup for an address in the whole domain
@@ -194,15 +192,27 @@ func (n *name) addIP(zr ZoneRecord, now time.Time) (*recordEntry, error) {
 	return ne, nil
 }
 
-// Delete an IP for this name
-func (n *name) deleteIP(ip IPv4) bool {
-	if ipRecord, found := n.ipv4[ip]; found {
-		ipRecord.notifyIPObservers()
-		n.notifyNameObservers()
-		delete(n.ipv4, ip)
-		return true
+// Delete by IP. Pass uninitialized value for wildcard
+func (n *name) deleteIP(ip net.IP) int {
+	count := 0
+	if !ip.Equal(net.IP{}) {
+		ipv4 := ipToIPv4(ip)
+		if ipv4V, found := n.ipv4[ipv4]; found {
+			ipv4V.notifyIPObservers()
+			delete(n.ipv4, ipv4)
+			count++
+		}
+	} else {
+		for ipv4K, ipv4V := range n.ipv4 {
+			ipv4V.notifyIPObservers()
+			delete(n.ipv4, ipv4K)
+			count++
+		}
 	}
-	return false
+	if count > 0 {
+		n.notifyNameObservers()
+	}
+	return count
 }
 
 // Update the list of IPs for this name, adding new ones and removing old IPs...
@@ -332,36 +342,25 @@ func (ns *namesSet) addIPToName(zr ZoneRecord, now time.Time) (*recordEntry, err
 	return n.addIP(zr, now)
 }
 
-// Delete a name in the names set
-func (ns *namesSet) deleteName(n string) error {
-	if name, found := ns.names[n]; found {
-		for _, ipRecord := range name.getAllEntries() {
-			ipRecord.notifyIPObservers()
+// Delete by name and IP. Pass uninitialized values for wildcard
+func (ns namesSet) deleteNameIP(name string, ip net.IP) int {
+	count := 0
+	if name != "" {
+		if nameV, found := ns.names[name]; found {
+			count += nameV.deleteIP(ip)
+			if nameV.empty() {
+				delete(ns.names, name)
+			}
 		}
-		name.notifyNameObservers()
-		delete(ns.names, n)
-		return nil
-	}
-	return LookupError(n)
-}
-
-// Delete an IPv4 from all names
-func (ns *namesSet) deleteIP(ip IPv4) error {
-	cnt := 0
-	for nkey, n := range ns.names {
-		if n.deleteIP(ip) {
-			cnt++
-		}
-		if n.empty() {
-			delete(ns.names, nkey)
+	} else {
+		for nameK, nameV := range ns.names {
+			count += nameV.deleteIP(ip)
+			if nameV.empty() {
+				delete(ns.names, nameK)
+			}
 		}
 	}
-
-	// we must return an error if no records have been found and deleted
-	if cnt == 0 {
-		return LookupError(fmt.Sprintf("%+v", ip))
-	}
-	return nil
+	return count
 }
 
 // Get the name query time
@@ -592,30 +591,16 @@ func (zone *ZoneDb) AddRecord(ident string, name string, ip net.IP) (err error)
 	return
 }
 
-// Delete all records for an ident
-func (zone *ZoneDb) DeleteRecordsFor(ident string) (err error) {
-	zone.mx.Lock()
-	defer zone.mx.Unlock()
-	Debug.Printf("[zonedb] Deleting records for ident '%s'", ident)
-	zone.deleteIdent(ident)
-	return
-}
-
-// Delete an IP in a ident
-func (zone *ZoneDb) DeleteRecord(ident string, ip net.IP) (err error) {
-	zone.mx.Lock()
-	defer zone.mx.Unlock()
-
-	Debug.Printf("[zonedb] Deleting record '%s'/[%s]", ident, ip)
-	if _, found := zone.idents[ident]; found {
-		err = zone.idents[ident].deleteIP(ipToIPv4(ip))
-		if zone.idents[ident].empty() {
-			zone.deleteIdent(ident)
-		}
-	} else {
-		err = LookupError(ident)
+// Delete matching records (uninitialised values act as wildcards)
+func (zone *ZoneDb) DeleteRecords(ident string, name string, ip net.IP) int {
+	if name != "" {
+		name = dns.Fqdn(name)
 	}
-	return
+
+	zone.mx.Lock()
+	defer zone.mx.Unlock()
+
+	return zone.deleteIdentNameIP(ident, name, ip)
 }
 
 // Observe a name.
@@ -684,7 +669,8 @@ func (zone *ZoneDb) String() string {
 // Notify that a container has died
 func (zone *ZoneDb) ContainerDied(ident string) error {
 	Info.Printf("[zonedb] Container %s down. Removing records", ident)
-	return zone.DeleteRecordsFor(ident)
+	zone.DeleteRecords(ident, "", net.IP{})
+	return nil
 }
 
 // Return true if a remote name is still relevant
@@ -762,16 +748,23 @@ func (zone *ZoneDb) getNamesSet(ident string) *namesSet {
 	return ns
 }
 
-// Delete a ident
-func (zone *ZoneDb) deleteIdent(ident string) {
-	Info.Printf("[zonedb] Removing everything for ident '%s'", ident)
-	if ns, found := zone.idents[ident]; found {
-		for _, n := range ns.names {
-			for _, entry := range n.getAllEntries() {
-				entry.notifyIPObservers()
+// Delete by ident, name and IP. Pass uninitialized value for wildcard
+func (zone *ZoneDb) deleteIdentNameIP(ident string, name string, ip net.IP) int {
+	count := 0
+	if ident != "" {
+		if identV, found := zone.idents[ident]; found {
+			count += identV.deleteNameIP(name, ip)
+			if identV.empty() {
+				delete(zone.idents, ident)
+			}
+		}
+	} else {
+		for identK, identV := range zone.idents {
+			count += identV.deleteNameIP(name, ip)
+			if identV.empty() {
+				delete(zone.idents, identK)
 			}
-			n.notifyNameObservers()
 		}
-		delete(zone.idents, ident)
 	}
+	return count
 }

nameserver/zone_lookup.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/miekg/dns"
 	. "github.com/weaveworks/weave/common"
+	"net"
 )
 
 type uniqZoneRecordKey struct {
@@ -128,7 +129,7 @@ func (zone *ZoneDb) DomainLookupName(name string) (res []ZoneRecord, err error)
 			// locally introduced entries are never expired: they always have TTL=0
 			if ze.hasExpired(now) {
 				Debug.Printf("[zonedb] '%s': expired entry '%s' ignored: removing", name, ze)
-				nameset.deleteName(name)
+				nameset.deleteNameIP(name, net.IP{})
 			} else {
 				uniq.add(ze)
 			}
@@ -173,7 +174,7 @@ func (zone *ZoneDb) DomainLookupInaddr(inaddr string) (res []ZoneRecord, err err
 			// locally introduced entries are never expired: they always have TTL=0
 			if ze.hasExpired(now) {
 				Debug.Printf("[zonedb] '%s': expired entry '%s' ignored: removing", revIPv4, ze)
-				nameset.deleteIP(revIPv4)
+				nameset.deleteNameIP("", revIPv4.toNetIP())
 			} else {
 				uniq.add(ze)
 				if identName != defaultRemoteIdent {
@@ -273,7 +274,7 @@ func (zone *ZoneDb) updater(num int) {
 			if !zone.IsNameRelevant(request.name) || zone.IsNameExpired(request.name) {
 				Debug.Printf("[zonedb] '%s' seem to be irrelevant now: removing any remote information", request.name)
 				zone.mx.Lock()
-				zone.getNamesSet(defaultRemoteIdent).deleteName(request.name)
+				zone.getNamesSet(defaultRemoteIdent).deleteNameIP(request.name, net.IP{})
 				zone.mx.Unlock()
 				continue
 			}
@@ -300,7 +301,7 @@ func (zone *ZoneDb) updater(num int) {
 			} else {
 				Debug.Printf("[zonedb] nobody knows about '%s'... removing", name)
 				zone.mx.Lock()
-				zone.getNamesSet(defaultRemoteIdent).deleteName(request.name)
+				zone.getNamesSet(defaultRemoteIdent).deleteNameIP(request.name, net.IP{})
 				zone.mx.Unlock()
 			}
 		}

nameserver/zone_test.go

@@ -98,8 +98,8 @@ func TestZone(t *testing.T) {
 
 	t.Logf("Zone database:\n%s", zone)
 	t.Logf("Deleting the %s in %s", ip1, container1)
-	err = zone.DeleteRecord(container1, ip1)
-	wt.AssertNoErr(t, err)
+	count := zone.DeleteRecords(container1, "", ip1)
+	wt.AssertEqualInt(t, count, 1, "delete failed")
 	t.Logf("Zone database:\n%s", zone)
 
 	t.Logf("Checking %s's observers have been notified on removal", name1)
@@ -114,19 +114,20 @@ func TestZone(t *testing.T) {
 
 	t.Logf("Checking %s is not found after removing %s it in %s and %s in %s",
 		name1, ip1, container2, ip2, container1)
-	err = zone.DeleteRecord(container1, ip2)
-	err = zone.DeleteRecord(container2, ip1)
-	wt.AssertNoErr(t, err)
+	count = zone.DeleteRecords(container1, "", ip2)
+	wt.AssertEqualInt(t, count, 1, "delete failed")
+	count = zone.DeleteRecords(container2, "", ip1)
+	wt.AssertEqualInt(t, count, 1, "delete failed")
 	t.Logf("Zone database:\n%s", zone)
 	_, err = zone.LookupName(name1)
 	wt.AssertErrorType(t, err, (*LookupError)(nil), "after deleting record")
 
 	t.Logf("Checking if removing an unknown record results in an error")
-	err = zone.DeleteRecord(container1, net.ParseIP("0.0.0.0"))
-	wt.AssertErrorType(t, err, (*LookupError)(nil), "when deleting record that doesn't exist")
+	count = zone.DeleteRecords(container1, "", net.ParseIP("0.0.0.0"))
+	wt.AssertEqualInt(t, count, 0, "delete failed")
 }
 
-func TestDeleteFor(t *testing.T) {
+func TestDeleteRecords(t *testing.T) {
 	var (
 		id    = "foobar"
 		name  = "foo.weave."
@@ -151,7 +152,8 @@ func TestDeleteFor(t *testing.T) {
 	_, err = zone.LookupName(name)
 	wt.AssertNoErr(t, err)
 
-	err = zone.DeleteRecordsFor(id)
+	count := zone.DeleteRecords(id, "", net.IP{})
+	wt.AssertEqualInt(t, count, 2, "wildcard delete failed")
 	_, err = zone.LookupName(name)
 	wt.AssertErrorType(t, err, (*LookupError)(nil), "after deleting records for ident")
 }

test/240_dns_add_name_test.sh

@@ -6,6 +6,7 @@ C1=10.2.0.78
 C2=10.2.0.34
 NAME1=seeone.weave.local
 NAME2=seetwo.weave.local
+NAME3=seethree.weave.local
 
 start_suite "Add and remove names on a single host"
 
@@ -19,11 +20,14 @@ weave_on $HOST1 dns-add $C2 c2 -h $NAME2
 assert_dns_record $HOST1 c1 $NAME2 $C2
 
 weave_on $HOST1 dns-add $C1 c1 -h $NAME1
+weave_on $HOST1 dns-add $C1 c1 -h $NAME3
 
-assert "exec_on $HOST1 c1 getent hosts $C1 | tr -s ' '" "$C1 $NAME1"
+assert_dns_a_record $HOST1 c1 $NAME1 $C1
+assert_dns_a_record $HOST1 c1 $NAME3 $C1
 
-weave_on $HOST1 dns-remove $C1 c1
+weave_on $HOST1 dns-remove $C1 c1 -h $NAME1
 
 assert_no_dns_record $HOST1 c1 $NAME1
+assert_dns_a_record $HOST1 c1 $NAME3 $C1
 
 end_suite

test/250_dns_test_restart.sh

@@ -0,0 +1,18 @@
+#! /bin/bash
+
+. ./config.sh
+
+C1=10.2.0.78
+C2=10.2.0.34
+NAME=seetwo.weave.local
+
+start_suite "Repopulation on weaveDNS restart"
+
+start_container          $HOST1 $C2/24 --name=c2 -h $NAME
+start_container_with_dns $HOST1 $C1/24 --name=c1
+
+weave_on $HOST1 launch-dns 10.2.254.1/24
+
+assert_dns_record $HOST1 c1 $NAME $C2
+
+end_suite

test/config.sh

@@ -143,6 +143,16 @@ assert_no_dns_record() {
     assert_raises "exec_on $host $container getent hosts $name" 2
 }
 
+# assert_dns_a_record <host> <container> <name> <ip>
+assert_dns_a_record() {
+    assert "exec_on $1 $2 getent hosts $3 | tr -s ' '" "$4 $3"
+}
+
+# assert_dns_ptr_record <host> <container> <name> <ip>
+assert_dns_ptr_record() {
+    assert "exec_on $1 $2 getent hosts $4 | tr -s ' '" "$4 $3"
+}
+
 start_suite() {
     for host in $HOST1 $HOST2; do
         [ -z "$DEBUG" ] || echo "Cleaning up on $host: removing all containers and resetting weave"

weave

@@ -35,7 +35,7 @@ usage() {
     echo "weave attach       [<cidr> ...] <container_id>"
     echo "weave detach       <cidr> [<cidr> ...] <container_id>"
     echo "weave dns-add      <ip_address> [<ip_address> ...] <container_id> [-h <fqdn>]"
-    echo "weave dns-remove   <ip_address> [<ip_address> ...] <container_id>"
+    echo "weave dns-remove   <ip_address> [<ip_address> ...] <container_id> [-h <fqdn>]"
     echo "weave expose       [<cidr> ...] [-h <fqdn>]"
     echo "weave hide         [<cidr> ...]"
     echo "weave ps           [<container_id> ...]"
@@ -644,34 +644,69 @@ dns_args() {
     DNS_ARGS="$DNS_ARG $DNS_SEARCH_ARG"
 }
 
-# Perform operation $1 on local DNS database, for container ID $2
-# with an FQDN obtained from the container's configured hostname and
-# domain, at addresses $3, $4 ...
-tell_dns() {
-    METHOD="$1"
-    CONTAINER_ID="$2"
-    shift 2
-    CONTAINER_FQDN=$(docker inspect --format='{{.Config.Hostname}}.{{.Config.Domainname}}.' $CONTAINER_ID 2>/dev/null) || true
-    tell_dns_fqdn $METHOD $CONTAINER_ID $CONTAINER_FQDN $@
+# Print an error to stderr and return with an indicative exit status
+# if the weaveDNS container does not exist or isn't running.
+check_dns_running() {
+    if ! STATUS=$(docker inspect --format='{{.State.Running}}' $DNS_CONTAINER_NAME 2>/dev/null); then
+        echo  "$DNS_CONTAINER_NAME container is not present. Have you launched it?" >&2
+        return 1
+    elif [ "$STATUS" != "true" ] ; then
+        echo "$DNS_CONTAINER_NAME container is not running." >&2
+        return 2
+    fi
 }
 
-# Perform operation $1 on local DNS database, for container ID $2
-# with FQDN $3, at addresses $4, $5 ... This function is only called
-# where we know $2 is a valid container name
-tell_dns_fqdn() {
-    METHOD="$1"
-    CONTAINER_ID="$2"
-    CONTAINER_FQDN="$3"
-    shift 3
-    if ! status=$(docker inspect --format='{{.State.Running}}' $DNS_CONTAINER_NAME 2>/dev/null) || [ "$status" != "true" ] ; then
-        # weavedns not running - silently return
-        return
+# Execute $@ only if the weaveDNS container is running
+when_dns_running() {
+    if check_dns_running 2>/dev/null; then
+        "$@"
     fi
-    # get the long form of the container ID
-    CONTAINER=$(docker inspect --format='{{.Id}}' $CONTAINER_ID 2>/dev/null)
-    MORE_ARGS="--data-urlencode fqdn=$CONTAINER_FQDN"
+}
+
+# Iff the container in $1 has an FQDN, invoke $2 as a command passing
+# the container as the first argument, the FQDN as the second argument
+# and $3.. as additional arguments
+with_container_fqdn() {
+    CONTAINER="$1"
+    COMMAND="$2"
+    shift 2
+
+    CONTAINER_FQDN=$(docker inspect --format='{{.Config.Hostname}}.{{.Config.Domainname}}.' $CONTAINER 2>/dev/null) || return 0
+
+    if echo "$CONTAINER_FQDN" | grep -qv '\.\.$' ; then
+        $COMMAND "$CONTAINER" "$CONTAINER_FQDN" "$@"
+    fi
+}
+
+# Register FQDN in $2 as names for addresses $3.. under full container ID $1
+put_dns_fqdn() {
+    CONTAINER_ID="$1"
+    FQDN="$2"
+    shift 2
+
     for ADDR in "$@" ; do
-        http_call $DNS_CONTAINER_NAME $DNS_HTTP_PORT $METHOD /name/$CONTAINER/${ADDR%/*} $MORE_ARGS || true
+        http_call $DNS_CONTAINER_NAME $DNS_HTTP_PORT PUT /name/$CONTAINER_ID/${ADDR%/*} --data-urlencode fqdn=$FQDN || true
+    done
+}
+
+# Delete all names for addresses $3.. under full container ID $1
+delete_dns() {
+    CONTAINER_ID="$1"
+    shift 1
+
+    for ADDR in "$@" ; do
+        http_call $DNS_CONTAINER_NAME $DNS_HTTP_PORT DELETE /name/$CONTAINER_ID/${ADDR%/*} || true
+    done
+}
+
+# Delete any FQDNs $2 from addresses $3.. under full container ID $1
+delete_dns_fqdn() {
+    CONTAINER_ID="$1"
+    FQDN="$2"
+    shift 2
+
+    for ADDR in "$@" ; do
+        http_call $DNS_CONTAINER_NAME $DNS_HTTP_PORT DELETE /name/$CONTAINER_ID/${ADDR%/*}?fqdn=$FQDN || true
     done
 }
 
@@ -682,17 +717,9 @@ populate_dns() {
         return 0
     fi
     for CONTAINER in $(docker ps -q --no-trunc) ; do
-        MORE_ARGS=$(docker inspect --format='--data-urlencode fqdn={{.Config.Hostname}}.{{.Config.Domainname}}.' $CONTAINER 2>/dev/null) || true
         if CONTAINER_IPS=$(with_container_netns $CONTAINER container_weave_addrs 2>&1 | sed -n -e 's/inet \([^/]*\)\/\(.*\)/\1/p') ; then
-            for IP in $CONTAINER_IPS ; do
-                # NB: CONTAINER_IP is the IP of the weavedns
-                # container; it is set by wait_for_status.
-                if ! http_call_ip $CONTAINER_IP $DNS_HTTP_PORT PUT /name/$CONTAINER/$IP $MORE_ARGS ; then
-                    echo "Failed to fully populate DNS."
-                    return 1
-                fi
-            done
-        fi
+            with_container_fqdn $CONTAINER put_dns_fqdn $CONTAINER_IPS
+        fi 
     done
 }
 
@@ -920,7 +947,7 @@ case "$COMMAND" in
         create_bridge
         get_ipam_cidr $CONTAINER
         with_container_netns $CONTAINER attach $CIDR_ARGS >/dev/null
-        tell_dns PUT $CONTAINER $CIDR_ARGS
+        when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS
         echo $CONTAINER
         ;;
     dns-args)
@@ -939,7 +966,7 @@ case "$COMMAND" in
         create_bridge
         get_ipam_cidr $CONTAINER
         with_container_netns $CONTAINER attach $CIDR_ARGS >/dev/null
-        tell_dns PUT $CONTAINER $CIDR_ARGS
+        when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS
         echo $CONTAINER
         ;;
     attach)
@@ -950,35 +977,39 @@ case "$COMMAND" in
         create_bridge
         get_ipam_cidr $CONTAINER
         with_container_netns $CONTAINER attach $CIDR_ARGS >/dev/null
-        tell_dns PUT $CONTAINER $CIDR_ARGS
+        when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS
         ;;
     detach)
         collect_cidr_args "$@"
         shift $CIDR_COUNT
         [ $# -eq 1 ] || usage
-        CONTAINER_ID="$1"
-        with_container_netns $CONTAINER_ID detach $CIDR_ARGS >/dev/null
-        tell_dns DELETE $CONTAINER_ID $CIDR_ARGS
+        CONTAINER=$(container_id $1)
+        with_container_netns $CONTAINER detach $CIDR_ARGS >/dev/null
+        when_dns_running with_container_fqdn $CONTAINER delete_dns_fqdn $CIDR_ARGS
         ;;
     dns-add)
         collect_ip_args "$@"
         shift $IP_COUNT
-        [ $# -ge 1 ] || usage
-        CONTAINER_ID="$1"
+        [ $# -eq 1 -o \( $# -eq 3 -a "$2" = "-h" \) ] || usage
+        check_dns_running
+        CONTAINER=$(container_id $1)
         if [ $# -eq 1 ] ; then
-            tell_dns PUT $CONTAINER_ID $IP_ARGS
+            with_container_fqdn $CONTAINER put_dns_fqdn $IP_ARGS
         else
-            [ $# -eq 3 -a "$2" = "-h" ] || usage
-            FQDN="$3"
-            tell_dns_fqdn PUT $CONTAINER_ID $FQDN $IP_ARGS
+            put_dns_fqdn $CONTAINER "$3" $IP_ARGS
         fi
         ;;
     dns-remove)
         collect_ip_args "$@"
         shift $IP_COUNT
-        [ $# -eq 1 ] || usage
-        CONTAINER_ID="$1"
-        tell_dns DELETE $CONTAINER_ID $IP_ARGS
+        [ $# -eq 1 -o \( $# -eq 3 -a "$2" = "-h" \) ] || usage
+        check_dns_running
+        CONTAINER=$(container_id $1)
+        if [ $# -eq 1 ] ; then
+            delete_dns $CONTAINER $IP_ARGS
+        else
+            delete_dns_fqdn $CONTAINER "$3" $IP_ARGS
+        fi
         ;;
     expose)
         collect_cidr_args "$@"
@@ -998,7 +1029,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 [ "$FQDN" ] ; then
-                    http_call $DNS_CONTAINER_NAME $DNS_HTTP_PORT PUT /name/weave:expose/${CIDR%/*} --data-urlencode "fqdn=$FQDN" 2>/dev/null || true
+                    when_dns_running put_dns_fqdn 'weave:expose' $FQDN $CIDR
                 fi
             fi
         done
@@ -1013,7 +1044,7 @@ case "$COMMAND" in
                 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
-                http_call $DNS_CONTAINER_NAME $DNS_HTTP_PORT DELETE /name/weave:expose/${CIDR%/*} 2>/dev/null || true
+                when_dns_running delete_dns 'weave:expose' $CIDR
             fi
         done
         [ $CIDR_COUNT -eq 0 ] && http_call $CONTAINER_NAME $HTTP_PORT DELETE /ip/weave:expose 2>/dev/null || true