From e21592d74fbf086cbbfc0ffb34cfcdabd6768e06 Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Tue, 12 May 2015 12:33:36 +0100 Subject: [PATCH 1/9] Add DeleteRecords method --- nameserver/mocks_test.go | 4 ++++ nameserver/zone.go | 43 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/nameserver/mocks_test.go b/nameserver/mocks_test.go index 7cf195b4..4c89941d 100644 --- a/nameserver/mocks_test.go +++ b/nameserver/mocks_test.go @@ -83,6 +83,10 @@ func (mz *mockedZoneWithRecords) AddRecord(ident string, name string, ip net.IP) notImplWarn() return nil } +func (mz *mockedZoneWithRecords) DeleteRecords(ident string, name string, ip net.IP) int { + notImplWarn() + return 0 +} func (mz *mockedZoneWithRecords) DeleteRecord(ident string, ip net.IP) error { notImplWarn() return nil diff --git a/nameserver/zone.go b/nameserver/zone.go index 4ad7cd06..704e6929 100644 --- a/nameserver/zone.go +++ b/nameserver/zone.go @@ -49,6 +49,8 @@ type Zone interface { Domain() string // Add a record in the local database AddRecord(ident string, name string, ip net.IP) error + // Delete matching records (uninitialised values act as wildcards) + DeleteRecords(ident string, name string, ip net.IP) (count int) // Delete a record in the local database DeleteRecord(ident string, ip net.IP) error // Delete all records for an ident in the local database @@ -592,6 +594,47 @@ func (zone *ZoneDb) AddRecord(ident string, name string, ip net.IP) (err error) return } +// Delete matching records (uninitialised values act as wildcards) +func (zone *ZoneDb) DeleteRecords(ident string, name string, ip net.IP) int { + zone.mx.Lock() + defer zone.mx.Unlock() + + count := 0 + + if name != "" { + name = dns.Fqdn(name) + } + + for identK, identV := range zone.idents { + if ident == "" || ident == identK { + for nameK, nameV := range identV.names { + nameModified := false + if name == "" || name == nameK { + for ipv4K, ipv4V := range nameV.ipv4 { + if ip.Equal(net.IP{}) || ip.Equal(ipv4K.toNetIP()) { + ipv4V.notifyIPObservers() + delete(nameV.ipv4, ipv4K) + nameModified = true + count++ + } + } + } + if nameModified { + nameV.notifyNameObservers() + if nameV.empty() { + delete(identV.names, nameK) + } + } + } + } + if identV.empty() { + delete(zone.idents, identK) + } + } + + return count +} + // Delete all records for an ident func (zone *ZoneDb) DeleteRecordsFor(ident string) (err error) { zone.mx.Lock() From e02bd8bb30bba61ab4110b4f5db82b0ca90c1f30 Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Tue, 12 May 2015 16:23:58 +0100 Subject: [PATCH 2/9] Add HTTP routes for wildcard deletion --- nameserver/http.go | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/nameserver/http.go b/nameserver/http.go index 64c0394a..ce438bee 100644 --- a/nameserver/http.go +++ b/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) From 9da5511484163b52e7b47d0ecf821be41260afff Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Wed, 13 May 2015 13:59:01 +0100 Subject: [PATCH 3/9] Support -h argument to dns-remove --- weave | 142 +++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 97 insertions(+), 45 deletions(-) diff --git a/weave b/weave index 4d4ea1e3..6097a83a 100755 --- a/weave +++ b/weave @@ -35,7 +35,7 @@ usage() { echo "weave attach [ ...] " echo "weave detach [ ...] " echo "weave dns-add [ ...] [-h ]" - echo "weave dns-remove [ ...] " + echo "weave dns-remove [ ...] [-h ]" echo "weave expose [ ...] [-h ]" echo "weave hide [ ...]" echo "weave ps [ ...]" @@ -644,34 +644,84 @@ 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 +when_has_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 +} + +# Convert the container name or short ID in $1 to its full form, then +# invoke $2 as a command passing the full form as the first argument and +# $3.. as additional arguments +with_full_container_id() { + CONTAINER="$1" + COMMAND="$2" + shift 2 + + if FULL_CONTAINER_ID=$(container_id $CONTAINER); then + $COMMAND $FULL_CONTAINER_ID "$@" + else + return $? + 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 +732,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 + when_has_fqdn $CONTAINER put_dns_fqdn $CONTAINER_IPS + fi done } @@ -920,7 +962,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 when_has_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS echo $CONTAINER ;; dns-args) @@ -939,7 +981,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 when_has_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS echo $CONTAINER ;; attach) @@ -950,7 +992,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 when_has_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS ;; detach) collect_cidr_args "$@" @@ -958,7 +1000,7 @@ case "$COMMAND" in [ $# -eq 1 ] || usage CONTAINER_ID="$1" with_container_netns $CONTAINER_ID detach $CIDR_ARGS >/dev/null - tell_dns DELETE $CONTAINER_ID $CIDR_ARGS + when_dns_running with_full_container_id $CONTAINER_ID when_has_fqdn delete_dns_fqdn $CIDR_ARGS ;; dns-add) collect_ip_args "$@" @@ -966,19 +1008,29 @@ case "$COMMAND" in [ $# -ge 1 ] || usage CONTAINER_ID="$1" if [ $# -eq 1 ] ; then - tell_dns PUT $CONTAINER_ID $IP_ARGS + check_dns_running + with_full_container_id $CONTAINER_ID when_has_fqdn put_dns_fqdn $IP_ARGS else [ $# -eq 3 -a "$2" = "-h" ] || usage FQDN="$3" - tell_dns_fqdn PUT $CONTAINER_ID $FQDN $IP_ARGS + check_dns_running + with_full_container_id $CONTAINER_ID put_dns_fqdn $FQDN $IP_ARGS fi ;; dns-remove) collect_ip_args "$@" shift $IP_COUNT - [ $# -eq 1 ] || usage + [ $# -ge 1 ] || usage CONTAINER_ID="$1" - tell_dns DELETE $CONTAINER_ID $IP_ARGS + if [ $# -eq 1 ] ; then + check_dns_running + with_full_container_id $CONTAINER_ID delete_dns $IP_ARGS + else + [ $# -eq 3 -a "$2" = "-h" ] || usage + FQDN="$3" + check_dns_running + with_full_container_id $CONTAINER_ID delete_dns_fqdn $FQDN $IP_ARGS + fi ;; expose) collect_cidr_args "$@" @@ -998,7 +1050,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 +1065,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 From 66de361fd6ea477f77749b831c0ac01d3a1b9c6e Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Fri, 15 May 2015 10:54:17 +0100 Subject: [PATCH 4/9] Smoketest dns-remove -h --- test/240_dns_add_name_test.sh | 8 ++++++-- test/250_dns_test_restart.sh | 18 ++++++++++++++++++ test/config.sh | 10 ++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) create mode 100755 test/250_dns_test_restart.sh diff --git a/test/240_dns_add_name_test.sh b/test/240_dns_add_name_test.sh index 59da781a..f29df2e4 100755 --- a/test/240_dns_add_name_test.sh +++ b/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 diff --git a/test/250_dns_test_restart.sh b/test/250_dns_test_restart.sh new file mode 100755 index 00000000..24df8e78 --- /dev/null +++ b/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 diff --git a/test/config.sh b/test/config.sh index 190fb90e..ed19b057 100644 --- a/test/config.sh +++ b/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 +assert_dns_a_record() { + assert "exec_on $1 $2 getent hosts $3 | tr -s ' '" "$4 $3" +} + +# assert_dns_ptr_record +assert_dns_ptr_record() { + assert "exec_on $1 $2 getent hosts $4 | tr -s ' '" "$4 $3" +} + start_suite() { for host in $HOST1 $HOST2; do echo "Cleaning up on $host: removing all containers and resetting weave" From 30447ada521586a76c3138b328c9e718f3d443a9 Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Wed, 20 May 2015 14:45:47 +0100 Subject: [PATCH 5/9] Remove obsolete methods --- nameserver/mocks_test.go | 7 +------ nameserver/server_cache_test.go | 12 ++++++------ nameserver/zone.go | 33 ++------------------------------- nameserver/zone_test.go | 20 +++++++++++--------- 4 files changed, 20 insertions(+), 52 deletions(-) diff --git a/nameserver/mocks_test.go b/nameserver/mocks_test.go index 4c89941d..1bec091d 100644 --- a/nameserver/mocks_test.go +++ b/nameserver/mocks_test.go @@ -87,12 +87,7 @@ func (mz *mockedZoneWithRecords) DeleteRecords(ident string, name string, ip net notImplWarn() return 0 } -func (mz *mockedZoneWithRecords) DeleteRecord(ident string, ip net.IP) error { - notImplWarn() - return nil -} -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 diff --git a/nameserver/server_cache_test.go b/nameserver/server_cache_test.go index 4474ac5e..16934db1 100644 --- a/nameserver/server_cache_test.go +++ b/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) diff --git a/nameserver/zone.go b/nameserver/zone.go index 704e6929..cfb784ac 100644 --- a/nameserver/zone.go +++ b/nameserver/zone.go @@ -51,10 +51,6 @@ type Zone interface { AddRecord(ident string, name string, ip net.IP) error // Delete matching records (uninitialised values act as wildcards) DeleteRecords(ident string, name string, ip net.IP) (count int) - // 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 // Lookup for a name in the whole domain DomainLookupName(name string) ([]ZoneRecord, error) // Lookup for an address in the whole domain @@ -635,32 +631,6 @@ func (zone *ZoneDb) DeleteRecords(ident string, name string, ip net.IP) int { return count } -// 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) - } - return -} - // Observe a name. // The name must have at least one valid IP address. // Name observers are notified when @@ -727,7 +697,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 diff --git a/nameserver/zone_test.go b/nameserver/zone_test.go index 63d98fb3..89dcfb20 100644 --- a/nameserver/zone_test.go +++ b/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") } From a4582fb5299022b19c55c81fc9169d70b4ac6cc8 Mon Sep 17 00:00:00 2001 From: Adam Harrison Date: Fri, 29 May 2015 16:27:22 +0100 Subject: [PATCH 6/9] Optimise DeleteRecords when explicit values are passed --- nameserver/zone.go | 131 ++++++++++++++++---------------------- nameserver/zone_lookup.go | 9 +-- 2 files changed, 60 insertions(+), 80 deletions(-) diff --git a/nameserver/zone.go b/nameserver/zone.go index cfb784ac..6c1a8f6f 100644 --- a/nameserver/zone.go +++ b/nameserver/zone.go @@ -192,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... @@ -330,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,43 +593,14 @@ func (zone *ZoneDb) AddRecord(ident string, name string, ip net.IP) (err error) // Delete matching records (uninitialised values act as wildcards) func (zone *ZoneDb) DeleteRecords(ident string, name string, ip net.IP) int { - zone.mx.Lock() - defer zone.mx.Unlock() - - count := 0 - if name != "" { name = dns.Fqdn(name) } - for identK, identV := range zone.idents { - if ident == "" || ident == identK { - for nameK, nameV := range identV.names { - nameModified := false - if name == "" || name == nameK { - for ipv4K, ipv4V := range nameV.ipv4 { - if ip.Equal(net.IP{}) || ip.Equal(ipv4K.toNetIP()) { - ipv4V.notifyIPObservers() - delete(nameV.ipv4, ipv4K) - nameModified = true - count++ - } - } - } - if nameModified { - nameV.notifyNameObservers() - if nameV.empty() { - delete(identV.names, nameK) - } - } - } - } - if identV.empty() { - delete(zone.idents, identK) - } - } + zone.mx.Lock() + defer zone.mx.Unlock() - return count + return zone.deleteIdentNameIP(ident, name, ip) } // Observe a name. @@ -776,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 } diff --git a/nameserver/zone_lookup.go b/nameserver/zone_lookup.go index 292648a9..9c310504 100644 --- a/nameserver/zone_lookup.go +++ b/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() } } From bf8e82ba162d966eb03a16027ea100072c05148c Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 1 Jun 2015 18:47:51 +0100 Subject: [PATCH 7/9] refactor: get rid of with_full_container_id --- weave | 35 +++++++++-------------------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/weave b/weave index 6097a83a..66ec8558 100755 --- a/weave +++ b/weave @@ -678,21 +678,6 @@ when_has_fqdn() { fi } -# Convert the container name or short ID in $1 to its full form, then -# invoke $2 as a command passing the full form as the first argument and -# $3.. as additional arguments -with_full_container_id() { - CONTAINER="$1" - COMMAND="$2" - shift 2 - - if FULL_CONTAINER_ID=$(container_id $CONTAINER); then - $COMMAND $FULL_CONTAINER_ID "$@" - else - return $? - fi -} - # Register FQDN in $2 as names for addresses $3.. under full container ID $1 put_dns_fqdn() { CONTAINER_ID="$1" @@ -998,38 +983,36 @@ case "$COMMAND" in collect_cidr_args "$@" shift $CIDR_COUNT [ $# -eq 1 ] || usage - CONTAINER_ID="$1" - with_container_netns $CONTAINER_ID detach $CIDR_ARGS >/dev/null - when_dns_running with_full_container_id $CONTAINER_ID when_has_fqdn delete_dns_fqdn $CIDR_ARGS + CONTAINER=$(container_id $1) + with_container_netns $CONTAINER detach $CIDR_ARGS >/dev/null + when_dns_running when_has_fqdn $CONTAINER delete_dns_fqdn $CIDR_ARGS ;; dns-add) collect_ip_args "$@" shift $IP_COUNT [ $# -ge 1 ] || usage - CONTAINER_ID="$1" + CONTAINER=$(container_id $1) if [ $# -eq 1 ] ; then check_dns_running - with_full_container_id $CONTAINER_ID when_has_fqdn put_dns_fqdn $IP_ARGS + when_has_fqdn $CONTAINER put_dns_fqdn $IP_ARGS else [ $# -eq 3 -a "$2" = "-h" ] || usage - FQDN="$3" check_dns_running - with_full_container_id $CONTAINER_ID put_dns_fqdn $FQDN $IP_ARGS + put_dns_fqdn $CONTAINER "$3" $IP_ARGS fi ;; dns-remove) collect_ip_args "$@" shift $IP_COUNT [ $# -ge 1 ] || usage - CONTAINER_ID="$1" + CONTAINER=$(container_id $1) if [ $# -eq 1 ] ; then check_dns_running - with_full_container_id $CONTAINER_ID delete_dns $IP_ARGS + delete_dns $CONTAINER $IP_ARGS else [ $# -eq 3 -a "$2" = "-h" ] || usage - FQDN="$3" check_dns_running - with_full_container_id $CONTAINER_ID delete_dns_fqdn $FQDN $IP_ARGS + delete_dns_fqdn $CONTAINER "$3" $IP_ARGS fi ;; expose) From dcedf241bbaf4f6287388068984dbe057a2361ee Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 1 Jun 2015 19:01:20 +0100 Subject: [PATCH 8/9] simplify dns-add/remove --- weave | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/weave b/weave index 66ec8558..50afd3a1 100755 --- a/weave +++ b/weave @@ -990,28 +990,24 @@ case "$COMMAND" in dns-add) collect_ip_args "$@" shift $IP_COUNT - [ $# -ge 1 ] || usage + [ $# -eq 1 -o \( $# -eq 3 -a "$2" = "-h" \) ] || usage + check_dns_running CONTAINER=$(container_id $1) if [ $# -eq 1 ] ; then - check_dns_running when_has_fqdn $CONTAINER put_dns_fqdn $IP_ARGS else - [ $# -eq 3 -a "$2" = "-h" ] || usage - check_dns_running put_dns_fqdn $CONTAINER "$3" $IP_ARGS fi ;; dns-remove) collect_ip_args "$@" shift $IP_COUNT - [ $# -ge 1 ] || usage + [ $# -eq 1 -o \( $# -eq 3 -a "$2" = "-h" \) ] || usage + check_dns_running CONTAINER=$(container_id $1) if [ $# -eq 1 ] ; then - check_dns_running delete_dns $CONTAINER $IP_ARGS else - [ $# -eq 3 -a "$2" = "-h" ] || usage - check_dns_running delete_dns_fqdn $CONTAINER "$3" $IP_ARGS fi ;; From 294aafde14bdf900d96d23911bc07d9540de442b Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Mon, 1 Jun 2015 19:13:05 +0100 Subject: [PATCH 9/9] cosmetic: s/when_has_fqdn/with_container_fqdn --- weave | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/weave b/weave index 50afd3a1..046dc08a 100755 --- a/weave +++ b/weave @@ -666,7 +666,7 @@ when_dns_running() { # 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 -when_has_fqdn() { +with_container_fqdn() { CONTAINER="$1" COMMAND="$2" shift 2 @@ -718,7 +718,7 @@ populate_dns() { fi for CONTAINER in $(docker ps -q --no-trunc) ; do if CONTAINER_IPS=$(with_container_netns $CONTAINER container_weave_addrs 2>&1 | sed -n -e 's/inet \([^/]*\)\/\(.*\)/\1/p') ; then - when_has_fqdn $CONTAINER put_dns_fqdn $CONTAINER_IPS + with_container_fqdn $CONTAINER put_dns_fqdn $CONTAINER_IPS fi done } @@ -947,7 +947,7 @@ case "$COMMAND" in create_bridge get_ipam_cidr $CONTAINER with_container_netns $CONTAINER attach $CIDR_ARGS >/dev/null - when_dns_running when_has_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS + when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS echo $CONTAINER ;; dns-args) @@ -966,7 +966,7 @@ case "$COMMAND" in create_bridge get_ipam_cidr $CONTAINER with_container_netns $CONTAINER attach $CIDR_ARGS >/dev/null - when_dns_running when_has_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS + when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS echo $CONTAINER ;; attach) @@ -977,7 +977,7 @@ case "$COMMAND" in create_bridge get_ipam_cidr $CONTAINER with_container_netns $CONTAINER attach $CIDR_ARGS >/dev/null - when_dns_running when_has_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS + when_dns_running with_container_fqdn $CONTAINER put_dns_fqdn $CIDR_ARGS ;; detach) collect_cidr_args "$@" @@ -985,7 +985,7 @@ case "$COMMAND" in [ $# -eq 1 ] || usage CONTAINER=$(container_id $1) with_container_netns $CONTAINER detach $CIDR_ARGS >/dev/null - when_dns_running when_has_fqdn $CONTAINER delete_dns_fqdn $CIDR_ARGS + when_dns_running with_container_fqdn $CONTAINER delete_dns_fqdn $CIDR_ARGS ;; dns-add) collect_ip_args "$@" @@ -994,7 +994,7 @@ case "$COMMAND" in check_dns_running CONTAINER=$(container_id $1) if [ $# -eq 1 ] ; then - when_has_fqdn $CONTAINER put_dns_fqdn $IP_ARGS + with_container_fqdn $CONTAINER put_dns_fqdn $IP_ARGS else put_dns_fqdn $CONTAINER "$3" $IP_ARGS fi