test(nemo-agents): add docker-mode agent deployment e2e test - #715
Conversation
|
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:
📝 WalkthroughWalkthroughAdds Docker daemon-aware test skipping, container-reachable subprocess networking, Docker runtime configuration, and a Linux-only E2E test that deploys and invokes a Nemo agent in Docker mode. ChangesDocker agent E2E flow
Sequence Diagram(s)sequenceDiagram
participant E2ETest
participant NeMoPlatform
participant DockerAgent
participant MockInferenceProvider
E2ETest->>NeMoPlatform: create and deploy Docker-mode agent
NeMoPlatform->>DockerAgent: start deployment container
E2ETest->>NeMoPlatform: invoke agent through gateway
DockerAgent->>MockInferenceProvider: request chat completion
MockInferenceProvider-->>DockerAgent: return mocked response
DockerAgent-->>E2ETest: return mocked response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@e2e/test_nemo_agents_docker.py`:
- Around line 261-265: Update the Docker image build subprocess invocation in
the relevant test setup to catch FileNotFoundError when the docker executable is
unavailable and skip the test consistently with the existing Docker connectivity
marker. Preserve normal subprocess behavior and build-result handling when the
CLI is present.
- Line 63: Replace the hardcoded _DOCKER_BRIDGE_HOST value with a dynamically
resolved active Docker bridge gateway or a configurable setting, ensuring the
e2e setup uses the daemon’s actual bridge host rather than assuming 172.17.0.1.
🪄 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: 72ab6b6e-787e-44f8-a071-c5f8ee15abad
📒 Files selected for processing (5)
conftest.pye2e/configs/local-docker-agents.yamle2e/services_pool.pye2e/test_nemo_agents_docker.pypytest.ini
|
3fe9791 to
5c1dfe4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@e2e/test_nemo_agents_docker.py`:
- Around line 270-274: Update the subprocess.run invocation for the Docker build
in the relevant test setup to include an explicit timeout, using the test
suite’s established duration constant or an appropriate bounded value. Preserve
the existing output capture and text handling while ensuring a hung docker build
raises a timeout instead of blocking until the CI job limit.
- Around line 243-245: Update the image setup flow around _AGENT_IMAGE to always
rebuild the test image from the current checkout instead of returning when
client.images.list finds an existing image. Remove or bypass the existing-image
early return while preserving the remaining Docker build and image-tag behavior.
🪄 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: fde94225-4e5b-40ba-ac5b-9559bfae3360
📒 Files selected for processing (5)
conftest.pye2e/configs/local-docker-agents.yamle2e/services_pool.pye2e/test_nemo_agents_docker.pypytest.ini
🚧 Files skipped from review as they are similar to previous changes (3)
- pytest.ini
- conftest.py
- e2e/configs/local-docker-agents.yaml
595a521 to
bbc6b8e
Compare
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 `@e2e/test_nemo_agents_docker.py`:
- Around line 365-369: Update the cleanup sequence in the test’s finally block
so _remove_agent_container_if_present and _delete_agent_if_exists always execute
even when _delete_deployment_if_exists or _wait_for_deployment_deleted raises.
Isolate deployment cleanup failures from the fallback cleanup while preserving
the existing cleanup calls and arguments.
🪄 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: 191bf7b4-baff-425c-88a0-7deaf4710988
📒 Files selected for processing (5)
conftest.pye2e/configs/local-docker-agents.yamle2e/services_pool.pye2e/test_nemo_agents_docker.pypytest.ini
🚧 Files skipped from review as they are similar to previous changes (3)
- pytest.ini
- conftest.py
- e2e/configs/local-docker-agents.yaml
bbc6b8e to
9f2a31b
Compare
Adds e2e/test_nemo_agents_docker.py, which deploys the calculator-agent example as a real Docker container through the nemo-deployments plugin and invokes it through the agents gateway, exercising the full container-mode chain: gateway proxy -> docker agent container (nat start fastapi) -> Inference Gateway -> mock provider short-circuit -> response. The test is hermetic and CI-friendly: - Builds a NAT runtime image from the calculator-agent example package, but registers the agent with a deterministic single-LLM chat_completion workflow (a mock LLM can't drive a multi-call ReAct loop), and asserts the exact mocked completion round-trips. - Uses the e2e mock inference provider, so no NVIDIA_API_KEY or model egress is needed. Supporting changes: - e2e/configs/local-docker-agents.yaml: platform.runtime=docker with a nemo-deployments docker executor (pull_images=false) and the agents plugin pointed at it. - e2e services harness: new subprocess-backend option container_base_url_host. When set, the harness binds the platform on all interfaces and rewrites platform.base_url to <host>:<actual bind port>, so the IGW URL injected into the agent container is reachable from inside the container (the docker bridge) while the platform's own in-process clients still reach it. The runner seeds NMP_BASE_URL from that platform.base_url host paired with the actual bind port. Markers: - subprocess_only: the test drives its own subprocess-harness platform wired with a docker executor, so it must not run against an external cluster (NMP_BASE_URL set, e.g. the Kind CPU e2e job) where the deployed Helm platform has no docker executor and the module's own e2e_config/harness are ignored (the docker-mode deploy would otherwise fail with 'No executor specified and no default_executor configured'). - docker: skips when no Docker daemon is reachable; the module also skips on non-Linux (Docker Desktop's host alias is not resolvable by the host platform process). Validated end-to-end on Linux and in the CI python-e2e-test job: the agent deploys in docker mode, reaches running (container-mode endpoints), and the mocked completion round-trips through the gateway. Signed-off-by: Ben McCown <bmccown@nvidia.com>
9f2a31b to
328556a
Compare
Signed-off-by: Ben McCown <bmccown@nvidia.com>
The docker-mode agent deployment test needs a NAT-capable image to deploy as a
container. Instead of building a bespoke image in the test, reuse the platform's
own nmp-api image (which already ships the NAT runtime and agent components) and
consume it prebuilt.
- New pytest marker needs_nmp_api_image: skips unless NMP_E2E_IMAGE_REGISTRY and
NMP_E2E_IMAGE_TAG are set (the existing e2e image convention). This keeps the
test out of the plain python-e2e-test job and the kind cluster job, which do
not provide the image.
- test_nemo_agents_docker.py deploys {registry}/nmp-api:{tag} from those env
vars; the in-test docker build (and its base-image/version/timeout concerns)
is removed. Retains subprocess_only + non-Linux skip.
- New CI job python-e2e-image-test (a sibling of python-e2e-test): depends on
build-cpu-smoke-images, logs in to GHCR, pulls nmp-api (timed), and runs only
-m needs_nmp_api_image. Gated like kind-cpu-e2e (publish_images) and, like it,
left out of the required aggregate gate since it skips on forks.
The deployments docker executor overrides the image entrypoint with
nat start fastapi, so nmp-api's own entrypoint is irrelevant.
Signed-off-by: Ben McCown <bmccown@nvidia.com>
…mage) Signed-off-by: Ben McCown <bmccown@nvidia.com>
mckornfield
left a comment
There was a problem hiding this comment.
we discussed in slack, it's probably fine to depend on the nmp-api image, yay more ci checks lol
Signed-off-by: Ben McCown <bmccown@nvidia.com>
crookedstorm
left a comment
There was a problem hiding this comment.
LGTM from devops side
* test(nemo-agents): add docker-mode agent deployment e2e test
Adds e2e/test_nemo_agents_docker.py, which deploys the calculator-agent
example as a real Docker container through the nemo-deployments plugin and
invokes it through the agents gateway, exercising the full container-mode
chain: gateway proxy -> docker agent container (nat start fastapi) ->
Inference Gateway -> mock provider short-circuit -> response.
The test is hermetic and CI-friendly:
- Builds a NAT runtime image from the calculator-agent example package, but
registers the agent with a deterministic single-LLM chat_completion
workflow (a mock LLM can't drive a multi-call ReAct loop), and asserts the
exact mocked completion round-trips.
- Uses the e2e mock inference provider, so no NVIDIA_API_KEY or model egress
is needed.
Supporting changes:
- e2e/configs/local-docker-agents.yaml: platform.runtime=docker with a
nemo-deployments docker executor (pull_images=false) and the agents plugin
pointed at it.
- e2e services harness: new subprocess-backend option
container_base_url_host. When set, the harness binds the platform on all
interfaces and rewrites platform.base_url to <host>:<actual bind port>, so
the IGW URL injected into the agent container is reachable from inside the
container (the docker bridge) while the platform's own in-process clients
still reach it. The runner seeds NMP_BASE_URL from that platform.base_url
host paired with the actual bind port.
Markers:
- subprocess_only: the test drives its own subprocess-harness platform wired
with a docker executor, so it must not run against an external cluster
(NMP_BASE_URL set, e.g. the Kind CPU e2e job) where the deployed Helm
platform has no docker executor and the module's own e2e_config/harness are
ignored (the docker-mode deploy would otherwise fail with 'No executor
specified and no default_executor configured').
- docker: skips when no Docker daemon is reachable; the module also skips on
non-Linux (Docker Desktop's host alias is not resolvable by the host
platform process).
Validated end-to-end on Linux and in the CI python-e2e-test job: the agent
deploys in docker mode, reaches running (container-mode endpoints), and the
mocked completion round-trips through the gateway.
Signed-off-by: Ben McCown <bmccown@nvidia.com>
* CR feedback
Signed-off-by: Ben McCown <bmccown@nvidia.com>
* test(e2e): run docker-agent e2e in a dedicated prebuilt-image job
The docker-mode agent deployment test needs a NAT-capable image to deploy as a
container. Instead of building a bespoke image in the test, reuse the platform's
own nmp-api image (which already ships the NAT runtime and agent components) and
consume it prebuilt.
- New pytest marker needs_nmp_api_image: skips unless NMP_E2E_IMAGE_REGISTRY and
NMP_E2E_IMAGE_TAG are set (the existing e2e image convention). This keeps the
test out of the plain python-e2e-test job and the kind cluster job, which do
not provide the image.
- test_nemo_agents_docker.py deploys {registry}/nmp-api:{tag} from those env
vars; the in-test docker build (and its base-image/version/timeout concerns)
is removed. Retains subprocess_only + non-Linux skip.
- New CI job python-e2e-image-test (a sibling of python-e2e-test): depends on
build-cpu-smoke-images, logs in to GHCR, pulls nmp-api (timed), and runs only
-m needs_nmp_api_image. Gated like kind-cpu-e2e (publish_images) and, like it,
left out of the required aggregate gate since it skips on forks.
The deployments docker executor overrides the image entrypoint with
nat start fastapi, so nmp-api's own entrypoint is irrelevant.
Signed-off-by: Ben McCown <bmccown@nvidia.com>
* docs: correct test docstring (deploys nmp-api image, not calculator image)
Signed-off-by: Ben McCown <bmccown@nvidia.com>
* CR feedback
Signed-off-by: Ben McCown <bmccown@nvidia.com>
---------
Signed-off-by: Ben McCown <bmccown@nvidia.com>
What
Adds the first e2e test that deploys an agent as a real Docker container (via the nemo-deployments plugin) and invokes it through the agents gateway — covering the container-mode path the existing subprocess-mode agent e2e tests don't.
Exercises the full chain end to end:
How
nmp-apiimage as the agent runtime. It already ships the NAT runtime (nvidia-nat-core/-langchainvianemo-agents-plugin), sonatis onPATHand thechat_completion/openaicomponents resolve. The deployments docker executor overrides the entrypoint withnat start fastapi, so the image's own entrypoint is irrelevant — no bespoke agent image is built.NMP_E2E_IMAGE_REGISTRY/NMP_E2E_IMAGE_TAGconvention (deploys{registry}/nmp-api:{tag}).chat_completionworkflow served by the e2e mock inference provider — noNVIDIA_API_KEYor model egress; asserts the exact mocked completion round-trips.container_base_url_hostharness option binds the platform on all interfaces and rewritesplatform.base_urlto the docker bridge, so the injected Inference Gateway URL is reachable from inside the agent container while the platform's own in-process clients still reach it.CI
New
python-e2e-image-testjob (sibling ofpython-e2e-test): depends onbuild-cpu-smoke-images, logs in to GHCR, pullsnmp-api(timed), and runs only-m needs_nmp_api_image. Gated likekind-cpu-e2e(publish_images) and, like it, left out of the required aggregate gate since it skips on forks. Theneeds_nmp_api_imagemarker skips the test in the plain subprocess e2e job and the kind cluster job (which don't provide the image).Changes
e2e/test_nemo_agents_docker.py— the test.e2e/configs/local-docker-agents.yaml— docker runtime + nemo-deployments docker executor.e2e/services_pool.py—container_base_url_hostsubprocess-harness option.conftest.py+pytest.ini—needs_nmp_api_imagemarker + skip logic..github/workflows/ci.yaml— thepython-e2e-image-testjob.