Fix/plan reuse current record (#145)

* inherit labels from dns provider records

* do not change import statements, rollback autoformatter changes

* add comment for the public method

endpoint/endpoint.go

@@ -43,3 +43,12 @@ func NewEndpoint(dnsName, target string) *Endpoint {
 		Labels:  map[string]string{},
 	}
 }
+
+// MergeLabels adds keys to labels if not defined for the endpoint
+func (e *Endpoint) MergeLabels(labels map[string]string) {
+	for k, v := range labels {
+		if e.Labels[k] == "" {
+			e.Labels[k] = v
+		}
+	}
+}

endpoint/endpoint_test.go

@@ -16,7 +16,10 @@ limitations under the License.
 
 package endpoint
 
-import "testing"
+import (
+	"reflect"
+	"testing"
+)
 
 func TestNewEndpoint(t *testing.T) {
 	e := NewEndpoint("example.org", "1.2.3.4")
@@ -32,3 +35,15 @@ func TestNewEndpoint(t *testing.T) {
 		t.Error("endpoint is not initialized correctly")
 	}
 }
+
+func TestMergeLabels(t *testing.T) {
+	e := NewEndpoint("abc.com", "1.2.3.4")
+	e.Labels = map[string]string{
+		"foo": "bar",
+		"baz": "qux",
+	}
+	e.MergeLabels(map[string]string{"baz": "baz", "new": "fox"})
+	if !reflect.DeepEqual(e.Labels, map[string]string{"foo": "bar", "baz": "qux", "new": "fox"}) {
+		t.Error("invalid merge result")
+	}
+}

internal/testutils/endpoint.go

@@ -24,7 +24,7 @@ import "github.com/kubernetes-incubator/external-dns/endpoint"
 // considers example.org. and example.org DNSName/Target as different endpoints
 // TODO:might need reconsideration regarding trailing dot
 func SameEndpoint(a, b *endpoint.Endpoint) bool {
-	return a.DNSName == b.DNSName && a.Target == b.Target
+	return a.DNSName == b.DNSName && a.Target == b.Target && a.Labels[endpoint.OwnerLabelKey] == b.Labels[endpoint.OwnerLabelKey]
 }
 
 // SameEndpoints compares two slices of endpoints regardless of order

plan/plan.go

@@ -63,6 +63,7 @@ func (p *Plan) Calculate() *Plan {
 
 		// If there already is a record update it if it changed.
 		if desired.Target != current.Target {
+			desired.MergeLabels(current.Labels) //inherit the labels from the dns provider, including Owner ID
 			changes.UpdateOld = append(changes.UpdateOld, current)
 			changes.UpdateNew = append(changes.UpdateNew, desired)
 		}

plan/plan_test.go

@@ -30,11 +30,16 @@ func TestCalculate(t *testing.T) {
 	// empty list of records
 	empty := []*endpoint.Endpoint{}
 	// a simple entry
-	fooV1 := []*endpoint.Endpoint{{DNSName: "foo", Target: "v1"}}
+	fooV1 := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v1")}
 	// the same entry but with different target
-	fooV2 := []*endpoint.Endpoint{{DNSName: "foo", Target: "v2"}}
+	fooV2 := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v2")}
 	// another simple entry
-	bar := []*endpoint.Endpoint{{DNSName: "bar", Target: "v1"}}
+	bar := []*endpoint.Endpoint{endpoint.NewEndpoint("bar", "v1")}
+
+	// test case with labels
+	noLabels := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v2")}
+	labeledV2 := []*endpoint.Endpoint{newEndpointWithOwner("foo", "v2", "123")}
+	labeledV1 := []*endpoint.Endpoint{newEndpointWithOwner("foo", "v1", "123")}
 
 	for _, tc := range []struct {
 		current, desired, create, updateOld, updateNew, delete []*endpoint.Endpoint
@@ -51,13 +56,14 @@ func TestCalculate(t *testing.T) {
 		{fooV1, fooV2, empty, fooV1, fooV2, empty},
 		// Both exist but are different creates desired and deletes current.
 		{fooV1, bar, bar, empty, empty, fooV1},
+		// Labels should be inherited
+		{labeledV1, noLabels, empty, labeledV1, labeledV2, empty},
 	} {
 		// setup plan
 		plan := &Plan{
 			Current: tc.current,
 			Desired: tc.desired,
 		}
-
 		// calculate actions
 		plan = plan.Calculate()
 
@@ -145,3 +151,9 @@ func validateEntries(t *testing.T, entries, expected []*endpoint.Endpoint) {
 		}
 	}
 }
+
+func newEndpointWithOwner(dnsName, target, ownerID string) *endpoint.Endpoint {
+	e := endpoint.NewEndpoint(dnsName, target)
+	e.Labels[endpoint.OwnerLabelKey] = ownerID
+	return e
+}