Skip to content

fix(router): honor Kubernetes worker namespace suffix - #12160

Merged
ishandhanani merged 3 commits into
ai-dynamo:mainfrom
cpakkamisaac-sae:codex/fix-router-k8s-worker-namespace
Jul 30, 2026
Merged

fix(router): honor Kubernetes worker namespace suffix#12160
ishandhanani merged 3 commits into
ai-dynamo:mainfrom
cpakkamisaac-sae:codex/fix-router-k8s-worker-namespace

Conversation

@cpakkamisaac-sae

@cpakkamisaac-sae cpakkamisaac-sae commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Overview:

Fix standalone router worker-endpoint resolution for Kubernetes deployments that publish worker-class components in a suffixed Dynamo namespace during rolling-update-aware DGD rendering.

Details:

  • Resolve the router's target worker endpoint with get_worker_namespace() so DYN_NAMESPACE_WORKER_SUFFIX is included when the operator injects it into worker-class router pods.
  • Preserve config.namespace as the base DYN_NAMESPACE, so the standalone router continues to serve its own endpoints in the DGD namespace while discovering backend workers in the suffixed worker namespace.
  • Add isolated router config tests covering default endpoint parsing, DYN_NAMESPACE, and DYN_NAMESPACE_WORKER_SUFFIX.
  • Add the router pytest marker to the new router test module.

Validation:

  • PYTHONPATH=components/src UV_CACHE_DIR=/tmp/codex-uv-cache uv run --no-project --with pytest --with pytest-asyncio python -m pytest -c /dev/null components/src/dynamo/router/tests/test_args.py components/src/dynamo/router/tests/test_standalone_router.py
  • Validated in AKS with aks-dynamo-thunderagent in resource group cpakkamisaac-sae-msft-east (eastus) using a Standard_NC4as_T4_v3 GPU node pool and model Qwen/Qwen3-0.6B.
  • Kubernetes runtime validation used Ashna's demo runtime image with an in-container startup patch equivalent to this PR change; no PR-specific container image was built.
  • Reproduced the namespace mismatch on the earlier dynamo-platform-1.2.1 chart: the router watched dynamo-system-thunderagent-demo.backend.generate while the vLLM worker published under dynamo-system-thunderagent-demo-588c6644.backend.generate; the second same-session ThunderAgent request returned HTTP 500 with no routable endpoints.
  • Validated the fix behavior on dynamo-platform-1.2.1: router discovery watched dynamo-system-thunderagent-demo-03302af8.backend.generate, the DGD became Ready, and first plus second same-session /v1/chat/completions calls returned HTTP 200.
  • Upgraded the same AKS cluster to latest Dynamo release v1.3.0 / chart dynamo-platform-1.3.0, applying the large Grove/KAI CRDs with server-side apply where needed. The DGD became Ready, router logs showed discovery against dynamo-system-thunderagent-demo-03302af8.backend.generate, /v1/models returned HTTP 200, and both first plus second same-session /v1/chat/completions calls returned HTTP 200.
  • v1.3.0 still does not list router as a valid DynamoComponentDeployment.spec.type enum value (frontend, worker, prefill, decode, planner, epp are valid), so the demo manifest still needs ThunderAgentRouter represented as componentType: worker. That is separate from the router worker-namespace bug fixed here.

Where should the reviewer start?

Start with components/src/dynamo/router/args.py, specifically DynamoRouterConfig.validate(), then review components/src/dynamo/router/tests/test_args.py for the namespace-resolution cases.

Related Issues

🔗 This PR is linked to an issue:

@cpakkamisaac-sae
cpakkamisaac-sae requested a review from a team as a code owner July 25, 2026 03:41
@copy-pr-bot

copy-pr-bot Bot commented Jul 25, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

Copy link
Copy Markdown
Contributor

👋 Hi cpakkamisaac-sae! Thank you for contributing to ai-dynamo/dynamo.

Just a reminder: The NVIDIA Test Github Validation CI runs an essential subset of the testing framework to quickly catch errors.Your PR reviewers may elect to test the changes comprehensively before approving your changes.

🚀

