From 60c39751d2f8d1ef6e5c81e38a74eda24e743880 Mon Sep 17 00:00:00 2001 From: Jim Ryan Date: Tue, 2 Jul 2024 14:57:33 +0100 Subject: [PATCH] Fix: GRPC healthcheck should not have keepalive time (#5899) * fix template file, and fix test * add test for GPRC health check fail message, remove flaky label from prior tests, clean up health check template * update snap * update snapshots * remove newline * fix tests --- .../__snapshots__/templates_test.snap | 204 +++++--------- internal/configs/version2/http.go | 1 + .../version2/nginx-plus.virtualserver.tmpl | 17 +- internal/configs/version2/templates_test.go | 253 ++++++++++-------- internal/configs/virtualserver.go | 1 + internal/configs/virtualserver_test.go | 2 + .../virtual-server-healthcheck-fail.yaml | 31 +++ .../virtual-server-healthcheck.yaml | 2 +- .../suite/test_v_s_route_upstream_options.py | 12 +- tests/suite/test_virtual_server_grpc.py | 69 ++++- .../test_virtual_server_upstream_options.py | 9 +- 11 files changed, 339 insertions(+), 262 deletions(-) create mode 100644 tests/data/virtual-server-grpc/virtual-server-healthcheck-fail.yaml diff --git a/internal/configs/version2/__snapshots__/templates_test.snap b/internal/configs/version2/__snapshots__/templates_test.snap index 60c9946a63..c39644ec92 100644 --- a/internal/configs/version2/__snapshots__/templates_test.snap +++ b/internal/configs/version2/__snapshots__/templates_test.snap @@ -190,23 +190,18 @@ server { proxy_read_timeout ; proxy_send_timeout ; proxy_pass http://coffee-v2; - health_check uri=/ port=50 interval=5s jitter=0s - fails=1 passes=1 - mandatory persistent - keepalive_time=; - } + health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; + + } location @hc-tea { grpc_connect_timeout ; grpc_read_timeout ; grpc_send_timeout ; grpc_pass grpc://tea-v3; - health_check port=50 interval=5s jitter=0s - fails=1 passes=1 - - type=grpc grpc_status=12 - grpc_service=tea-servicev2 keepalive_time=; - } + health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; + + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -627,23 +622,18 @@ server { proxy_read_timeout ; proxy_send_timeout ; proxy_pass http://coffee-v2; - health_check uri=/ port=50 interval=5s jitter=0s - fails=1 passes=1 - mandatory persistent - keepalive_time=; - } + health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; + + } location @hc-tea { grpc_connect_timeout ; grpc_read_timeout ; grpc_send_timeout ; grpc_pass grpc://tea-v3; - health_check port=50 interval=5s jitter=0s - fails=1 passes=1 - - type=grpc grpc_status=12 - grpc_service=tea-servicev2 keepalive_time=; - } + health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; + + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -1999,23 +1989,18 @@ server { proxy_read_timeout ; proxy_send_timeout ; proxy_pass http://coffee-v2; - health_check uri=/ port=50 interval=5s jitter=0s - fails=1 passes=1 - mandatory persistent - keepalive_time=; - } + health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; + + } location @hc-tea { grpc_connect_timeout ; grpc_read_timeout ; grpc_send_timeout ; grpc_pass grpc://tea-v3; - health_check port=50 interval=5s jitter=0s - fails=1 passes=1 - - type=grpc grpc_status=12 - grpc_service=tea-servicev2 keepalive_time=; - } + health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; + + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -2371,23 +2356,18 @@ server { proxy_read_timeout ; proxy_send_timeout ; proxy_pass http://coffee-v2; - health_check uri=/ port=50 interval=5s jitter=0s - fails=1 passes=1 - mandatory persistent - keepalive_time=; - } + health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; + + } location @hc-tea { grpc_connect_timeout ; grpc_read_timeout ; grpc_send_timeout ; grpc_pass grpc://tea-v3; - health_check port=50 interval=5s jitter=0s - fails=1 passes=1 - - type=grpc grpc_status=12 - grpc_service=tea-servicev2 keepalive_time=; - } + health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; + + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -2800,23 +2780,18 @@ server { proxy_read_timeout ; proxy_send_timeout ; proxy_pass http://coffee-v2; - health_check uri=/ port=50 interval=5s jitter=0s - fails=1 passes=1 - mandatory persistent - keepalive_time=; - } + health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; + + } location @hc-tea { grpc_connect_timeout ; grpc_read_timeout ; grpc_send_timeout ; grpc_pass grpc://tea-v3; - health_check port=50 interval=5s jitter=0s - fails=1 passes=1 - - type=grpc grpc_status=12 - grpc_service=tea-servicev2 keepalive_time=; - } + health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; + + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -3227,23 +3202,18 @@ server { proxy_read_timeout ; proxy_send_timeout ; proxy_pass http://coffee-v2; - health_check uri=/ port=50 interval=5s jitter=0s - fails=1 passes=1 - mandatory persistent - keepalive_time=; - } + health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; + + } location @hc-tea { grpc_connect_timeout ; grpc_read_timeout ; grpc_send_timeout ; grpc_pass grpc://tea-v3; - health_check port=50 interval=5s jitter=0s - fails=1 passes=1 - - type=grpc grpc_status=12 - grpc_service=tea-servicev2 keepalive_time=; - } + health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; + + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -3654,23 +3624,18 @@ server { proxy_read_timeout ; proxy_send_timeout ; proxy_pass http://coffee-v2; - health_check uri=/ port=50 interval=5s jitter=0s - fails=1 passes=1 - mandatory persistent - keepalive_time=; - } + health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; + + } location @hc-tea { grpc_connect_timeout ; grpc_read_timeout ; grpc_send_timeout ; grpc_pass grpc://tea-v3; - health_check port=50 interval=5s jitter=0s - fails=1 passes=1 - - type=grpc grpc_status=12 - grpc_service=tea-servicev2 keepalive_time=; - } + health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; + + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -4083,23 +4048,18 @@ server { proxy_read_timeout ; proxy_send_timeout ; proxy_pass http://coffee-v2; - health_check uri=/ port=50 interval=5s jitter=0s - fails=1 passes=1 - mandatory persistent - keepalive_time=; - } + health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; + + } location @hc-tea { grpc_connect_timeout ; grpc_read_timeout ; grpc_send_timeout ; grpc_pass grpc://tea-v3; - health_check port=50 interval=5s jitter=0s - fails=1 passes=1 - - type=grpc grpc_status=12 - grpc_service=tea-servicev2 keepalive_time=; - } + health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; + + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -4512,23 +4472,18 @@ server { proxy_read_timeout ; proxy_send_timeout ; proxy_pass http://coffee-v2; - health_check uri=/ port=50 interval=5s jitter=0s - fails=1 passes=1 - mandatory persistent - keepalive_time=; - } + health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; + + } location @hc-tea { grpc_connect_timeout ; grpc_read_timeout ; grpc_send_timeout ; grpc_pass grpc://tea-v3; - health_check port=50 interval=5s jitter=0s - fails=1 passes=1 - - type=grpc grpc_status=12 - grpc_service=tea-servicev2 keepalive_time=; - } + health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; + + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -4941,23 +4896,18 @@ server {gunzip on; proxy_read_timeout ; proxy_send_timeout ; proxy_pass http://coffee-v2; - health_check uri=/ port=50 interval=5s jitter=0s - fails=1 passes=1 - mandatory persistent - keepalive_time=; - } + health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; + + } location @hc-tea { grpc_connect_timeout ; grpc_read_timeout ; grpc_send_timeout ; grpc_pass grpc://tea-v3; - health_check port=50 interval=5s jitter=0s - fails=1 passes=1 - - type=grpc grpc_status=12 - grpc_service=tea-servicev2 keepalive_time=; - } + health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; + + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -5370,23 +5320,18 @@ server { proxy_read_timeout ; proxy_send_timeout ; proxy_pass http://coffee-v2; - health_check uri=/ port=50 interval=5s jitter=0s - fails=1 passes=1 - mandatory persistent - keepalive_time=; - } + health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; + + } location @hc-tea { grpc_connect_timeout ; grpc_read_timeout ; grpc_send_timeout ; grpc_pass grpc://tea-v3; - health_check port=50 interval=5s jitter=0s - fails=1 passes=1 - - type=grpc grpc_status=12 - grpc_service=tea-servicev2 keepalive_time=; - } + health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; + + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -6243,23 +6188,18 @@ server { proxy_read_timeout ; proxy_send_timeout ; proxy_pass http://coffee-v2; - health_check uri=/ port=50 interval=5s jitter=0s - fails=1 passes=1 - mandatory persistent - keepalive_time=; - } + health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; + + } location @hc-tea { grpc_connect_timeout ; grpc_read_timeout ; grpc_send_timeout ; grpc_pass grpc://tea-v3; - health_check port=50 interval=5s jitter=0s - fails=1 passes=1 - - type=grpc grpc_status=12 - grpc_service=tea-servicev2 keepalive_time=; - } + health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; + + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go index f05a999ca9..a2266fc98e 100644 --- a/internal/configs/version2/http.go +++ b/internal/configs/version2/http.go @@ -286,6 +286,7 @@ type HealthCheck struct { Mandatory bool Persistent bool KeepaliveTime string + IsGRPC bool } // TLSRedirect defines a redirect in a Server. diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index cdd8eea60e..935d11e686 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -332,12 +332,17 @@ server { {{- else }} proxy_pass {{ $hc.ProxyPass }}; {{- end }} - 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 }} - {{ if $hc.GRPCService }} grpc_service={{ $hc.GRPCService }}{{ end }}{{ end }} keepalive_time={{ $hc.KeepaliveTime }}; - } + 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 {{ end -}} + {{- if $hc.Persistent }} persistent {{ end -}} + {{- if not $hc.IsGRPC }} keepalive_time={{ $hc.KeepaliveTime }}{{ end -}} + {{- if $hc.GRPCPass }} type=grpc{{- if $hc.GRPCStatus }} grpc_status={{ $hc.GRPCStatus }}{{- end -}} + {{- if $hc.GRPCService }} grpc_service={{ $hc.GRPCService }}{{- end -}}{{ end -}}; + + } {{- end }} {{- range $e := $s.ErrorPageLocations }} diff --git a/internal/configs/version2/templates_test.go b/internal/configs/version2/templates_test.go index adadc78cca..b01bb5c38b 100644 --- a/internal/configs/version2/templates_test.go +++ b/internal/configs/version2/templates_test.go @@ -707,16 +707,18 @@ func vsConfig() VirtualServerConfig { }, HealthChecks: []HealthCheck{ { - Name: "coffee", - URI: "/", - Interval: "5s", - Jitter: "0s", - Fails: 1, - Passes: 1, - Port: 50, - ProxyPass: "http://coffee-v2", - Mandatory: true, - Persistent: true, + Name: "coffee", + URI: "/", + Interval: "5s", + Jitter: "0s", + Fails: 1, + Passes: 1, + Port: 50, + ProxyPass: "http://coffee-v2", + Mandatory: true, + Persistent: true, + KeepaliveTime: "60s", + IsGRPC: false, }, { Name: "tea", @@ -729,6 +731,7 @@ func vsConfig() VirtualServerConfig { GRPCPass: "grpc://tea-v3", GRPCStatus: createPointerFromInt(12), GRPCService: "tea-servicev2", + IsGRPC: true, }, }, Locations: []Location{ @@ -1055,16 +1058,18 @@ var ( }, HealthChecks: []HealthCheck{ { - Name: "coffee", - URI: "/", - Interval: "5s", - Jitter: "0s", - Fails: 1, - Passes: 1, - Port: 50, - ProxyPass: "http://coffee-v2", - Mandatory: true, - Persistent: true, + Name: "coffee", + URI: "/", + Interval: "5s", + Jitter: "0s", + Fails: 1, + Passes: 1, + Port: 50, + ProxyPass: "http://coffee-v2", + Mandatory: true, + Persistent: true, + KeepaliveTime: "60s", + IsGRPC: false, }, { Name: "tea", @@ -1077,6 +1082,7 @@ var ( GRPCPass: "grpc://tea-v3", GRPCStatus: createPointerFromInt(12), GRPCService: "tea-servicev2", + IsGRPC: true, }, }, Locations: []Location{ @@ -1401,16 +1407,18 @@ var ( }, HealthChecks: []HealthCheck{ { - Name: "coffee", - URI: "/", - Interval: "5s", - Jitter: "0s", - Fails: 1, - Passes: 1, - Port: 50, - ProxyPass: "http://coffee-v2", - Mandatory: true, - Persistent: true, + Name: "coffee", + URI: "/", + Interval: "5s", + Jitter: "0s", + Fails: 1, + Passes: 1, + Port: 50, + ProxyPass: "http://coffee-v2", + Mandatory: true, + Persistent: true, + KeepaliveTime: "60s", + IsGRPC: false, }, { Name: "tea", @@ -1423,6 +1431,7 @@ var ( GRPCPass: "grpc://tea-v3", GRPCStatus: createPointerFromInt(12), GRPCService: "tea-servicev2", + IsGRPC: true, }, }, Locations: []Location{ @@ -1747,16 +1756,18 @@ var ( }, HealthChecks: []HealthCheck{ { - Name: "coffee", - URI: "/", - Interval: "5s", - Jitter: "0s", - Fails: 1, - Passes: 1, - Port: 50, - ProxyPass: "http://coffee-v2", - Mandatory: true, - Persistent: true, + Name: "coffee", + URI: "/", + Interval: "5s", + Jitter: "0s", + Fails: 1, + Passes: 1, + Port: 50, + ProxyPass: "http://coffee-v2", + Mandatory: true, + Persistent: true, + KeepaliveTime: "60s", + IsGRPC: false, }, { Name: "tea", @@ -1769,6 +1780,7 @@ var ( GRPCPass: "grpc://tea-v3", GRPCStatus: createPointerFromInt(12), GRPCService: "tea-servicev2", + IsGRPC: true, }, }, Locations: []Location{ @@ -2094,16 +2106,18 @@ var ( }, HealthChecks: []HealthCheck{ { - Name: "coffee", - URI: "/", - Interval: "5s", - Jitter: "0s", - Fails: 1, - Passes: 1, - Port: 50, - ProxyPass: "http://coffee-v2", - Mandatory: true, - Persistent: true, + Name: "coffee", + URI: "/", + Interval: "5s", + Jitter: "0s", + Fails: 1, + Passes: 1, + Port: 50, + ProxyPass: "http://coffee-v2", + Mandatory: true, + Persistent: true, + KeepaliveTime: "60s", + IsGRPC: false, }, { Name: "tea", @@ -2116,6 +2130,7 @@ var ( GRPCPass: "grpc://tea-v3", GRPCStatus: createPointerFromInt(12), GRPCService: "tea-servicev2", + IsGRPC: true, }, }, Locations: []Location{ @@ -2441,16 +2456,18 @@ var ( }, HealthChecks: []HealthCheck{ { - Name: "coffee", - URI: "/", - Interval: "5s", - Jitter: "0s", - Fails: 1, - Passes: 1, - Port: 50, - ProxyPass: "http://coffee-v2", - Mandatory: true, - Persistent: true, + Name: "coffee", + URI: "/", + Interval: "5s", + Jitter: "0s", + Fails: 1, + Passes: 1, + Port: 50, + ProxyPass: "http://coffee-v2", + Mandatory: true, + Persistent: true, + KeepaliveTime: "60s", + IsGRPC: false, }, { Name: "tea", @@ -2463,6 +2480,7 @@ var ( GRPCPass: "grpc://tea-v3", GRPCStatus: createPointerFromInt(12), GRPCService: "tea-servicev2", + IsGRPC: true, }, }, Locations: []Location{ @@ -2787,16 +2805,18 @@ var ( }, HealthChecks: []HealthCheck{ { - Name: "coffee", - URI: "/", - Interval: "5s", - Jitter: "0s", - Fails: 1, - Passes: 1, - Port: 50, - ProxyPass: "http://coffee-v2", - Mandatory: true, - Persistent: true, + Name: "coffee", + URI: "/", + Interval: "5s", + Jitter: "0s", + Fails: 1, + Passes: 1, + Port: 50, + ProxyPass: "http://coffee-v2", + Mandatory: true, + Persistent: true, + KeepaliveTime: "60s", + IsGRPC: false, }, { Name: "tea", @@ -2809,6 +2829,7 @@ var ( GRPCPass: "grpc://tea-v3", GRPCStatus: createPointerFromInt(12), GRPCService: "tea-servicev2", + IsGRPC: true, }, }, Locations: []Location{ @@ -3140,16 +3161,18 @@ var ( }, HealthChecks: []HealthCheck{ { - Name: "coffee", - URI: "/", - Interval: "5s", - Jitter: "0s", - Fails: 1, - Passes: 1, - Port: 50, - ProxyPass: "http://coffee-v2", - Mandatory: true, - Persistent: true, + Name: "coffee", + URI: "/", + Interval: "5s", + Jitter: "0s", + Fails: 1, + Passes: 1, + Port: 50, + ProxyPass: "http://coffee-v2", + Mandatory: true, + Persistent: true, + KeepaliveTime: "60s", + IsGRPC: false, }, { Name: "tea", @@ -3162,6 +3185,7 @@ var ( GRPCPass: "grpc://tea-v3", GRPCStatus: createPointerFromInt(12), GRPCService: "tea-servicev2", + IsGRPC: true, }, }, Locations: []Location{ @@ -3764,16 +3788,18 @@ var ( }, HealthChecks: []HealthCheck{ { - Name: "coffee", - URI: "/", - Interval: "5s", - Jitter: "0s", - Fails: 1, - Passes: 1, - Port: 50, - ProxyPass: "http://coffee-v2", - Mandatory: true, - Persistent: true, + Name: "coffee", + URI: "/", + Interval: "5s", + Jitter: "0s", + Fails: 1, + Passes: 1, + Port: 50, + ProxyPass: "http://coffee-v2", + Mandatory: true, + Persistent: true, + KeepaliveTime: "60s", + IsGRPC: false, }, { Name: "tea", @@ -3786,6 +3812,7 @@ var ( GRPCPass: "grpc://tea-v3", GRPCStatus: createPointerFromInt(12), GRPCService: "tea-servicev2", + IsGRPC: true, }, }, Locations: []Location{ @@ -4113,16 +4140,18 @@ var ( }, HealthChecks: []HealthCheck{ { - Name: "coffee", - URI: "/", - Interval: "5s", - Jitter: "0s", - Fails: 1, - Passes: 1, - Port: 50, - ProxyPass: "http://coffee-v2", - Mandatory: true, - Persistent: true, + Name: "coffee", + URI: "/", + Interval: "5s", + Jitter: "0s", + Fails: 1, + Passes: 1, + Port: 50, + ProxyPass: "http://coffee-v2", + Mandatory: true, + Persistent: true, + KeepaliveTime: "60s", + IsGRPC: false, }, { Name: "tea", @@ -4135,6 +4164,7 @@ var ( GRPCPass: "grpc://tea-v3", GRPCStatus: createPointerFromInt(12), GRPCService: "tea-servicev2", + IsGRPC: true, }, }, Locations: []Location{ @@ -4462,16 +4492,18 @@ var ( }, HealthChecks: []HealthCheck{ { - Name: "coffee", - URI: "/", - Interval: "5s", - Jitter: "0s", - Fails: 1, - Passes: 1, - Port: 50, - ProxyPass: "http://coffee-v2", - Mandatory: true, - Persistent: true, + Name: "coffee", + URI: "/", + Interval: "5s", + Jitter: "0s", + Fails: 1, + Passes: 1, + Port: 50, + ProxyPass: "http://coffee-v2", + Mandatory: true, + Persistent: true, + KeepaliveTime: "60s", + IsGRPC: false, }, { Name: "tea", @@ -4484,6 +4516,7 @@ var ( GRPCPass: "grpc://tea-v3", GRPCStatus: createPointerFromInt(12), GRPCService: "tea-servicev2", + IsGRPC: true, }, }, Locations: []Location{ diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index de5fba197f..d2fc020f8d 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -265,6 +265,7 @@ func newHealthCheckWithDefaults(upstream conf_v1.Upstream, upstreamName string, ProxySendTimeout: generateTimeWithDefault(upstream.ProxySendTimeout, cfgParams.ProxySendTimeout), Headers: make(map[string]string), GRPCPass: generateGRPCPass(isGRPC(upstream.Type), upstream.TLS.Enable, upstreamName), + IsGRPC: isGRPC(upstream.Type), } } diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index f03e313e76..abb4dc1886 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -13278,6 +13278,7 @@ func TestGenerateGrpcHealthCheck(t *testing.T) { "Host": "my.service", "User-Agent": "nginx", }, + IsGRPC: true, }, msg: "HealthCheck with changed parameters", }, @@ -13305,6 +13306,7 @@ func TestGenerateGrpcHealthCheck(t *testing.T) { Fails: 1, Passes: 1, Headers: make(map[string]string), + IsGRPC: true, }, msg: "HealthCheck with default parameters from Upstream", }, diff --git a/tests/data/virtual-server-grpc/virtual-server-healthcheck-fail.yaml b/tests/data/virtual-server-grpc/virtual-server-healthcheck-fail.yaml new file mode 100644 index 0000000000..f0d85621f6 --- /dev/null +++ b/tests/data/virtual-server-grpc/virtual-server-healthcheck-fail.yaml @@ -0,0 +1,31 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServer +metadata: + name: virtual-server +spec: + host: virtual-server.example.com + tls: + secret: virtual-server-tls-grpc-secret + upstreams: + - name: grpc1 + service: grpc1-svc + port: 50051 + type: grpc + healthCheck: + enable: true + interval: "1s" + jitter: "2s" + port: 50051 + grpcStatus: 0 # This should faill the health check, as an implemented service will not return 0, which means OK + grpcService: "none.None" + - name: grpc2 + service: grpc2-svc + port: 50051 + type: grpc + routes: + - path: "/helloworld.Greeter" + action: + pass: grpc1 + - path: "/notimplemented" + action: + pass: grpc2 diff --git a/tests/data/virtual-server-grpc/virtual-server-healthcheck.yaml b/tests/data/virtual-server-grpc/virtual-server-healthcheck.yaml index 4afe29b611..c482e5ad70 100644 --- a/tests/data/virtual-server-grpc/virtual-server-healthcheck.yaml +++ b/tests/data/virtual-server-grpc/virtual-server-healthcheck.yaml @@ -17,7 +17,7 @@ spec: jitter: "2s" port: 50051 grpcStatus: 12 - grpcService: "helloworld.Greeter" + grpcService: "none.None" # This does not need to exist to be a valid health check, as grpc status 12 means "Unimplemented" - name: grpc2 service: grpc2-svc port: 50051 diff --git a/tests/suite/test_v_s_route_upstream_options.py b/tests/suite/test_v_s_route_upstream_options.py index 88bf20b200..74c6e7e187 100644 --- a/tests/suite/test_v_s_route_upstream_options.py +++ b/tests/suite/test_v_s_route_upstream_options.py @@ -576,8 +576,7 @@ class TestOptionsSpecificForPlus: }, }, [ - "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 keepalive_time=60s;", "slow_start=3h", "queue 100 timeout=60s;", "sticky cookie TestCookie expires=max domain=virtual-server-route.example.com httponly secure path=/some-valid/path;", @@ -590,6 +589,7 @@ class TestOptionsSpecificForPlus: "enable": True, "path": "/health", "interval": "15s", + "keepalive-time": "120s", "jitter": "3", "fails": 2, "passes": 2, @@ -605,16 +605,14 @@ class TestOptionsSpecificForPlus: "queue": {"size": 1000, "timeout": "66s"}, }, [ - "health_check uri=/health port=8080 interval=15s jitter=3", - "fails=2 passes=2 match=", - "proxy_pass https://vs", + "slow_start=0s", "status 200;", "proxy_connect_timeout 35s;", "proxy_read_timeout 45s;", "proxy_send_timeout 55s;", 'proxy_set_header Host "virtual-server.example.com";', - "slow_start=0s", - "queue 1000 timeout=66s;", + "proxy_pass https://vs", + "health_check uri=/health port=8080 interval=15s jitter=3s fails=2 passes=2 match=vs_backends-namespace_virtual-server-route_vsr_backend2-namespace_backend2_backend2_match keepalive_time=120s;", ], ), ], diff --git a/tests/suite/test_virtual_server_grpc.py b/tests/suite/test_virtual_server_grpc.py index 4de034df57..afa6859c99 100644 --- a/tests/suite/test_virtual_server_grpc.py +++ b/tests/suite/test_virtual_server_grpc.py @@ -297,7 +297,7 @@ def test_config_after_enable_healthcheck( param_list = [ "health_check port=50051 interval=1s jitter=2s", "type=grpc grpc_status=12", - "grpc_service=helloworld.Greeter keepalive_time=60s;", + "grpc_service=none.None;", ] for p in param_list: assert p in config @@ -327,3 +327,70 @@ def test_grpc_healthcheck_validation( assert_vs_conf_not_exists( kube_apis, ic_pod_name, ingress_controller_prerequisites.namespace, virtual_server_setup ) + + @pytest.mark.parametrize("backend_setup", [{"app_type": "grpc-vs"}], indirect=True) + def test_grpc_healthcheck_send_hello( + self, kube_apis, ingress_controller_prerequisites, crd_ingress_controller, backend_setup, virtual_server_setup + ): + ic_pod_name = get_first_pod_name(kube_apis.v1, ingress_controller_prerequisites.namespace) + patch_virtual_server_from_yaml( + kube_apis.custom_objects, + virtual_server_setup.vs_name, + f"{TEST_DATA}/virtual-server-grpc/virtual-server-healthcheck.yaml", + virtual_server_setup.namespace, + ) + + # Ensure the initial health check configuration + config = get_vs_nginx_template_conf( + kube_apis.v1, + virtual_server_setup.namespace, + virtual_server_setup.vs_name, + ic_pod_name, + ingress_controller_prerequisites.namespace, + ) + assert "grpc_service=none.None" in config + + # Attempt to send a Hello message + cert = get_certificate( + virtual_server_setup.public_endpoint.public_ip, + virtual_server_setup.vs_host, + virtual_server_setup.public_endpoint.port_ssl, + ) + target = f"{virtual_server_setup.public_endpoint.public_ip}:{virtual_server_setup.public_endpoint.port_ssl}" + credentials = grpc.ssl_channel_credentials(root_certificates=cert.encode()) + options = (("grpc.ssl_target_name_override", virtual_server_setup.vs_host),) + + try: + with grpc.secure_channel(target, credentials, options) as channel: + stub = GreeterStub(channel) + response = stub.SayHello(HelloRequest(name="")) + valid_message = "Hello " + assert response.message == valid_message, f"Expected '{valid_message}', but got '{response.message}'" + except grpc.RpcError as e: + print(e.details()) + pytest.fail("RPC error was not expected during call, exiting...") + + # Update health check to expect gRPC status code 0 + patch_virtual_server_from_yaml( + kube_apis.custom_objects, + virtual_server_setup.vs_name, + f"{TEST_DATA}/virtual-server-grpc/virtual-server-healthcheck-fail.yaml", + virtual_server_setup.namespace, + ) + wait_before_test() + + # Re-fetch the config to confirm the update + config = get_vs_nginx_template_conf( + kube_apis.v1, + virtual_server_setup.namespace, + virtual_server_setup.vs_name, + ic_pod_name, + ingress_controller_prerequisites.namespace, + ) + assert "grpc_status=0" in config + + # Verify that the health check fails + log_contents = kube_apis.v1.read_namespaced_pod_log(ic_pod_name, ingress_controller_prerequisites.namespace) + assert ( + "peer is unhealthy while checking grpc response" in log_contents + ), "Expected 'peer is unhealthy while checking grpc response' in logs but it was not found" diff --git a/tests/suite/test_virtual_server_upstream_options.py b/tests/suite/test_virtual_server_upstream_options.py index 29049153a2..3ce241f4cd 100644 --- a/tests/suite/test_virtual_server_upstream_options.py +++ b/tests/suite/test_virtual_server_upstream_options.py @@ -507,7 +507,7 @@ class TestOptionsSpecificForPlus: [ "health_check uri=/ interval=5s jitter=0s", "fails=1 passes=1", - "mandatory persistent", + "mandatory persistent", "keepalive_time=60s;", "slow_start=3h", "queue 100 timeout=60s;", @@ -536,7 +536,7 @@ class TestOptionsSpecificForPlus: [ "health_check uri=/ interval=5s jitter=0s", "fails=1 passes=1", - "mandatory persistent", + "mandatory persistent", "keepalive_time=60s;", "slow_start=3h", "queue 100 timeout=60s;", @@ -565,7 +565,7 @@ class TestOptionsSpecificForPlus: [ "health_check uri=/ interval=5s jitter=0s", "fails=1 passes=1", - "mandatory persistent", + "mandatory persistent", "keepalive_time=60s;", "slow_start=3h", "queue 100 timeout=60s;", @@ -597,8 +597,7 @@ class TestOptionsSpecificForPlus: "ntlm": True, }, [ - "health_check uri=/health port=8080 interval=15s jitter=3", - "fails=2 passes=2 match=", + "health_check uri=/health port=8080 interval=15s jitter=3s fails=2 passes=2 match=", "proxy_pass https://vs", "status 200;", "proxy_connect_timeout 35s;",