Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix healthcheck ports #2378

Merged
merged 1 commit into from
Feb 11, 2022
Merged

Fix healthcheck ports #2378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/content/configuration/transportserver-resource.md
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion docs/content/configuration/virtualserver-and-virtualserverroute-resources.md
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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