feat(nemo-agents): support nemo agents pacakge for fabric backed agents - #1036
Conversation
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
|
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPackaging detects NAT and Fabric configurations and routes them through separate validation, rendering, and image-building paths. Fabric support adds artifact checks, dedicated Dockerfiles, Platform gateway credential handling, deployment environment propagation, and regression coverage. ChangesAgent packaging and Fabric runtime
Sequence Diagram(s)sequenceDiagram
participant CLI
participant FormatDetector
participant FabricBuilder
participant FabricValidator
participant Docker
CLI->>FormatDetector: detect configuration format
FormatDetector-->>CLI: Fabric format
CLI->>FabricBuilder: build_fabric_agent_image
FabricBuilder->>FabricValidator: validate and plan package
FabricValidator-->>FabricBuilder: validation result
FabricBuilder->>Docker: render and build image
Possibly related PRs
Suggested labels: 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
plugins/nemo-agents/tests/unit/test_container.py (2)
381-395: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix the stale docstring and render through
_render_context.Line 382 states the renderer is not wired yet.
TestRenderFabricDockerfilebelow wires it in this same PR. Also,asdict(params)bypasses_render_context, so the flattening path the production renderers use is not exercised here.♻️ Proposed change
class TestFabricDockerfileTemplate: - """Direct contract tests for the Fabric template before its renderer is wired.""" + """Direct contract tests for the Fabric Dockerfile template.""" `@staticmethod` def _render(**overrides: object) -> str: - from dataclasses import asdict - from nemo_agents_plugin.container.template import ( FABRIC_DOCKERFILE_TEMPLATE, FabricRenderParams, _jinja_env, + _render_context, ) params = FabricRenderParams(contract_version="1.2.3", **overrides) - return _jinja_env().from_string(FABRIC_DOCKERFILE_TEMPLATE).render(**asdict(params)) + return _jinja_env().from_string(FABRIC_DOCKERFILE_TEMPLATE).render(**_render_context(params))🤖 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/tests/unit/test_container.py` around lines 381 - 395, Update TestFabricDockerfileTemplate’s docstring to reflect that the Fabric renderer is wired, and change its _render helper to render using _render_context with the FabricRenderParams instance instead of converting params via asdict. Preserve the existing overrides and template rendering behavior while exercising the production flattening path.
535-554: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd Fabric non-root hardening coverage.
Every Fabric test either sets
allow_root=Trueor does not assert the user block.TestRenderNatDockerfile.test_non_root_user_by_defaultlocks the uid-1000 reclaim guards for NAT. The Fabric template carries the same block at template lines 275-281 with no equivalent test. A regression there ships a root-running agent image.Add a test that renders Fabric with default
allow_rootand assertsUSER agent,getent passwd 1000, andchown -R agent:agent /workspace.🤖 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/tests/unit/test_container.py` around lines 535 - 554, Add a Fabric Dockerfile test alongside test_allow_root_and_sandbox_profile that calls render_fabric_dockerfile with the default allow_root setting, then asserts the rendered output contains USER agent, getent passwd 1000, and chown -R agent:agent /workspace. Keep the existing root-enabled test unchanged.plugins/nemo-agents/src/nemo_agents_plugin/container/builder.py (1)
354-378: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared render-build-cleanup block.
Lines 354-378 duplicate lines 208-248 of
build_nat_agent_imageexactly. The safety comments that explain the.dockerignorepre-existence guard exist only in the NAT copy. A future fix to one copy will miss the other.Extract a helper such as
_build_with_generated_dockerfile(context_dir, content, tag, build_args, generate_ignore, platforms, push)and call it from both builders.🤖 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/container/builder.py` around lines 354 - 378, Extract the duplicated generated-Dockerfile render/build/cleanup flow from build_nat_agent_image and the corresponding builder block into a shared helper such as _build_with_generated_dockerfile. Move the Dockerfile refusal check and .dockerignore pre-existence cleanup guard into that helper, then update both builders to call it while preserving their existing arguments and return behavior.plugins/nemo-agents/src/nemo_agents_plugin/container/fabric_validator.py (1)
10-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
FabricConfignormally instead of underTYPE_CHECKING.
nemo_fabricis already a runtime dependency of this package (nemo_agents_plugin.fabric.translatorimports it at module level). The guarded import gives no benefit here and blocks runtime introspection ofFabricPackageValidationResult.♻️ Proposed change
-from typing import TYPE_CHECKING, Any +from typing import Any +from nemo_fabric import FabricConfig from nemo_agents_plugin.agent_config import AgentConfig, AgentConfigLoadError, load_agent_config from nemo_agents_plugin.fabric.translator import FabricTranslationError, translate_agent_config from nemo_agents_plugin.fabric.validation import FabricValidationError, plan_fabric_config - -if TYPE_CHECKING: - from nemo_fabric import FabricConfigAs per coding guidelines: "In Python code, prefer concrete type hints over string-based type hints, and do not import those types only under
TYPE_CHECKING; import them normally when possible."🤖 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/container/fabric_validator.py` around lines 10 - 17, Import FabricConfig directly from nemo_fabric in fabric_validator.py instead of guarding it with TYPE_CHECKING, and remove the now-unused TYPE_CHECKING import. Preserve the existing FabricConfig annotations and runtime introspection of FabricPackageValidationResult.Source: Coding guidelines
plugins/nemo-agents/src/nemo_agents_plugin/fabric/validation.py (1)
91-108: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider matching
doctor_fabric_configerror handling inplan_fabric_config.
plan_fabric_configcatches onlyFabricConfigErrorand applies no timeout.doctor_fabric_configcatches all exceptions and bounds runtime withFABRIC_VALIDATION_TIMEOUT_SECONDS. The packaging path incontainer/fabric_validator.pywraps onlyFabricValidationError, so any other Fabric failure escapes as a raw traceback duringnemo agents package. A blockingplancall in a worker thread also cannot be cancelled.♻️ Proposed change
try: - return await asyncio.to_thread(fabric_client.plan, fabric_config, base_dir=base_dir) + return await asyncio.wait_for( + asyncio.to_thread(fabric_client.plan, fabric_config, base_dir=base_dir), + timeout=FABRIC_VALIDATION_TIMEOUT_SECONDS, + ) + except asyncio.TimeoutError as error: + raise FabricValidationError(f"Fabric plan timed out after {FABRIC_VALIDATION_TIMEOUT_SECONDS:g}s.") from error except FabricConfigError as error: raise FabricValidationError(f"Fabric plan failed: {error}") from error🤖 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/fabric/validation.py` around lines 91 - 108, Update plan_fabric_config to mirror doctor_fabric_config: run fabric_client.plan through the existing FABRIC_VALIDATION_TIMEOUT_SECONDS timeout mechanism and catch all exceptions, converting them into FabricValidationError with the original exception chained. Preserve the current planning arguments and injected fabric client behavior.plugins/nemo-agents/tests/unit/test_fabric_gateway_credentials.py (1)
56-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the legacy
settings.base_urlbranch.
_is_platform_gateway_modelalso matches the gateway marker insidemodel["settings"]["base_url"]. No test exercises that path.💚 Proposed test
+def test_legacy_settings_base_url_receives_runtime_only_binding() -> None: + config = {"models": {"default": {"provider": "nvidia", "model": "m", "settings": {"base_url": _IGW_URL}}}} + + assert platform_gateway_credential_env(config) == {PLATFORM_IGW_API_KEY_ENV: PLATFORM_IGW_API_KEY_PLACEHOLDER}🤖 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/tests/unit/test_fabric_gateway_credentials.py` around lines 56 - 87, Add a unit test alongside test_translated_igw_model_receives_runtime_only_key_reference that builds a model with the platform gateway URL under model["settings"]["base_url"], invokes bind_platform_gateway_model_credential, and verifies it receives PLATFORM_IGW_API_KEY_ENV while the original model remains without api_key_env. This should cover the legacy settings.base_url branch of _is_platform_gateway_model.
🤖 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/container/builder.py`:
- Around line 381-393: Update detect_agent_config_format to catch OSError and
UnicodeDecodeError from agent_config.read_text, converting them into ValueError
messages consistent with validate_agent_config so the CLI’s existing ValueError
handling reports structured errors without a traceback. Preserve the existing
YAML parsing and format validation behavior.
In `@plugins/nemo-agents/src/nemo_agents_plugin/container/template.py`:
- Around line 232-247: The Fabric install flow must not pin nemo-platform to the
unresolved "0.0.0" fallback from get_contract_version(). Validate the resolved
contract version before rendering the install commands in the template, and fail
with an explicit message when it is unavailable; otherwise preserve the
exact-version pin for valid versions in both has_pyproject branches.
- Around line 216-222: Update the Nemo Relay installer block to fetch the script
from an immutable tag or commit matching PINNED_NEMO_RELAY_CLI_VERSION, verify
the downloaded script against a pinned checksum before execution, and only then
run it as root; keep the cleanup and version validation steps intact and correct
the adjacent comment if its guarantee is not fully accurate.
---
Nitpick comments:
In `@plugins/nemo-agents/src/nemo_agents_plugin/container/builder.py`:
- Around line 354-378: Extract the duplicated generated-Dockerfile
render/build/cleanup flow from build_nat_agent_image and the corresponding
builder block into a shared helper such as _build_with_generated_dockerfile.
Move the Dockerfile refusal check and .dockerignore pre-existence cleanup guard
into that helper, then update both builders to call it while preserving their
existing arguments and return behavior.
In `@plugins/nemo-agents/src/nemo_agents_plugin/container/fabric_validator.py`:
- Around line 10-17: Import FabricConfig directly from nemo_fabric in
fabric_validator.py instead of guarding it with TYPE_CHECKING, and remove the
now-unused TYPE_CHECKING import. Preserve the existing FabricConfig annotations
and runtime introspection of FabricPackageValidationResult.
In `@plugins/nemo-agents/src/nemo_agents_plugin/fabric/validation.py`:
- Around line 91-108: Update plan_fabric_config to mirror doctor_fabric_config:
run fabric_client.plan through the existing FABRIC_VALIDATION_TIMEOUT_SECONDS
timeout mechanism and catch all exceptions, converting them into
FabricValidationError with the original exception chained. Preserve the current
planning arguments and injected fabric client behavior.
In `@plugins/nemo-agents/tests/unit/test_container.py`:
- Around line 381-395: Update TestFabricDockerfileTemplate’s docstring to
reflect that the Fabric renderer is wired, and change its _render helper to
render using _render_context with the FabricRenderParams instance instead of
converting params via asdict. Preserve the existing overrides and template
rendering behavior while exercising the production flattening path.
- Around line 535-554: Add a Fabric Dockerfile test alongside
test_allow_root_and_sandbox_profile that calls render_fabric_dockerfile with the
default allow_root setting, then asserts the rendered output contains USER
agent, getent passwd 1000, and chown -R agent:agent /workspace. Keep the
existing root-enabled test unchanged.
In `@plugins/nemo-agents/tests/unit/test_fabric_gateway_credentials.py`:
- Around line 56-87: Add a unit test alongside
test_translated_igw_model_receives_runtime_only_key_reference that builds a
model with the platform gateway URL under model["settings"]["base_url"], invokes
bind_platform_gateway_model_credential, and verifies it receives
PLATFORM_IGW_API_KEY_ENV while the original model remains without api_key_env.
This should cover the legacy settings.base_url branch of
_is_platform_gateway_model.
🪄 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: c82940fe-eadf-42de-99ec-57ef1cc8fc4a
📒 Files selected for processing (18)
plugins/nemo-agents/src/nemo_agents_plugin/cli.pyplugins/nemo-agents/src/nemo_agents_plugin/container/builder.pyplugins/nemo-agents/src/nemo_agents_plugin/container/fabric_validator.pyplugins/nemo-agents/src/nemo_agents_plugin/container/metadata.pyplugins/nemo-agents/src/nemo_agents_plugin/container/template.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/gateway_credentials.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/translator.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/validation.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/in_memory.pyplugins/nemo-agents/tests/unit/test_container.pyplugins/nemo-agents/tests/unit/test_fabric_gateway_credentials.pyplugins/nemo-agents/tests/unit/test_fabric_package_validation.pyplugins/nemo-agents/tests/unit/test_fabric_server.pyplugins/nemo-agents/tests/unit/test_fabric_translator.pyplugins/nemo-agents/tests/unit/test_fabric_validation.pyplugins/nemo-agents/tests/unit/test_runner_deployments.pyplugins/nemo-agents/tests/unit/test_runner_in_memory.py
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Summary
This PR extends
nemo agents packageto build Docker images for Fabric-backed NeMo Agents as part of AIRCORE-970. It also includes the runtime-only inference-gateway credential binding needed by Fabric adapters from AIRCORE-943.The package command now detects the agent configuration format and routes to a runtime-specific packaging path while retaining the existing NAT workflow:
The generated Fabric image contains the agent bundle, the release-matched NeMo Agents runtime and supported Fabric adapters, the pinned NeMo Relay CLI, and the Platform-owned Fabric server used by Docker and Kubernetes deployments.
Changes
nemo agents packageto detectconfig_formatbefore rendering or building:nemo-agents-spec-v1routes to Fabric packaging.nat-workflow-v1and configs withoutconfig_formatroute to NAT packaging.build_nat_agent_imagebuild_fabric_agent_imageagent.yamlschema.FabricConfig.Fabric.planagainst the agent config directory.SKILL.md./workspaceso paths remain relative toagent.yaml.nemo-platform[nemo-agents-plugin]package.--pyprojectis provided./workspace/.venvas the runtime environment.8000and startsnemo_agents_plugin.fabric.server.NAT_BASE_IMAGE_URL->NEMO_AGENTS_BASE_IMAGE_URLNAT_BASE_IMAGE_TAG->NEMO_AGENTS_BASE_IMAGE_TAGNAT_PYTHON_VERSION->NEMO_AGENTS_PYTHON_VERSIONNAT_UV_VERSION->NEMO_AGENTS_UV_VERSIONNAT_VERSIONas NAT-only configuration and reject an explicit--nat-versionfor Fabric agents.0.9.14, matching the repository's supported uv constraint.com.nemo.agent.framework=nemo_platform_agent.nemo-platformdistribution version as the runtime contract version.Design Choices
The CLI owns format routing
nemo agents packagedetects the configuration format before calling a builder. Each builder therefore receives only the parameters supported by its runtime.This keeps
build_nat_agent_imagefree of Fabric branches and keepsbuild_fabric_agent_imagefree of NAT-only inputs such asnat_versionandNAT_CONFIG_FILE.NAT and Fabric have separate builders and render contracts
The two image types share infrastructure such as Docker invocation, build-context selection, OCI metadata, sandbox setup, and generated-file cleanup. Their runtime dependencies, validation, environment variables, ports, and entrypoints remain separate.
This avoids a compatibility alias for the old internal
build_agent_imagehelper. The CLI was its only production caller, so the call sites and tests were updated directly.Packaging runs Fabric plan, not doctor
Fabric.planvalidates the translated configuration and selected adapter contract without requiring the producer machine to contain the binaries that will be installed in the image.Fabric.doctorremains part of runtime/deployment validation, where the complete image environment, adapter binaries, and Relay CLI are available. Running doctor during packaging would incorrectly couple image production to the host environment.The complete build context is the packaged agent bundle
In config-only mode, the directory containing
agent.yamlis the Docker build context. In project mode, the directory containingpyproject.tomlis the build context, andagent.yamlmust be located within it.The image copies that complete context into
/workspace. Referenced skill paths must stay inside the context and remain valid relative to the packagedagent.yaml.Runtime dependencies follow the Platform release contract
Fabric images install
nemo-platform[nemo-agents-plugin]at the same version as the package command that rendered the Dockerfile. The plugin owns the supported Fabric and adapter dependency set, so users do not need to select or version individual adapters while packaging.The Relay CLI is installed separately because Codex and Claude Relay adapters launch it as an external executable. Its version is pinned independently from the Python dependency graph.
Project mode resolves the runtime and project together
When
--pyprojectis supplied, the release-matched NeMo Agents runtime and the user's project are installed in one resolution. Conflicting project constraints therefore fail during the image build instead of producing a partially compatible runtime.Config-only mode installs only the release package and uses the directory containing
agent.yamlas the bundle.Shared environment variables are runtime-neutral
Base image, Python, and uv settings apply to both NAT and Fabric images, so their environment variables now use the
NEMO_AGENTS_prefix.NAT_VERSIONremains unchanged because it is meaningful only for NAT packaging.The CLI distinguishes an explicitly supplied
--nat-versionfrom an ambientNAT_VERSION. An explicit NAT-only flag is rejected for Fabric, while an ambient NAT setting does not interfere with Fabric packaging and remains available to the NAT resolver.Image identity includes the runtime contract
The default image tag remains
<agent-name>-<agent-id>:<agent-version>, but Fabric image identity includes the Platform contract, Relay CLI, base image, Python, and uv inputs. Changing a runtime-defining input therefore produces a distinct agent ID rather than silently reusing an incompatible image identity.Gateway credentials remain runtime-only
Some Fabric model adapters require
api_key_enveven when requests are routed through the authenticated Platform inference gateway and no upstream API key is needed. Platform supplies a non-secret placeholder to the child runtime for that case.The binding is derived after deployment gateway resolution, is not persisted in
agent.yaml, and is not applied to direct third-party endpoints. This avoids creating a temporary public config convention while keeping the same behavior across subprocess, Docker, and Kubernetes runners.Error Behavior
config_format: package command exits before rendering.--nat-versionwith a Fabric agent: package command exits with a NAT-only flag error.Dockerfile.generatedand generated.dockerignorefiles.Out of Scope
Validation
Affected NeMo Agents unit suites:
This includes NAT and Fabric package routing, Dockerfile rendering, metadata and image identity, Fabric package validation, gateway credential binding, Fabric server startup, translation and validation, and subprocess/Docker/Kubernetes runner configuration.
Repository Python style and formatting:
Manual Docker end-to-end validation:
nemo-agents-spec-v1agent.yaml.pending -> starting -> running.Local CLI flow
The branch version is not available from the package index, so local validation rendered the release Dockerfile and replaced its published-package install with a frozen workspace sync. Released builds use the generated Dockerfile directly and do not need the
perlsubstitution.Optional cleanup:
Summary by CodeRabbit
New Features
NEMO_AGENTS_*packaging environment variables.Bug Fixes