Stop noisy modifications (#1517)

* Stop noisy modifications

* Review comments, tests

* More coverage

* vet

* Make it obvious new port name is not a typo

2 files changed, 152 insertions, 3 deletions

diff --git a/plugin/kubernetes/controller.go b/plugin/kubernetes/controller.go
index 3262f299e..39d4ead64 100644
--- a/plugin/kubernetes/controller.go
+++ b/plugin/kubernetes/controller.go
@@ -431,6 +431,64 @@ func (dns *dnsControl) updateModifed() {
 	atomic.StoreInt64(&dns.modified, unix)
 }
 
-func (dns *dnsControl) Add(obj interface{})               { dns.updateModifed() }
-func (dns *dnsControl) Delete(obj interface{})            { dns.updateModifed() }
-func (dns *dnsControl) Update(objOld, newObj interface{}) { dns.updateModifed() }
+func (dns *dnsControl) Add(obj interface{})    { dns.updateModifed() }
+func (dns *dnsControl) Delete(obj interface{}) { dns.updateModifed() }
+
+func (dns *dnsControl) Update(objOld, newObj interface{}) {
+	// endpoint updates can come frequently, make sure
+	// it's a change we care about
+	if o, ok := objOld.(*api.Endpoints); ok {
+		n := newObj.(*api.Endpoints)
+		if endpointsEquivalent(o, n) {
+			return
+		}
+	}
+	dns.updateModifed()
+}
+
+// endpointsEquivalent checks if the update to an endpoint is something
+// that matters to us: ready addresses, host names, ports (including names for SRV)
+func endpointsEquivalent(a, b *api.Endpoints) bool {
+	// supposedly we should be able to rely on
+	// these being sorted and able to be compared
+	// they are supposed to be in a canonical format
+
+	if len(a.Subsets) != len(b.Subsets) {
+		return false
+	}
+
+	for i, sa := range a.Subsets {
+		// check the Addresses and Ports. Ignore unready addresses.
+		sb := b.Subsets[i]
+		if len(sa.Addresses) != len(sb.Addresses) {
+			return false
+		}
+		if len(sa.Ports) != len(sb.Ports) {
+			return false
+		}
+
+		for addr, aaddr := range sa.Addresses {
+			baddr := sb.Addresses[addr]
+			if aaddr.IP != baddr.IP {
+				return false
+			}
+			if aaddr.Hostname != baddr.Hostname {
+				return false
+			}
+		}
+
+		for port, aport := range sa.Ports {
+			bport := sb.Ports[port]
+			if aport.Name != bport.Name {
+				return false
+			}
+			if aport.Port != bport.Port {
+				return false
+			}
+			if aport.Protocol != bport.Protocol {
+				return false
+			}
+		}
+	}
+	return true
+}
diff --git a/plugin/kubernetes/xfr_test.go b/plugin/kubernetes/xfr_test.go
index 81be775dc..ef02d427a 100644
--- a/plugin/kubernetes/xfr_test.go
+++ b/plugin/kubernetes/xfr_test.go
@@ -7,6 +7,8 @@ import (
 	"github.com/coredns/coredns/plugin/pkg/dnstest"
 	"github.com/coredns/coredns/plugin/test"
 	"golang.org/x/net/context"
+	api "k8s.io/api/core/v1"
+	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/miekg/dns"
 )
@@ -109,3 +111,92 @@ func difference(testRRs []dns.RR, gotRRs []dns.RR) []dns.RR {
 	}
 	return foundRRs
 }
+
+func TestEndpointsEquivalent(t *testing.T) {
+	epA := api.Endpoints{
+		ObjectMeta: meta.ObjectMeta{ResourceVersion: "1230"},
+		Subsets: []api.EndpointSubset{{
+			Addresses:         []api.EndpointAddress{{IP: "1.2.3.4", Hostname: "foo"}},
+			NotReadyAddresses: []api.EndpointAddress{{IP: "1.2.3.5", Hostname: "foobar"}},
+		}},
+	}
+	epB := api.Endpoints{
+		ObjectMeta: meta.ObjectMeta{ResourceVersion: "1234"},
+		Subsets: []api.EndpointSubset{{
+			Addresses:         []api.EndpointAddress{{IP: "1.2.3.4", Hostname: "foo"}},
+			NotReadyAddresses: []api.EndpointAddress{{IP: "1.1.1.1", Hostname: "foobar"}},
+		}},
+	}
+	epC := api.Endpoints{
+		Subsets: []api.EndpointSubset{{
+			Addresses: []api.EndpointAddress{{IP: "1.2.3.5", Hostname: "foo"}},
+		}},
+	}
+	epD := api.Endpoints{
+		Subsets: []api.EndpointSubset{{
+			Addresses: []api.EndpointAddress{{IP: "1.2.3.5", Hostname: "foo"}},
+		},
+			{
+				Addresses: []api.EndpointAddress{{IP: "1.2.2.2", Hostname: "foofoo"}},
+			}},
+	}
+	epE := api.Endpoints{
+		Subsets: []api.EndpointSubset{{
+			Addresses: []api.EndpointAddress{{IP: "1.2.3.5", Hostname: "foo"}, {IP: "1.1.1.1"}},
+		}},
+	}
+	epF := api.Endpoints{
+		Subsets: []api.EndpointSubset{{
+			Addresses: []api.EndpointAddress{{IP: "1.2.3.4", Hostname: "foofoo"}},
+		}},
+	}
+	epG := api.Endpoints{
+		Subsets: []api.EndpointSubset{{
+			Addresses: []api.EndpointAddress{{IP: "1.2.3.4", Hostname: "foo"}},
+			Ports:     []api.EndpointPort{{Name: "http", Port: 80, Protocol: "TCP"}},
+		}},
+	}
+	epH := api.Endpoints{
+		Subsets: []api.EndpointSubset{{
+			Addresses: []api.EndpointAddress{{IP: "1.2.3.4", Hostname: "foo"}},
+			Ports:     []api.EndpointPort{{Name: "newportname", Port: 80, Protocol: "TCP"}},
+		}},
+	}
+	epI := api.Endpoints{
+		Subsets: []api.EndpointSubset{{
+			Addresses: []api.EndpointAddress{{IP: "1.2.3.4", Hostname: "foo"}},
+			Ports:     []api.EndpointPort{{Name: "http", Port: 8080, Protocol: "TCP"}},
+		}},
+	}
+	epJ := api.Endpoints{
+		Subsets: []api.EndpointSubset{{
+			Addresses: []api.EndpointAddress{{IP: "1.2.3.4", Hostname: "foo"}},
+			Ports:     []api.EndpointPort{{Name: "http", Port: 80, Protocol: "UDP"}},
+		}},
+	}
+
+	tests := []struct {
+		equiv bool
+		a     *api.Endpoints
+		b     *api.Endpoints
+	}{
+		{true, &epA, &epB},
+		{false, &epA, &epC},
+		{false, &epA, &epD},
+		{false, &epA, &epE},
+		{false, &epA, &epF},
+		{false, &epF, &epG},
+		{false, &epG, &epH},
+		{false, &epG, &epI},
+		{false, &epG, &epJ},
+	}
+
+	for i, tc := range tests {
+		if tc.equiv && !endpointsEquivalent(tc.a, tc.b) {
+			t.Errorf("Test %d: expected endpoints to be equivalent and they are not.", i)
+		}
+		if !tc.equiv && endpointsEquivalent(tc.a, tc.b) {
+			t.Errorf("Test %d: expected endpoints to be seen as different but they were not.", i)
+		}
+	}
+}