Skip to content

Commit

Permalink
Enable setting mandatory and persistent in healthchecks
Browse files Browse the repository at this point in the history
  • Loading branch information
ciarams87 committed Dec 17, 2021
1 parent 07058ef commit 1532ee7
Show file tree
Hide file tree
Showing 15 changed files with 87 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -511,10 +511,14 @@ spec:
type: string
jitter:
type: string
mandatory:
type: boolean
passes:
type: integer
path:
type: string
persistent:
type: boolean
port:
type: integer
read-timeout:
Expand Down
4 changes: 4 additions & 0 deletions deployments/common/crds/k8s.nginx.org_virtualservers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -543,10 +543,14 @@ spec:
type: string
jitter:
type: string
mandatory:
type: boolean
passes:
type: integer
path:
type: string
persistent:
type: boolean
port:
type: integer
read-timeout:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -511,10 +511,14 @@ spec:
type: string
jitter:
type: string
mandatory:
type: boolean
passes:
type: integer
path:
type: string
persistent:
type: boolean
port:
type: integer
read-timeout:
Expand Down
4 changes: 4 additions & 0 deletions deployments/helm-chart/crds/k8s.nginx.org_virtualservers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -543,10 +543,14 @@ spec:
type: string
jitter:
type: string
mandatory:
type: boolean
passes:
type: integer
path:
type: string
persistent:
type: boolean
port:
type: integer
read-timeout:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,12 +327,13 @@ Note: This feature is supported only in NGINX Plus.

### Upstream.Healthcheck

The Healthcheck defines an [active health check](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-health-check/). In the example below we enable a health check for an upstream and configure all the available parameters:
The Healthcheck defines an [active health check](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-health-check/). In the example below we enable a health check for an upstream and configure all the available parameters, including the `slow-start` parameter combined with [`mandatory` and `persistent`](https://docs.nginx.com/nginx/admin-guide/load-balancer/http-health-check/#mandatory-health-checks):

```yaml
name: tea
service: tea-svc
port: 80
slow-start: 30s
healthCheck:
enable: true
path: /healthz
Expand All @@ -350,6 +351,8 @@ healthCheck:
- name: Host
value: my.service
statusMatch: "! 500"
mandatory: true
persistent: true
```

Note: This feature is supported only in NGINX Plus.
Expand All @@ -372,6 +375,8 @@ Note: This feature is supported only in NGINX Plus.
|``statusMatch`` | The expected response status codes of a health check. By default, the response should have status code 2xx or 3xx. Examples: ``"200"`` , ``"! 500"`` , ``"301-303 307"``. See the documentation of the [match](https://nginx.org/en/docs/http/ngx_http_upstream_hc_module.html?#match) directive. This not supported for gRPC type upstreams. | ``string`` | No |
|``grpcStatus`` | The expected [gRPC status code](https://github.com/grpc/grpc/blob/master/doc/statuscodes.md#status-codes-and-their-use-in-grpc) of the upstream server response to the [Check method](https://github.com/grpc/grpc/blob/master/doc/health-checking.md). Configure this field only if your gRPC services do not implement the gRPC health checking protocol. For example, configure ``12`` if the upstream server responds with `12 (UNIMPLEMENTED)` status code. Only valid on gRPC type upstreams. | ``int`` | No |
|``grpcService`` | The gRPC service to be monitored on the upstream server. Only valid on gRPC type upstreams. | ``string`` | No |
|``mandatory`` | Require every newly added server to pass all configured health checks before NGINX Plus sends traffic to it. If this is not specified, or is set to false, the server will be initially considered healthy. When combined with [slow-start](https://nginx.org/en/docs/http/ngx_http_upstream_module.html#slow_start), it gives a new server more time to connect to databases and “warm up” before being asked to handle their full share of traffic. | ``bool`` | No |
|``persistent`` | Set the initial “up” state for a server after reload if the server was considered healthy before reload. Enabling persistent requires that the mandatory parameter is also set to `true`. | ``bool`` | No |
{{% /table %}}

### Upstream.SessionCookie
Expand Down
2 changes: 2 additions & 0 deletions internal/configs/version2/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,8 @@ type HealthCheck struct {
GRPCPass string
GRPCStatus *int
GRPCService string
Mandatory bool
Persistent bool
}

// TLSRedirect defines a redirect in a Server.
Expand Down
1 change: 1 addition & 0 deletions internal/configs/version2/nginx-plus.virtualserver.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ server {
{{ end }}
health_check {{ if $hc.URI }}uri={{ $hc.URI }} {{ end }}port={{ $hc.Port }} 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 }}
{{ if $hc.GRPCService }} grpc_service={{ $hc.GRPCService }}{{ end }}{{ end }};
}
Expand Down
18 changes: 10 additions & 8 deletions internal/configs/version2/templates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,16 @@ var virtualServerCfg = VirtualServerConfig{
},
HealthChecks: []HealthCheck{
{
Name: "coffee",
URI: "/",
Interval: "5s",
Jitter: "0s",
Fails: 1,
Passes: 1,
Port: 50,
ProxyPass: "http://coffee-v2",
Name: "coffee",
URI: "/",
Interval: "5s",
Jitter: "0s",
Fails: 1,
Passes: 1,
Port: 50,
ProxyPass: "http://coffee-v2",
Mandatory: true,
Persistent: true,
},
{
Name: "tea",
Expand Down
4 changes: 4 additions & 0 deletions internal/configs/virtualserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1349,6 +1349,10 @@ func generateHealthCheck(
hc.Match = generateStatusMatchName(upstreamName)
}

hc.Mandatory = upstream.HealthCheck.Mandatory

hc.Persistent = upstream.HealthCheck.Persistent

hc.GRPCStatus = upstream.HealthCheck.GRPCStatus

hc.GRPCService = upstream.HealthCheck.GRPCService
Expand Down
29 changes: 29 additions & 0 deletions internal/configs/virtualserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6800,6 +6800,35 @@ func TestGenerateHealthCheck(t *testing.T) {
},
msg: "HealthCheck with time parameters have correct format",
},
{
upstream: conf_v1.Upstream{
HealthCheck: &conf_v1.HealthCheck{
Enable: true,
Mandatory: true,
Persistent: true,
},
ProxyConnectTimeout: "30s",
ProxyReadTimeout: "30s",
ProxySendTimeout: "30s",
},
upstreamName: upstreamName,
expected: &version2.HealthCheck{
Name: upstreamName,
ProxyConnectTimeout: "30s",
ProxyReadTimeout: "30s",
ProxySendTimeout: "30s",
ProxyPass: fmt.Sprintf("http://%v", upstreamName),
URI: "/",
Interval: "5s",
Jitter: "0s",
Fails: 1,
Passes: 1,
Headers: make(map[string]string),
Mandatory: true,
Persistent: true,
},
msg: "HealthCheck with mandatory and persistent set",
},
}

baseCfgParams := &ConfigParams{
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/configuration/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ type HealthCheck struct {
StatusMatch string `json:"statusMatch"`
GRPCStatus *int `json:"grpcStatus"`
GRPCService string `json:"grpcService"`
Mandatory bool `json:"mandatory"`
Persistent bool `json:"persistent"`
}

// Header defines an HTTP Header.
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/configuration/validation/virtualserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,10 @@ func validateUpstreamHealthCheck(hc *v1.HealthCheck, typeName string, fieldPath
}
}

if hc.Persistent && !hc.Mandatory {
allErrs = append(allErrs, field.Required(fieldPath.Child("mandatory"), "must be true when `persistent` is true"))
}

return allErrs
}

Expand Down
9 changes: 9 additions & 0 deletions pkg/apis/configuration/validation/virtualserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2397,6 +2397,8 @@ func TestValidateUpstreamHealthCheck(t *testing.T) {
},
},
StatusMatch: "! 500",
Mandatory: true,
Persistent: true,
}

