Merge pull request #1231 from spencerhance/cp-1209-release-1-10

Cherry-pick #1209  [Expose custom health check ports in the firewall] into release-1-10

cmd/e2e-test/healthcheck_test.go

@@ -61,7 +61,7 @@ func TestHealthCheck(t *testing.T) {
 			beConfig: fuzz.NewBackendConfigBuilder("", "backendconfig-1").Build(),
 			want: &backendconfig.HealthCheckConfig{
 				RequestPath: pstring("/my-path"),
-				Port:        pint64(8080), // Matches the targetPort
+				Port:        pint64(9000), // Purposely does not match deployment, we verify it separately
 			},
 		},
 	} {
@@ -106,9 +106,15 @@ func TestHealthCheck(t *testing.T) {
 			}
 			t.Logf("Ingress created (%s/%s)", s.Namespace, ing.Name)
 
-			ing, err = e2e.WaitForIngress(s, ing, nil)
-			if err != nil {
-				t.Fatalf("error waiting for Ingress to stabilize: %v", err)
+			// If health check port does not match deployment, then the deployment will be unreachable
+			if tc.want.Port != nil {
+				options := &e2e.WaitForIngressOptions{ExpectUnreachable: true}
+				ing, _ = e2e.WaitForIngress(s, ing, options)
+			} else {
+				ing, err = e2e.WaitForIngress(s, ing, nil)
+				if err != nil {
+					t.Fatalf("error waiting for Ingress to stabilize: %v", err)
+				}
 			}
 			t.Logf("GCLB resources created (%s/%s)", s.Namespace, ing.Name)
 

pkg/firewalls/controller.go

@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	"reflect"
+	"strconv"
 	"time"
 
 	apiv1 "k8s.io/api/core/v1"
@@ -181,8 +182,15 @@ func (fwc *FirewallController) sync(key string) error {
 		}
 	}
 
+	var additionalPorts []string
+	if flags.F.EnableBackendConfigHealthCheck {
+		hcPorts := fwc.getCustomHealthCheckPorts(gceSvcPorts)
+		additionalPorts = append(additionalPorts, hcPorts...)
+	}
+	additionalPorts = append(additionalPorts, negPorts...)
+
 	// Ensure firewall rule for the cluster and pass any NEG endpoint ports.
-	if err := fwc.firewallPool.Sync(nodeNames, negPorts, additionalRanges); err != nil {
+	if err := fwc.firewallPool.Sync(nodeNames, additionalPorts, additionalRanges); err != nil {
 		if fwErr, ok := err.(*FirewallXPNError); ok {
 			// XPN: Raise an event on each ingress
 			for _, ing := range gceIngresses {
@@ -217,3 +225,15 @@ func (fwc *FirewallController) ilbFirewallSrcRange(gceIngresses []*v1beta1.Ingre
 
 	return "", ErrNoILBIngress
 }
+
+func (fwc *FirewallController) getCustomHealthCheckPorts(svcPorts []utils.ServicePort) []string {
+	var result []string
+
+	for _, svcPort := range svcPorts {
+		if svcPort.BackendConfig != nil && svcPort.BackendConfig.Spec.HealthCheck != nil && svcPort.BackendConfig.Spec.HealthCheck.Port != nil {
+			result = append(result, strconv.FormatInt(*svcPort.BackendConfig.Spec.HealthCheck.Port, 10))
+		}
+	}
+
+	return result
+}

pkg/firewalls/controller_test.go

@@ -21,10 +21,12 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/go-cmp/cmp"
 	api_v1 "k8s.io/api/core/v1"
 	"k8s.io/api/networking/v1beta1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/fake"
+	v1 "k8s.io/ingress-gce/pkg/apis/backendconfig/v1"
 	backendconfigclient "k8s.io/ingress-gce/pkg/backendconfig/client/clientset/versioned/fake"
 	test "k8s.io/ingress-gce/pkg/test"
 	"k8s.io/ingress-gce/pkg/utils"
@@ -103,3 +105,39 @@ func TestFirewallCreateDelete(t *testing.T) {
 		t.Fatalf("cloud.GetFirewall(%v) = _, %v, want _, 404 error", ruleName, err)
 	}
 }
+
+func TestGetCustomHealthCheckPorts(t *testing.T) {
+	t.Parallel()
+
+	testCases := []struct {
+		desc     string
+		svcPorts []utils.ServicePort
+		expect   []string
+	}{
+		{
+			desc:     "One service port with custom port",
+			svcPorts: []utils.ServicePort{utils.ServicePort{BackendConfig: &v1.BackendConfig{Spec: v1.BackendConfigSpec{HealthCheck: &v1.HealthCheckConfig{Port: utils.NewInt64Pointer(8000)}}}}},
+			expect:   []string{"8000"},
+		},
+		{
+			desc: "Two service ports with custom port",
+			svcPorts: []utils.ServicePort{utils.ServicePort{BackendConfig: &v1.BackendConfig{Spec: v1.BackendConfigSpec{HealthCheck: &v1.HealthCheckConfig{Port: utils.NewInt64Pointer(8000)}}}},
+				utils.ServicePort{BackendConfig: &v1.BackendConfig{Spec: v1.BackendConfigSpec{HealthCheck: &v1.HealthCheckConfig{Port: utils.NewInt64Pointer(9000)}}}}},
+			expect: []string{"8000", "9000"},
+		},
+		{
+			desc:   "No service ports",
+			expect: nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			fwc := newFirewallController()
+			result := fwc.getCustomHealthCheckPorts(tc.svcPorts)
+			if diff := cmp.Diff(tc.expect, result); diff != "" {
+				t.Errorf("unexpected diff of ports (-want +got): \n%s", diff)
+			}
+		})
+	}
+}

pkg/firewalls/firewalls_test.go

@@ -17,13 +17,13 @@ limitations under the License.
 package firewalls
 
 import (
-	"k8s.io/kubernetes/pkg/util/slice"
 	"strings"
 	"testing"
 
 	"google.golang.org/api/compute/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/ingress-gce/pkg/utils/namer"
+	"k8s.io/kubernetes/pkg/util/slice"
 	"k8s.io/legacy-cloud-providers/gce"
 )
 

pkg/utils/utils.go

@@ -604,3 +604,8 @@ func MakeL4ILBServiceDescription(svcName, ip string, version meta.Version) (stri
 func NewStringPointer(s string) *string {
 	return &s
 }
+
+// NewInt64Pointer returns a pointer to the provided int64 literal
+func NewInt64Pointer(i int64) *int64 {
+	return &i
+}