fix: set envoy upstream idle timeout - #1101
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds configurable API keep-alive timeouts and Envoy upstream idle timeouts. CLI, embedded services, platform runner, and Helm deployments propagate the settings. Helm and CI tests validate timeout ordering and rendered Envoy configurations. ChangesConfigurable keep-alive timeouts
Helm and Envoy timeout configuration
Sequence Diagram(s)sequenceDiagram
participant CLI
participant PlatformAppConfig
participant run_platform
participant Uvicorn
CLI->>PlatformAppConfig: keep_alive_timeout_seconds
PlatformAppConfig->>run_platform: resolved timeout
run_platform->>Uvicorn: timeout_keep_alive
sequenceDiagram
participant HelmValues
participant HelmValidation
participant EnvoyConfigMap
participant Envoy
HelmValues->>HelmValidation: API and upstream idle timeouts
HelmValidation->>EnvoyConfigMap: validated timeout configuration
EnvoyConfigMap->>Envoy: rendered Envoy configuration
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/auth_idp/conftest.py`:
- Around line 334-352: The workspace creation flow around sdk.workspaces.create
must pass exist_ok=True so a committed request followed by a 409 ConflictError
is treated as successful and cleanup can proceed. Keep retrying only the
intended transient Envoy failure, but match the complete error signature rather
than checking solely for “connection termination”; preserve immediate
propagation for other errors and exhausted attempts.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dd2d2b03-841a-45fb-9e53-ed70774f9ad7
📒 Files selected for processing (1)
tests/auth_idp/conftest.py
Contributor
|
ironcommit
force-pushed
the
fix/ci-job-92312717122
branch
from
August 7, 2026 00:07
3fb4f53 to
0bd9a73
Compare
ironcommit
changed the title
ironcommit
changed the title
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
ironcommit
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@k8s/helm/values.yaml`:
- Around line 1050-1051: Set the Envoy upstream idle timeout to 4s in both
k8s/helm/values.yaml: envoyProxy.timeouts.upstreamIdle, and
contrib/auth/authentik/gateway/envoy.yaml: the static nemo cluster value. Keep
the timeout below Uvicorn’s 5-second keep-alive setting to prevent stale pooled
connections.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5821541f-7646-464e-912a-2f5ecbb229b2
📒 Files selected for processing (8)
contrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/helm/templates/_envoy-config.tplk8s/helm/README.mdk8s/helm/templates/proxy/envoy-configmap.yamlk8s/helm/values.yamltests/auth_idp/static/test_authentik_kubernetes_demo.pytests/auth_idp/static/test_provider_layout.pytests/unit/test_helm_clickhouse.py
ironcommit
force-pushed
the
fix/ci-job-92312717122
branch
from
August 7, 2026 00:48
0bd9a73 to
8547381
Compare
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
mckornfield
approved these changes
Aug 7, 2026
ironcommit
enabled auto-merge
August 7, 2026 14:57
ironcommit
force-pushed
the
fix/ci-job-92312717122
branch
from
August 7, 2026 15:50
8547381 to
1027720
Compare
Contributor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/services/cli.py`:
- Around line 518-524: Reject non-positive timeout values during CLI parsing for
all three options in
packages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/services/cli.py:214-220,
332-338, and 518-524. Add a minimum-value constraint of 1 or reuse a shared
Typer validator for the timeout option declarations, including
keep_alive_timeout_seconds, so invalid values are rejected before
restart_services calls stop_instance().
In `@packages/nmp_platform_runner/src/nmp/platform_runner/config.py`:
- Around line 115-119: Update validate_keep_alive_timeout_seconds to reject bool
and non-int values before checking positivity, while continuing to accept only
positive integers. Add coverage for -1, 1.5, and True, preserving the existing
ValueError behavior for invalid inputs.
In `@tests/unit/test_envoy_config_validation.py`:
- Line 15: Update _validate_envoy_config to accept the expected Envoy image as a
parameter, or introduce separate image constants, so Helm validation uses
v1.37.0 while static Compose gateway validation uses v1.36.2. Ensure each test
passes the image corresponding to its deployment.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 95bae833-71d4-4f23-96c5-265bbeaae52c
⛔ Files ignored due to path filters (6)
sdk/python/nemo-platform/src/nemo_platform/cli/commands/services/cli.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/local/process.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/local/services.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_services.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/cli/commands/test_services_process.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/vendored/nemo_platform_ext/local/test_services.pyis excluded by!sdk/**
📒 Files selected for processing (25)
contrib/auth/authentik/gateway/envoy.yamlcontrib/auth/authentik/helm/templates/_envoy-config.tplcontrib/auth/authentik/helm/values.yamldocs/cli/reference.mdxk8s/helm/README.mdk8s/helm/templates/_helpers.tplk8s/helm/templates/api/api-deployment.yamlk8s/helm/templates/proxy/envoy-configmap.yamlk8s/helm/values.yamlpackages/nemo_platform_ext/src/nemo_platform_ext/cli/commands/services/cli.pypackages/nemo_platform_ext/src/nemo_platform_ext/local/process.pypackages/nemo_platform_ext/src/nemo_platform_ext/local/services.pypackages/nemo_platform_ext/tests/cli/commands/test_services.pypackages/nemo_platform_ext/tests/cli/commands/test_services_process.pypackages/nemo_platform_ext/tests/local/test_services.pypackages/nmp_platform_runner/src/nmp/platform_runner/config.pypackages/nmp_platform_runner/src/nmp/platform_runner/run.pypackages/nmp_platform_runner/src/nmp/platform_runner/server.pypackages/nmp_platform_runner/tests/test_config.pypackages/nmp_platform_runner/tests/test_run.pypackages/nmp_platform_runner/tests/test_server.pytests/auth_idp/static/test_authentik_kubernetes_demo.pytests/auth_idp/static/test_provider_layout.pytests/unit/test_envoy_config_validation.pytests/unit/test_helm_clickhouse.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/auth_idp/static/test_authentik_kubernetes_demo.py
- tests/auth_idp/static/test_provider_layout.py
- contrib/auth/authentik/helm/templates/_envoy-config.tpl
ironcommit
force-pushed
the
fix/ci-job-92312717122
branch
from
August 7, 2026 17:03
1027720 to
de351af
Compare
ironcommit
force-pushed
the
fix/ci-job-92312717122
branch
3 times, most recently
from
August 7, 2026 20:50
1ccaf72 to
9387711
Compare
ironcommit
changed the title
ironcommit
force-pushed
the
fix/ci-job-92312717122
branch
from
August 7, 2026 21:52
9387711 to
8601723
Compare
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>
ironcommit
force-pushed
the
fix/ci-job-92312717122
branch
from
August 7, 2026 22:42
8601723 to
5faf1fb
Compare
svvarom
approved these changes
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This centralizes the connection keep-alive fix in this PR. Before this change, Envoy could keep pooled upstream HTTP/1 connections around longer than the Uvicorn API kept idle sockets open, which allowed intermittent reuse of a closed backend connection. This PR makes the API keep-alive timeout explicit, sets Envoy's upstream idle timeout below it, and validates the rendered Envoy configs in chart-aware CI instead of general unit tests.
Changes
keep_alive_timeout_secondsplatform runner setting with validation and propagates it through direct, socket-bound, reload, embedded, and background Uvicorn startup paths.--keep-alive-timeout-secondstonemo services run,start, andrestart, preserving the previous value on restart unless explicitly overridden.4supstream idle timeout against the5sAPI keep-alive default.v1.37.0) and the static Compose gateway (v1.36.2).Type of Change
Quality Gates
Verification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified belowTargeted validation:
uv run --frozen pytest tests/auth_idp/static -v— 101 passeduv run --frozen pytest tests/unit/test_helm_clickhouse.py -v— 6 passeduv run --frozen pytest tests/auth_idp/static/test_envoy_config_validation.py tests/auth_idp/static/test_authentik_kubernetes_demo.py::test_authentik_static_ci_prepares_envoy_validation_inputs -v— 3 passedtools/lint/lint-helm.sh— passed, including rendered Envoy validationuv run --frozen ruff check tests/auth_idp/static/test_envoy_config_validation.py tests/auth_idp/static/test_authentik_kubernetes_demo.py— passeduv run --frozen ruff format --check tests/auth_idp/static/test_envoy_config_validation.py tests/auth_idp/static/test_authentik_kubernetes_demo.py— passedgit diff --check— passeduv run pre-commit run -a— passedCI spot-check after the latest push:
Python auth-idp static tests— passed ond065da0626fd950b2ed627c86a6e9b7f8421a0acHelm lint— passed ond065da0626fd950b2ed627c86a6e9b7f8421a0acHelm chart verifier— passed ond065da0626fd950b2ed627c86a6e9b7f8421a0ac