@github-actions github-actions Bot added external-contribution Pull request is from an external contributor fix router Relates to routing, KV-aware routing, etc. labels Jul 25, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread components/src/dynamo/router/args.py
Comment thread components/src/dynamo/router/args.py
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

Router namespace resolution

Layer / File(s) Summary
Namespace resolution during validation
components/src/dynamo/router/args.py
DynamoRouterConfig.validate reads DYN_NAMESPACE, resolves the worker namespace, rewrites the endpoint when needed, and logs the update.
Namespace resolution test coverage
components/src/dynamo/router/tests/test_args.py
Adds isolated configuration stubs and tests for default namespaces, configured namespaces, worker suffixes, endpoint rewriting, and log messages.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly matches the namespace-suffix router fix and is concise.
Description check ✅ Passed The description covers overview, details, reviewer start point, and links the PR to an issue.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@components/src/dynamo/router/tests/test_args.py`:
- Line 12: Update the module-level pytestmark declaration in test_args.py to
include the required pytest.mark.router marker alongside the existing pre_merge,
unit, and gpu_0 markers.
🪄 Autofix (Beta)

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: 13c3c42d-b50b-4a72-9e0f-59a1b64aed5b

📥 Commits

Reviewing files that changed from the base of the PR and between daf44c6 and 0dd7e60.

📒 Files selected for processing (2)
  • components/src/dynamo/router/args.py
  • components/src/dynamo/router/tests/test_args.py

Comment thread components/src/dynamo/router/tests/test_args.py Outdated
@cpakkamisaac-sae
cpakkamisaac-sae temporarily deployed to external_collaborator July 25, 2026 04:07 — with GitHub Actions Inactive
@cpakkamisaac-sae

cpakkamisaac-sae commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Hi @ishandhanani, could you please review this PR when you have a chance? It fixes the namespace mismatch described in #12016 by honoring the Kubernetes worker-namespace suffix for router endpoints used by the ThunderAgent demo. I added unit coverage and validated the fix on AKS against Dynamo Platform v1.3.0.

@ishandhanani ishandhanani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nits. Pls ping me on slack when this is ready

Comment thread components/src/dynamo/router/args.py Outdated
Comment thread components/src/dynamo/router/tests/test_args.py Outdated
Comment thread components/src/dynamo/router/args.py Outdated
@cpakkamisaac-sae

Copy link
Copy Markdown
Contributor Author

@ishandhanani updated based on your recommendations.

Changes made:

  • Removed the extra namespace-resolution log and logging import.
  • Removed the added router/tests/test_args.py file.
  • Kept the runtime namespace fix minimal in components/src/dynamo/router/args.py.

I also revalidated the behavior end-to-end on AKS with the ThunderAgent deployment from #12016: the router resolved the suffixed worker namespace, discovered the vLLM worker, and both /v1/models and /v1/chat/completions returned successfully.

@ishandhanani
ishandhanani enabled auto-merge (squash) July 29, 2026 18:09
@ishandhanani

Copy link
Copy Markdown
Contributor

/ok-to-test 452b571

@datadog-official

This comment has been minimized.

Signed-off-by: cpakkamisaac-sae <cpakkamisaac@nvidia.com>
Signed-off-by: cpakkamisaac-sae <cpakkamisaac@nvidia.com>
Signed-off-by: cpakkamisaac-sae <cpakkamisaac@nvidia.com>
auto-merge was automatically disabled July 30, 2026 04:20

Head branch was pushed to by a user without write access

@cpakkamisaac-sae
cpakkamisaac-sae force-pushed the codex/fix-router-k8s-worker-namespace branch from 452b571 to 9a53ad2 Compare July 30, 2026 04:20
@cpakkamisaac-sae
cpakkamisaac-sae temporarily deployed to external_collaborator July 30, 2026 04:20 — with GitHub Actions Inactive
@ishandhanani

Copy link
Copy Markdown
Contributor

/ok-to-test 9a53ad2

@ishandhanani
ishandhanani enabled auto-merge (squash) July 30, 2026 17:43
@ishandhanani
ishandhanani merged commit bd0bf84 into ai-dynamo:main Jul 30, 2026
100 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external-contribution Pull request is from an external contributor fix router Relates to routing, KV-aware routing, etc. size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants