Don't enqueue ingress for some service changes

internal/handlers/service.go

@@ -2,6 +2,7 @@ package handlers
 
 import (
 	"reflect"
+	"sort"
 
 	"github.com/golang/glog"
 	"github.com/nginxinc/kubernetes-ingress/internal/controller"
@@ -47,14 +48,52 @@ func CreateServiceHandlers(lbc *controller.LoadBalancerController) cache.Resourc
 		},
 		UpdateFunc: func(old, cur interface{}) {
 			if !reflect.DeepEqual(old, cur) {
-				svc := cur.(*api_v1.Service)
-				if lbc.IsExternalServiceForStatus(svc) {
-					lbc.AddSyncQueue(svc)
+				curSvc := cur.(*api_v1.Service)
+				if lbc.IsExternalServiceForStatus(curSvc) {
+					lbc.AddSyncQueue(curSvc)
 					return
 				}
-				glog.V(3).Infof("Service %v changed, syncing", svc.Name)
-				lbc.EnqueueIngressForService(svc)
+				oldSvc := old.(*api_v1.Service)
+				if hasServicePortChanges(oldSvc.Spec.Ports, curSvc.Spec.Ports) {
+					glog.V(3).Infof("Service %v changed, syncing", curSvc.Name)
+					lbc.EnqueueIngressForService(curSvc)
+				}
 			}
 		},
 	}
 }
+
+type portSort []api_v1.ServicePort
+
+func (a portSort) Len() int {
+	return len(a)
+}
+
+func (a portSort) Swap(i, j int) {
+	a[i], a[j] = a[j], a[i]
+}
+
+func (a portSort) Less(i, j int) bool {
+	if a[i].Name == a[j].Name {
+		return a[i].Port < a[j].Port
+	}
+	return a[i].Name < a[j].Name
+}
+
+// hasServicePortChanges only compares ServicePort.Name and .Port.
+func hasServicePortChanges(oldServicePorts []api_v1.ServicePort, curServicePorts []api_v1.ServicePort) bool {
+	if len(oldServicePorts) != len(curServicePorts) {
+		return true
+	}
+
+	sort.Sort(portSort(oldServicePorts))
+	sort.Sort(portSort(curServicePorts))
+
+	for i := range oldServicePorts {
+		if oldServicePorts[i].Port != curServicePorts[i].Port ||
+			oldServicePorts[i].Name != curServicePorts[i].Name {
+			return true
+		}
+	}
+	return false
+}

internal/handlers/service_test.go

@@ -0,0 +1,156 @@
+package handlers
+
+import (
+	"testing"
+
+	"k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+)
+
+func TestHasServicePortChanges(t *testing.T) {
+	cases := []struct {
+		a      []v1.ServicePort
+		b      []v1.ServicePort
+		result bool
+		reason string
+	}{
+		{
+			[]v1.ServicePort{},
+			[]v1.ServicePort{},
+			false,
+			"Empty should report no changes",
+		},
+		{
+			[]v1.ServicePort{{
+				Port: 80,
+			}},
+			[]v1.ServicePort{{
+				Port: 8080,
+			}},
+			true,
+			"Different Ports",
+		},
+		{
+			[]v1.ServicePort{{
+				Port: 80,
+			}},
+			[]v1.ServicePort{{
+				Port: 80,
+			}},
+			false,
+			"Same Ports",
+		},
+		{
+			[]v1.ServicePort{{
+				Name: "asdf",
+				Port: 80,
+			}},
+			[]v1.ServicePort{{
+				Name: "asdf",
+				Port: 80,
+			}},
+			false,
+			"Same Port and Name",
+		},
+		{
+			[]v1.ServicePort{{
+				Name: "foo",
+				Port: 80,
+			}},
+			[]v1.ServicePort{{
+				Name: "bar",
+				Port: 80,
+			}},
+			true,
+			"Different Name same Port",
+		},
+		{
+			[]v1.ServicePort{{
+				Name: "foo",
+				Port: 8080,
+			}},
+			[]v1.ServicePort{{
+				Name: "bar",
+				Port: 80,
+			}},
+			true,
+			"Different Name different Port",
+		},
+		{
+			[]v1.ServicePort{{
+				Name: "foo",
+			}},
+			[]v1.ServicePort{{
+				Name: "fooo",
+			}},
+			true,
+			"Very similar Name",
+		},
+		{
+			[]v1.ServicePort{{
+				Name: "asdf",
+				Port: 80,
+				TargetPort: intstr.IntOrString{
+					IntVal: 80,
+				},
+			}},
+			[]v1.ServicePort{{
+				Name: "asdf",
+				Port: 80,
+				TargetPort: intstr.IntOrString{
+					IntVal: 8080,
+				},
+			}},
+			false,
+			"TargetPort should be ignored",
+		},
+		{
+			[]v1.ServicePort{{
+				Name: "foo",
+			}, {
+				Name: "bar",
+			}},
+			[]v1.ServicePort{{
+				Name: "foo",
+			}, {
+				Name: "bar",
+			}},
+			false,
+			"Multiple same names",
+		},
+		{
+			[]v1.ServicePort{{
+				Name: "foo",
+			}, {
+				Name: "bar",
+			}},
+			[]v1.ServicePort{{
+				Name: "foo",
+			}, {
+				Name: "bars",
+			}},
+			true,
+			"Multiple different names",
+		},
+		{
+			[]v1.ServicePort{{
+				Name: "foo",
+			}, {
+				Port: 80,
+			}},
+			[]v1.ServicePort{{
+				Port: 80,
+			}, {
+				Name: "foo",
+			}},
+			false,
+			"Some names some ports",
+		},
+	}
+
+	for _, c := range cases {
+		if c.result != hasServicePortChanges(c.a, c.b) {
+			t.Errorf("hasServicePortChanges returned %v, but expected %v for %q case", c.result, !c.result, c.reason)
+		}
+	}
+}