diff --git a/nameserver/http.go b/nameserver/http.go index 7ab19550..615a00a2 100644 --- a/nameserver/http.go +++ b/nameserver/http.go @@ -61,15 +61,12 @@ func ListenHttp(domain string, db Zone, port int) { if dns.IsSubDomain(domain, name) { Info.Printf("Adding %s -> %s", name, ipStr) if err = db.AddRecord(ident, name, ip); err != nil { - if dup, ok := err.(DuplicateError); !ok { + if _, ok := err.(DuplicateError); !ok { httpErrorAndLog( Error, w, "Internal error", http.StatusInternalServerError, "Unexpected error from DB: %s", err) return - } else if dup.Ident != ident { - http.Error(w, err.Error(), http.StatusConflict) - return - } // else we are golden + } // oh, I already know this. whatever. } } else { Info.Printf("Ignoring name %s, not in %s", name, domain) diff --git a/nameserver/http_test.go b/nameserver/http_test.go index 71874e7d..fb8a6970 100644 --- a/nameserver/http_test.go +++ b/nameserver/http_test.go @@ -55,27 +55,32 @@ func TestHttp(t *testing.T) { assertNoErr(t, err) assertStatus(t, resp.StatusCode, http.StatusOK, "http success response for duplicate add") - // Now try adding the same address again with a different ident - should fail + // Now try adding the same address again with a different ident -- + // again should be fine otherUrl := fmt.Sprintf("http://localhost:%d/name/%s/%s", port, "other", addrParts[0]) resp, err = genForm("PUT", otherUrl, url.Values{"fqdn": {successTestName}, "local_ip": {dockerIP}, "routing_prefix": {addrParts[1]}}) assertNoErr(t, err) - assertStatus(t, resp.StatusCode, http.StatusConflict, "http response") + assertStatus(t, resp.StatusCode, http.StatusOK, "http response") // Delete the address resp, err = genForm("DELETE", addrUrl, nil) assertNoErr(t, err) assertStatus(t, resp.StatusCode, http.StatusOK, "http response") - // Check that the address is not there now. + // Check that the address is still resolvable. _, err = zone.LookupLocal(successTestName) - assertErrorType(t, err, (*LookupError)(nil), "nonexistent lookup") + assertNoErr(t, err) - // Delete the address again, it should accept this - resp, err = genForm("DELETE", addrUrl, nil) + // Delete the address record mentioning the other container + resp, err = genForm("DELETE", otherUrl, nil) assertNoErr(t, err) assertStatus(t, resp.StatusCode, http.StatusOK, "http response") + // Check that the address is gone + _, err = zone.LookupLocal(successTestName) + assertErrorType(t, err, (*LookupError)(nil), "fully-removed address") + // Would like to shut down the http server at the end of this test // but it's complicated. // See https://groups.google.com/forum/#!topic/golang-nuts/vLHWa5sHnCE diff --git a/nameserver/testing_utils.go b/nameserver/testing_utils.go index 6183d5f8..b018143b 100644 --- a/nameserver/testing_utils.go +++ b/nameserver/testing_utils.go @@ -19,7 +19,7 @@ func assertStatus(t *testing.T, got int, wanted int, desc string) { func assertErrorInterface(t *testing.T, got interface{}, wanted interface{}, desc string) { if got == nil { - t.Fatalf("Expected %s but got nil", reflect.TypeOf(wanted).Elem()) + t.Fatalf("Expected %s but got nil (%s)", reflect.TypeOf(wanted).Elem(), desc) } gotT, wantedT := reflect.TypeOf(got), reflect.TypeOf(wanted).Elem() if !gotT.Implements(wantedT) { @@ -29,7 +29,7 @@ func assertErrorInterface(t *testing.T, got interface{}, wanted interface{}, des func assertErrorType(t *testing.T, got interface{}, wanted interface{}, desc string) { if got == nil { - t.Fatalf("Expected %s but got nil", reflect.TypeOf(wanted).Elem()) + t.Fatalf("Expected %s but got nil (%s)", reflect.TypeOf(wanted).Elem(), desc) } gotT, wantedT := reflect.TypeOf(got), reflect.TypeOf(wanted).Elem() if gotT != wantedT { diff --git a/nameserver/zone.go b/nameserver/zone.go index db3ba489..44c194b9 100644 --- a/nameserver/zone.go +++ b/nameserver/zone.go @@ -34,13 +34,10 @@ func (ops LookupError) Error() string { } type DuplicateError struct { - Name string - IP net.IP - Ident string } -func (err DuplicateError) Error() string { - return "Duplicate " + err.Name + "," + err.IP.String() + " for identity " + err.Ident +func (dup DuplicateError) Error() string { + return "Tried to add a duplicate entry" } func (zone *ZoneDb) indexOf(match func(Record) bool) int { @@ -79,8 +76,10 @@ func (zone *ZoneDb) AddRecord(ident string, name string, ip net.IP) error { defer zone.mx.Unlock() fqdn := dns.Fqdn(name) if index := zone.indexOf( - func(r Record) bool { return r.Name == fqdn && r.IP.Equal(ip) }); index != -1 { - return DuplicateError{fqdn, ip, zone.recs[index].Ident} + func(r Record) bool { + return r.Name == fqdn && r.IP.Equal(ip) && r.Ident == ident + }); index != -1 { + return DuplicateError{} } zone.recs = append(zone.recs, Record{ident, fqdn, ip}) return nil @@ -90,7 +89,9 @@ func (zone *ZoneDb) DeleteRecord(ident string, ip net.IP) error { zone.mx.Lock() defer zone.mx.Unlock() if index := zone.indexOf( - func(r Record) bool { return r.Ident == ident && r.IP.Equal(ip) }); index == -1 { + func(r Record) bool { + return r.Ident == ident && r.IP.Equal(ip) + }); index == -1 { return LookupError(ident) } else { zone.recs = append(zone.recs[:index], zone.recs[index+1:]...) diff --git a/nameserver/zone_test.go b/nameserver/zone_test.go index 68b6aca1..3cb80f56 100644 --- a/nameserver/zone_test.go +++ b/nameserver/zone_test.go @@ -7,9 +7,10 @@ import ( func TestZone(t *testing.T) { var ( - containerID = "deadbeef" - successTestName = "test1.weave." - testAddr1 = "10.0.2.1/24" + containerID = "deadbeef" + otherContainerID = "cowjuice" + successTestName = "test1.weave." + testAddr1 = "10.0.2.1/24" ) var zone = new(ZoneDb) @@ -40,14 +41,20 @@ func TestZone(t *testing.T) { t.Fatal("Unexpected result for", ip, foundName) } - // Now try to add the same address again err = zone.AddRecord(containerID, successTestName, ip) assertErrorType(t, err, (*DuplicateError)(nil), "duplicate add") - // Now delete the record + err = zone.AddRecord(otherContainerID, successTestName, ip) + // Delete the record for the original container err = zone.DeleteRecord(containerID, ip) assertNoErr(t, err) + _, err = zone.LookupLocal(successTestName) + assertNoErr(t, err) + + err = zone.DeleteRecord(otherContainerID, ip) + assertNoErr(t, err) + // Check that the address is not there now. _, err = zone.LookupLocal(successTestName) assertErrorType(t, err, (*LookupError)(nil), "after deleting record") @@ -59,10 +66,10 @@ func TestZone(t *testing.T) { func TestDeleteFor(t *testing.T) { var ( - id = "foobar" - name = "foo.weave." - addr1 = "10.1.2.3/24" - addr2 = "10.2.7.8/24" + id = "foobar" + name = "foo.weave." + addr1 = "10.1.2.3/24" + addr2 = "10.2.7.8/24" ) zone := new(ZoneDb) for _, addr := range []string{addr1, addr2} {