feat(nemo-agents): serve Fabric agents via Docker and Kubernetes deployments - #979
Merged
tylersbray merged 2 commits intoJul 30, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughFabric deployments now support Fabric-specific configuration paths, server commands, gateway URL rewriting, runtime labels, Docker/Kubernetes wiring, and auth-proxy behavior. Configuration descriptions and deployment tests were updated accordingly. ChangesFabric deployment support
Sequence Diagram(s)sequenceDiagram
participant create_deployment
participant build_deployment_config
participant FabricServer
create_deployment->>build_deployment_config: agent_config and deployment mode
build_deployment_config->>FabricServer: agent.yaml path and server command
FabricServer-->>create_deployment: deployment configuration
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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.
🧹 Nitpick comments (2)
plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py (2)
266-266: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
nat_configtoagent_config.The parameter now accepts either a NAT workflow config or a Fabric spec; the name misleads at every call site.
🤖 Prompt for 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. In `@plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py` at line 266, Rename the deployments backend parameter nat_config to agent_config and update every reference and call site consistently, preserving its support for both NAT workflow configurations and Fabric specs.
324-367: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the env-var name from the constant and factor out the shared materialization shell.
Line 331 hardcodes
$AGENT_CONFIG_YAMLwhile line 327 uses_AGENT_CONFIG_YAML_ENV; renaming the constant silently breaks the container. Both branches also duplicate the samemkdir -p && printf && execshape.♻️ Suggested consolidation
+def _materialize_and_exec(*, config_path: str, yaml_env: str, argv: list[str]) -> list[str]: + return [ + f'mkdir -p "$(dirname "{config_path}")" ' + f'&& printf "%s" "${yaml_env}" > "{config_path}" ' + f"&& exec {' '.join(argv)}" + ]if is_fabric: fabric_args = ["python", *_fabric_server_cli_args(config_path=config_path, port=port)] if mode == "docker": env.append(EnvVar(name=_AGENT_CONFIG_YAML_ENV, value=config_yaml)) command = ["sh", "-c"] - args = [ - f'mkdir -p "$(dirname "{config_path}")" ' - f'&& printf "%s" "$AGENT_CONFIG_YAML" > "{config_path}" ' - f"&& exec {' '.join(fabric_args)}" - ] + args = _materialize_and_exec( + config_path=config_path, yaml_env=_AGENT_CONFIG_YAML_ENV, argv=fabric_args + )🤖 Prompt for 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. In `@plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py` around lines 324 - 367, Update the docker handling in the deployment runner to derive the shell variable reference from _AGENT_CONFIG_YAML_ENV instead of hardcoding AGENT_CONFIG_YAML, so it remains aligned with the EnvVar name. Factor the shared mkdir/printf/exec materialization command used by the fabric and NAT branches into a reusable local helper or shared construction, while preserving each branch’s config path, environment variable, and executable arguments.
🤖 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.
Nitpick comments:
In `@plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py`:
- Line 266: Rename the deployments backend parameter nat_config to agent_config
and update every reference and call site consistently, preserving its support
for both NAT workflow configurations and Fabric specs.
- Around line 324-367: Update the docker handling in the deployment runner to
derive the shell variable reference from _AGENT_CONFIG_YAML_ENV instead of
hardcoding AGENT_CONFIG_YAML, so it remains aligned with the EnvVar name. Factor
the shared mkdir/printf/exec materialization command used by the fabric and NAT
branches into a reusable local helper or shared construction, while preserving
each branch’s config path, environment variable, and executable arguments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f7466eac-6b6e-48ad-95e4-100852a3afcc
📒 Files selected for processing (3)
plugins/nemo-agents/src/nemo_agents_plugin/config.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.pyplugins/nemo-agents/tests/unit/test_runner_deployments.py
Contributor
|
tylersbray
added a commit
that referenced
this pull request
Jul 30, 2026
…onfig materialization Rename build_deployment_config's nat_config to agent_config now that it accepts both NAT workflow configs and Fabric specs, and derive the docker shell config variable from the EnvVar constants via a shared materialize-and-exec helper so the two branches cannot drift. Addresses review feedback on #979. Signed-off-by: Tyler Bray <tbray@nvidia.com>
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 `@plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.py`:
- Around line 227-234: Update _materialize_config_and_exec to shell-escape every
dynamic value with shlex.quote before constructing the command: quote
config_path consistently in both mkdir/printf usage and quote each argv element
before joining for exec. Preserve the existing command sequence and YAML
environment expansion behavior while ensuring paths and arguments containing
spaces, quotes, or shell metacharacters are passed literally.
🪄 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: 26b3234e-b78f-4f20-a220-7d923b21a182
📒 Files selected for processing (2)
plugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.pyplugins/nemo-agents/tests/unit/test_runner_deployments.py
mmogallapalli
approved these changes
Jul 30, 2026
mmogallapalli
left a comment
mmogallapalli
left a comment
Contributor
There was a problem hiding this comment.
LGTM, thanks for taking this on. I’ll handle the packaging/dependency wiring in a follow-up PR shortly.
…oyments Branch DeploymentsRunnerBackend compile/rewrite on nemo-agents-spec-v1 so container modes start the Fabric FastAPI shim with agent.yaml, while leaving NAT nat start fastapi paths unchanged. Signed-off-by: Tyler Bray <tbray@nvidia.com>
…onfig materialization Rename build_deployment_config's nat_config to agent_config now that it accepts both NAT workflow configs and Fabric specs, and derive the docker shell config variable from the EnvVar constants via a shared materialize-and-exec helper so the two branches cannot drift. Addresses review feedback on #979. Signed-off-by: Tyler Bray <tbray@nvidia.com>
tylersbray
force-pushed
the
AIRCORE-947-fabric-agent-serving-docker-k8s/tbray
branch
from
July 30, 2026 19:58
e337834 to
5b8f8bd
Compare
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
2 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
DeploymentsRunnerBackendsonemo-agents-spec-v1agents compile to the Platform Fabric FastAPI shim (python -m nemo_agents_plugin.fabric.server) for Docker and Kubernetes, withagent.yamlinjection/mount and/healthreadiness.rewrite_fabric_config_base_urls()so Fabric model/harness IGW URLs are rebased for container reachability (including auth-proxy), parallel to the existing NAT rewriter.nat start fastapicontainer behavior unchanged; add unit coverage for Fabric docker/k8s compile, rewrite, create, and auth-proxy paths.Closes AIRCORE-947.
Test plan
uv run --frozen pytest plugins/nemo-agents/tests/unit/test_runner_deployments.py -v(44 passed)ty check/ pre-commit hooks on commitSummary by CodeRabbit
base_urlvalues to the inference gateway host when applicable.base_urlrewriting through an auth proxy.