From 60d5d6e25d65702c91727f4d807de7aca1bbe62d Mon Sep 17 00:00:00 2001
From: LorcanMcVeigh <lorcan.mcveigh@gmail.com>
Date: Tue, 16 Feb 2021 13:53:59 +0000
Subject: [PATCH 1/9] Add health checks to transport server

---
 .../configuration/transportserver-resource.md | 58 +++++++++++++++
 internal/configs/transportserver.go           | 30 +++++++-
 internal/configs/transportserver_test.go      | 30 ++++++++
 .../version2/nginx-plus.transportserver.tmpl  |  6 ++
 internal/configs/version2/stream.go           |  7 ++
 .../v1alpha1/zz_generated.deepcopy.go         | 16 ++++
 .../validation/transportserver.go             | 24 ++++++
 .../validation/transportserver_test.go        | 73 +++++++++++++++++++
 8 files changed, 241 insertions(+), 3 deletions(-)

diff --git a/docs-web/configuration/transportserver-resource.md b/docs-web/configuration/transportserver-resource.md
index 5752f6c354..2cee171b34 100644
--- a/docs-web/configuration/transportserver-resource.md
+++ b/docs-web/configuration/transportserver-resource.md
@@ -192,6 +192,64 @@ failTimeout: 30s
 
 ```
 
+
+### Upstream.Healthcheck
+
+The Healthcheck defines an [active health check](https://nginx.org/en/docs/stream/ngx_stream_upstream_hc_module.html?#health_check). In the example below we enable a health check for an upstream and configure all the available parameters:
+
+```yaml
+name: secure-app 
+service: secure-app
+port: 8443
+healthCheck:
+  enable: true
+  interval: 20s
+  timeout: 30s
+  jitter: 3s
+  fails: 5
+  passes: 5
+  port: 8080
+
+```
+
+```eval_rst
+.. list-table::
+   :header-rows: 1
+
+   * - Field
+     - Description
+     - Type
+     - Required
+   * - ``enable``
+     - Enables a health check for an upstream server. The default is ``false``.
+     - ``boolean``
+     - No
+   * - ``interval``
+     - The interval between two consecutive health checks. The default is ``5s``.
+     - ``string``
+     - No
+   * - ``timeout``
+     - This overrides the timeout set by `proxy_timeout <http://nginx.org/en/docs/stream/ngx_stream_proxy_module.html#proxy_timeout>`_ which is set in [SessionParameters](#sessionparameters) for health checks. The default value is ``5s``.
+     - ``string``
+     - No
+   * - ``jitter``
+     - The time within which each health check will be randomly delayed. By default, there is no delay.
+     - ``string``
+     - No
+   * - ``fails``
+     - The number of consecutive failed health checks of a particular upstream server after which this server will be considered unhealthy. The default is ``1``.
+     - ``integer``
+     - No
+   * - ``passes``
+     - The number of consecutive passed health checks of a particular upstream server after which the server will be considered healthy. The default is ``1``.
+     - ``integer``
+     - No
+   * - ``port``
+     - The port used for health check requests. By default, the port of the upstream is used. Note: in contrast with the port of the upstream, this port is not a service port, but a port of a pod.
+     - ``integer``
+     - No
+```
+
 ### UpstreamParameters
 
 The upstream parameters define various parameters for the upstreams:
diff --git a/internal/configs/transportserver.go b/internal/configs/transportserver.go
index c1d8e7f873..47913c3c0d 100644
--- a/internal/configs/transportserver.go
+++ b/internal/configs/transportserver.go
@@ -32,7 +32,7 @@ func (tsEx *TransportServerEx) String() string {
 func generateTransportServerConfig(transportServerEx *TransportServerEx, listenerPort int, isPlus bool) version2.TransportServerConfig {
 	upstreamNamer := newUpstreamNamerForTransportServer(transportServerEx.TransportServer)
 
-	upstreams := generateStreamUpstreams(transportServerEx, upstreamNamer, isPlus)
+	upstreams, healthCheck := generateStreamUpstreams(transportServerEx, upstreamNamer, isPlus)
 
 	var proxyRequests, proxyResponses *int
 	var connectTimeout, nextUpstreamTimeout string
@@ -80,6 +80,13 @@ func generateTransportServerConfig(transportServerEx *TransportServerEx, listene
 			ProxyNextUpstream:        nextUpstream,
 			ProxyNextUpstreamTimeout: generateTime(nextUpstreamTimeout, "0"),
 			ProxyNextUpstreamTries:   nextUpstreamTries,
+			HealthCheck:              healthCheck.Enabled,
+			HCInterval:               healthCheck.Interval,
+			HCPort:                   healthCheck.Port,
+			HCPasses:                 healthCheck.Passes,
+			HCJitter:                 healthCheck.Jitter,
+			HCFails:                  healthCheck.Fails,
+			HCTimeout:                healthCheck.Timeout,
 		},
 		Upstreams: upstreams,
 	}
@@ -93,14 +100,19 @@ func generateUnixSocket(transportServerEx *TransportServerEx) string {
 	return ""
 }
 
-func generateStreamUpstreams(transportServerEx *TransportServerEx, upstreamNamer *upstreamNamer, isPlus bool) []version2.StreamUpstream {
+func generateStreamUpstreams(transportServerEx *TransportServerEx, upstreamNamer *upstreamNamer, isPlus bool) ([]version2.StreamUpstream, *conf_v1alpha1.HealthCheck) {
 	var upstreams []version2.StreamUpstream
+	var hc *conf_v1alpha1.HealthCheck
 
+	hc = generateTransportServerHealthCheckWithDefaults()
 	for _, u := range transportServerEx.TransportServer.Spec.Upstreams {
 
 		// subselector is not supported yet in TransportServer upstreams. That's why we pass "nil" here
 		endpointsKey := GenerateEndpointsKey(transportServerEx.TransportServer.Namespace, u.Service, nil, uint16(u.Port))
 		endpoints := transportServerEx.Endpoints[endpointsKey]
+		if u.HealthCheck != nil {
+			hc = u.HealthCheck
+		}
 
 		ups := generateStreamUpstream(&u, upstreamNamer, endpoints, isPlus)
 
@@ -112,7 +124,19 @@ func generateStreamUpstreams(transportServerEx *TransportServerEx, upstreamNamer
 		upstreams = append(upstreams, ups)
 	}
 
-	return upstreams
+	return upstreams, hc
+}
+
+func generateTransportServerHealthCheckWithDefaults() *conf_v1alpha1.HealthCheck {
+	return &conf_v1alpha1.HealthCheck{
+		Enabled:  false,
+		Timeout:  "5s",
+		Jitter:   "0",
+		Port:     0,
+		Interval: "5s",
+		Passes:   1,
+		Fails:    1,
+	}
 }
 
 func generateStreamUpstream(upstream *conf_v1alpha1.Upstream, upstreamNamer *upstreamNamer, endpoints []string, isPlus bool) version2.StreamUpstream {
diff --git a/internal/configs/transportserver_test.go b/internal/configs/transportserver_test.go
index 23ade7b3ca..668a1a8ccc 100644
--- a/internal/configs/transportserver_test.go
+++ b/internal/configs/transportserver_test.go
@@ -134,6 +134,13 @@ func TestGenerateTransportServerConfigForTCP(t *testing.T) {
 			ProxyNextUpstreamTries:   0,
 			ProxyNextUpstreamTimeout: "0s",
 			ProxyTimeout:             "50s",
+			HealthCheck:              false,
+			HCTimeout:                "5s",
+			HCJitter:                 "0",
+			HCPort:                   0,
+			HCInterval:               "5s",
+			HCPasses:                 1,
+			HCFails:                  1,
 		},
 	}
 
@@ -217,6 +224,13 @@ func TestGenerateTransportServerConfigForTLSPasstrhough(t *testing.T) {
 			ProxyNextUpstreamTimeout: "0s",
 			ProxyNextUpstreamTries:   0,
 			ProxyTimeout:             "10m",
+			HealthCheck:              false,
+			HCTimeout:                "5s",
+			HCJitter:                 "0",
+			HCPort:                   0,
+			HCInterval:               "5s",
+			HCPasses:                 1,
+			HCFails:                  1,
 		},
 	}
 
@@ -247,6 +261,15 @@ func TestGenerateTransportServerConfigForUDP(t *testing.T) {
 						Name:    "udp-app",
 						Service: "udp-app-svc",
 						Port:    5001,
+						HealthCheck: &conf_v1alpha1.HealthCheck{
+							Enabled:  true,
+							Timeout:  "10s",
+							Jitter:   "4",
+							Port:     90,
+							Interval: "8s",
+							Passes:   3,
+							Fails:    6,
+						},
 					},
 				},
 				UpstreamParameters: &conf_v1alpha1.UpstreamParameters{
@@ -304,6 +327,13 @@ func TestGenerateTransportServerConfigForUDP(t *testing.T) {
 			ProxyNextUpstreamTimeout: "0s",
 			ProxyNextUpstreamTries:   0,
 			ProxyTimeout:             "10m",
+			HealthCheck:              true,
+			HCTimeout:                "10s",
+			HCJitter:                 "4",
+			HCPort:                   90,
+			HCInterval:               "8s",
+			HCPasses:                 3,
+			HCFails:                  6,
 		},
 	}
 
diff --git a/internal/configs/version2/nginx-plus.transportserver.tmpl b/internal/configs/version2/nginx-plus.transportserver.tmpl
index 286fbf51e4..f20a562163 100644
--- a/internal/configs/version2/nginx-plus.transportserver.tmpl
+++ b/internal/configs/version2/nginx-plus.transportserver.tmpl
@@ -30,6 +30,12 @@ server {
 
     proxy_pass {{ $s.ProxyPass }};
 
+    {{ if $s.HealthCheck }}
+    health_check interval={{ $s.HCInterval }} port={{ $s.HCPort }} 
+        passes={{ $s.HCPasses }} jitter={{ $s.HCJitter }} fails={{ $s.HCFails }} {{ if $s.UDP }}udp{{ end }};
+    health_check_timeout {{ $s.HCTimeout }};
+    {{ end }}
+
     proxy_timeout {{ $s.ProxyTimeout }};
     proxy_connect_timeout {{ $s.ProxyConnectTimeout }};
 
diff --git a/internal/configs/version2/stream.go b/internal/configs/version2/stream.go
index 469fd669d8..6445b83d7e 100644
--- a/internal/configs/version2/stream.go
+++ b/internal/configs/version2/stream.go
@@ -37,6 +37,13 @@ type StreamServer struct {
 	ProxyNextUpstream        bool
 	ProxyNextUpstreamTimeout string
 	ProxyNextUpstreamTries   int
+	HealthCheck              bool
+	HCInterval               string
+	HCPort                   int
+	HCPasses                 int
+	HCJitter                 string
+	HCFails                  int
+	HCTimeout                string
 }
 
 // TLSPassthroughHostsConfig defines a mapping between TLS Passthrough hosts and the corresponding unix sockets.
diff --git a/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go
index 0adca67c12..cef814fd9c 100644
--- a/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go
+++ b/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go
@@ -105,6 +105,22 @@ func (in *GlobalConfigurationSpec) DeepCopy() *GlobalConfigurationSpec {
 	return out
 }
 
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *HealthCheck) DeepCopyInto(out *HealthCheck) {
+	*out = *in
+	return
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HealthCheck.
+func (in *HealthCheck) DeepCopy() *HealthCheck {
+	if in == nil {
+		return nil
+	}
+	out := new(HealthCheck)
+	in.DeepCopyInto(out)
+	return out
+}
+
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *Listener) DeepCopyInto(out *Listener) {
 	*out = *in
diff --git a/pkg/apis/configuration/validation/transportserver.go b/pkg/apis/configuration/validation/transportserver.go
index f4065eb462..2a3162c9af 100644
--- a/pkg/apis/configuration/validation/transportserver.go
+++ b/pkg/apis/configuration/validation/transportserver.go
@@ -154,11 +154,35 @@ func validateTransportServerUpstreams(upstreams []v1alpha1.Upstream, fieldPath *
 		for _, msg := range validation.IsValidPortNum(u.Port) {
 			allErrs = append(allErrs, field.Invalid(idxPath.Child("port"), u.Port, msg))
 		}
+
+		allErrs = append(allErrs, validateTSUpstreamHealthChecks(u.HealthCheck, idxPath.Child("healthChecks"))...)
 	}
 
 	return allErrs, upstreamNames
 }
 
+func validateTSUpstreamHealthChecks(hc *v1alpha1.HealthCheck, fieldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+
+	if hc == nil {
+		return allErrs
+	}
+
+	allErrs = append(allErrs, validateTime(hc.Timeout, fieldPath.Child("timeout"))...)
+	allErrs = append(allErrs, validateTime(hc.Interval, fieldPath.Child("interval"))...)
+	allErrs = append(allErrs, validateTime(hc.Jitter, fieldPath.Child("jitter"))...)
+	allErrs = append(allErrs, validatePositiveIntOrZero(hc.Fails, fieldPath.Child("fails"))...)
+	allErrs = append(allErrs, validatePositiveIntOrZero(hc.Passes, fieldPath.Child("passes"))...)
+
+	if hc.Port > 0 {
+		for _, msg := range validation.IsValidPortNum(hc.Port) {
+			allErrs = append(allErrs, field.Invalid(fieldPath.Child("port"), hc.Port, msg))
+		}
+	}
+
+	return allErrs
+}
+
 func validateTransportServerUpstreamParameters(upstreamParameters *v1alpha1.UpstreamParameters, fieldPath *field.Path, protocol string) field.ErrorList {
 	allErrs := field.ErrorList{}
 
diff --git a/pkg/apis/configuration/validation/transportserver_test.go b/pkg/apis/configuration/validation/transportserver_test.go
index 2a30173ce3..2ebf43e237 100644
--- a/pkg/apis/configuration/validation/transportserver_test.go
+++ b/pkg/apis/configuration/validation/transportserver_test.go
@@ -387,6 +387,79 @@ func TestValidateListenerProtocol(t *testing.T) {
 	}
 }
 
+func TestValidateTSUpstreamHealthChecks(t *testing.T) {
+	tests := []struct {
+		healthCheck *v1alpha1.HealthCheck
+		msg         string
+	}{
+		{
+			healthCheck: nil,
+			msg:         "nil health check",
+		},
+		{
+			healthCheck: &v1alpha1.HealthCheck{},
+			msg:         "non nil health check",
+		},
+		{
+			healthCheck: &v1alpha1.HealthCheck{
+				Enabled:  true,
+				Timeout:  "30s",
+				Jitter:   "5s",
+				Port:     88,
+				Interval: "10",
+				Passes:   3,
+				Fails:    4,
+			},
+			msg: "valid Health check",
+		},
+	}
+	for _, test := range tests {
+		allErrs := validateTSUpstreamHealthChecks(test.healthCheck, field.NewPath("healthCheck"))
+		if len(allErrs) > 0 {
+			t.Errorf("validateUpstreamHealthChecks() returned errors %v  for valid input for the case of %s", allErrs, test.msg)
+		}
+	}
+}
+
+func TestValidateTSUpstreamHealthChecksFails(t *testing.T) {
+	tests := []struct {
+		healthCheck *v1alpha1.HealthCheck
+		msg         string
+	}{
+		{
+			healthCheck: &v1alpha1.HealthCheck{
+				Enabled:  true,
+				Timeout:  "-30s",
+				Jitter:   "5s",
+				Port:     88,
+				Interval: "10",
+				Passes:   3,
+				Fails:    4,
+			},
+			msg: "invalid timeout",
+		},
+		{
+			healthCheck: &v1alpha1.HealthCheck{
+				Enabled:  true,
+				Timeout:  "30s",
+				Jitter:   "5s",
+				Port:     4000000000000000,
+				Interval: "10",
+				Passes:   3,
+				Fails:    4,
+			},
+			msg: "invalid port number",
+		},
+	}
+
+	for _, test := range tests {
+		allErrs := validateTSUpstreamHealthChecks(test.healthCheck, field.NewPath("healthCheck"))
+		if len(allErrs) == 0 {
+			t.Errorf("validateUpstreamHealthChecks() returned no error for invalid input %v", test.msg)
+		}
+	}
+}
+
 func TestValidateUpstreamParameters(t *testing.T) {
 	tests := []struct {
 		parameters *v1alpha1.UpstreamParameters

From fe76bd12ee2538eb90d7f4b270fd5f3c084e9247 Mon Sep 17 00:00:00 2001
From: LorcanMcVeigh <lorcan.mcveigh@gmail.com>
Date: Thu, 18 Feb 2021 15:03:55 +0000
Subject: [PATCH 2/9] Feedback

---
 .../configuration/transportserver-resource.md |  4 +-
 internal/configs/transportserver.go           | 60 ++++++++++++-----
 internal/configs/transportserver_test.go      | 64 +++++++++----------
 .../version2/nginx-plus.transportserver.tmpl  |  8 +--
 internal/configs/version2/stream.go           | 18 ++++--
 internal/configs/version2/templates_test.go   |  9 +++
 .../validation/transportserver_test.go        | 50 ++++++++++++++-
 7 files changed, 150 insertions(+), 63 deletions(-)

diff --git a/docs-web/configuration/transportserver-resource.md b/docs-web/configuration/transportserver-resource.md
index 2cee171b34..36292ffdb4 100644
--- a/docs-web/configuration/transportserver-resource.md
+++ b/docs-web/configuration/transportserver-resource.md
@@ -14,6 +14,7 @@ This document is the reference documentation for the TransportServer resource. T
   - [TransportServer Specification](#transportserver-specification)
     - [Listener](#listener)
     - [Upstream](#upstream)
+      - [Health Checks](#upstream-healthchecks)
     - [UpstreamParameters](#upstreamparameters)
     - [SessionParameters](#sessionparameters)
     - [Action](#action)
@@ -192,7 +193,6 @@ failTimeout: 30s
 
 ```
 
-
 ### Upstream.Healthcheck
 
 The Healthcheck defines an [active health check](https://nginx.org/en/docs/stream/ngx_stream_upstream_hc_module.html?#health_check). In the example below we enable a health check for an upstream and configure all the available parameters:
@@ -229,7 +229,7 @@ healthCheck:
      - ``string``
      - No
    * - ``timeout``
-     - This overrides the timeout set by `proxy_timeout <http://nginx.org/en/docs/stream/ngx_stream_proxy_module.html#proxy_timeout>`_ which is set in [SessionParameters](#sessionparameters) for health checks. The default value is ``5s``.
+     - This overrides the timeout set by `proxy_timeout <http://nginx.org/en/docs/stream/ngx_stream_proxy_module.html#proxy_timeout>`_ which is set in `SessionParameters` for health checks. The default value is ``5s``.
      - ``string``
      - No
    * - ``jitter``
diff --git a/internal/configs/transportserver.go b/internal/configs/transportserver.go
index 47913c3c0d..61687baae9 100644
--- a/internal/configs/transportserver.go
+++ b/internal/configs/transportserver.go
@@ -32,7 +32,7 @@ func (tsEx *TransportServerEx) String() string {
 func generateTransportServerConfig(transportServerEx *TransportServerEx, listenerPort int, isPlus bool) version2.TransportServerConfig {
 	upstreamNamer := newUpstreamNamerForTransportServer(transportServerEx.TransportServer)
 
-	upstreams, healthCheck := generateStreamUpstreams(transportServerEx, upstreamNamer, isPlus)
+	upstreams, healthCheck := generateStreamUpstreamsAndHealthCheck(transportServerEx, upstreamNamer, isPlus)
 
 	var proxyRequests, proxyResponses *int
 	var connectTimeout, nextUpstreamTimeout string
@@ -80,13 +80,7 @@ func generateTransportServerConfig(transportServerEx *TransportServerEx, listene
 			ProxyNextUpstream:        nextUpstream,
 			ProxyNextUpstreamTimeout: generateTime(nextUpstreamTimeout, "0"),
 			ProxyNextUpstreamTries:   nextUpstreamTries,
-			HealthCheck:              healthCheck.Enabled,
-			HCInterval:               healthCheck.Interval,
-			HCPort:                   healthCheck.Port,
-			HCPasses:                 healthCheck.Passes,
-			HCJitter:                 healthCheck.Jitter,
-			HCFails:                  healthCheck.Fails,
-			HCTimeout:                healthCheck.Timeout,
+			HealthCheck:              healthCheck,
 		},
 		Upstreams: upstreams,
 	}
@@ -100,19 +94,17 @@ func generateUnixSocket(transportServerEx *TransportServerEx) string {
 	return ""
 }
 
-func generateStreamUpstreams(transportServerEx *TransportServerEx, upstreamNamer *upstreamNamer, isPlus bool) ([]version2.StreamUpstream, *conf_v1alpha1.HealthCheck) {
+func generateStreamUpstreamsAndHealthCheck(transportServerEx *TransportServerEx, upstreamNamer *upstreamNamer, isPlus bool) ([]version2.StreamUpstream, *version2.StreamHealthCheck) {
 	var upstreams []version2.StreamUpstream
-	var hc *conf_v1alpha1.HealthCheck
+	var hc *version2.StreamHealthCheck
 
-	hc = generateTransportServerHealthCheckWithDefaults()
 	for _, u := range transportServerEx.TransportServer.Spec.Upstreams {
 
 		// subselector is not supported yet in TransportServer upstreams. That's why we pass "nil" here
 		endpointsKey := GenerateEndpointsKey(transportServerEx.TransportServer.Namespace, u.Service, nil, uint16(u.Port))
 		endpoints := transportServerEx.Endpoints[endpointsKey]
-		if u.HealthCheck != nil {
-			hc = u.HealthCheck
-		}
+
+		hc = generateTransportServerHealthCheck(u.HealthCheck)
 
 		ups := generateStreamUpstream(&u, upstreamNamer, endpoints, isPlus)
 
@@ -127,8 +119,44 @@ func generateStreamUpstreams(transportServerEx *TransportServerEx, upstreamNamer
 	return upstreams, hc
 }
 
-func generateTransportServerHealthCheckWithDefaults() *conf_v1alpha1.HealthCheck {
-	return &conf_v1alpha1.HealthCheck{
+func generateTransportServerHealthCheck(healthCheck *conf_v1alpha1.HealthCheck) *version2.StreamHealthCheck {
+	if healthCheck == nil || !healthCheck.Enabled {
+		return generateTransportServerHealthCheckWithDefaults()
+	}
+
+	hc := generateTransportServerHealthCheckWithDefaults()
+
+	hc.Enabled = healthCheck.Enabled
+
+	if healthCheck.Interval != "" {
+		hc.Interval = healthCheck.Interval
+	}
+
+	if healthCheck.Jitter != "" {
+		hc.Jitter = healthCheck.Jitter
+	}
+
+	if healthCheck.Fails > 0 {
+		hc.Fails = healthCheck.Fails
+	}
+
+	if healthCheck.Passes > 0 {
+		hc.Passes = healthCheck.Passes
+	}
+
+	if healthCheck.Port > 0 {
+		hc.Port = healthCheck.Port
+	}
+
+	if healthCheck.Timeout != "" {
+		hc.Timeout = healthCheck.Timeout
+	}
+
+	return hc
+}
+
+func generateTransportServerHealthCheckWithDefaults() *version2.StreamHealthCheck {
+	return &version2.StreamHealthCheck{
 		Enabled:  false,
 		Timeout:  "5s",
 		Jitter:   "0",
diff --git a/internal/configs/transportserver_test.go b/internal/configs/transportserver_test.go
index 668a1a8ccc..117dfd6cea 100644
--- a/internal/configs/transportserver_test.go
+++ b/internal/configs/transportserver_test.go
@@ -134,13 +134,15 @@ func TestGenerateTransportServerConfigForTCP(t *testing.T) {
 			ProxyNextUpstreamTries:   0,
 			ProxyNextUpstreamTimeout: "0s",
 			ProxyTimeout:             "50s",
-			HealthCheck:              false,
-			HCTimeout:                "5s",
-			HCJitter:                 "0",
-			HCPort:                   0,
-			HCInterval:               "5s",
-			HCPasses:                 1,
-			HCFails:                  1,
+			HealthCheck: &version2.StreamHealthCheck{
+				Enabled:  false,
+				Timeout:  "5s",
+				Jitter:   "0",
+				Port:     0,
+				Interval: "5s",
+				Passes:   1,
+				Fails:    1,
+			},
 		},
 	}
 
@@ -224,13 +226,15 @@ func TestGenerateTransportServerConfigForTLSPasstrhough(t *testing.T) {
 			ProxyNextUpstreamTimeout: "0s",
 			ProxyNextUpstreamTries:   0,
 			ProxyTimeout:             "10m",
-			HealthCheck:              false,
-			HCTimeout:                "5s",
-			HCJitter:                 "0",
-			HCPort:                   0,
-			HCInterval:               "5s",
-			HCPasses:                 1,
-			HCFails:                  1,
+			HealthCheck: &version2.StreamHealthCheck{
+				Enabled:  false,
+				Timeout:  "5s",
+				Jitter:   "0",
+				Port:     0,
+				Interval: "5s",
+				Passes:   1,
+				Fails:    1,
+			},
 		},
 	}
 
@@ -258,18 +262,10 @@ func TestGenerateTransportServerConfigForUDP(t *testing.T) {
 				},
 				Upstreams: []conf_v1alpha1.Upstream{
 					{
-						Name:    "udp-app",
-						Service: "udp-app-svc",
-						Port:    5001,
-						HealthCheck: &conf_v1alpha1.HealthCheck{
-							Enabled:  true,
-							Timeout:  "10s",
-							Jitter:   "4",
-							Port:     90,
-							Interval: "8s",
-							Passes:   3,
-							Fails:    6,
-						},
+						Name:        "udp-app",
+						Service:     "udp-app-svc",
+						Port:        5001,
+						HealthCheck: &conf_v1alpha1.HealthCheck{},
 					},
 				},
 				UpstreamParameters: &conf_v1alpha1.UpstreamParameters{
@@ -327,13 +323,15 @@ func TestGenerateTransportServerConfigForUDP(t *testing.T) {
 			ProxyNextUpstreamTimeout: "0s",
 			ProxyNextUpstreamTries:   0,
 			ProxyTimeout:             "10m",
-			HealthCheck:              true,
-			HCTimeout:                "10s",
-			HCJitter:                 "4",
-			HCPort:                   90,
-			HCInterval:               "8s",
-			HCPasses:                 3,
-			HCFails:                  6,
+			HealthCheck: &version2.StreamHealthCheck{
+				Enabled:  false,
+				Timeout:  "5s",
+				Jitter:   "0",
+				Port:     0,
+				Interval: "5s",
+				Passes:   1,
+				Fails:    1,
+			},
 		},
 	}
 
diff --git a/internal/configs/version2/nginx-plus.transportserver.tmpl b/internal/configs/version2/nginx-plus.transportserver.tmpl
index f20a562163..276d45c751 100644
--- a/internal/configs/version2/nginx-plus.transportserver.tmpl
+++ b/internal/configs/version2/nginx-plus.transportserver.tmpl
@@ -30,10 +30,10 @@ server {
 
     proxy_pass {{ $s.ProxyPass }};
 
-    {{ if $s.HealthCheck }}
-    health_check interval={{ $s.HCInterval }} port={{ $s.HCPort }} 
-        passes={{ $s.HCPasses }} jitter={{ $s.HCJitter }} fails={{ $s.HCFails }} {{ if $s.UDP }}udp{{ end }};
-    health_check_timeout {{ $s.HCTimeout }};
+    {{ if $s.HealthCheck.Enabled }}
+    health_check interval={{ $s.HealthCheck.Interval }} port={{ $s.HealthCheck.Port }} 
+        passes={{ $s.HealthCheck.Passes }} jitter={{ $s.HealthCheck.Jitter }} fails={{ $s.HealthCheck.Fails }} {{ if $s.UDP }}udp{{ end }};
+    health_check_timeout {{ $s.HealthCheck.Timeout }};
     {{ end }}
 
     proxy_timeout {{ $s.ProxyTimeout }};
diff --git a/internal/configs/version2/stream.go b/internal/configs/version2/stream.go
index 6445b83d7e..de01a0ac45 100644
--- a/internal/configs/version2/stream.go
+++ b/internal/configs/version2/stream.go
@@ -37,13 +37,17 @@ type StreamServer struct {
 	ProxyNextUpstream        bool
 	ProxyNextUpstreamTimeout string
 	ProxyNextUpstreamTries   int
-	HealthCheck              bool
-	HCInterval               string
-	HCPort                   int
-	HCPasses                 int
-	HCJitter                 string
-	HCFails                  int
-	HCTimeout                string
+	HealthCheck              *StreamHealthCheck
+}
+
+type StreamHealthCheck struct {
+	Enabled  bool
+	Interval string
+	Port     int
+	Passes   int
+	Jitter   string
+	Fails    int
+	Timeout  string
 }
 
 // TLSPassthroughHostsConfig defines a mapping between TLS Passthrough hosts and the corresponding unix sockets.
diff --git a/internal/configs/version2/templates_test.go b/internal/configs/version2/templates_test.go
index da972cc897..fab7ffe4f4 100644
--- a/internal/configs/version2/templates_test.go
+++ b/internal/configs/version2/templates_test.go
@@ -342,6 +342,15 @@ var transportServerCfg = TransportServerConfig{
 		ProxyNextUpstream:        true,
 		ProxyNextUpstreamTimeout: "10s",
 		ProxyNextUpstreamTries:   5,
+		HealthCheck: &StreamHealthCheck{
+			Enabled:  false,
+			Timeout:  "5s",
+			Jitter:   "0",
+			Port:     0,
+			Interval: "5s",
+			Passes:   1,
+			Fails:    1,
+		},
 	},
 }
 
diff --git a/pkg/apis/configuration/validation/transportserver_test.go b/pkg/apis/configuration/validation/transportserver_test.go
index 2ebf43e237..e41221f1b0 100644
--- a/pkg/apis/configuration/validation/transportserver_test.go
+++ b/pkg/apis/configuration/validation/transportserver_test.go
@@ -175,7 +175,7 @@ func TestValidateTransportServerUpstreamsFails(t *testing.T) {
 	for _, test := range tests {
 		allErrs, resultUpstreamNames := validateTransportServerUpstreams(test.upstreams, field.NewPath("upstreams"))
 		if len(allErrs) == 0 {
-			t.Errorf("validateTransportServerUpstreams() returned errors no errors for the case of %s", test.msg)
+			t.Errorf("validateTransportServerUpstreams() returned no errors for the case of %s", test.msg)
 		}
 		if !resultUpstreamNames.Equal(test.expectedUpstreamNames) {
 			t.Errorf("validateTransportServerUpstreams() returned %v expected %v for the case of %s", resultUpstreamNames, test.expectedUpstreamNames, test.msg)
@@ -450,6 +450,54 @@ func TestValidateTSUpstreamHealthChecksFails(t *testing.T) {
 			},
 			msg: "invalid port number",
 		},
+		{
+			healthCheck: &v1alpha1.HealthCheck{
+				Enabled:  true,
+				Timeout:  "30s",
+				Jitter:   "5s",
+				Port:     40,
+				Interval: "10",
+				Passes:   -3,
+				Fails:    4,
+			},
+			msg: "invalid passes value",
+		},
+		{
+			healthCheck: &v1alpha1.HealthCheck{
+				Enabled:  true,
+				Timeout:  "30s",
+				Jitter:   "5s",
+				Port:     40,
+				Interval: "10",
+				Passes:   3,
+				Fails:    -4,
+			},
+			msg: "invalid fails value",
+		},
+		{
+			healthCheck: &v1alpha1.HealthCheck{
+				Enabled:  true,
+				Timeout:  "30s",
+				Jitter:   "5s",
+				Port:     40,
+				Interval: "ten",
+				Passes:   3,
+				Fails:    4,
+			},
+			msg: "invalid interval value",
+		},
+		{
+			healthCheck: &v1alpha1.HealthCheck{
+				Enabled:  true,
+				Timeout:  "30s",
+				Jitter:   "5sec",
+				Port:     40,
+				Interval: "10",
+				Passes:   3,
+				Fails:    4,
+			},
+			msg: "invalid jitter value",
+		},
 	}
 
 	for _, test := range tests {

From d3955f002dfb4d85e861edc0f5d021c3ddda3726 Mon Sep 17 00:00:00 2001
From: LorcanMcVeigh <lorcan.mcveigh@gmail.com>
Date: Fri, 19 Feb 2021 10:43:57 +0000
Subject: [PATCH 3/9] Feedback

---
 .../configuration/transportserver-resource.md |  2 +-
 internal/configs/transportserver.go           |  2 +-
 internal/configs/transportserver_test.go      | 30 ++-----------------
 .../version2/nginx-plus.transportserver.tmpl  |  2 +-
 internal/configs/version2/stream.go           |  1 +
 5 files changed, 7 insertions(+), 30 deletions(-)

diff --git a/docs-web/configuration/transportserver-resource.md b/docs-web/configuration/transportserver-resource.md
index 36292ffdb4..859b562d64 100644
--- a/docs-web/configuration/transportserver-resource.md
+++ b/docs-web/configuration/transportserver-resource.md
@@ -14,7 +14,7 @@ This document is the reference documentation for the TransportServer resource. T
   - [TransportServer Specification](#transportserver-specification)
     - [Listener](#listener)
     - [Upstream](#upstream)
-      - [Health Checks](#upstream-healthchecks)
+      - [Upstream.Healthcheck](#upstream-healthcheck)
     - [UpstreamParameters](#upstreamparameters)
     - [SessionParameters](#sessionparameters)
     - [Action](#action)
diff --git a/internal/configs/transportserver.go b/internal/configs/transportserver.go
index 61687baae9..aa10133918 100644
--- a/internal/configs/transportserver.go
+++ b/internal/configs/transportserver.go
@@ -121,7 +121,7 @@ func generateStreamUpstreamsAndHealthCheck(transportServerEx *TransportServerEx,
 
 func generateTransportServerHealthCheck(healthCheck *conf_v1alpha1.HealthCheck) *version2.StreamHealthCheck {
 	if healthCheck == nil || !healthCheck.Enabled {
-		return generateTransportServerHealthCheckWithDefaults()
+		return nil
 	}
 
 	hc := generateTransportServerHealthCheckWithDefaults()
diff --git a/internal/configs/transportserver_test.go b/internal/configs/transportserver_test.go
index 117dfd6cea..47a4f7ee39 100644
--- a/internal/configs/transportserver_test.go
+++ b/internal/configs/transportserver_test.go
@@ -134,15 +134,7 @@ func TestGenerateTransportServerConfigForTCP(t *testing.T) {
 			ProxyNextUpstreamTries:   0,
 			ProxyNextUpstreamTimeout: "0s",
 			ProxyTimeout:             "50s",
-			HealthCheck: &version2.StreamHealthCheck{
-				Enabled:  false,
-				Timeout:  "5s",
-				Jitter:   "0",
-				Port:     0,
-				Interval: "5s",
-				Passes:   1,
-				Fails:    1,
-			},
+			HealthCheck:              nil,
 		},
 	}
 
@@ -226,15 +218,7 @@ func TestGenerateTransportServerConfigForTLSPasstrhough(t *testing.T) {
 			ProxyNextUpstreamTimeout: "0s",
 			ProxyNextUpstreamTries:   0,
 			ProxyTimeout:             "10m",
-			HealthCheck: &version2.StreamHealthCheck{
-				Enabled:  false,
-				Timeout:  "5s",
-				Jitter:   "0",
-				Port:     0,
-				Interval: "5s",
-				Passes:   1,
-				Fails:    1,
-			},
+			HealthCheck:              nil,
 		},
 	}
 
@@ -323,15 +307,7 @@ func TestGenerateTransportServerConfigForUDP(t *testing.T) {
 			ProxyNextUpstreamTimeout: "0s",
 			ProxyNextUpstreamTries:   0,
 			ProxyTimeout:             "10m",
-			HealthCheck: &version2.StreamHealthCheck{
-				Enabled:  false,
-				Timeout:  "5s",
-				Jitter:   "0",
-				Port:     0,
-				Interval: "5s",
-				Passes:   1,
-				Fails:    1,
-			},
+			HealthCheck:              nil,
 		},
 	}
 
diff --git a/internal/configs/version2/nginx-plus.transportserver.tmpl b/internal/configs/version2/nginx-plus.transportserver.tmpl
index 276d45c751..1c593f3d23 100644
--- a/internal/configs/version2/nginx-plus.transportserver.tmpl
+++ b/internal/configs/version2/nginx-plus.transportserver.tmpl
@@ -30,7 +30,7 @@ server {
 
     proxy_pass {{ $s.ProxyPass }};
 
-    {{ if $s.HealthCheck.Enabled }}
+    {{ if $s.HealthCheck }}
     health_check interval={{ $s.HealthCheck.Interval }} port={{ $s.HealthCheck.Port }} 
         passes={{ $s.HealthCheck.Passes }} jitter={{ $s.HealthCheck.Jitter }} fails={{ $s.HealthCheck.Fails }} {{ if $s.UDP }}udp{{ end }};
     health_check_timeout {{ $s.HealthCheck.Timeout }};
diff --git a/internal/configs/version2/stream.go b/internal/configs/version2/stream.go
index de01a0ac45..da065791c5 100644
--- a/internal/configs/version2/stream.go
+++ b/internal/configs/version2/stream.go
@@ -40,6 +40,7 @@ type StreamServer struct {
 	HealthCheck              *StreamHealthCheck
 }
 
+// StreamHealthCheck defines a health check for a StreamUpstream in a StreamServer.
 type StreamHealthCheck struct {
 	Enabled  bool
 	Interval string

From d47b37a72e3d2efed9505a01e94fd3278a6608cd Mon Sep 17 00:00:00 2001
From: LorcanMcVeigh <lorcan.mcveigh@gmail.com>
Date: Fri, 19 Feb 2021 11:05:09 +0000
Subject: [PATCH 4/9] Resolve conflicts

---
 .../k8s.nginx.org_transportservers.yaml       | 18 ++++++
 .../crds/k8s.nginx.org_transportservers.yaml  | 18 ++++++
 internal/configs/transportserver_test.go      | 56 +++++++++++++++++++
 pkg/apis/configuration/v1alpha1/types.go      | 22 ++++++--
 .../v1alpha1/zz_generated.deepcopy.go         |  5 ++
 5 files changed, 114 insertions(+), 5 deletions(-)

diff --git a/deployments/common/crds-v1beta1/k8s.nginx.org_transportservers.yaml b/deployments/common/crds-v1beta1/k8s.nginx.org_transportservers.yaml
index b2372e0665..cb003c4282 100644
--- a/deployments/common/crds-v1beta1/k8s.nginx.org_transportservers.yaml
+++ b/deployments/common/crds-v1beta1/k8s.nginx.org_transportservers.yaml
@@ -79,6 +79,24 @@ spec:
                 properties:
                   failTimeout:
                     type: string
+                  healthCheck:
+                    description: HealthCheck defines the parameters for active Upstream HealthChecks.
+                    type: object
+                    properties:
+                      enable:
+                        type: boolean
+                      fails:
+                        type: integer
+                      intervals:
+                        type: string
+                      jitter:
+                        type: string
+                      passes:
+                        type: integer
+                      port:
+                        type: integer
+                      timeout:
+                        type: string
                   maxFails:
                     type: integer
                   name:
diff --git a/deployments/common/crds/k8s.nginx.org_transportservers.yaml b/deployments/common/crds/k8s.nginx.org_transportservers.yaml
index 472b2447bb..f4e276ad8e 100644
--- a/deployments/common/crds/k8s.nginx.org_transportservers.yaml
+++ b/deployments/common/crds/k8s.nginx.org_transportservers.yaml
@@ -80,6 +80,24 @@ spec:
                     properties:
                       failTimeout:
                         type: string
+                      healthCheck:
+                        description: HealthCheck defines the parameters for active Upstream HealthChecks.
+                        type: object
+                        properties:
+                          enable:
+                            type: boolean
+                          fails:
+                            type: integer
+                          intervals:
+                            type: string
+                          jitter:
+                            type: string
+                          passes:
+                            type: integer
+                          port:
+                            type: integer
+                          timeout:
+                            type: string
                       maxFails:
                         type: integer
                       name:
diff --git a/internal/configs/transportserver_test.go b/internal/configs/transportserver_test.go
index 47a4f7ee39..e677e77b45 100644
--- a/internal/configs/transportserver_test.go
+++ b/internal/configs/transportserver_test.go
@@ -4,6 +4,7 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/nginxinc/kubernetes-ingress/internal/configs/version2"
 	conf_v1alpha1 "github.com/nginxinc/kubernetes-ingress/pkg/apis/configuration/v1alpha1"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -349,6 +350,61 @@ func TestGenerateUnixSocket(t *testing.T) {
 	}
 }
 
+func TestGenerateTransportServerHealthChecks(t *testing.T) {
+	tests := []struct {
+		healthCheck *conf_v1alpha1.HealthCheck
+		expected    *version2.StreamHealthCheck
+		msg         string
+	}{
+		{
+			healthCheck: &conf_v1alpha1.HealthCheck{
+				Enabled:  false,
+				Timeout:  "30s",
+				Jitter:   "30s",
+				Port:     80,
+				Interval: "20s",
+				Passes:   4,
+				Fails:    5,
+			},
+			expected: nil,
+			msg:      "health checks disabled",
+		},
+		{
+			healthCheck: &conf_v1alpha1.HealthCheck{},
+			expected:    nil,
+			msg:         "nil health checks",
+		},
+		{
+			healthCheck: &conf_v1alpha1.HealthCheck{
+				Enabled:  true,
+				Timeout:  "30s",
+				Jitter:   "30s",
+				Port:     80,
+				Interval: "20s",
+				Passes:   4,
+				Fails:    5,
+			},
+			expected: &version2.StreamHealthCheck{
+				Enabled:  true,
+				Timeout:  "30s",
+				Jitter:   "30s",
+				Port:     80,
+				Interval: "20s",
+				Passes:   4,
+				Fails:    5,
+			},
+			msg: "valid health checks",
+		},
+	}
+
+	for _, test := range tests {
+		result := generateTransportServerHealthCheck(test.healthCheck)
+		if diff := cmp.Diff(result, test.expected); diff != "" {
+			t.Errorf("generateTransportServerHealthCheck() returned %v but expected %v", result, test.expected)
+		}
+	}
+}
+
 func intPointer(value int) *int {
 	return &value
 }
diff --git a/pkg/apis/configuration/v1alpha1/types.go b/pkg/apis/configuration/v1alpha1/types.go
index b078ae4921..a70d77f66f 100644
--- a/pkg/apis/configuration/v1alpha1/types.go
+++ b/pkg/apis/configuration/v1alpha1/types.go
@@ -77,11 +77,23 @@ type TransportServerListener struct {
 
 // Upstream defines an upstream.
 type Upstream struct {
-	Name        string `json:"name"`
-	Service     string `json:"service"`
-	Port        int    `json:"port"`
-	FailTimeout string `json:"failTimeout"`
-	MaxFails    *int   `json:"maxFails"`
+	Name        string       `json:"name"`
+	Service     string       `json:"service"`
+	Port        int          `json:"port"`
+	FailTimeout string       `json:"failTimeout"`
+	MaxFails    *int         `json:"maxFails"`
+	HealthCheck *HealthCheck `json:"healthCheck"`
+}
+
+// HealthCheck defines the parameters for active Upstream HealthChecks.
+type HealthCheck struct {
+	Enabled  bool   `json:"enable"`
+	Timeout  string `json:"timeout"`
+	Jitter   string `json:"jitter"`
+	Port     int    `json:"port"`
+	Interval string `json:"intervals"`
+	Passes   int    `json:"passes"`
+	Fails    int    `json:"fails"`
 }
 
 // UpstreamParameters defines parameters for an upstream.
diff --git a/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go
index cef814fd9c..d46d30ec7c 100644
--- a/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go
+++ b/pkg/apis/configuration/v1alpha1/zz_generated.deepcopy.go
@@ -276,6 +276,11 @@ func (in *Upstream) DeepCopyInto(out *Upstream) {
 		*out = new(int)
 		**out = **in
 	}
+	if in.HealthCheck != nil {
+		in, out := &in.HealthCheck, &out.HealthCheck
+		*out = new(HealthCheck)
+		**out = **in
+	}
 	return
 }
 

From f8a442bbb791cc6e7865503df508332e4cc11aca Mon Sep 17 00:00:00 2001
From: LorcanMcVeigh <lorcan.mcveigh@gmail.com>
Date: Mon, 22 Feb 2021 09:54:06 +0000
Subject: [PATCH 5/9] Feedback 3

---
 docs-web/configuration/transportserver-resource.md    |  4 ++++
 internal/configs/transportserver.go                   | 11 +++--------
 internal/configs/transportserver_test.go              |  2 +-
 .../configuration/validation/transportserver_test.go  |  4 ++--
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/docs-web/configuration/transportserver-resource.md b/docs-web/configuration/transportserver-resource.md
index 859b562d64..0b061e8e57 100644
--- a/docs-web/configuration/transportserver-resource.md
+++ b/docs-web/configuration/transportserver-resource.md
@@ -190,6 +190,10 @@ failTimeout: 30s
      - Sets the `time <https://nginx.org/en/docs/stream/ngx_stream_upstream_module.html#fail_timeout>`_ during which the specified number of unsuccessful attempts to communicate with the server should happen to consider the server unavailable and the period of time the server will be considered unavailable. The default is ``10s``.
      - ``string``
      - No
+   * - ``healthCheck``
+     - The health check configuration for the Upstream. See the `health_check <https://nginx.org/en/docs/stream/ngx_stream_upstream_hc_module.html#health_check>`_ directive. Note: this feature is supported only in NGINX Plus.
+     - `healthcheck <#upstream-healthcheck>`_
+     - No
 
 ```
 
diff --git a/internal/configs/transportserver.go b/internal/configs/transportserver.go
index aa10133918..4e2049f006 100644
--- a/internal/configs/transportserver.go
+++ b/internal/configs/transportserver.go
@@ -127,14 +127,9 @@ func generateTransportServerHealthCheck(healthCheck *conf_v1alpha1.HealthCheck)
 	hc := generateTransportServerHealthCheckWithDefaults()
 
 	hc.Enabled = healthCheck.Enabled
-
-	if healthCheck.Interval != "" {
-		hc.Interval = healthCheck.Interval
-	}
-
-	if healthCheck.Jitter != "" {
-		hc.Jitter = healthCheck.Jitter
-	}
+	hc.Interval = generateTime(healthCheck.Interval)
+	hc.Jitter = generateTime(healthCheck.Jitter)
+	hc.Timeout = generateTime(healthCheck.Timeout)
 
 	if healthCheck.Fails > 0 {
 		hc.Fails = healthCheck.Fails
diff --git a/internal/configs/transportserver_test.go b/internal/configs/transportserver_test.go
index e677e77b45..7a7c185759 100644
--- a/internal/configs/transportserver_test.go
+++ b/internal/configs/transportserver_test.go
@@ -400,7 +400,7 @@ func TestGenerateTransportServerHealthChecks(t *testing.T) {
 	for _, test := range tests {
 		result := generateTransportServerHealthCheck(test.healthCheck)
 		if diff := cmp.Diff(result, test.expected); diff != "" {
-			t.Errorf("generateTransportServerHealthCheck() returned %v but expected %v", result, test.expected)
+			t.Errorf("generateTransportServerHealthCheck() '%v' mismatch (-want +got):\n%s", test.msg, diff)
 		}
 	}
 }
diff --git a/pkg/apis/configuration/validation/transportserver_test.go b/pkg/apis/configuration/validation/transportserver_test.go
index e41221f1b0..36231b9bac 100644
--- a/pkg/apis/configuration/validation/transportserver_test.go
+++ b/pkg/apis/configuration/validation/transportserver_test.go
@@ -416,7 +416,7 @@ func TestValidateTSUpstreamHealthChecks(t *testing.T) {
 	for _, test := range tests {
 		allErrs := validateTSUpstreamHealthChecks(test.healthCheck, field.NewPath("healthCheck"))
 		if len(allErrs) > 0 {
-			t.Errorf("validateUpstreamHealthChecks() returned errors %v  for valid input for the case of %s", allErrs, test.msg)
+			t.Errorf("validateTSUpstreamHealthChecks() returned errors %v  for valid input for the case of %s", allErrs, test.msg)
 		}
 	}
 }
@@ -503,7 +503,7 @@ func TestValidateTSUpstreamHealthChecksFails(t *testing.T) {
 	for _, test := range tests {
 		allErrs := validateTSUpstreamHealthChecks(test.healthCheck, field.NewPath("healthCheck"))
 		if len(allErrs) == 0 {
-			t.Errorf("validateUpstreamHealthChecks() returned no error for invalid input %v", test.msg)
+			t.Errorf("validateTSUpstreamHealthChecks() returned no error for invalid input %v", test.msg)
 		}
 	}
 }

From c9e5418733d7c3cde14975181ba03b841249dcc5 Mon Sep 17 00:00:00 2001
From: LorcanMcVeigh <lorcan.mcveigh@gmail.com>
Date: Mon, 22 Feb 2021 12:37:46 +0000
Subject: [PATCH 6/9] Address feedback 3

---
 internal/configs/transportserver.go      | 65 ++++++++--------
 internal/configs/transportserver_test.go | 96 ++++++++++++++++++------
 2 files changed, 107 insertions(+), 54 deletions(-)

diff --git a/internal/configs/transportserver.go b/internal/configs/transportserver.go
index 4e2049f006..661edcca1f 100644
--- a/internal/configs/transportserver.go
+++ b/internal/configs/transportserver.go
@@ -32,8 +32,10 @@ func (tsEx *TransportServerEx) String() string {
 func generateTransportServerConfig(transportServerEx *TransportServerEx, listenerPort int, isPlus bool) version2.TransportServerConfig {
 	upstreamNamer := newUpstreamNamerForTransportServer(transportServerEx.TransportServer)
 
-	upstreams, healthCheck := generateStreamUpstreamsAndHealthCheck(transportServerEx, upstreamNamer, isPlus)
+	upstreams := generateStreamUpstreams(transportServerEx, upstreamNamer, isPlus)
 
+	healthCheck := generateTransportServerHealthCheck(transportServerEx.TransportServer.Spec.Action.Pass,
+		transportServerEx.TransportServer.Spec.Upstreams)
 	var proxyRequests, proxyResponses *int
 	var connectTimeout, nextUpstreamTimeout string
 	var nextUpstream bool
@@ -94,9 +96,8 @@ func generateUnixSocket(transportServerEx *TransportServerEx) string {
 	return ""
 }
 
-func generateStreamUpstreamsAndHealthCheck(transportServerEx *TransportServerEx, upstreamNamer *upstreamNamer, isPlus bool) ([]version2.StreamUpstream, *version2.StreamHealthCheck) {
+func generateStreamUpstreams(transportServerEx *TransportServerEx, upstreamNamer *upstreamNamer, isPlus bool) []version2.StreamUpstream {
 	var upstreams []version2.StreamUpstream
-	var hc *version2.StreamHealthCheck
 
 	for _, u := range transportServerEx.TransportServer.Spec.Upstreams {
 
@@ -104,8 +105,6 @@ func generateStreamUpstreamsAndHealthCheck(transportServerEx *TransportServerEx,
 		endpointsKey := GenerateEndpointsKey(transportServerEx.TransportServer.Namespace, u.Service, nil, uint16(u.Port))
 		endpoints := transportServerEx.Endpoints[endpointsKey]
 
-		hc = generateTransportServerHealthCheck(u.HealthCheck)
-
 		ups := generateStreamUpstream(&u, upstreamNamer, endpoints, isPlus)
 
 		ups.UpstreamLabels.Service = u.Service
@@ -116,38 +115,38 @@ func generateStreamUpstreamsAndHealthCheck(transportServerEx *TransportServerEx,
 		upstreams = append(upstreams, ups)
 	}
 
-	return upstreams, hc
+	return upstreams
 }
 
-func generateTransportServerHealthCheck(healthCheck *conf_v1alpha1.HealthCheck) *version2.StreamHealthCheck {
-	if healthCheck == nil || !healthCheck.Enabled {
-		return nil
-	}
-
-	hc := generateTransportServerHealthCheckWithDefaults()
-
-	hc.Enabled = healthCheck.Enabled
-	hc.Interval = generateTime(healthCheck.Interval)
-	hc.Jitter = generateTime(healthCheck.Jitter)
-	hc.Timeout = generateTime(healthCheck.Timeout)
-
-	if healthCheck.Fails > 0 {
-		hc.Fails = healthCheck.Fails
-	}
-
-	if healthCheck.Passes > 0 {
-		hc.Passes = healthCheck.Passes
-	}
-
-	if healthCheck.Port > 0 {
-		hc.Port = healthCheck.Port
-	}
-
-	if healthCheck.Timeout != "" {
-		hc.Timeout = healthCheck.Timeout
+func generateTransportServerHealthCheck(upstreamHealthCheckName string, upstreams []conf_v1alpha1.Upstream) *version2.StreamHealthCheck {
+	var hc *version2.StreamHealthCheck
+	for _, u := range upstreams {
+		if u.Name == upstreamHealthCheckName {
+			if u.HealthCheck == nil || !u.HealthCheck.Enabled {
+				return nil
+			}
+			hc = generateTransportServerHealthCheckWithDefaults()
+
+			hc.Enabled = u.HealthCheck.Enabled
+			hc.Interval = generateTime(u.HealthCheck.Interval)
+			hc.Jitter = generateTime(u.HealthCheck.Jitter)
+			hc.Timeout = generateTime(u.HealthCheck.Timeout)
+
+			if u.HealthCheck.Fails > 0 {
+				hc.Fails = u.HealthCheck.Fails
+			}
+
+			if u.HealthCheck.Passes > 0 {
+				hc.Passes = u.HealthCheck.Passes
+			}
+
+			if u.HealthCheck.Port > 0 {
+				hc.Port = u.HealthCheck.Port
+			}
+		}
 	}
-
 	return hc
+
 }
 
 func generateTransportServerHealthCheckWithDefaults() *version2.StreamHealthCheck {
diff --git a/internal/configs/transportserver_test.go b/internal/configs/transportserver_test.go
index 7a7c185759..0cc8b76795 100644
--- a/internal/configs/transportserver_test.go
+++ b/internal/configs/transportserver_test.go
@@ -351,54 +351,108 @@ func TestGenerateUnixSocket(t *testing.T) {
 }
 
 func TestGenerateTransportServerHealthChecks(t *testing.T) {
+	upstreamName := "dns-tcp"
 	tests := []struct {
-		healthCheck *conf_v1alpha1.HealthCheck
-		expected    *version2.StreamHealthCheck
-		msg         string
+		upstreams []conf_v1alpha1.Upstream
+		expected  *version2.StreamHealthCheck
+		msg       string
 	}{
 		{
-			healthCheck: &conf_v1alpha1.HealthCheck{
-				Enabled:  false,
-				Timeout:  "30s",
-				Jitter:   "30s",
-				Port:     80,
-				Interval: "20s",
-				Passes:   4,
-				Fails:    5,
+			upstreams: []conf_v1alpha1.Upstream{
+				{
+					Name: "dns-tcp",
+					HealthCheck: &conf_v1alpha1.HealthCheck{
+						Enabled:  false,
+						Timeout:  "30s",
+						Jitter:   "30s",
+						Port:     80,
+						Interval: "20s",
+						Passes:   4,
+						Fails:    5,
+					},
+				},
 			},
 			expected: nil,
 			msg:      "health checks disabled",
 		},
 		{
-			healthCheck: &conf_v1alpha1.HealthCheck{},
-			expected:    nil,
-			msg:         "nil health checks",
+			upstreams: []conf_v1alpha1.Upstream{
+				{
+					Name:        "dns-tcp",
+					HealthCheck: &conf_v1alpha1.HealthCheck{},
+				},
+			},
+			expected: nil,
+			msg:      "empty health check",
 		},
 		{
-			healthCheck: &conf_v1alpha1.HealthCheck{
+			upstreams: []conf_v1alpha1.Upstream{
+				{
+					Name: "dns-tcp",
+					HealthCheck: &conf_v1alpha1.HealthCheck{
+						Enabled:  true,
+						Timeout:  "40s",
+						Jitter:   "30s",
+						Port:     88,
+						Interval: "20s",
+						Passes:   4,
+						Fails:    5,
+					},
+				},
+			},
+			expected: &version2.StreamHealthCheck{
 				Enabled:  true,
-				Timeout:  "30s",
+				Timeout:  "40s",
 				Jitter:   "30s",
-				Port:     80,
+				Port:     88,
 				Interval: "20s",
 				Passes:   4,
 				Fails:    5,
 			},
+			msg: "valid health checks",
+		},
+		{
+			upstreams: []conf_v1alpha1.Upstream{
+				{
+					Name: "dns-tcp",
+					HealthCheck: &conf_v1alpha1.HealthCheck{
+						Enabled:  true,
+						Timeout:  "40s",
+						Jitter:   "30s",
+						Port:     88,
+						Interval: "20s",
+						Passes:   4,
+						Fails:    5,
+					},
+				},
+				{
+					Name: "dns-tcp-2",
+					HealthCheck: &conf_v1alpha1.HealthCheck{
+						Enabled:  false,
+						Timeout:  "50s",
+						Jitter:   "60s",
+						Port:     89,
+						Interval: "10s",
+						Passes:   9,
+						Fails:    7,
+					},
+				},
+			},
 			expected: &version2.StreamHealthCheck{
 				Enabled:  true,
-				Timeout:  "30s",
+				Timeout:  "40s",
 				Jitter:   "30s",
-				Port:     80,
+				Port:     88,
 				Interval: "20s",
 				Passes:   4,
 				Fails:    5,
 			},
-			msg: "valid health checks",
+			msg: "valid 2 health checks",
 		},
 	}
 
 	for _, test := range tests {
-		result := generateTransportServerHealthCheck(test.healthCheck)
+		result := generateTransportServerHealthCheck(upstreamName, test.upstreams)
 		if diff := cmp.Diff(result, test.expected); diff != "" {
 			t.Errorf("generateTransportServerHealthCheck() '%v' mismatch (-want +got):\n%s", test.msg, diff)
 		}

From b43181125cc34f39e268764e1795836ee034958d Mon Sep 17 00:00:00 2001
From: LorcanMcVeigh <lorcan.mcveigh@gmail.com>
Date: Tue, 23 Feb 2021 11:20:21 +0000
Subject: [PATCH 7/9] Fix default port problem

---
 internal/configs/transportserver.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/internal/configs/transportserver.go b/internal/configs/transportserver.go
index 661edcca1f..b425b96825 100644
--- a/internal/configs/transportserver.go
+++ b/internal/configs/transportserver.go
@@ -125,7 +125,7 @@ func generateTransportServerHealthCheck(upstreamHealthCheckName string, upstream
 			if u.HealthCheck == nil || !u.HealthCheck.Enabled {
 				return nil
 			}
-			hc = generateTransportServerHealthCheckWithDefaults()
+			hc = generateTransportServerHealthCheckWithDefaults(u)
 
 			hc.Enabled = u.HealthCheck.Enabled
 			hc.Interval = generateTime(u.HealthCheck.Interval)
@@ -149,12 +149,12 @@ func generateTransportServerHealthCheck(upstreamHealthCheckName string, upstream
 
 }
 
-func generateTransportServerHealthCheckWithDefaults() *version2.StreamHealthCheck {
+func generateTransportServerHealthCheckWithDefaults(up conf_v1alpha1.Upstream) *version2.StreamHealthCheck {
 	return &version2.StreamHealthCheck{
 		Enabled:  false,
 		Timeout:  "5s",
 		Jitter:   "0",
-		Port:     0,
+		Port:     up.Port,
 		Interval: "5s",
 		Passes:   1,
 		Fails:    1,

From cc61ff5cfc87252306403480d8f9a56097195a68 Mon Sep 17 00:00:00 2001
From: LorcanMcVeigh <lorcan.mcveigh@gmail.com>
Date: Tue, 23 Feb 2021 11:21:42 +0000
Subject: [PATCH 8/9] Feedback 4

---
 internal/configs/transportserver.go      | 1 -
 internal/configs/transportserver_test.go | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/internal/configs/transportserver.go b/internal/configs/transportserver.go
index b425b96825..51b742b6d9 100644
--- a/internal/configs/transportserver.go
+++ b/internal/configs/transportserver.go
@@ -146,7 +146,6 @@ func generateTransportServerHealthCheck(upstreamHealthCheckName string, upstream
 		}
 	}
 	return hc
-
 }
 
 func generateTransportServerHealthCheckWithDefaults(up conf_v1alpha1.Upstream) *version2.StreamHealthCheck {
diff --git a/internal/configs/transportserver_test.go b/internal/configs/transportserver_test.go
index 0cc8b76795..76eacfe026 100644
--- a/internal/configs/transportserver_test.go
+++ b/internal/configs/transportserver_test.go
@@ -453,7 +453,7 @@ func TestGenerateTransportServerHealthChecks(t *testing.T) {
 
 	for _, test := range tests {
 		result := generateTransportServerHealthCheck(upstreamName, test.upstreams)
-		if diff := cmp.Diff(result, test.expected); diff != "" {
+		if diff := cmp.Diff(test.expected, result); diff != "" {
 			t.Errorf("generateTransportServerHealthCheck() '%v' mismatch (-want +got):\n%s", test.msg, diff)
 		}
 	}

From dc5dedc1814ac062961cd0ef53c2c4cb886ca338 Mon Sep 17 00:00:00 2001
From: LorcanMcVeigh <lorcan.mcveigh@gmail.com>
Date: Thu, 25 Feb 2021 10:21:14 +0000
Subject: [PATCH 9/9] Add health check to crd

---
 .../crds/k8s.nginx.org_transportservers.yaml   | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/deployments/helm-chart/crds/k8s.nginx.org_transportservers.yaml b/deployments/helm-chart/crds/k8s.nginx.org_transportservers.yaml
index 472b2447bb..f4e276ad8e 100644
--- a/deployments/helm-chart/crds/k8s.nginx.org_transportservers.yaml
+++ b/deployments/helm-chart/crds/k8s.nginx.org_transportservers.yaml
@@ -80,6 +80,24 @@ spec:
                     properties:
                       failTimeout:
                         type: string
+                      healthCheck:
+                        description: HealthCheck defines the parameters for active Upstream HealthChecks.
+                        type: object
+                        properties:
+                          enable:
+                            type: boolean
+                          fails:
+                            type: integer
+                          intervals:
+                            type: string
+                          jitter:
+                            type: string
+                          passes:
+                            type: integer
+                          port:
+                            type: integer
+                          timeout:
+                            type: string
                       maxFails:
                         type: integer
                       name: