Skip health checks when there is one server that backs the route

Previously we had attempted to suppress the health checks, but it was
looking at the endpoints of the service, not the servers that back the
route.  The problem is that if we just look at endpoints, then if a
route has two backing services, each with one endpoint, we would
incorrectly suppress the health checks.

Fixes bug 1507664 (https://bugzilla.redhat.com/show_bug.cgi?id=1507664)

Author: knobunc

pkg/router/template/template_helper.go

@@ -157,16 +157,28 @@ func processEndpointsForAlias(alias ServiceAliasConfig, svc ServiceUnit, action
 }
 
 func endpointsForAlias(alias ServiceAliasConfig, svc ServiceUnit) []Endpoint {
-	if len(alias.PreferPort) == 0 {
+	if len(alias.PreferPort) == 0 && len(svc.EndpointTable) != 1 {
+		// We can skip the work.  They are selecting everything, and since they don't have one endpoint,
+		// we can't disable the health checks.  So we can just use their list.
 		return svc.EndpointTable
 	}
+
 	endpoints := make([]Endpoint, 0, len(svc.EndpointTable))
-	for i := range svc.EndpointTable {
-		endpoint := svc.EndpointTable[i]
-		if endpoint.PortName == alias.PreferPort || endpoint.Port == alias.PreferPort {
+	for _, endpoint := range svc.EndpointTable {
+		// Filter the list:
+		// - If PreferPort length is 0, match everything
+		// - Otherwise, if the PortName or Port matches PreferPort, then we have a match
+		if len(alias.PreferPort) == 0 || endpoint.PortName == alias.PreferPort || endpoint.Port == alias.PreferPort {
 			endpoints = append(endpoints, endpoint)
 		}
 	}
+
+	// We want to disable endpoint checks if there is only one endpoint since there's no point in
+	// testing and removing it from the set ahead of time since there are no other eps to replace it.
+	if len(endpoints) == 1 {
+		endpoints[0].NoHealthCheck = true
+	}
+
 	return endpoints
 }
 

pkg/router/template/template_helper_test.go

@@ -1,6 +1,7 @@
 package templaterouter
 
 import (
+	"reflect"
 	"regexp"
 	"testing"
 )
@@ -297,3 +298,168 @@ func TestMatchPattern(t *testing.T) {
 		}
 	}
 }
+
+func TestEndpointsForAlias(t *testing.T) {
+	testEndpoints := []struct {
+		name       string
+		preferPort string
+		eps        []Endpoint
+		expected   []Endpoint
+	}{
+		{
+			name:       "Test no preference with a single endpoint",
+			preferPort: "",
+			eps: []Endpoint{
+				{
+					ID:       "ep1",
+					Port:     "80",
+					PortName: "http",
+				},
+			},
+			expected: []Endpoint{
+				{
+					ID:            "ep1",
+					Port:          "80",
+					PortName:      "http",
+					NoHealthCheck: true,
+				},
+			},
+		},
+		{
+			name:       "Test no preference with multiple",
+			preferPort: "",
+			eps: []Endpoint{
+				{
+					ID:       "ep1",
+					Port:     "80",
+					PortName: "http",
+				},
+				{
+					ID:       "ep1",
+					Port:     "443",
+					PortName: "https",
+				},
+				{
+					ID:       "ep2",
+					Port:     "80",
+					PortName: "http",
+				},
+				{
+					ID:       "ep3",
+					Port:     "80",
+					PortName: "http",
+				},
+			},
+			expected: []Endpoint{
+				{
+					ID:       "ep1",
+					Port:     "80",
+					PortName: "http",
+				},
+				{
+					ID:       "ep1",
+					Port:     "443",
+					PortName: "https",
+				},
+				{
+					ID:       "ep2",
+					Port:     "80",
+					PortName: "http",
+				},
+				{
+					ID:       "ep3",
+					Port:     "80",
+					PortName: "http",
+				},
+			},
+		},
+		{
+			name:       "Test a preference resulting in multiple",
+			preferPort: "http",
+			eps: []Endpoint{
+				{
+					ID:       "ep1",
+					Port:     "80",
+					PortName: "http",
+				},
+				{
+					ID:       "ep1",
+					Port:     "443",
+					PortName: "https",
+				},
+				{
+					ID:       "ep2",
+					Port:     "80",
+					PortName: "http",
+				},
+				{
+					ID:       "ep3",
+					Port:     "80",
+					PortName: "http",
+				},
+			},
+			expected: []Endpoint{
+				{
+					ID:       "ep1",
+					Port:     "80",
+					PortName: "http",
+				},
+				{
+					ID:       "ep2",
+					Port:     "80",
+					PortName: "http",
+				},
+				{
+					ID:       "ep3",
+					Port:     "80",
+					PortName: "http",
+				},
+			},
+		},
+		{
+			name:       "Test a preference resulting in one",
+			preferPort: "https",
+			eps: []Endpoint{
+				{
+					ID:       "ep1",
+					Port:     "80",
+					PortName: "http",
+				},
+				{
+					ID:       "ep1",
+					Port:     "443",
+					PortName: "https",
+				},
+				{
+					ID:       "ep2",
+					Port:     "80",
+					PortName: "http",
+				},
+				{
+					ID:       "ep3",
+					Port:     "80",
+					PortName: "http",
+				},
+			},
+			expected: []Endpoint{
+				{
+					ID:            "ep1",
+					Port:          "443",
+					PortName:      "https",
+					NoHealthCheck: true,
+				},
+			},
+		},
+	}
+
+	for _, te := range testEndpoints {
+		eps := endpointsForAlias(
+			ServiceAliasConfig{PreferPort: te.preferPort},
+			ServiceUnit{EndpointTable: te.eps},
+		)
+
+		if !reflect.DeepEqual(te.expected, eps) {
+			t.Errorf("%s: expected result: %#v, got: %#v", te.name, te.expected, eps)
+		}
+	}
+}