Fix handling of DNS updates for RFC2136 provider.

When receiving an update, we should be removing the old record and
adding the new.  Previously, we were removing and readding the new
record, which would cause the old DNS record to be left lying around.

provider/rfc2136/rfc2136.go

@@ -221,14 +221,14 @@ func (r rfc2136Provider) ApplyChanges(ctx context.Context, changes *plan.Changes
 
 		r.AddRecord(m, ep)
 	}
-	for _, ep := range changes.UpdateNew {
+	for index, ep := range changes.UpdateNew {
 
 		if !r.domainFilter.Match(ep.DNSName) {
 			log.Debugf("Skipping record %s because it was filtered out by the specified --domain-filter", ep.DNSName)
 			continue
 		}
 
-		r.UpdateRecord(m, ep)
+		r.UpdateRecord(m, changes.UpdateOld[index], ep)
 	}
 	for _, ep := range changes.Delete {
 
@@ -251,13 +251,13 @@ func (r rfc2136Provider) ApplyChanges(ctx context.Context, changes *plan.Changes
 	return nil
 }
 
-func (r rfc2136Provider) UpdateRecord(m *dns.Msg, ep *endpoint.Endpoint) error {
-	err := r.RemoveRecord(m, ep)
+func (r rfc2136Provider) UpdateRecord(m *dns.Msg, oldEp *endpoint.Endpoint, newEp *endpoint.Endpoint) error {
+	err := r.RemoveRecord(m, oldEp)
 	if err != nil {
 		return err
 	}
 
-	return r.AddRecord(m, ep)
+	return r.AddRecord(m, newEp)
 }
 
 func (r rfc2136Provider) AddRecord(m *dns.Msg, ep *endpoint.Endpoint) error {

provider/rfc2136/rfc2136_test.go

@@ -248,6 +248,75 @@ func TestRfc2136ApplyChangesWithDifferentTTLs(t *testing.T) {
 
 }
 
+func TestRfc2136ApplyChangesWithUpdate(t *testing.T) {
+	stub := newStub()
+
+	provider, err := createRfc2136StubProvider(stub)
+	assert.NoError(t, err)
+
+	p := &plan.Changes{
+		Create: []*endpoint.Endpoint{
+			{
+				DNSName:    "v1.foo.com",
+				RecordType: "A",
+				Targets:    []string{"1.2.3.4"},
+				RecordTTL:  endpoint.TTL(400),
+			},
+			{
+				DNSName:    "v1.foobar.com",
+				RecordType: "TXT",
+				Targets:    []string{"boom"},
+			},
+		},
+	}
+
+	err = provider.ApplyChanges(context.Background(), p)
+	assert.NoError(t, err)
+
+	p = &plan.Changes{
+		UpdateOld: []*endpoint.Endpoint{
+			{
+				DNSName:    "v1.foo.com",
+				RecordType: "A",
+				Targets:    []string{"1.2.3.4"},
+				RecordTTL:  endpoint.TTL(400),
+			},
+			{
+				DNSName:    "v1.foobar.com",
+				RecordType: "TXT",
+				Targets:    []string{"boom"},
+			},
+		},
+		UpdateNew: []*endpoint.Endpoint{
+			{
+				DNSName:    "v1.foo.com",
+				RecordType: "A",
+				Targets:    []string{"1.2.3.5"},
+				RecordTTL:  endpoint.TTL(400),
+			},
+			{
+				DNSName:    "v1.foobar.com",
+				RecordType: "TXT",
+				Targets:    []string{"kablui"},
+			},
+		},
+	}
+
+	err = provider.ApplyChanges(context.Background(), p)
+	assert.NoError(t, err)
+
+	createRecords := extractAuthoritySectionFromMessage(stub.createMsgs[0])
+	assert.Equal(t, 2, len(createRecords))
+	assert.True(t, strings.Contains(createRecords[0], "v1.foo.com"))
+	assert.False(t, strings.Contains(createRecords[0], "1.2.3.4"))
+	assert.True(t, strings.Contains(createRecords[0], "1.2.3.5"))
+	assert.True(t, strings.Contains(createRecords[0], "400"))
+	assert.True(t, strings.Contains(createRecords[1], "v1.foobar.com"))
+	assert.False(t, strings.Contains(createRecords[1], "boom"))
+	assert.True(t, strings.Contains(createRecords[1], "kablui"))
+
+}
+
 func contains(arr []*endpoint.Endpoint, name string) bool {
 	for _, a := range arr {
 		if a.DNSName == name {