Cope with duplicate name->IP entries
The zone DB no longer considers duplciates of (IP, name); it will reject duplicates of (IP, name, ident), but this ultimately doesn't matter since those are all the fields it records anyway. The HTTP interface treats these duplicates as trivial success (i.e., it acts as if it's idempotent, which it is). This changes the behaviour for removals: if (IP, name) is recorded for two different containers, removing one will leave the other extant.
This commit is contained in:
1 parent
a17425989e
commit
b08c28c038
5 files changed
+40
-30
No files matched your search
+2
-5
@@ -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)
|
||||
|
||||
+11
-6
@@ -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
|
||||
|
||||
@@ -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 {
|
||||
|
||||
+9
-8
@@ -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:]...)
|
||||
|
||||
+16
-9
@@ -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} {
|
||||
|
||||
Reference in new issue
Block a user