From 5faf1fb0bab69c26187a9ea602552faf10832e1c Mon Sep 17 00:00:00 2001 From: Ryan S <267728323+ironcommit@users.noreply.github.com> Date: Wed, 5 Aug 2026 11:48:55 -0700 Subject: [PATCH] fix: set Envoy upstream idle timeout Set Envoy's upstream HTTP connection-pool idle timeout to 20s in the base Helm chart and the Authentik reference Envoy configs. Envoy defaults this timeout to 1h, which can keep pooled backend API connections around after the backend has closed its keep-alive socket. Retiring idle upstream connections sooner reduces stale connection reuse and addresses the auth-idp Kubernetes failure that surfaced as: upstream connect error or disconnect/reset before headers. reset reason: connection termination Add inline Envoy comments, Helm README documentation, and render/static assertions for the base chart, Authentik chart override, and Authentik compose gateway. Signed-off-by: Ryan S <267728323+ironcommit@users.noreply.github.com> --- .github/workflows/ci.yaml | 4 + contrib/auth/authentik/gateway/envoy.yaml | 10 ++ .../helm/templates/_envoy-config.tpl | 10 ++ contrib/auth/authentik/helm/values.yaml | 2 + docs/cli/reference.mdx | 3 + k8s/helm/README.md | 3 + k8s/helm/templates/_helpers.tpl | 21 +++ k8s/helm/templates/api/api-deployment.yaml | 1 + k8s/helm/templates/proxy/envoy-configmap.yaml | 11 ++ k8s/helm/values.yaml | 6 + .../cli/commands/services/cli.py | 42 ++++- .../src/nemo_platform_ext/local/process.py | 1 + .../src/nemo_platform_ext/local/services.py | 22 ++- .../tests/cli/commands/test_services.py | 61 +++++++- .../cli/commands/test_services_process.py | 3 + .../tests/local/test_services.py | 43 +++++- .../src/nmp/platform_runner/config.py | 14 ++ .../src/nmp/platform_runner/run.py | 9 +- .../src/nmp/platform_runner/server.py | 51 +++++- .../nmp_platform_runner/tests/test_config.py | 21 +++ .../nmp_platform_runner/tests/test_run.py | 10 +- .../nmp_platform_runner/tests/test_server.py | 70 ++++++++- .../cli/commands/services/cli.py | 42 ++++- .../src/nemo_platform/local/process.py | 1 + .../src/nemo_platform/local/services.py | 22 ++- .../cli/commands/test_services.py | 61 +++++++- .../cli/commands/test_services_process.py | 3 + .../nemo_platform_ext/local/test_services.py | 43 +++++- .../static/test_authentik_kubernetes_demo.py | 19 +++ .../static/test_envoy_config_validation.py | 146 ++++++++++++++++++ tests/auth_idp/static/test_provider_layout.py | 7 + tests/unit/test_helm_clickhouse.py | 63 ++++++++ tools/lint/lint-helm.sh | 48 +++++- 33 files changed, 847 insertions(+), 26 deletions(-) create mode 100644 tests/auth_idp/static/test_envoy_config_validation.py diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d0f9939ab5..4e45c75c05 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -1060,6 +1060,7 @@ jobs: !cancelled() && ( github.event_name == 'workflow_dispatch' || needs.changes.outputs.cpu-smoke == 'true' || + needs.changes.outputs.helm == 'true' || needs.changes.outputs.auth-idp == 'true' ) runs-on: ubuntu-latest @@ -1078,6 +1079,9 @@ jobs: cache-dependency-glob: uv.lock - name: Run auth-idp static tests run: | + set -euo pipefail + docker pull docker.io/envoyproxy/envoy:v1.37.0 + docker pull envoyproxy/envoy:v1.36.2 helm dependency build k8s/helm helm dependency build contrib/auth/authentik/helm uv run --frozen pytest tests/auth_idp/static -v diff --git a/contrib/auth/authentik/gateway/envoy.yaml b/contrib/auth/authentik/gateway/envoy.yaml index 861ad81668..1dd8597bc2 100644 --- a/contrib/auth/authentik/gateway/envoy.yaml +++ b/contrib/auth/authentik/gateway/envoy.yaml @@ -251,6 +251,16 @@ static_resources: - name: nemo connect_timeout: 5s type: LOGICAL_DNS + # Envoy's default upstream HTTP idle timeout is 1h. Keep this below the + # API keep-alive timeout so stale pooled API connections are retired + # before the backend closes them. + typed_extension_protocol_options: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + common_http_protocol_options: + idle_timeout: "4s" + explicit_http_config: + http_protocol_options: {} load_assignment: cluster_name: nemo endpoints: diff --git a/contrib/auth/authentik/helm/templates/_envoy-config.tpl b/contrib/auth/authentik/helm/templates/_envoy-config.tpl index f2987b488e..1ccd850966 100644 --- a/contrib/auth/authentik/helm/templates/_envoy-config.tpl +++ b/contrib/auth/authentik/helm/templates/_envoy-config.tpl @@ -257,6 +257,16 @@ static_resources: - name: nemo connect_timeout: 5s type: LOGICAL_DNS + # Envoy's default upstream HTTP idle timeout is 1h. Keep this below the + # API keep-alive timeout so stale pooled API connections are retired + # before the backend closes them. + typed_extension_protocol_options: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + common_http_protocol_options: + idle_timeout: {{ .Values.envoyProxy.timeouts.upstreamIdle | quote }} + explicit_http_config: + http_protocol_options: {} load_assignment: cluster_name: nemo endpoints: diff --git a/contrib/auth/authentik/helm/values.yaml b/contrib/auth/authentik/helm/values.yaml index 578ecff364..0a24f1bfb0 100644 --- a/contrib/auth/authentik/helm/values.yaml +++ b/contrib/auth/authentik/helm/values.yaml @@ -188,6 +188,8 @@ nemo-platform: - name: workload-token-tls secret: secretName: *workloadTokenTlsSecretName + timeouts: + upstreamIdle: "4s" configOverride: '{{ include "nemo-platform-authentik.envoyConfig" . }}' platformConfig: auth: diff --git a/docs/cli/reference.mdx b/docs/cli/reference.mdx index c118633a94..ad5e42edba 100644 --- a/docs/cli/reference.mdx +++ b/docs/cli/reference.mdx @@ -342,6 +342,7 @@ nemo services run [OPTIONS] * `--config`: Path to a platform configuration YAML file. * `--host`: Host to bind to. [default: 127.0.0.1] * `--port `: Port to bind to. [default: 8080] +* `--keep-alive-timeout-seconds `: Seconds Uvicorn keeps idle HTTP connections open. [default: 5] * `--instance`: Instance name. Defaults to a name derived from the working directory and port. **Help:** @@ -377,6 +378,7 @@ nemo services start [OPTIONS] * `--config`: Path to a platform configuration YAML file. * `--host`: Host to bind to. [default: 127.0.0.1] * `--port `: Port to bind to. [default: 8080] +* `--keep-alive-timeout-seconds `: Seconds Uvicorn keeps idle HTTP connections open. [default: 5] * `--instance`: Instance name. Defaults to a name derived from the working directory and port. **Help:** @@ -446,6 +448,7 @@ nemo services restart [OPTIONS] * `--config`: Path to a platform configuration YAML file. * `--host`: Host to bind to. Defaults to previous value or 127.0.0.1. * `--port `: Port to bind to. Defaults to previous value or 8080. +* `--keep-alive-timeout-seconds `: Seconds Uvicorn keeps idle HTTP connections open. Defaults to the previous value or 5. * `--instance`: Instance name. Defaults to a name derived from the working directory and port. **Help:** diff --git a/k8s/helm/README.md b/k8s/helm/README.md index 7f1ba1caa2..5653902791 100644 --- a/k8s/helm/README.md +++ b/k8s/helm/README.md @@ -160,6 +160,8 @@ and | api.replicaCount | int | `1` | Number of replicas for the API service. | | api.resources | object | `{}` | Kubernetes deployment resources configuration for the API service. Utilization-based autoscaling requires a matching resource request. | | api.securityContext | object | `{}` | Container-level security context settings for the API service. | +| api.server | object | `{"keepAliveTimeoutSeconds":5}` | Platform API server settings. | +| api.server.keepAliveTimeoutSeconds | int | `5` | Seconds Uvicorn keeps idle HTTP connections open. Must be greater than envoyProxy.timeouts.upstreamIdle when Envoy is enabled. | | api.service | object | This object has the following default values for the service configuration. | Service configuration for the API service. | | api.service.annotations | object | `{}` | Annotations for the API service. | | api.service.port | int | `8080` | The port number to expose for the service. | @@ -331,6 +333,7 @@ and | envoyProxy.timeouts.requestHeaders | string | `"60s"` | Time to receive full request headers. 0 = disabled. | | envoyProxy.timeouts.route | string | `"0s"` | Per-route timeout for the passthrough to backend. 0 = disabled. | | envoyProxy.timeouts.streamIdle | string | `"0s"` | Stream idle timeout. Time with no activity before stream is closed. 0 = disabled (required for long-lived streams). | +| envoyProxy.timeouts.upstreamIdle | string | `"4s"` | Positive whole-second upstream connection idle timeout. Must be less than api.server.keepAliveTimeoutSeconds when Envoy is enabled. | | envoyProxy.tolerations | list | `[]` | Tolerations configuration for the Envoy pods. | | envoyProxy.topologySpreadConstraints | list | `[]` | Topology spread constraints for the Envoy pods. See https://kubernetes.io/docs/concepts/scheduling-eviction/topology-spread-constraints/ | | existingSecret | string | `"ngc-api"` | You can use an existing Kubernetes secret for communicating with the NGC API for downloading models. The chart uses the `ngcAPIKey` value to generate the secret if you set this to an empty string. | diff --git a/k8s/helm/templates/_helpers.tpl b/k8s/helm/templates/_helpers.tpl index 54d8cf256b..9da289f6bc 100644 --- a/k8s/helm/templates/_helpers.tpl +++ b/k8s/helm/templates/_helpers.tpl @@ -141,6 +141,27 @@ local process instead of the cluster Service. {{- printf "http://localhost:%s" (toString .Values.api.service.port) -}} {{- end -}} +{{/* +Validate that Envoy retires idle upstream API connections before Uvicorn closes +them. This avoids reusing a backend connection that the API already dropped. +*/}} +{{- define "nemo-platform.validateEnvoyKeepAliveTimeouts" -}} +{{- if and .Values.api.enabled (include "nemo-platform.authEnabled" .) .Values.envoyProxy.enabled -}} +{{- $apiKeepAliveSeconds := .Values.api.server.keepAliveTimeoutSeconds | int -}} +{{- if lt $apiKeepAliveSeconds 1 -}} +{{- fail "api.server.keepAliveTimeoutSeconds must be greater than 0" -}} +{{- end -}} +{{- $upstreamIdle := .Values.envoyProxy.timeouts.upstreamIdle | toString -}} +{{- if not (regexMatch "^[1-9][0-9]*s$" $upstreamIdle) -}} +{{- fail "envoyProxy.timeouts.upstreamIdle must be a positive whole-second duration like \"4s\"" -}} +{{- end -}} +{{- $upstreamIdleSeconds := trimSuffix "s" $upstreamIdle | int -}} +{{- if ge $upstreamIdleSeconds $apiKeepAliveSeconds -}} +{{- fail (printf "envoyProxy.timeouts.upstreamIdle (%s) must be less than api.server.keepAliveTimeoutSeconds (%ds)" $upstreamIdle $apiKeepAliveSeconds) -}} +{{- end -}} +{{- end -}} +{{- end -}} + {{/* Pod annotations */}} diff --git a/k8s/helm/templates/api/api-deployment.yaml b/k8s/helm/templates/api/api-deployment.yaml index cfd2ca4a20..b112ebd047 100644 --- a/k8s/helm/templates/api/api-deployment.yaml +++ b/k8s/helm/templates/api/api-deployment.yaml @@ -52,6 +52,7 @@ spec: - "--service-group=all" - "--host={{ include "nemo-platform.bindHost" . }}" - "--port={{ .Values.api.service.port }}" + - "--keep-alive-timeout-seconds={{ .Values.api.server.keepAliveTimeoutSeconds }}" {{- range .Values.api.extraArgs }} - {{ . | quote }} {{- end }} diff --git a/k8s/helm/templates/proxy/envoy-configmap.yaml b/k8s/helm/templates/proxy/envoy-configmap.yaml index b446d29672..672d131595 100644 --- a/k8s/helm/templates/proxy/envoy-configmap.yaml +++ b/k8s/helm/templates/proxy/envoy-configmap.yaml @@ -1,3 +1,4 @@ +{{- include "nemo-platform.validateEnvoyKeepAliveTimeouts" . }} {{- if and (include "nemo-platform.authEnabled" .) .Values.envoyProxy.enabled }} apiVersion: v1 kind: ConfigMap @@ -59,6 +60,16 @@ data: type: STRICT_DNS lb_policy: ROUND_ROBIN connect_timeout: {{ .Values.envoyProxy.timeouts.connect | quote }} + # Envoy's default upstream HTTP idle timeout is 1h. The chart default + # stays below the API keep-alive timeout so stale pooled API + # connections are retired before the backend closes them. + typed_extension_protocol_options: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + common_http_protocol_options: + idle_timeout: {{ .Values.envoyProxy.timeouts.upstreamIdle | quote }} + explicit_http_config: + http_protocol_options: {} load_assignment: cluster_name: backend_cluster endpoints: diff --git a/k8s/helm/values.yaml b/k8s/helm/values.yaml index 957311b6fc..adcdc892ee 100644 --- a/k8s/helm/values.yaml +++ b/k8s/helm/values.yaml @@ -647,6 +647,10 @@ api: # -- Number of replicas for the API service. replicaCount: 1 + # -- Platform API server settings. + server: + # -- Seconds Uvicorn keeps idle HTTP connections open. Must be greater than envoyProxy.timeouts.upstreamIdle when Envoy is enabled. + keepAliveTimeoutSeconds: 5 # -- Additional arguments to pass to the Platform API service extraArgs: [] # -- Additional volume mounts to add to the Platform API container. @@ -1053,6 +1057,8 @@ envoyProxy: route: "0s" # -- Cluster connect timeout (time to establish connection to backend). connect: "30s" + # -- Positive whole-second upstream connection idle timeout. Must be less than api.server.keepAliveTimeoutSeconds when Envoy is enabled. + upstreamIdle: "4s" # -- Kubernetes deployment resources configuration for the Envoy service. Utilization-based autoscaling requires a matching resource request. resources: {} diff --git a/packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/services/cli.py b/packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/services/cli.py index a4009af5d3..e71a09da26 100644 --- a/packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/services/cli.py +++ b/packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/services/cli.py @@ -37,7 +37,11 @@ stop_instance, write_descriptor, ) -from nmp.platform_runner.config import DEFAULT_LOCAL_SERVICES_BIND_HOST, PlatformAppConfig +from nmp.platform_runner.config import ( + DEFAULT_LOCAL_SERVICES_BIND_HOST, + DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, + PlatformAppConfig, +) logger = logging.getLogger(__name__) @@ -207,6 +211,14 @@ def run_services( ] = None, host: Annotated[str, typer.Option("--host", help="Host to bind to.")] = DEFAULT_LOCAL_SERVICES_BIND_HOST, port: Annotated[int, typer.Option("--port", help="Port to bind to.")] = _DEFAULT_PORT, + keep_alive_timeout_seconds: Annotated[ + int, + typer.Option( + "--keep-alive-timeout-seconds", + min=1, + help="Seconds Uvicorn keeps idle HTTP connections open.", + ), + ] = DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, instance: Annotated[ str | None, typer.Option( @@ -244,6 +256,7 @@ def run_services( scope=scope, host=host, port=port, + keep_alive_timeout_seconds=keep_alive_timeout_seconds, state_root=base_dir, ) @@ -317,6 +330,14 @@ def start_services( ] = None, host: Annotated[str, typer.Option("--host", help="Host to bind to.")] = DEFAULT_LOCAL_SERVICES_BIND_HOST, port: Annotated[int, typer.Option("--port", help="Port to bind to.")] = _DEFAULT_PORT, + keep_alive_timeout_seconds: Annotated[ + int, + typer.Option( + "--keep-alive-timeout-seconds", + min=1, + help="Seconds Uvicorn keeps idle HTTP connections open.", + ), + ] = DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, instance: Annotated[ str | None, typer.Option( @@ -358,6 +379,7 @@ def start_services( scope=scope, host=host, port=port, + keep_alive_timeout_seconds=keep_alive_timeout_seconds, state_root=base_dir, ) @@ -495,6 +517,14 @@ def restart_services( int | None, typer.Option("--port", help="Port to bind to. Defaults to previous value or 8080."), ] = None, + keep_alive_timeout_seconds: Annotated[ + int | None, + typer.Option( + "--keep-alive-timeout-seconds", + min=1, + help="Seconds Uvicorn keeps idle HTTP connections open. Defaults to the previous value or 5.", + ), + ] = None, instance: Annotated[ str | None, typer.Option( @@ -561,6 +591,15 @@ def restart_services( host if host is not None else (previous_config.host if previous_config else DEFAULT_LOCAL_SERVICES_BIND_HOST) ) effective_port = port if port is not None else (previous_config.port if previous_config else _DEFAULT_PORT) + effective_keep_alive_timeout_seconds = ( + keep_alive_timeout_seconds + if keep_alive_timeout_seconds is not None + else ( + previous_config.keep_alive_timeout_seconds + if previous_config + else DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS + ) + ) _warn_bind_all(effective_host) @@ -575,6 +614,7 @@ def restart_services( scope=scope, host=effective_host, port=effective_port, + keep_alive_timeout_seconds=effective_keep_alive_timeout_seconds, state_root=base_dir, ) diff --git a/packages/nemo_platform_ext/src/nemo_platform_ext/local/process.py b/packages/nemo_platform_ext/src/nemo_platform_ext/local/process.py index f988924142..e6ebb59510 100644 --- a/packages/nemo_platform_ext/src/nemo_platform_ext/local/process.py +++ b/packages/nemo_platform_ext/src/nemo_platform_ext/local/process.py @@ -827,6 +827,7 @@ def start_background( if config.config_path: args += ["--config", config.config_path] args += ["--host", config.host, "--port", str(config.port)] + args += ["--keep-alive-timeout-seconds", str(config.keep_alive_timeout_seconds)] args += ["--instance", config.scope] env = os.environ.copy() diff --git a/packages/nemo_platform_ext/src/nemo_platform_ext/local/services.py b/packages/nemo_platform_ext/src/nemo_platform_ext/local/services.py index d115fe06ec..6af3f5f04f 100644 --- a/packages/nemo_platform_ext/src/nemo_platform_ext/local/services.py +++ b/packages/nemo_platform_ext/src/nemo_platform_ext/local/services.py @@ -35,9 +35,11 @@ ) from nmp.platform_runner.config import ( DEFAULT_SCOPE, + DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, PlatformAppConfig, default_runtime_root, default_state_root, + validate_keep_alive_timeout_seconds, validate_scope, ) @@ -151,6 +153,7 @@ class ServiceRunConfig: data_dir: str | Path | None = None readiness_timeout: float = 60.0 readiness_poll_interval: float = 0.5 + keep_alive_timeout_seconds: int = DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS mode: ServiceMode | str = ServiceMode.DAEMON def __post_init__(self) -> None: @@ -176,6 +179,7 @@ def __post_init__(self) -> None: raise ValueError("readiness_timeout must be greater than 0") if self.readiness_poll_interval <= 0: raise ValueError("readiness_poll_interval must be greater than 0") + self.keep_alive_timeout_seconds = validate_keep_alive_timeout_seconds(self.keep_alive_timeout_seconds) self.scope = validate_scope(self.scope) @property @@ -215,6 +219,7 @@ def to_platform_app_config(self) -> PlatformAppConfig: socket_path=_optional_str(self.resolved_socket_path), state_root=_optional_str(self.state_root), runtime_root=_optional_str(self.runtime_dir), + keep_alive_timeout_seconds=self.keep_alive_timeout_seconds, ) def to_child_payload(self) -> dict[str, object]: @@ -239,6 +244,7 @@ def to_child_payload(self) -> dict[str, object]: "data_dir": _optional_str(self.data_dir), "readiness_timeout": self.readiness_timeout, "readiness_poll_interval": self.readiness_poll_interval, + "keep_alive_timeout_seconds": self.keep_alive_timeout_seconds, } @@ -486,9 +492,21 @@ def serve_embedded_app(app: Any, cfg: ServiceRunConfig, socket_path: Path | None if socket_path is not None: from nmp.platform_runner.server import _run_server_on_bound_sockets - _run_server_on_bound_sockets(app, host=cfg.host, port=cfg.port, socket_path=str(socket_path)) + _run_server_on_bound_sockets( + app, + host=cfg.host, + port=cfg.port, + socket_path=str(socket_path), + keep_alive_timeout_seconds=cfg.keep_alive_timeout_seconds, + ) else: - uvicorn.run(app, host=cfg.host, port=cfg.port, log_config=None) + uvicorn.run( + app, + host=cfg.host, + port=cfg.port, + log_config=None, + timeout_keep_alive=cfg.keep_alive_timeout_seconds, + ) def run_services( diff --git a/packages/nemo_platform_ext/tests/cli/commands/test_services.py b/packages/nemo_platform_ext/tests/cli/commands/test_services.py index 7c939fcb8f..e286b61f4b 100644 --- a/packages/nemo_platform_ext/tests/cli/commands/test_services.py +++ b/packages/nemo_platform_ext/tests/cli/commands/test_services.py @@ -78,6 +78,18 @@ def test_services_help_lists_all_commands(): assert cmd in result.stdout, f"'{cmd}' not in help output" +@pytest.mark.parametrize("command", ["run", "start", "restart"]) +@pytest.mark.parametrize("timeout", ["0", "-1"]) +def test_services_reject_non_positive_keep_alive_timeout(command: str, timeout: str): + with patch(f"{_CLI_MODULE}.stop_instance") as mock_stop: + result = runner.invoke(app, ["services", command, "--keep-alive-timeout-seconds", timeout]) + + assert result.exit_code != 0 + assert "--keep-alive-timeout-seconds" in f"{result.stdout}{result.stderr}" + if command == "restart": + mock_stop.assert_not_called() + + # --------------------------------------------------------------------------- # run (foreground) # --------------------------------------------------------------------------- @@ -113,6 +125,8 @@ def test_run_invokes_runner(base_dir: Path): "127.0.0.1", "--port", "9000", + "--keep-alive-timeout-seconds", + "12", "--instance", "test-run", ], @@ -130,6 +144,7 @@ def test_run_invokes_runner(base_dir: Path): assert config.config_path is None assert config.host == "127.0.0.1" assert config.port == 9000 + assert config.keep_alive_timeout_seconds == 12 assert kwargs["on_shutdown"] is not None @@ -208,16 +223,18 @@ def test_start_launches_background(base_dir: Path): with ( patch(f"{_CLI_MODULE}._require_services_extra"), - patch(f"{_CLI_MODULE}.start_background", return_value=mock_proc), + patch(f"{_CLI_MODULE}.start_background", return_value=mock_proc) as mock_start, patch(f"{_CLI_MODULE}._wait_for_healthy", return_value=True), ): result = runner.invoke( app, - ["services", "start", "--instance", "bg-test"], + ["services", "start", "--instance", "bg-test", "--keep-alive-timeout-seconds", "12"], ) assert result.exit_code == 0 assert "99999" in result.stdout + config = mock_start.call_args.args[0] + assert config.keep_alive_timeout_seconds == 12 def test_start_refuses_when_already_running(base_dir: Path): @@ -463,6 +480,7 @@ def test_restart_preserves_previous_args(self, base_dir: Path): controllers=["jobs"], host="127.0.0.1", port=9000, + keep_alive_timeout_seconds=12, ), mode="background", create_time=1.0, @@ -494,6 +512,45 @@ def test_restart_preserves_previous_args(self, base_dir: Path): assert config.controllers == ["jobs"] assert config.host == "127.0.0.1" assert config.port == 9000 + assert config.keep_alive_timeout_seconds == 12 + + def test_restart_overrides_previous_keep_alive_timeout(self, base_dir: Path): + scope = "override-keep-alive-test" + fd = acquire_lock(scope, base_dir=base_dir) + desc = InstanceDescriptor( + pid=os.getpid(), + config=PlatformAppConfig( + scope=scope, + host="127.0.0.1", + port=9000, + keep_alive_timeout_seconds=12, + ), + mode="background", + create_time=1.0, + ) + write_descriptor(desc, base_dir=base_dir) + + mock_proc = MagicMock() + mock_proc.pid = 22223 + mock_proc.poll.return_value = None + + try: + with ( + patch(f"{_CLI_MODULE}._require_services_extra"), + patch(f"{_CLI_MODULE}.stop_instance", return_value=StopResult(stopped_pids=[os.getpid()])), + patch(f"{_CLI_MODULE}.start_background", return_value=mock_proc) as mock_start, + patch(f"{_CLI_MODULE}._wait_for_healthy", return_value=True), + ): + result = runner.invoke( + app, + ["services", "restart", "--instance", scope, "--keep-alive-timeout-seconds", "15"], + ) + finally: + os.close(fd) + + assert result.exit_code == 0 + config = mock_start.call_args.args[0] + assert config.keep_alive_timeout_seconds == 15 # --------------------------------------------------------------------------- diff --git a/packages/nemo_platform_ext/tests/cli/commands/test_services_process.py b/packages/nemo_platform_ext/tests/cli/commands/test_services_process.py index 5b18e6a342..3d5b896b48 100644 --- a/packages/nemo_platform_ext/tests/cli/commands/test_services_process.py +++ b/packages/nemo_platform_ext/tests/cli/commands/test_services_process.py @@ -644,6 +644,7 @@ def test_launches_detached_subprocess(self, base_dir: Path) -> None: controllers=["jobs"], host="127.0.0.1", port=8080, + keep_alive_timeout_seconds=12, state_root=base_dir, ), ) @@ -653,6 +654,8 @@ def test_launches_detached_subprocess(self, base_dir: Path) -> None: assert call_kwargs["start_new_session"] is True assert call_kwargs["stdin"] == subprocess.DEVNULL assert call_kwargs["close_fds"] is True + args = mock_popen.call_args.args[0] + assert args[args.index("--keep-alive-timeout-seconds") + 1] == "12" def test_injects_nmp_data_dir_when_unset(self, base_dir: Path, monkeypatch) -> None: monkeypatch.delenv("NMP_DATA_DIR", raising=False) diff --git a/packages/nemo_platform_ext/tests/local/test_services.py b/packages/nemo_platform_ext/tests/local/test_services.py index a6716995f0..aede65e5ea 100644 --- a/packages/nemo_platform_ext/tests/local/test_services.py +++ b/packages/nemo_platform_ext/tests/local/test_services.py @@ -55,6 +55,7 @@ def test_service_run_config_converts_to_platform_app_config(tmp_path: Path) -> N sidecars=["adapters"], config_path=tmp_path / "local.yaml", socket_path=tmp_path / "nemo.sock", + keep_alive_timeout_seconds=12, mode="embedded", ) @@ -69,6 +70,12 @@ def test_service_run_config_converts_to_platform_app_config(tmp_path: Path) -> N assert app_config.runtime_dir() == tmp_path assert app_config.host == "127.0.0.1" assert app_config.port == 8080 + assert app_config.keep_alive_timeout_seconds == 12 + + +def test_service_run_config_rejects_non_positive_keep_alive_timeout() -> None: + with pytest.raises(ValueError, match="keep_alive_timeout_seconds must be greater than 0"): + ServiceRunConfig(keep_alive_timeout_seconds=0) def test_instance_descriptor_converts_from_service_run_config( @@ -862,14 +869,46 @@ def test_run_services_serves_embedded_app_with_socket_path(monkeypatch: pytest.M def test_serve_embedded_app_with_socket_path_listens_on_tcp_and_uds(tmp_path: Path) -> None: - cfg = ServiceRunConfig(transport="tcp", host="127.0.0.1", port=9090) + cfg = ServiceRunConfig( + transport="tcp", + host="127.0.0.1", + port=9090, + keep_alive_timeout_seconds=12, + ) app = object() socket_path = tmp_path / "nemo.sock" with patch("nmp.platform_runner.server._run_server_on_bound_sockets") as run_bound_sockets: services.serve_embedded_app(app, cfg, socket_path) - run_bound_sockets.assert_called_once_with(app, host="127.0.0.1", port=9090, socket_path=str(socket_path)) + run_bound_sockets.assert_called_once_with( + app, + host="127.0.0.1", + port=9090, + socket_path=str(socket_path), + keep_alive_timeout_seconds=12, + ) + + +def test_serve_embedded_app_without_socket_path_sets_keep_alive_timeout() -> None: + cfg = ServiceRunConfig( + transport="tcp", + host="127.0.0.1", + port=9090, + keep_alive_timeout_seconds=12, + ) + app = object() + + with patch("uvicorn.run") as uvicorn_run: + services.serve_embedded_app(app, cfg, None) + + uvicorn_run.assert_called_once_with( + app, + host="127.0.0.1", + port=9090, + log_config=None, + timeout_keep_alive=12, + ) def test_run_services_cleans_lock_when_log_path_resolution_fails( diff --git a/packages/nmp_platform_runner/src/nmp/platform_runner/config.py b/packages/nmp_platform_runner/src/nmp/platform_runner/config.py index 06c9acc189..454a7fa7f4 100644 --- a/packages/nmp_platform_runner/src/nmp/platform_runner/config.py +++ b/packages/nmp_platform_runner/src/nmp/platform_runner/config.py @@ -35,6 +35,7 @@ DEFAULT_SCOPE = "default" DEFAULT_PLATFORM_BIND_HOST = "0.0.0.0" DEFAULT_LOCAL_SERVICES_BIND_HOST = "127.0.0.1" +DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS = 5 _SCOPE_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") _INSTANCES_DIRNAME = "instances" @@ -59,6 +60,7 @@ class PlatformAppConfig: state_root: str | Path | None = None runtime_root: str | Path | None = None log_path: str | Path | None = None + keep_alive_timeout_seconds: int = DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS def __post_init__(self) -> None: self.scope = validate_scope(self.scope) @@ -66,6 +68,7 @@ def __post_init__(self) -> None: self.state_root = _resolve_absolute_path(self.state_root, "state root") self.runtime_root = _resolve_absolute_path(self.runtime_root, "runtime root") self.log_path = _resolve_absolute_path(self.log_path, "log path") + self.keep_alive_timeout_seconds = validate_keep_alive_timeout_seconds(self.keep_alive_timeout_seconds) @property def state_root_path(self) -> Path: @@ -109,6 +112,15 @@ def validate_scope(scope: str) -> str: return scope +def validate_keep_alive_timeout_seconds(timeout_seconds: int) -> int: + """Ensure Uvicorn keep-alive timeout is a positive whole-second value.""" + if isinstance(timeout_seconds, bool) or not isinstance(timeout_seconds, int): + raise ValueError("keep_alive_timeout_seconds must be greater than 0") + if timeout_seconds <= 0: + raise ValueError("keep_alive_timeout_seconds must be greater than 0") + return timeout_seconds + + def default_state_root() -> Path: """Return the local services state root.""" xdg = os.environ.get("XDG_STATE_HOME") @@ -146,6 +158,7 @@ class ResolvedRunConfiguration: socket_path: str | None = None available_services: dict[str, str | Service] = field(default_factory=dict) available_controllers: dict[str, str | ControllerRunFunc] = field(default_factory=dict) + keep_alive_timeout_seconds: int = DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS def default_config_path() -> str: @@ -236,6 +249,7 @@ def resolve_run_configuration( socket_path=resolved_socket_path, available_services=available_services, available_controllers=available_controllers, + keep_alive_timeout_seconds=config.keep_alive_timeout_seconds, ) diff --git a/packages/nmp_platform_runner/src/nmp/platform_runner/run.py b/packages/nmp_platform_runner/src/nmp/platform_runner/run.py index d3a54eac98..98da70493e 100644 --- a/packages/nmp_platform_runner/src/nmp/platform_runner/run.py +++ b/packages/nmp_platform_runner/src/nmp/platform_runner/run.py @@ -145,13 +145,20 @@ def signal_handler(signum: int, _frame: object) -> None: reload_app_factory or "nmp.platform_runner.server:create_default_app", host=resolved.host, port=resolved.port, + keep_alive_timeout_seconds=resolved.keep_alive_timeout_seconds, ) else: if controller_run_funcs: controller_threads.extend(run_controllers_in_threads(controller_run_funcs, controller_stop_signal)) if sidecar_run_funcs: controller_threads.extend(run_controllers_in_threads(sidecar_run_funcs, controller_stop_signal)) - run_server(service_instances, host=resolved.host, port=resolved.port, socket_path=resolved.socket_path) + run_server( + service_instances, + host=resolved.host, + port=resolved.port, + socket_path=resolved.socket_path, + keep_alive_timeout_seconds=resolved.keep_alive_timeout_seconds, + ) except ValueError as error: logger.error("Configuration error: %s", error) raise SystemExit(1) from error diff --git a/packages/nmp_platform_runner/src/nmp/platform_runner/server.py b/packages/nmp_platform_runner/src/nmp/platform_runner/server.py index 01f38787b0..d1cfca8eb2 100644 --- a/packages/nmp_platform_runner/src/nmp/platform_runner/server.py +++ b/packages/nmp_platform_runner/src/nmp/platform_runner/server.py @@ -27,7 +27,7 @@ from nmp.common.observability.context import create_app_context_dependency from nmp.common.pyleak import detect_blocking from nmp.common.service import Service -from nmp.platform_runner.config import PlatformAppConfig +from nmp.platform_runner.config import DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, PlatformAppConfig from nmp.platform_runner.health import ReadinessCheck, create_platform_health_router, get_platform_resource_attributes from nmp.platform_runner.loader import ( ControllerRunFunc, @@ -340,20 +340,51 @@ def run_server( host: str = "0.0.0.0", port: int = 8080, socket_path: str | None = None, + keep_alive_timeout_seconds: int = DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, ) -> None: """Run the platform API server.""" preflight_embedded_auth_policy_wasm(get_auth_config()) app = create_app(services or []) setup_fastapi_instrumentations(app) if socket_path: - _run_server_on_bound_sockets(app, host=host, port=port, socket_path=socket_path) + _run_server_on_bound_sockets( + app, + host=host, + port=port, + socket_path=socket_path, + keep_alive_timeout_seconds=keep_alive_timeout_seconds, + ) else: - uvicorn.run(app, host=host, port=port, log_config=None) + uvicorn.run( + app, + host=host, + port=port, + log_config=None, + timeout_keep_alive=keep_alive_timeout_seconds, + ) -def _run_server_on_bound_sockets(app: FastAPI, *, host: str, port: int, socket_path: str) -> None: - tcp_config = uvicorn.Config(app, host=host, port=port, log_config=None) - uds_config = uvicorn.Config(app, uds=socket_path, log_config=None) +def _run_server_on_bound_sockets( + app: FastAPI, + *, + host: str, + port: int, + socket_path: str, + keep_alive_timeout_seconds: int = DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, +) -> None: + tcp_config = uvicorn.Config( + app, + host=host, + port=port, + log_config=None, + timeout_keep_alive=keep_alive_timeout_seconds, + ) + uds_config = uvicorn.Config( + app, + uds=socket_path, + log_config=None, + timeout_keep_alive=keep_alive_timeout_seconds, + ) sockets = [tcp_config.bind_socket(), uds_config.bind_socket()] try: asyncio.run(uvicorn.Server(tcp_config).serve(sockets=sockets)) @@ -362,7 +393,12 @@ def _run_server_on_bound_sockets(app: FastAPI, *, host: str, port: int, socket_p sock.close() -def run_server_with_reload(app_factory: str, host: str = "0.0.0.0", port: int = 8080) -> None: +def run_server_with_reload( + app_factory: str, + host: str = "0.0.0.0", + port: int = 8080, + keep_alive_timeout_seconds: int = DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, +) -> None: """Run the platform API server with uvicorn reload enabled.""" preflight_embedded_auth_policy_wasm(get_auth_config()) reload_dirs = [ @@ -382,6 +418,7 @@ def run_server_with_reload(app_factory: str, host: str = "0.0.0.0", port: int = access_log=False, log_level="warning", factory=True, + timeout_keep_alive=keep_alive_timeout_seconds, ) diff --git a/packages/nmp_platform_runner/tests/test_config.py b/packages/nmp_platform_runner/tests/test_config.py index 738ae21fb8..aae07fb54e 100644 --- a/packages/nmp_platform_runner/tests/test_config.py +++ b/packages/nmp_platform_runner/tests/test_config.py @@ -2,11 +2,13 @@ # SPDX-License-Identifier: Apache-2.0 from pathlib import Path +from typing import Any import pytest from nmp.platform_runner import registry from nmp.platform_runner.config import ( DEFAULT_PLATFORM_BIND_HOST, + DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, PlatformAppConfig, ResolvedRunConfiguration, apply_run_environment, @@ -86,6 +88,18 @@ def test_platform_app_config_uses_explicit_log_path(tmp_path: Path): assert config.log_file_path() == tmp_path / "logs" / "nemo.log" +def test_platform_app_config_defaults_keep_alive_timeout(): + config = PlatformAppConfig() + + assert config.keep_alive_timeout_seconds == DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS + + +@pytest.mark.parametrize("timeout_seconds", [0, -1, 1.5, True]) +def test_platform_app_config_rejects_invalid_keep_alive_timeout(timeout_seconds: Any): + with pytest.raises(ValueError, match="keep_alive_timeout_seconds must be greater than 0"): + PlatformAppConfig(keep_alive_timeout_seconds=timeout_seconds) + + def test_platform_app_config_rejects_relative_socket_path(): with pytest.raises(ValueError, match="UDS socket path must be absolute"): PlatformAppConfig(socket_path="relative/path") @@ -120,6 +134,13 @@ def test_resolve_run_configuration_accepts_platform_app_config(): assert resolved.controllers == set() assert resolved.host == "127.0.0.1" assert resolved.port == 9090 + assert resolved.keep_alive_timeout_seconds == DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS + + +def test_resolve_run_configuration_preserves_keep_alive_timeout(): + resolved = resolve(keep_alive_timeout_seconds=12) + + assert resolved.keep_alive_timeout_seconds == 12 def test_no_arguments_defaults_to_all_services_and_default_controllers(): diff --git a/packages/nmp_platform_runner/tests/test_run.py b/packages/nmp_platform_runner/tests/test_run.py index def553aadc..787f7dfcf3 100644 --- a/packages/nmp_platform_runner/tests/test_run.py +++ b/packages/nmp_platform_runner/tests/test_run.py @@ -62,6 +62,7 @@ def test_run_platform_marks_loaded_services_local_before_starting_controllers(mo host="127.0.0.1", port=8080, config_path="", + keep_alive_timeout_seconds=12, ) services = [_StubService("jobs"), _StubService("entities")] @@ -76,7 +77,13 @@ def test_run_platform_marks_loaded_services_local_before_starting_controllers(mo lambda names, registry, kind: {"jobs": lambda stop_signal: None} if kind == "controller" else {}, ) monkeypatch.setattr(runner, "_display_banner", lambda **_: None) - monkeypatch.setattr(runner, "run_server", lambda services, host, port, socket_path=None: None) + monkeypatch.setattr( + runner, + "run_server", + lambda services, host, port, socket_path=None, keep_alive_timeout_seconds=None: captured.update( + {"keep_alive_timeout_seconds": str(keep_alive_timeout_seconds)} + ), + ) monkeypatch.setattr(runner.signal, "signal", lambda *args: None) def capture_controller_start( @@ -94,3 +101,4 @@ def capture_controller_start( Configuration.clear_cache() assert captured["services"] == "entities,jobs" + assert captured["keep_alive_timeout_seconds"] == "12" diff --git a/packages/nmp_platform_runner/tests/test_server.py b/packages/nmp_platform_runner/tests/test_server.py index e368b64e51..8903e26342 100644 --- a/packages/nmp_platform_runner/tests/test_server.py +++ b/packages/nmp_platform_runner/tests/test_server.py @@ -3,6 +3,7 @@ import asyncio import builtins +import inspect import os import sys import threading @@ -19,6 +20,7 @@ from nmp.common.service import RouterConfig, Service from nmp.platform_runner import config as runner_config from nmp.platform_runner import server +from nmp.platform_runner.config import DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS from nmp.platform_runner.health import ReadinessCheck, create_platform_health_router from pydantic import BaseModel @@ -351,11 +353,12 @@ def test_run_server_runs_embedded_auth_preflight(): patch("nmp.platform_runner.server.setup_fastapi_instrumentations"), patch("nmp.platform_runner.server.uvicorn.run") as uvicorn_run, ): - server.run_server(services=[], host="127.0.0.1", port=9999) + server.run_server(services=[], host="127.0.0.1", port=9999, keep_alive_timeout_seconds=12) assert calls == [auth_cfg] create_app.assert_called_once_with([]) uvicorn_run.assert_called_once() + assert uvicorn_run.call_args.kwargs["timeout_keep_alive"] == 12 def test_run_server_can_bind_tcp_and_unix_domain_socket(): @@ -372,11 +375,76 @@ def test_run_server_can_bind_tcp_and_unix_domain_socket(): run_bound_sockets.assert_called_once() assert run_bound_sockets.call_args.kwargs == { "host": "127.0.0.1", + "keep_alive_timeout_seconds": DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, "port": 9999, "socket_path": "/tmp/nemo-platform.sock", } +def test_run_server_on_bound_sockets_sets_keep_alive_timeout(): + app = FastAPI() + tcp_socket = MagicMock() + uds_socket = MagicMock() + tcp_config = MagicMock() + uds_config = MagicMock() + tcp_config.bind_socket.return_value = tcp_socket + uds_config.bind_socket.return_value = uds_socket + + with ( + patch("nmp.platform_runner.server.uvicorn.Config", side_effect=[tcp_config, uds_config]) as config_cls, + patch("nmp.platform_runner.server.uvicorn.Server") as server_cls, + patch("nmp.platform_runner.server.asyncio.run") as asyncio_run, + ): + server._run_server_on_bound_sockets( + app, + host="127.0.0.1", + port=9999, + socket_path="/tmp/nemo-platform.sock", + keep_alive_timeout_seconds=12, + ) + + assert config_cls.call_args_list[0].kwargs["timeout_keep_alive"] == 12 + assert config_cls.call_args_list[1].kwargs["timeout_keep_alive"] == 12 + server_cls.assert_called_once_with(tcp_config) + server_cls.return_value.serve.assert_called_once_with(sockets=[tcp_socket, uds_socket]) + asyncio_run.assert_called_once_with(server_cls.return_value.serve.return_value) + tcp_socket.close.assert_called_once_with() + uds_socket.close.assert_called_once_with() + + +def test_run_server_with_reload_sets_keep_alive_timeout(): + auth_cfg = _make_auth_config(enabled=True) + with ( + patch("nmp.platform_runner.server.get_auth_config", return_value=auth_cfg), + patch("nmp.platform_runner.server.preflight_embedded_auth_policy_wasm"), + patch("nmp.platform_runner.server.uvicorn.run") as uvicorn_run, + ): + server.run_server_with_reload( + "nmp.platform_runner.server:create_default_app", + host="127.0.0.1", + port=9999, + keep_alive_timeout_seconds=12, + ) + + uvicorn_run.assert_called_once() + assert uvicorn_run.call_args.kwargs["timeout_keep_alive"] == 12 + + +def test_server_default_keep_alive_matches_runner_config_default(): + assert ( + inspect.signature(server.run_server).parameters["keep_alive_timeout_seconds"].default + == DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS + ) + assert ( + inspect.signature(server._run_server_on_bound_sockets).parameters["keep_alive_timeout_seconds"].default + == DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS + ) + assert ( + inspect.signature(server.run_server_with_reload).parameters["keep_alive_timeout_seconds"].default + == DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS + ) + + def test_create_default_app_raises_for_unknown_service_from_env(monkeypatch): monkeypatch.setattr(server, "_obs_initialized", True) monkeypatch.setenv("NMP_SERVICES", "missing-service") diff --git a/sdk/python/nemo-platform/src/nemo_platform/cli/commands/services/cli.py b/sdk/python/nemo-platform/src/nemo_platform/cli/commands/services/cli.py index 8a7e3022bd..f3855c4c39 100644 --- a/sdk/python/nemo-platform/src/nemo_platform/cli/commands/services/cli.py +++ b/sdk/python/nemo-platform/src/nemo_platform/cli/commands/services/cli.py @@ -37,7 +37,11 @@ stop_instance, write_descriptor, ) -from nmp.platform_runner.config import DEFAULT_LOCAL_SERVICES_BIND_HOST, PlatformAppConfig +from nmp.platform_runner.config import ( + DEFAULT_LOCAL_SERVICES_BIND_HOST, + DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, + PlatformAppConfig, +) logger = logging.getLogger(__name__) @@ -207,6 +211,14 @@ def run_services( ] = None, host: Annotated[str, typer.Option("--host", help="Host to bind to.")] = DEFAULT_LOCAL_SERVICES_BIND_HOST, port: Annotated[int, typer.Option("--port", help="Port to bind to.")] = _DEFAULT_PORT, + keep_alive_timeout_seconds: Annotated[ + int, + typer.Option( + "--keep-alive-timeout-seconds", + min=1, + help="Seconds Uvicorn keeps idle HTTP connections open.", + ), + ] = DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, instance: Annotated[ str | None, typer.Option( @@ -244,6 +256,7 @@ def run_services( scope=scope, host=host, port=port, + keep_alive_timeout_seconds=keep_alive_timeout_seconds, state_root=base_dir, ) @@ -317,6 +330,14 @@ def start_services( ] = None, host: Annotated[str, typer.Option("--host", help="Host to bind to.")] = DEFAULT_LOCAL_SERVICES_BIND_HOST, port: Annotated[int, typer.Option("--port", help="Port to bind to.")] = _DEFAULT_PORT, + keep_alive_timeout_seconds: Annotated[ + int, + typer.Option( + "--keep-alive-timeout-seconds", + min=1, + help="Seconds Uvicorn keeps idle HTTP connections open.", + ), + ] = DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, instance: Annotated[ str | None, typer.Option( @@ -358,6 +379,7 @@ def start_services( scope=scope, host=host, port=port, + keep_alive_timeout_seconds=keep_alive_timeout_seconds, state_root=base_dir, ) @@ -495,6 +517,14 @@ def restart_services( int | None, typer.Option("--port", help="Port to bind to. Defaults to previous value or 8080."), ] = None, + keep_alive_timeout_seconds: Annotated[ + int | None, + typer.Option( + "--keep-alive-timeout-seconds", + min=1, + help="Seconds Uvicorn keeps idle HTTP connections open. Defaults to the previous value or 5.", + ), + ] = None, instance: Annotated[ str | None, typer.Option( @@ -561,6 +591,15 @@ def restart_services( host if host is not None else (previous_config.host if previous_config else DEFAULT_LOCAL_SERVICES_BIND_HOST) ) effective_port = port if port is not None else (previous_config.port if previous_config else _DEFAULT_PORT) + effective_keep_alive_timeout_seconds = ( + keep_alive_timeout_seconds + if keep_alive_timeout_seconds is not None + else ( + previous_config.keep_alive_timeout_seconds + if previous_config + else DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS + ) + ) _warn_bind_all(effective_host) @@ -575,6 +614,7 @@ def restart_services( scope=scope, host=effective_host, port=effective_port, + keep_alive_timeout_seconds=effective_keep_alive_timeout_seconds, state_root=base_dir, ) diff --git a/sdk/python/nemo-platform/src/nemo_platform/local/process.py b/sdk/python/nemo-platform/src/nemo_platform/local/process.py index f988924142..e6ebb59510 100644 --- a/sdk/python/nemo-platform/src/nemo_platform/local/process.py +++ b/sdk/python/nemo-platform/src/nemo_platform/local/process.py @@ -827,6 +827,7 @@ def start_background( if config.config_path: args += ["--config", config.config_path] args += ["--host", config.host, "--port", str(config.port)] + args += ["--keep-alive-timeout-seconds", str(config.keep_alive_timeout_seconds)] args += ["--instance", config.scope] env = os.environ.copy() diff --git a/sdk/python/nemo-platform/src/nemo_platform/local/services.py b/sdk/python/nemo-platform/src/nemo_platform/local/services.py index bd904c9cfd..9930d331d6 100644 --- a/sdk/python/nemo-platform/src/nemo_platform/local/services.py +++ b/sdk/python/nemo-platform/src/nemo_platform/local/services.py @@ -35,9 +35,11 @@ ) from nmp.platform_runner.config import ( DEFAULT_SCOPE, + DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS, PlatformAppConfig, default_runtime_root, default_state_root, + validate_keep_alive_timeout_seconds, validate_scope, ) @@ -151,6 +153,7 @@ class ServiceRunConfig: data_dir: str | Path | None = None readiness_timeout: float = 60.0 readiness_poll_interval: float = 0.5 + keep_alive_timeout_seconds: int = DEFAULT_UVICORN_KEEP_ALIVE_TIMEOUT_SECONDS mode: ServiceMode | str = ServiceMode.DAEMON def __post_init__(self) -> None: @@ -176,6 +179,7 @@ def __post_init__(self) -> None: raise ValueError("readiness_timeout must be greater than 0") if self.readiness_poll_interval <= 0: raise ValueError("readiness_poll_interval must be greater than 0") + self.keep_alive_timeout_seconds = validate_keep_alive_timeout_seconds(self.keep_alive_timeout_seconds) self.scope = validate_scope(self.scope) @property @@ -215,6 +219,7 @@ def to_platform_app_config(self) -> PlatformAppConfig: socket_path=_optional_str(self.resolved_socket_path), state_root=_optional_str(self.state_root), runtime_root=_optional_str(self.runtime_dir), + keep_alive_timeout_seconds=self.keep_alive_timeout_seconds, ) def to_child_payload(self) -> dict[str, object]: @@ -239,6 +244,7 @@ def to_child_payload(self) -> dict[str, object]: "data_dir": _optional_str(self.data_dir), "readiness_timeout": self.readiness_timeout, "readiness_poll_interval": self.readiness_poll_interval, + "keep_alive_timeout_seconds": self.keep_alive_timeout_seconds, } @@ -486,9 +492,21 @@ def serve_embedded_app(app: Any, cfg: ServiceRunConfig, socket_path: Path | None if socket_path is not None: from nmp.platform_runner.server import _run_server_on_bound_sockets - _run_server_on_bound_sockets(app, host=cfg.host, port=cfg.port, socket_path=str(socket_path)) + _run_server_on_bound_sockets( + app, + host=cfg.host, + port=cfg.port, + socket_path=str(socket_path), + keep_alive_timeout_seconds=cfg.keep_alive_timeout_seconds, + ) else: - uvicorn.run(app, host=cfg.host, port=cfg.port, log_config=None) + uvicorn.run( + app, + host=cfg.host, + port=cfg.port, + log_config=None, + timeout_keep_alive=cfg.keep_alive_timeout_seconds, + ) def run_services( diff --git a/sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_services.py b/sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_services.py index ee18d2a9ee..75008034f8 100644 --- a/sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_services.py +++ b/sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_services.py @@ -78,6 +78,18 @@ def test_services_help_lists_all_commands(): assert cmd in result.stdout, f"'{cmd}' not in help output" +@pytest.mark.parametrize("command", ["run", "start", "restart"]) +@pytest.mark.parametrize("timeout", ["0", "-1"]) +def test_services_reject_non_positive_keep_alive_timeout(command: str, timeout: str): + with patch(f"{_CLI_MODULE}.stop_instance") as mock_stop: + result = runner.invoke(app, ["services", command, "--keep-alive-timeout-seconds", timeout]) + + assert result.exit_code != 0 + assert "--keep-alive-timeout-seconds" in f"{result.stdout}{result.stderr}" + if command == "restart": + mock_stop.assert_not_called() + + # --------------------------------------------------------------------------- # run (foreground) # --------------------------------------------------------------------------- @@ -113,6 +125,8 @@ def test_run_invokes_runner(base_dir: Path): "127.0.0.1", "--port", "9000", + "--keep-alive-timeout-seconds", + "12", "--instance", "test-run", ], @@ -130,6 +144,7 @@ def test_run_invokes_runner(base_dir: Path): assert config.config_path is None assert config.host == "127.0.0.1" assert config.port == 9000 + assert config.keep_alive_timeout_seconds == 12 assert kwargs["on_shutdown"] is not None @@ -208,16 +223,18 @@ def test_start_launches_background(base_dir: Path): with ( patch(f"{_CLI_MODULE}._require_services_extra"), - patch(f"{_CLI_MODULE}.start_background", return_value=mock_proc), + patch(f"{_CLI_MODULE}.start_background", return_value=mock_proc) as mock_start, patch(f"{_CLI_MODULE}._wait_for_healthy", return_value=True), ): result = runner.invoke( app, - ["services", "start", "--instance", "bg-test"], + ["services", "start", "--instance", "bg-test", "--keep-alive-timeout-seconds", "12"], ) assert result.exit_code == 0 assert "99999" in result.stdout + config = mock_start.call_args.args[0] + assert config.keep_alive_timeout_seconds == 12 def test_start_refuses_when_already_running(base_dir: Path): @@ -463,6 +480,7 @@ def test_restart_preserves_previous_args(self, base_dir: Path): controllers=["jobs"], host="127.0.0.1", port=9000, + keep_alive_timeout_seconds=12, ), mode="background", create_time=1.0, @@ -494,6 +512,45 @@ def test_restart_preserves_previous_args(self, base_dir: Path): assert config.controllers == ["jobs"] assert config.host == "127.0.0.1" assert config.port == 9000 + assert config.keep_alive_timeout_seconds == 12 + + def test_restart_overrides_previous_keep_alive_timeout(self, base_dir: Path): + scope = "override-keep-alive-test" + fd = acquire_lock(scope, base_dir=base_dir) + desc = InstanceDescriptor( + pid=os.getpid(), + config=PlatformAppConfig( + scope=scope, + host="127.0.0.1", + port=9000, + keep_alive_timeout_seconds=12, + ), + mode="background", + create_time=1.0, + ) + write_descriptor(desc, base_dir=base_dir) + + mock_proc = MagicMock() + mock_proc.pid = 22223 + mock_proc.poll.return_value = None + + try: + with ( + patch(f"{_CLI_MODULE}._require_services_extra"), + patch(f"{_CLI_MODULE}.stop_instance", return_value=StopResult(stopped_pids=[os.getpid()])), + patch(f"{_CLI_MODULE}.start_background", return_value=mock_proc) as mock_start, + patch(f"{_CLI_MODULE}._wait_for_healthy", return_value=True), + ): + result = runner.invoke( + app, + ["services", "restart", "--instance", scope, "--keep-alive-timeout-seconds", "15"], + ) + finally: + os.close(fd) + + assert result.exit_code == 0 + config = mock_start.call_args.args[0] + assert config.keep_alive_timeout_seconds == 15 # --------------------------------------------------------------------------- diff --git a/sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_services_process.py b/sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_services_process.py index d03f44830a..8b3d324b4a 100644 --- a/sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_services_process.py +++ b/sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_services_process.py @@ -644,6 +644,7 @@ def test_launches_detached_subprocess(self, base_dir: Path) -> None: controllers=["jobs"], host="127.0.0.1", port=8080, + keep_alive_timeout_seconds=12, state_root=base_dir, ), ) @@ -653,6 +654,8 @@ def test_launches_detached_subprocess(self, base_dir: Path) -> None: assert call_kwargs["start_new_session"] is True assert call_kwargs["stdin"] == subprocess.DEVNULL assert call_kwargs["close_fds"] is True + args = mock_popen.call_args.args[0] + assert args[args.index("--keep-alive-timeout-seconds") + 1] == "12" def test_injects_nmp_data_dir_when_unset(self, base_dir: Path, monkeypatch) -> None: monkeypatch.delenv("NMP_DATA_DIR", raising=False) diff --git a/sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/local/test_services.py b/sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/local/test_services.py index a17a2c2bdb..090d4e45c0 100644 --- a/sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/local/test_services.py +++ b/sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/local/test_services.py @@ -55,6 +55,7 @@ def test_service_run_config_converts_to_platform_app_config(tmp_path: Path) -> N sidecars=["adapters"], config_path=tmp_path / "local.yaml", socket_path=tmp_path / "nemo.sock", + keep_alive_timeout_seconds=12, mode="embedded", ) @@ -69,6 +70,12 @@ def test_service_run_config_converts_to_platform_app_config(tmp_path: Path) -> N assert app_config.runtime_dir() == tmp_path assert app_config.host == "127.0.0.1" assert app_config.port == 8080 + assert app_config.keep_alive_timeout_seconds == 12 + + +def test_service_run_config_rejects_non_positive_keep_alive_timeout() -> None: + with pytest.raises(ValueError, match="keep_alive_timeout_seconds must be greater than 0"): + ServiceRunConfig(keep_alive_timeout_seconds=0) def test_instance_descriptor_converts_from_service_run_config( @@ -862,14 +869,46 @@ def test_run_services_serves_embedded_app_with_socket_path(monkeypatch: pytest.M def test_serve_embedded_app_with_socket_path_listens_on_tcp_and_uds(tmp_path: Path) -> None: - cfg = ServiceRunConfig(transport="tcp", host="127.0.0.1", port=9090) + cfg = ServiceRunConfig( + transport="tcp", + host="127.0.0.1", + port=9090, + keep_alive_timeout_seconds=12, + ) app = object() socket_path = tmp_path / "nemo.sock" with patch("nmp.platform_runner.server._run_server_on_bound_sockets") as run_bound_sockets: services.serve_embedded_app(app, cfg, socket_path) - run_bound_sockets.assert_called_once_with(app, host="127.0.0.1", port=9090, socket_path=str(socket_path)) + run_bound_sockets.assert_called_once_with( + app, + host="127.0.0.1", + port=9090, + socket_path=str(socket_path), + keep_alive_timeout_seconds=12, + ) + + +def test_serve_embedded_app_without_socket_path_sets_keep_alive_timeout() -> None: + cfg = ServiceRunConfig( + transport="tcp", + host="127.0.0.1", + port=9090, + keep_alive_timeout_seconds=12, + ) + app = object() + + with patch("uvicorn.run") as uvicorn_run: + services.serve_embedded_app(app, cfg, None) + + uvicorn_run.assert_called_once_with( + app, + host="127.0.0.1", + port=9090, + log_config=None, + timeout_keep_alive=12, + ) def test_run_services_cleans_lock_when_log_path_resolution_fails( diff --git a/tests/auth_idp/static/test_authentik_kubernetes_demo.py b/tests/auth_idp/static/test_authentik_kubernetes_demo.py index 81588ef733..409f042c76 100644 --- a/tests/auth_idp/static/test_authentik_kubernetes_demo.py +++ b/tests/auth_idp/static/test_authentik_kubernetes_demo.py @@ -218,6 +218,17 @@ def test_authentik_tutorial_tests_scoped_access_keys() -> None: assert "unset ACCESS_KEY ACCESS_KEY_CONTEXT INVALID_ACCESS_KEY INVALID_STATUS" in tutorial +def test_authentik_static_ci_prepares_envoy_validation_inputs() -> None: + ci_workflow = Path(".github/workflows/ci.yaml").read_text(encoding="utf-8") + job = _workflow_job_block(ci_workflow, "python-auth-idp-static-test") + + assert "needs.changes.outputs.helm == 'true'" in job + assert "docker pull docker.io/envoyproxy/envoy:v1.37.0" in job + assert "docker pull envoyproxy/envoy:v1.36.2" in job + assert "helm dependency build k8s/helm" in job + assert "helm dependency build contrib/auth/authentik/helm" in job + + def test_authentik_e2e_ci_requires_published_nmp_api_image() -> None: ci_workflow = Path(".github/workflows/ci.yaml").read_text(encoding="utf-8") job = _workflow_job_block(ci_workflow, "python-auth-idp-e2e-test") @@ -739,6 +750,7 @@ def test_authentik_umbrella_values_configure_nemo_envoy_as_the_only_edge_proxy() envoy_config = _load_rendered_authentik_envoy_config() http_manager = envoy_config["static_resources"]["listeners"][0]["filter_chains"][0]["filters"][0]["typed_config"] routes = http_manager["route_config"]["virtual_hosts"][0]["routes"] + clusters = envoy_config["static_resources"]["clusters"] forwarded_proto_header = [ { "header": {"key": "x-forwarded-proto", "value": "https"}, @@ -746,6 +758,13 @@ def test_authentik_umbrella_values_configure_nemo_envoy_as_the_only_edge_proxy() } ] assert envoy["configOverride"] == '{{ include "nemo-platform-authentik.envoyConfig" . }}' + assert envoy["timeouts"]["upstreamIdle"] == "4s" + nemo_cluster = next(cluster for cluster in clusters if cluster["name"] == "nemo") + http_options = nemo_cluster["typed_extension_protocol_options"][ + "envoy.extensions.upstreams.http.v3.HttpProtocolOptions" + ] + assert http_options["common_http_protocol_options"] == {"idle_timeout": "4s"} + assert http_options["explicit_http_config"] == {"http_protocol_options": {}} gateway_ready_route = next(route for route in routes if route["match"] == {"path": "/health/gateway/ready"}) health_route = next(route for route in routes if route["match"] == {"prefix": "/health/"}) assert routes.index(gateway_ready_route) < routes.index(health_route) diff --git a/tests/auth_idp/static/test_envoy_config_validation.py b/tests/auth_idp/static/test_envoy_config_validation.py new file mode 100644 index 0000000000..8318b1586d --- /dev/null +++ b/tests/auth_idp/static/test_envoy_config_validation.py @@ -0,0 +1,146 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +import shutil +import subprocess +from pathlib import Path + +import pytest +import yaml + +pytestmark = [pytest.mark.auth_idp] + +ROOT = Path(__file__).parent.parent.parent.parent +AUTHENTIK_HELM_DIR = ROOT / "contrib" / "auth" / "authentik" / "helm" +AUTHENTIK_GATEWAY_ENVOY_CONFIG = ROOT / "contrib" / "auth" / "authentik" / "gateway" / "envoy.yaml" +AUTHENTIK_UMBRELLA_ENVOY_IMAGE = "docker.io/envoyproxy/envoy:v1.37.0" +AUTHENTIK_STATIC_GATEWAY_ENVOY_IMAGE = "envoyproxy/envoy:v1.36.2" +HELM_TEMPLATE_TIMEOUT_SECONDS = 60 +ENVOY_VALIDATE_TIMEOUT_SECONDS = 60 +TLS_CERT_TIMEOUT_SECONDS = 30 + + +def _require_envoy_validation_image(image: str) -> None: + if shutil.which("docker") is None: + pytest.skip("docker is required to run envoy --mode validate") + + completed = subprocess.run( + ["docker", "image", "inspect", image], + capture_output=True, + check=False, + text=True, + timeout=ENVOY_VALIDATE_TIMEOUT_SECONDS, + ) + if completed.returncode != 0: + pytest.skip(f"{image} is required locally to run envoy --mode validate") + + +def _helm_template(release_name: str, chart_dir: Path, *args: str) -> list[dict]: + if shutil.which("helm") is None: + pytest.skip("helm is required to render Envoy configs") + + completed = subprocess.run( + ["helm", "template", release_name, str(chart_dir), *args], + capture_output=True, + check=False, + text=True, + timeout=HELM_TEMPLATE_TIMEOUT_SECONDS, + ) + assert completed.returncode == 0, completed.stderr + return [document for document in yaml.safe_load_all(completed.stdout) if document] + + +def _envoy_config_from_config_map(documents: list[dict]) -> dict: + config_map = next( + document + for document in documents + if document["kind"] == "ConfigMap" and document["metadata"]["name"] == "nemo-platform-envoy" + ) + return yaml.safe_load(config_map["data"]["envoy.yaml"]) + + +def _write_dummy_tls_certificate(tmp_path: Path) -> Path: + if shutil.which("openssl") is None: + pytest.skip("openssl is required to create TLS material for envoy --mode validate") + + tls_dir = tmp_path / "tls" + tls_dir.mkdir() + + completed = subprocess.run( + [ + "openssl", + "req", + "-x509", + "-newkey", + "rsa:2048", + "-nodes", + "-keyout", + str(tls_dir / "tls.key"), + "-out", + str(tls_dir / "tls.crt"), + "-days", + "1", + "-subj", + "/CN=localhost", + ], + capture_output=True, + check=False, + text=True, + timeout=TLS_CERT_TIMEOUT_SECONDS, + ) + assert completed.returncode == 0, completed.stdout + completed.stderr + + (tls_dir / "tls.crt").chmod(0o644) + (tls_dir / "tls.key").chmod(0o644) + return tls_dir + + +def _validate_envoy_config(config: dict, tmp_path: Path, image: str) -> None: + _require_envoy_validation_image(image) + + config_path = tmp_path / "envoy.yaml" + config_path.write_text(yaml.safe_dump(config), encoding="utf-8") + tls_dir = _write_dummy_tls_certificate(tmp_path) + + completed = subprocess.run( + [ + "docker", + "run", + "--rm", + "-v", + f"{config_path}:/etc/envoy/envoy.yaml:ro", + "-v", + f"{tls_dir}:/etc/envoy/tls:ro", + "-v", + f"{tls_dir}:/etc/nmp/workload-token-tls:ro", + image, + "--mode", + "validate", + "-c", + "/etc/envoy/envoy.yaml", + ], + capture_output=True, + check=False, + text=True, + timeout=ENVOY_VALIDATE_TIMEOUT_SECONDS, + ) + assert completed.returncode == 0, completed.stdout + completed.stderr + + +def test_authentik_umbrella_envoy_config_validates_with_envoy(tmp_path: Path) -> None: + documents = _helm_template( + "authentik-demo", + AUTHENTIK_HELM_DIR, + "-n", + "nemo-authentik", + "--show-only", + "charts/nemo-platform/templates/proxy/envoy-configmap.yaml", + ) + + _validate_envoy_config(_envoy_config_from_config_map(documents), tmp_path, AUTHENTIK_UMBRELLA_ENVOY_IMAGE) + + +def test_authentik_static_gateway_envoy_config_validates_with_envoy(tmp_path: Path) -> None: + config = yaml.safe_load(AUTHENTIK_GATEWAY_ENVOY_CONFIG.read_text(encoding="utf-8")) + + _validate_envoy_config(config, tmp_path, AUTHENTIK_STATIC_GATEWAY_ENVOY_IMAGE) diff --git a/tests/auth_idp/static/test_provider_layout.py b/tests/auth_idp/static/test_provider_layout.py index 688e791f30..62931f898d 100644 --- a/tests/auth_idp/static/test_provider_layout.py +++ b/tests/auth_idp/static/test_provider_layout.py @@ -98,6 +98,7 @@ def test_authentik_compose_uses_liveness_for_container_health_and_routes_status_ http_manager = envoy["static_resources"]["listeners"][0]["filter_chains"][0]["filters"][0]["typed_config"] routes = http_manager["route_config"]["virtual_hosts"][0]["routes"] + clusters = envoy["static_resources"]["clusters"] route_matches = [route["match"] for route in routes if route.get("route", {}).get("cluster") == "nemo"] forwarded_proto_header = [ { @@ -105,6 +106,12 @@ def test_authentik_compose_uses_liveness_for_container_health_and_routes_status_ "append_action": "OVERWRITE_IF_EXISTS_OR_ADD", } ] + nemo_cluster = next(cluster for cluster in clusters if cluster["name"] == "nemo") + http_options = nemo_cluster["typed_extension_protocol_options"][ + "envoy.extensions.upstreams.http.v3.HttpProtocolOptions" + ] + assert http_options["common_http_protocol_options"] == {"idle_timeout": "4s"} + assert http_options["explicit_http_config"] == {"http_protocol_options": {}} gateway_ready_route = next(route for route in routes if route["match"] == {"path": "/health/gateway/ready"}) health_route = next(route for route in routes if route["match"] == {"prefix": "/health/"}) assert routes.index(gateway_ready_route) < routes.index(health_route) diff --git a/tests/unit/test_helm_clickhouse.py b/tests/unit/test_helm_clickhouse.py index 49f6895357..e42d972a25 100644 --- a/tests/unit/test_helm_clickhouse.py +++ b/tests/unit/test_helm_clickhouse.py @@ -27,6 +27,21 @@ def _helm_template(*args: str) -> list[dict]: return [document for document in yaml.safe_load_all(completed.stdout) if document] +def _helm_template_failure(*args: str) -> str: + if shutil.which("helm") is None: + pytest.skip("helm is required to render the NeMo Platform chart") + + completed = subprocess.run( + ["helm", "template", "nemo-platform", str(HELM_DIR), *args], + check=False, + capture_output=True, + text=True, + timeout=HELM_TEMPLATE_TIMEOUT_SECONDS, + ) + assert completed.returncode != 0, completed.stdout + return completed.stderr + + def _clickhouse_resources(documents: list[dict]) -> list[dict]: return [ document @@ -45,6 +60,15 @@ def _api_container(documents: list[dict]) -> dict: return deployment["spec"]["template"]["spec"]["containers"][0] +def _envoy_config(documents: list[dict]) -> dict: + config_map = next( + document + for document in documents + if document["kind"] == "ConfigMap" and document["metadata"]["name"] == "nemo-platform-envoy" + ) + return yaml.safe_load(config_map["data"]["envoy.yaml"]) + + def _env_by_name(container: dict) -> dict[str, dict]: return {env["name"]: env for env in container["env"]} @@ -95,3 +119,42 @@ def test_external_clickhouse_disables_embedded_dependency() -> None: assert env["NMP_INTAKE_CLICKHOUSE_URL"]["value"] == "http://clickhouse.example.internal:8123" password_ref = env["NMP_INTAKE_CLICKHOUSE_PASSWORD"]["valueFrom"]["secretKeyRef"] assert password_ref == {"name": "clickhouse-credentials", "key": "password"} + + +def test_envoy_backend_cluster_uses_short_upstream_idle_timeout() -> None: + documents = _helm_template( + "--set", + "platformConfig.auth.enabled=true", + ) + envoy_config = _envoy_config(documents) + backend_cluster = next( + cluster for cluster in envoy_config["static_resources"]["clusters"] if cluster["name"] == "backend_cluster" + ) + http_options = backend_cluster["typed_extension_protocol_options"][ + "envoy.extensions.upstreams.http.v3.HttpProtocolOptions" + ] + + assert http_options["common_http_protocol_options"] == {"idle_timeout": "4s"} + assert http_options["explicit_http_config"] == {"http_protocol_options": {}} + + +def test_envoy_upstream_idle_must_be_less_than_api_keep_alive() -> None: + stderr = _helm_template_failure( + "--set", + "platformConfig.auth.enabled=true", + "--set", + "api.server.keepAliveTimeoutSeconds=5", + "--set", + "envoyProxy.timeouts.upstreamIdle=5s", + ) + + assert ("envoyProxy.timeouts.upstreamIdle (5s) must be less than api.server.keepAliveTimeoutSeconds (5s)") in stderr + + +def test_api_deployment_passes_keep_alive_timeout_to_services_run() -> None: + documents = _helm_template( + "--set", + "api.server.keepAliveTimeoutSeconds=9", + ) + + assert "--keep-alive-timeout-seconds=9" in _api_container(documents)["args"] diff --git a/tools/lint/lint-helm.sh b/tools/lint/lint-helm.sh index e8c66561e3..9266b6876a 100755 --- a/tools/lint/lint-helm.sh +++ b/tools/lint/lint-helm.sh @@ -4,6 +4,7 @@ set -xeo pipefail HELM_FOLDER=${HELM_FOLDER:-k8s/helm} HELM_RELEASE_NAME=${HELM_RELEASE_NAME:-nemo-platform} +HELM_ENVOY_IMAGE=${HELM_ENVOY_IMAGE:-docker.io/envoyproxy/envoy:v1.37.0} OPENSHIFT_VERSION=${OPENSHIFT_VERSION:-4.1.0} # Cache dir for kubeconform so schemas are downloaded once per run instead of per file @@ -11,16 +12,59 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" PROJECT_ROOT="${CI_PROJECT_DIR:-$(cd "${SCRIPT_DIR}/../.." && pwd)}" KUBECONFORM_CACHE="${KUBECONFORM_CACHE:-${PROJECT_ROOT}/.kubeconform-cache}" mkdir -p "${KUBECONFORM_CACHE}" +lint_tmp=$(mktemp -d) +trap 'rm -rf "${lint_tmp}"' EXIT + +validate_rendered_envoy_config() { + if ! command -v docker >/dev/null 2>&1; then + echo "Docker is unavailable; skipping Envoy config validation" >&2 + return + fi + + docker pull "${HELM_ENVOY_IMAGE}" + + local envoy_config_map="${lint_tmp}/envoy-configmap.yaml" + local envoy_config="${lint_tmp}/envoy.yaml" + + helm template "${HELM_RELEASE_NAME}" "${HELM_FOLDER}" \ + --set platformConfig.auth.enabled=true \ + --show-only templates/proxy/envoy-configmap.yaml \ + > "${envoy_config_map}" + + awk ' + $0 == " envoy.yaml: |" { in_block = 1; next } + in_block && /^ [^[:space:]][^:]*:/ { exit } + in_block { + if ($0 == "") { + print "" + next + } + if (substr($0, 1, 4) != " ") { + print "unexpected indentation while extracting envoy.yaml: " $0 > "/dev/stderr" + exit 1 + } + print substr($0, 5) + } + ' "${envoy_config_map}" > "${envoy_config}" + test -s "${envoy_config}" + + docker run --rm \ + -v "${envoy_config}:/etc/envoy/envoy.yaml:ro" \ + "${HELM_ENVOY_IMAGE}" \ + --mode validate \ + -c /etc/envoy/envoy.yaml +} # Fetch chart dependencies so subchart templates (e.g. postgresql) are available during lint/template helm dependency update "${HELM_FOLDER}" # Lint the Helm chart helm lint --strict "${HELM_FOLDER}" +validate_rendered_envoy_config # StatefulSet volumeClaimTemplates are immutable, so chart metadata must not change them. -postgres_claim_tmp=$(mktemp -d) -trap 'rm -rf "${postgres_claim_tmp}"' EXIT +postgres_claim_tmp="${lint_tmp}/postgres-claim" +mkdir -p "${postgres_claim_tmp}" for version in 1 2; do helm package "${HELM_FOLDER}" \