Skip to content

Commit

Permalink
Fix healthcheck ports
Browse files Browse the repository at this point in the history
  • Loading branch information
ciarams87 committed Feb 11, 2022
1 parent 3958b3e commit f239812
Show file tree
Hide file tree
Showing 10 changed files with 13 additions and 22 deletions.
2 changes: 1 addition & 1 deletion docs/content/configuration/transportserver-resource.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ Note: This feature is supported only in NGINX Plus.
|``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 |
|``port`` | The port used for health check requests. By default, the [server port is used](https://nginx.org/en/docs/stream/ngx_stream_upstream_hc_module.html#health_check_port). Note: in contrast with the port of the upstream, this port is not a service port, but a port of a pod. | ``integer`` | No |
|``match`` | Controls the data to send and the response to expect for the healthcheck. | [match](#upstreamhealthcheckmatch) | No |
{{% /table %}}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ Note: This feature is supported only in NGINX Plus.
|``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 |
|``port`` | The port used for health check requests. By default, the [server port is used](https://nginx.org/en/docs/http/ngx_http_upstream_hc_module.html#health_check_port). Note: in contrast with the port of the upstream, this port is not a service port, but a port of a pod. | ``integer`` | No |
|``tls`` | The TLS configuration used for health check requests. By default, the ``tls`` field of the upstream is used. | [upstream.tls](#upstreamtls) | No |
|``connect-timeout`` | The timeout for establishing a connection with an upstream server. By default, the ``connect-timeout`` of the upstream is used. | ``string`` | No |
|``read-timeout`` | The timeout for reading a response from an upstream server. By default, the ``read-timeout`` of the upstream is used. | ``string`` | No |
Expand Down
6 changes: 1 addition & 5 deletions internal/configs/transportserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ func generateTransportServerHealthCheck(upstreamName string, generatedUpstreamNa
hc.Interval = generateTimeWithDefault(u.HealthCheck.Interval, hc.Interval)
hc.Jitter = generateTimeWithDefault(u.HealthCheck.Jitter, hc.Jitter)
hc.Timeout = generateTimeWithDefault(u.HealthCheck.Timeout, hc.Timeout)
hc.Port = u.HealthCheck.Port

if u.HealthCheck.Fails > 0 {
hc.Fails = u.HealthCheck.Fails
Expand All @@ -153,10 +154,6 @@ func generateTransportServerHealthCheck(upstreamName string, generatedUpstreamNa
hc.Passes = u.HealthCheck.Passes
}

if u.HealthCheck.Port > 0 {
hc.Port = u.HealthCheck.Port
}

if u.HealthCheck.Match != nil {
name := "match_" + generatedUpstreamName
match = generateHealthCheckMatch(u.HealthCheck.Match, name)
Expand All @@ -175,7 +172,6 @@ func generateTransportServerHealthCheckWithDefaults(up conf_v1alpha1.Upstream) *
Enabled: false,
Timeout: "5s",
Jitter: "0s",
Port: up.Port,
Interval: "5s",
Passes: 1,
Fails: 1,
Expand Down
2 changes: 0 additions & 2 deletions internal/configs/transportserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,6 @@ func TestGenerateTransportServerHealthChecks(t *testing.T) {
Enabled: true,
Timeout: "5s",
Jitter: "0s",
Port: 90,
Interval: "5s",
Passes: 1,
Fails: 1,
Expand All @@ -668,7 +667,6 @@ func TestGenerateTransportServerHealthChecks(t *testing.T) {
Enabled: true,
Timeout: "5s",
Jitter: "0s",
Port: 90,
Interval: "5s",
Passes: 1,
Fails: 1,
Expand Down
2 changes: 1 addition & 1 deletion internal/configs/version2/nginx-plus.transportserver.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ server {
proxy_pass {{ $s.ProxyPass }};

{{ if $s.HealthCheck }}
health_check interval={{ $s.HealthCheck.Interval }} port={{ $s.HealthCheck.Port }}
health_check interval={{ $s.HealthCheck.Interval }} {{ if $s.HealthCheck.Port }} port={{ $s.HealthCheck.Port }}{{ end }}
passes={{ $s.HealthCheck.Passes }} jitter={{ $s.HealthCheck.Jitter }} fails={{ $s.HealthCheck.Fails }}{{ if $s.UDP }} udp{{ end }}{{ if $s.HealthCheck.Match }} match={{ $s.HealthCheck.Match }}{{ end }};
health_check_timeout {{ $s.HealthCheck.Timeout }};
{{ end }}
Expand Down
2 changes: 1 addition & 1 deletion internal/configs/version2/nginx-plus.virtualserver.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ server {
{{ else }}
proxy_pass {{ $hc.ProxyPass }};
{{ end }}
health_check {{ if $hc.URI }}uri={{ $hc.URI }} {{ end }}port={{ $hc.Port }} interval={{ $hc.Interval }} jitter={{ $hc.Jitter }}
health_check {{ if $hc.URI }}uri={{ $hc.URI }} {{ end }}{{ if $hc.Port }}port={{ $hc.Port }} {{ end }}interval={{ $hc.Interval }} jitter={{ $hc.Jitter }}
fails={{ $hc.Fails }} passes={{ $hc.Passes }}{{ if $hc.Match }} match={{ $hc.Match }}{{ end }}
{{ if $hc.Mandatory }} mandatory{{ if $hc.Persistent }} persistent{{ end }}{{ end }}
{{ if $hc.GRPCPass }} type=grpc{{ if $hc.GRPCStatus }} grpc_status={{ $hc.GRPCStatus }}{{ end }}
Expand Down
7 changes: 2 additions & 5 deletions internal/configs/virtualserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ func newHealthCheckWithDefaults(upstream conf_v1.Upstream, upstreamName string,
Jitter: "0s",
Fails: 1,
Passes: 1,
Port: int(upstream.Port),
ProxyPass: fmt.Sprintf("%v://%v", generateProxyPassProtocol(upstream.TLS.Enable), upstreamName),
ProxyConnectTimeout: generateTimeWithDefault(upstream.ProxyConnectTimeout, cfgParams.ProxyConnectTimeout),
ProxyReadTimeout: generateTimeWithDefault(upstream.ProxyReadTimeout, cfgParams.ProxyReadTimeout),
Expand Down Expand Up @@ -1321,10 +1320,6 @@ func generateHealthCheck(
hc.Passes = upstream.HealthCheck.Passes
}

if upstream.HealthCheck.Port > 0 {
hc.Port = upstream.HealthCheck.Port
}

if upstream.HealthCheck.ConnectTimeout != "" {
hc.ProxyConnectTimeout = generateTime(upstream.HealthCheck.ConnectTimeout)
}
Expand All @@ -1349,6 +1344,8 @@ func generateHealthCheck(
hc.Match = generateStatusMatchName(upstreamName)
}

hc.Port = upstream.HealthCheck.Port

hc.Mandatory = upstream.HealthCheck.Mandatory

hc.Persistent = upstream.HealthCheck.Persistent
Expand Down
4 changes: 2 additions & 2 deletions tests/suite/test_transport_server_tcp_load_balance.py
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ def test_tcp_passing_healthcheck_with_match(

match = f"match_ts_{transport_server_setup.namespace}_transport-server_tcp-app"

assert "health_check interval=5s port=3333" in result_conf
assert "health_check interval=5s" in result_conf
assert f"passes=1 jitter=0s fails=1 match={match}" in result_conf
assert "health_check_timeout 3s;"
assert 'send "health"' in result_conf
Expand Down Expand Up @@ -559,7 +559,7 @@ def test_tcp_failing_healthcheck_with_match(

match = f"match_ts_{transport_server_setup.namespace}_transport-server_tcp-app"

assert "health_check interval=5s port=3333" in result_conf
assert "health_check interval=5s" in result_conf
assert f"passes=1 jitter=0s fails=1 match={match}" in result_conf
assert "health_check_timeout 3s"
assert 'send "health"' in result_conf
Expand Down
4 changes: 2 additions & 2 deletions tests/suite/test_transport_server_udp_load_balance.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ def test_udp_passing_healthcheck_with_match(

match = f"match_ts_{transport_server_setup.namespace}_transport-server_udp-app"

assert "health_check interval=5s port=3334" in result_conf
assert "health_check interval=5s" in result_conf
assert f"passes=1 jitter=0s fails=1 udp match={match}" in result_conf
assert "health_check_timeout 3s;"
assert 'send "health"' in result_conf
Expand Down Expand Up @@ -350,7 +350,7 @@ def test_udp_failing_healthcheck_with_match(

match = f"match_ts_{transport_server_setup.namespace}_transport-server_udp-app"

assert "health_check interval=5s port=3334" in result_conf
assert "health_check interval=5s" in result_conf
assert f"passes=1 jitter=0s fails=1 udp match={match}" in result_conf
assert "health_check_timeout 3s;"
assert 'send "health"' in result_conf
Expand Down
4 changes: 2 additions & 2 deletions tests/suite/test_virtual_server_upstream_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ def test_openapi_validation_flow(self, kube_apis, ingress_controller_prerequisit
class TestOptionsSpecificForPlus:
@pytest.mark.parametrize('options, expected_strings', [
({"lb-method": "least_conn",
"healthCheck": {"enable": True, "port": 8080, "mandatory": True, "persistent": True},
"healthCheck": {"enable": True, "mandatory": True, "persistent": True},
"slow-start": "3h",
"queue": {"size": 100},
"ntlm": True,
Expand All @@ -311,7 +311,7 @@ class TestOptionsSpecificForPlus:
"path": "/some-valid/path",
"expires": "max",
"domain": "virtual-server-route.example.com", "httpOnly": True, "secure": True}},
["health_check uri=/ port=8080 interval=5s jitter=0s", "fails=1 passes=1", "mandatory persistent", ";",
["health_check uri=/ interval=5s jitter=0s", "fails=1 passes=1", "mandatory persistent", ";",
"slow_start=3h", "queue 100 timeout=60s;", "ntlm;",
"sticky cookie TestCookie expires=max domain=virtual-server-route.example.com httponly secure path=/some-valid/path;"]),
({"lb-method": "least_conn",
Expand Down

0 comments on commit f239812

Please sign in to comment.