allErrs := validateUpstreamHealthCheck(hc, "", field.NewPath("healthCheck"))
Expand Down Expand Up @@ -2480,6 +2482,13 @@ func TestValidateUpstreamHealthCheckFails(t *testing.T) {
GRPCService: "tea-servicev2",
},
},
{
hc: &v1.HealthCheck{
Enable: true,
Path: "/healthz",
Persistent: true,
},
},
}

for _, test := range tests {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ spec:
statusMatch: "invalid"
grpcService: "notimplemented"
grpcStatus: 12
persistent: True
slow-start: "-3s"
queue:
size: -100
Expand Down
6 changes: 3 additions & 3 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},
"healthCheck": {"enable": True, "port": 8080, "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", ";",
["health_check uri=/ port=8080 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 Expand Up @@ -398,7 +398,7 @@ def test_validation_flow(self, kube_apis, ingress_controller_prerequisites,
"upstreams[0].healthCheck.read-timeout", "upstreams[0].healthCheck.send-timeout",
"upstreams[0].healthCheck.headers[0].name", "upstreams[0].healthCheck.headers[0].value",
"upstreams[0].healthCheck.statusMatch", "upstreams[0].healthCheck.grpcStatus",
"upstreams[0].healthCheck.grpcService",
"upstreams[0].healthCheck.grpcService", "upstreams[0].healthCheck.mandatory",
"upstreams[0].slow-start",
"upstreams[0].queue.size", "upstreams[0].queue.timeout",
"upstreams[0].sessionCookie.name", "upstreams[0].sessionCookie.path",
Expand Down

0 comments on commit 1532ee7

Please sign in to comment.