feat(models): deployments_plugin cutover, drop legacy backends and k8s-nim-operator (AIRCORE-759)#705
Open
tylersbray wants to merge 18 commits into
Open
feat(models): deployments_plugin cutover, drop legacy backends and k8s-nim-operator (AIRCORE-759)#705tylersbray wants to merge 18 commits into
tylersbray wants to merge 18 commits into
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 Models service now standardizes deployment execution on ChangesDeployments plugin migration
Sequence Diagram(s)sequenceDiagram
participant ModelsController
participant ModelDeploymentReconciler
participant DeploymentsPluginServiceBackend
participant DeploymentsController
ModelsController->>ModelDeploymentReconciler: reconcile deployment
ModelDeploymentReconciler->>DeploymentsPluginServiceBackend: delete_model_deployment with elapsed time
DeploymentsPluginServiceBackend->>DeploymentsController: stop and delete deployment entities
DeploymentsPluginServiceBackend-->>ModelDeploymentReconciler: DELETING, DELETED, or timeout ERROR
ModelDeploymentReconciler-->>ModelsController: persist deployment status
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.py (1)
182-210: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
FAILEDdeleting timeouts re-fire on every reconcile tick
reconcile_onekeeps routing STOPPED deployments into_reconcile_delete, and_check_deleting_timeoutrebuilds a newFAILEDmessage/error_details from the originalDELETINGtimestamp each time. That bypasses the dedup check, appends anotherstatus_historyevent, and rewrites the entity on every loop. Stop re-evaluating the deleting timeout once the deployment is alreadyFAILEDfordeleting_timeout(or gate the delete path on terminal status).🤖 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-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.py` around lines 182 - 210, The delete reconciliation flow around _reconcile_delete and _check_deleting_timeout must stop re-evaluating an already terminal deleting timeout. When the deployment is already FAILED with error_details indicating deleting_timeout, bypass the timeout check and subsequent status rewrite (or gate this delete path on terminal status), while preserving timeout handling for active DELETING deployments.
🧹 Nitpick comments (1)
services/core/models/tests/unit/test_config.py (1)
54-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the runtime-specific executor mappings.
These tests do not cover the newly added
docker_executor,k8s_executor, anddefault_executorvalues.Proposed assertions
assert backends["deployments_plugin"].enabled is True +assert backends["deployments_plugin"].docker_executor == "local-docker" +assert backends["deployments_plugin"].default_executor == "local-docker" assert backends["deployments_plugin"].enabled is True +assert backends["deployments_plugin"].k8s_executor == "local-k8s" +assert backends["deployments_plugin"].default_executor == "local-k8s"🤖 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 `@services/core/models/tests/unit/test_config.py` around lines 54 - 79, Extend the runtime-specific tests around get_default_backends_for_runtime to assert the expected docker_executor for Runtime.DOCKER, k8s_executor for Runtime.KUBERNETES, and default_executor for Runtime.NONE. Verify each mapping’s value matches the runtime-specific executor configuration while preserving the existing backend assertions.
🤖 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 @.github/actions/setup-kind-cluster/action.yaml:
- Around line 134-147: Fix the YAML indentation for the “Pre-pull GHCR images
into kind” step: dedent its “- name:” entry by two spaces so it aligns with the
other steps, while keeping its shell, env, and run properties at the
corresponding nested indentation.
In `@e2e/test_models_deployments_plugin.py`:
- Around line 95-103: Update the deployment deletion polling helper around
sdk.inference.deployments.retrieve so it checks each retrieved deployment’s
status for ERROR, fails immediately when encountered, and includes the
deployment’s error details in the failure message. Preserve the existing
NotFoundError success path and timeout handling for other statuses.
In `@k8s/helm/README.md`:
- Line 264: Update the platformConfig documentation entry in the Helm README to
replace the hardcoded “NeMo Platform” product name with the repository’s
configured documentation substitution, preserving the surrounding
config-reference link and wording.
In `@k8s/helm/values.yaml`:
- Line 249: Update the configuration path in the comment near the NIM default
StorageClass example to use
models.controller.backends.deployments_plugin.default_storage_class, removing
the nonexistent config segment.
In `@services/core/inference-gateway/tests/integration/conftest.py`:
- Around line 370-385: Wrap the fixture teardown calls after the yield in a
finally block so controller.shutdown() always executes even when
deployments_controller.on_shutdown() raises. Update the teardown flow around
deployments_controller.on_shutdown() and controller.shutdown(), preserving their
existing order.
In
`@services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.py`:
- Around line 194-205: Update the tool_call_inits initialization in the
deployment compiler so separately configured tool-call plugin filesets are
preserved for unweighted image-backed NIM deployments instead of being gated
solely by weighted. Ensure plugin storage and tool_call_plugin_path are enabled
whenever the plugin configuration exists, independently of the weights volume;
otherwise explicitly reject the unsupported configuration.
- Around line 302-309: Align override_config behavior across
apply_nim_override_config in compiler.py and its OpenAPI schema documentation:
either implement merging for the promised arbitrary configuration fragments, or
validate and reject unsupported keys before applying the override. Update
platform.openapi.yaml to document the actual supported subset when arbitrary
fragments remain unsupported, ensuring validation, implementation, and
documentation describe the same behavior.
- Around line 156-161: Update the backend configuration selection around
build_k8s_deployment_backend_config so legacy operator fields are mapped only
for NIM engines, not vLLM or generic engines, while retaining the
engine-specific security defaults. In
openapi/ga/individual/platform.openapi.yaml lines 8995-9002, make no change and
retain the existing NIM-only contract statement.
In
`@services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/nim_compiler.py`:
- Around line 98-135: The tool-plugin initialization in compile_model_deployment
currently depends on the weights volume and weighted deployments. Update the
related container setup and invocation to use only the scratch volume, and
ensure plugin compilation runs independently of the weighted model path so
non-files-service NIM deployments also install tool plugins.
- Around line 326-335: Update apply_container_resources to deep-merge the
container’s existing requests and limits maps with the provided resource
overrides, preserving unspecified existing keys while replacing only supplied
keys; retain the current string conversion and early-return behavior. In
services/core/models/src/nmp/core/models/schemas.py lines 1463-1470, make no
direct change because the existing deep-merge description is correct once
apply_container_resources is fixed.
- Around line 174-181: Update the weighted=False branch around NIM_MODEL_NAME
and NIM_MODEL_PATH so it does not assign the unmounted _WEIGHTS_MOUNT
(/model-store) when no weights volume exists. Remove or conditionally skip these
path variables unless the corresponding volume is provisioned, while preserving
the existing NIM_SERVED_MODEL_NAME handling.
- Around line 480-483: Update the override handling in the deployment compiler
to check whether “command” and “args” keys exist in override rather than whether
their values are truthy. Preserve the existing string conversion while allowing
explicit empty lists to clear container.command and container.args.
- Around line 493-503: Update the readiness and liveness override handling
around _probe_from_nim_service_probe so an explicitly disabled probe clears the
generated container probe instead of leaving it unchanged. When readinessProbe
or livenessProbe is a dictionary, assign the conversion result to the
corresponding container field even when it is None; preserve the existing
behavior for absent or non-dictionary overrides.
In `@services/core/models/tests/integration/test_deployments_plugin_lifecycle.py`:
- Around line 98-112: Update wait_for_container_deleted to advance
reconciliation on each retry by calling reconcile(controller), while polling
both the Docker container state and
sdk.inference.providers.retrieve(provider_name, workspace=provider_workspace).
Return only once the container is deleted and the provider retrieval raises
NotFoundError, preserving the existing retry behavior and assertion.
---
Outside diff comments:
In
`@plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.py`:
- Around line 182-210: The delete reconciliation flow around _reconcile_delete
and _check_deleting_timeout must stop re-evaluating an already terminal deleting
timeout. When the deployment is already FAILED with error_details indicating
deleting_timeout, bypass the timeout check and subsequent status rewrite (or
gate this delete path on terminal status), while preserving timeout handling for
active DELETING deployments.
---
Nitpick comments:
In `@services/core/models/tests/unit/test_config.py`:
- Around line 54-79: Extend the runtime-specific tests around
get_default_backends_for_runtime to assert the expected docker_executor for
Runtime.DOCKER, k8s_executor for Runtime.KUBERNETES, and default_executor for
Runtime.NONE. Verify each mapping’s value matches the runtime-specific executor
configuration while preserving the existing backend assertions.
🪄 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: 6fa6a1dc-dc3c-4bc3-bbd2-e0c035d1c1a0
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (79)
.github/actions/setup-kind-cluster/action.yaml.github/workflows/ci.yamldocs/set-up/config-reference.mdxdocs/set-up/helm/persistent-volumes.mdxe2e/k8s/values/default.yamle2e/k8s/values/kind.yamle2e/k8s/values/minikube-auth-portforward.yamle2e/k8s/values/minikube-auth.yamle2e/k8s/values/minikube.yamle2e/test_models_deployments_plugin.pyk8s/helm/Chart.yamlk8s/helm/README.mdk8s/helm/templates/core/controller-role.yamlk8s/helm/values.yamlopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nmp_platform/config/local.yamlpackages/nmp_platform_runner/src/nmp/platform_runner/config/local.yamlplugins/nemo-deployments/src/nemo_deployments_plugin/config.pyplugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.pyplugins/nemo-deployments/tests/unit/backends/k8s/test_rbac_manifest.pyplugins/nemo-deployments/tests/unit/reconciler/test_deployment_reconciler.pyplugins/nemo-deployments/tests/unit/test_config.pyservices/core/inference-gateway/tests/integration/conftest.pyservices/core/models/config/local.yamlservices/core/models/pyproject.tomlservices/core/models/scripts/generate_nim_types.pyservices/core/models/scripts/update-types.shservices/core/models/src/nmp/core/models/config.pyservices/core/models/src/nmp/core/models/controllers/backends/__init__.pyservices/core/models/src/nmp/core/models/controllers/backends/backends.pyservices/core/models/src/nmp/core/models/controllers/backends/common.pyservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.pyservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.pyservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/config.pyservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/nim_compiler.pyservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/status.pyservices/core/models/src/nmp/core/models/controllers/backends/docker/__init__.pyservices/core/models/src/nmp/core/models/controllers/backends/docker/backend.pyservices/core/models/src/nmp/core/models/controllers/backends/docker/config.pyservices/core/models/src/nmp/core/models/controllers/backends/docker/creation_reconciler.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/__init__.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/backend.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/config.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/nimservice_compiler.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/base.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/k8s.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/nim_operator.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/resource_deleter.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/status_projector.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/types/README.mdservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/types/__init__.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/types/nimcache.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/vllm_k8s_compiler.pyservices/core/models/src/nmp/core/models/controllers/backends/none_backend.pyservices/core/models/src/nmp/core/models/controllers/backends/registry.pyservices/core/models/src/nmp/core/models/controllers/deployment_reconciler.pyservices/core/models/src/nmp/core/models/schemas.pyservices/core/models/tests/integration/README.mdservices/core/models/tests/integration/conftest.pyservices/core/models/tests/integration/test_chat_template_tool_calling.pyservices/core/models/tests/integration/test_deployments_plugin_lifecycle.pyservices/core/models/tests/integration/test_models.pyservices/core/models/tests/integration/test_models_controller.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_compiler.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_nim_compiler.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_status.pyservices/core/models/tests/unit/controllers/backends/test_vllm_k8s_compiler.pyservices/core/models/tests/unit/controllers/test_backend_config_fields.pyservices/core/models/tests/unit/controllers/test_backend_registry.pyservices/core/models/tests/unit/controllers/test_deployment_reconciler.pyservices/core/models/tests/unit/controllers/test_docker_backend.pyservices/core/models/tests/unit/controllers/test_k8s_nim_operator_backend.pyservices/core/models/tests/unit/controllers/test_nimservice_compiler.pyservices/core/models/tests/unit/nimservice_spec_types.pyservices/core/models/tests/unit/test_config.pyservices/core/models/tests/unit/test_schemas_override_config.py
💤 Files with no reviewable changes (28)
- services/core/models/src/nmp/core/models/controllers/backends/none_backend.py
- services/core/models/scripts/update-types.sh
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/config.py
- services/core/models/src/nmp/core/models/controllers/backends/init.py
- e2e/k8s/values/minikube.yaml
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/base.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/types/init.py
- e2e/k8s/values/default.yaml
- services/core/models/src/nmp/core/models/controllers/backends/docker/backend.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/types/README.md
- services/core/models/src/nmp/core/models/controllers/backends/docker/init.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/init.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/backend.py
- k8s/helm/Chart.yaml
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/nim_operator.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/vllm_k8s_compiler.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/types/nimcache.py
- services/core/models/tests/unit/controllers/backends/test_vllm_k8s_compiler.py
- services/core/models/src/nmp/core/models/controllers/backends/docker/creation_reconciler.py
- k8s/helm/templates/core/controller-role.yaml
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/status_projector.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/resource_deleter.py
- services/core/models/scripts/generate_nim_types.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/nimservice_compiler.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/k8s.py
- services/core/models/tests/unit/controllers/test_nimservice_compiler.py
- services/core/models/src/nmp/core/models/controllers/backends/docker/config.py
- services/core/models/tests/unit/controllers/test_backend_config_fields.py
tylersbray
added a commit
that referenced
this pull request
Jul 15, 2026
Fix kind CI action YAML, stop re-firing deleting timeouts, improve NIM compiler parity (tool-call plugins, resource merge, override_config), and tighten delete polling in e2e/integration tests. 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
`@services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/nim_compiler.py`:
- Around line 145-148: Update the deployment command construction around the
plugin move operation to skip moving the file when the downloaded source path
already equals plugin_path, including the /scratch/plugin/plugin.py case.
Preserve the existing move behavior for different paths so the init container
continues copying unweighted plugins as before.
🪄 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: f409f8e2-023d-436e-8196-85c40b61a43d
📒 Files selected for processing (13)
.github/actions/setup-kind-cluster/action.yamle2e/test_models_deployments_plugin.pyk8s/helm/README.mdk8s/helm/values.yamlplugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.pyplugins/nemo-deployments/tests/unit/reconciler/test_deployment_reconciler.pyservices/core/inference-gateway/tests/integration/conftest.pyservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.pyservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/nim_compiler.pyservices/core/models/src/nmp/core/models/schemas.pyservices/core/models/tests/integration/test_deployments_plugin_lifecycle.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_nim_compiler.pyservices/core/models/tests/unit/test_config.py
🚧 Files skipped from review as they are similar to previous changes (11)
- k8s/helm/README.md
- services/core/models/tests/integration/test_deployments_plugin_lifecycle.py
- .github/actions/setup-kind-cluster/action.yaml
- plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.py
- plugins/nemo-deployments/tests/unit/reconciler/test_deployment_reconciler.py
- k8s/helm/values.yaml
- services/core/models/tests/unit/test_config.py
- services/core/models/src/nmp/core/models/schemas.py
- e2e/test_models_deployments_plugin.py
- services/core/models/tests/unit/controllers/backends/deployments_plugin/test_nim_compiler.py
- services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.py
tylersbray
added a commit
that referenced
this pull request
Jul 15, 2026
Correct deployments_plugin storage-class config path, skip no-op plugin mv on unweighted NIM, sync override_config OpenAPI docs, and fix helm doc example. Signed-off-by: Tyler Bray <tbray@nvidia.com>
tylersbray
added a commit
that referenced
this pull request
Jul 15, 2026
Fix kind CI action YAML, stop re-firing deleting timeouts, improve NIM compiler parity (tool-call plugins, resource merge, override_config), and tighten delete polling in e2e/integration tests. Signed-off-by: Tyler Bray <tbray@nvidia.com>
tylersbray
added a commit
that referenced
this pull request
Jul 15, 2026
Correct deployments_plugin storage-class config path, skip no-op plugin mv on unweighted NIM, sync override_config OpenAPI docs, and fix helm doc example. Signed-off-by: Tyler Bray <tbray@nvidia.com>
tylersbray
force-pushed
the
759-models-plugin-cutover-pr2/tbray
branch
from
July 15, 2026 23:35
df1efa3 to
5704324
Compare
Contributor
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 `@services/core/models/tests/integration/conftest.py`:
- Around line 478-479: Update the teardown sequence around
deployments_controller.on_shutdown() so models_controller.shutdown() is
guaranteed to run even when on_shutdown() raises, using a finally-style cleanup
path while preserving propagation of the original teardown error.
🪄 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: 4f26042c-cab3-43bc-8358-488ceef1b50b
📒 Files selected for processing (79)
.github/actions/setup-kind-cluster/action.yaml.github/workflows/ci.yamldocs/set-up/config-reference.mdxdocs/set-up/helm/persistent-volumes.mdxe2e/k8s/values/default.yamle2e/k8s/values/kind.yamle2e/k8s/values/minikube-auth-portforward.yamle2e/k8s/values/minikube-auth.yamle2e/k8s/values/minikube.yamle2e/test_models_deployments_plugin.pyk8s/helm/Chart.yamlk8s/helm/README.mdk8s/helm/templates/core/controller-role.yamlk8s/helm/values.yamlopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlpackages/nmp_platform/config/local.yamlpackages/nmp_platform_runner/src/nmp/platform_runner/config/local.yamlplugins/nemo-deployments/src/nemo_deployments_plugin/config.pyplugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.pyplugins/nemo-deployments/tests/unit/backends/k8s/test_rbac_manifest.pyplugins/nemo-deployments/tests/unit/reconciler/test_deployment_reconciler.pyplugins/nemo-deployments/tests/unit/test_config.pyservices/core/inference-gateway/tests/integration/conftest.pyservices/core/models/config/local.yamlservices/core/models/pyproject.tomlservices/core/models/scripts/generate_nim_types.pyservices/core/models/scripts/update-types.shservices/core/models/src/nmp/core/models/config.pyservices/core/models/src/nmp/core/models/controllers/backends/__init__.pyservices/core/models/src/nmp/core/models/controllers/backends/backends.pyservices/core/models/src/nmp/core/models/controllers/backends/common.pyservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.pyservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.pyservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/config.pyservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/nim_compiler.pyservices/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/status.pyservices/core/models/src/nmp/core/models/controllers/backends/docker/__init__.pyservices/core/models/src/nmp/core/models/controllers/backends/docker/backend.pyservices/core/models/src/nmp/core/models/controllers/backends/docker/config.pyservices/core/models/src/nmp/core/models/controllers/backends/docker/creation_reconciler.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/__init__.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/backend.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/config.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/nimservice_compiler.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/base.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/k8s.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/nim_operator.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/resource_deleter.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/status_projector.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/types/README.mdservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/types/__init__.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/types/nimcache.pyservices/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/vllm_k8s_compiler.pyservices/core/models/src/nmp/core/models/controllers/backends/none_backend.pyservices/core/models/src/nmp/core/models/controllers/backends/registry.pyservices/core/models/src/nmp/core/models/controllers/deployment_reconciler.pyservices/core/models/src/nmp/core/models/schemas.pyservices/core/models/tests/integration/README.mdservices/core/models/tests/integration/conftest.pyservices/core/models/tests/integration/test_chat_template_tool_calling.pyservices/core/models/tests/integration/test_deployments_plugin_lifecycle.pyservices/core/models/tests/integration/test_models.pyservices/core/models/tests/integration/test_models_controller.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_compiler.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_nim_compiler.pyservices/core/models/tests/unit/controllers/backends/deployments_plugin/test_status.pyservices/core/models/tests/unit/controllers/backends/test_vllm_k8s_compiler.pyservices/core/models/tests/unit/controllers/test_backend_config_fields.pyservices/core/models/tests/unit/controllers/test_backend_registry.pyservices/core/models/tests/unit/controllers/test_deployment_reconciler.pyservices/core/models/tests/unit/controllers/test_docker_backend.pyservices/core/models/tests/unit/controllers/test_k8s_nim_operator_backend.pyservices/core/models/tests/unit/controllers/test_nimservice_compiler.pyservices/core/models/tests/unit/nimservice_spec_types.pyservices/core/models/tests/unit/test_config.pyservices/core/models/tests/unit/test_schemas_override_config.py
💤 Files with no reviewable changes (28)
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/types/README.md
- k8s/helm/Chart.yaml
- services/core/models/src/nmp/core/models/controllers/backends/init.py
- services/core/models/scripts/update-types.sh
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/init.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/resource_deleter.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/types/init.py
- e2e/k8s/values/minikube.yaml
- services/core/models/src/nmp/core/models/controllers/backends/docker/init.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/base.py
- k8s/helm/templates/core/controller-role.yaml
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/types/nimcache.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/config.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/backend.py
- services/core/models/tests/unit/controllers/test_nimservice_compiler.py
- e2e/k8s/values/default.yaml
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/status_projector.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/nim_operator.py
- services/core/models/src/nmp/core/models/controllers/backends/none_backend.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/vllm_k8s_compiler.py
- services/core/models/src/nmp/core/models/controllers/backends/docker/backend.py
- services/core/models/src/nmp/core/models/controllers/backends/docker/creation_reconciler.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/nimservice_compiler.py
- services/core/models/tests/unit/controllers/backends/test_vllm_k8s_compiler.py
- services/core/models/src/nmp/core/models/controllers/backends/k8s_nim_operator/reconcilers/k8s.py
- services/core/models/src/nmp/core/models/controllers/backends/docker/config.py
- services/core/models/tests/unit/controllers/test_backend_config_fields.py
- services/core/models/scripts/generate_nim_types.py
🚧 Files skipped from review as they are similar to previous changes (43)
- services/core/models/src/nmp/core/models/controllers/backends/backends.py
- services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/config.py
- services/core/models/src/nmp/core/models/controllers/deployment_reconciler.py
- services/core/models/src/nmp/core/models/controllers/backends/common.py
- services/core/models/config/local.yaml
- packages/nmp_platform/config/local.yaml
- plugins/nemo-deployments/tests/unit/test_config.py
- docs/set-up/helm/persistent-volumes.mdx
- services/core/models/tests/integration/README.md
- services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/status.py
- k8s/helm/README.md
- services/core/models/tests/unit/controllers/backends/deployments_plugin/test_status.py
- plugins/nemo-deployments/src/nemo_deployments_plugin/config.py
- e2e/k8s/values/minikube-auth-portforward.yaml
- services/core/models/tests/unit/controllers/backends/deployments_plugin/test_compiler.py
- services/core/models/tests/unit/test_schemas_override_config.py
- plugins/nemo-deployments/src/nemo_deployments_plugin/reconciler/deployment_reconciler.py
- services/core/models/tests/integration/test_models.py
- services/core/models/tests/integration/test_deployments_plugin_lifecycle.py
- plugins/nemo-deployments/tests/unit/backends/k8s/test_rbac_manifest.py
- e2e/k8s/values/minikube-auth.yaml
- openapi/ga/openapi.yaml
- services/core/models/pyproject.toml
- services/core/models/src/nmp/core/models/controllers/backends/registry.py
- .github/workflows/ci.yaml
- openapi/openapi.yaml
- e2e/test_models_deployments_plugin.py
- services/core/models/tests/unit/controllers/test_deployment_reconciler.py
- k8s/helm/values.yaml
- services/core/models/src/nmp/core/models/config.py
- services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/backend.py
- services/core/models/tests/unit/controllers/backends/deployments_plugin/test_backend.py
- services/core/models/tests/unit/test_config.py
- packages/nmp_platform_runner/src/nmp/platform_runner/config/local.yaml
- services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.py
- openapi/ga/individual/platform.openapi.yaml
- services/core/models/tests/unit/controllers/test_backend_registry.py
- plugins/nemo-deployments/tests/unit/reconciler/test_deployment_reconciler.py
- services/core/models/src/nmp/core/models/schemas.py
- docs/set-up/config-reference.mdx
- services/core/models/tests/integration/test_models_controller.py
- services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/nim_compiler.py
- services/core/models/tests/unit/nimservice_spec_types.py
Contributor
|
tylersbray
changed the title
tylersbray
added a commit
that referenced
this pull request
Jul 16, 2026
Fix kind CI action YAML, stop re-firing deleting timeouts, improve NIM compiler parity (tool-call plugins, resource merge, override_config), and tighten delete polling in e2e/integration tests. Signed-off-by: Tyler Bray <tbray@nvidia.com>
tylersbray
added a commit
that referenced
this pull request
Jul 16, 2026
Correct deployments_plugin storage-class config path, skip no-op plugin mv on unweighted NIM, sync override_config OpenAPI docs, and fix helm doc example. Signed-off-by: Tyler Bray <tbray@nvidia.com>
tylersbray
force-pushed
the
759-models-plugin-cutover-pr2/tbray
branch
from
July 16, 2026 00:37
c0be7ed to
464f530
Compare
Contributor
Author
|
@coderabbitai review fixes for open comments. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
gh pr view 705 --repo NVIDIA-NeMo/nemo-platform --json comments,reviews -q '.reviews[] | select(.body != "") | {author: .author.login, state: .state}' | head -50
echo "---REVIEW THREADS---"
gh api graphql -f query='
query {
repository(owner: "NVIDIA-NeMo", name: "nemo-platform") {
pullRequest(number: 705) {
reviewThreads(first: 100) {
nodes {
id
isResolved
isOutdated
path
line
comments(first: 10) {
nodes {
author { login }
body
createdAt
url
}
}
}
}
}
}
}' --jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved==false) | {path, line, isOutdated, comments: [.comments.nodes[] | {author: .author.login, body: .body[0:300], url}]}'Length of output: 2819 🏁 Script executed: #!/bin/bash
echo "=== k8s/helm/README.md around line 264 ==="
sed -n '1,50p' k8s/helm/README.md | grep -n "NeMo Platform"
grep -n "NeMo Platform" k8s/helm/README.md
echo "=== compiler.py override_config section ==="
sed -n '280,340p' services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/compiler.py
echo "=== nim_compiler.py apply_nim_override_config ==="
rg -n "def apply_nim_override_config" -A 60 services/core/models/src/nmp/core/models/controllers/backends/deployments_plugin/nim_compiler.py | head -100
echo "=== conftest.py around line 471 ==="
sed -n '440,500p' services/core/models/tests/integration/conftest.pyLength of output: 7417 🏁 Script executed: #!/bin/bash
gh api graphql -f query='
query {
repository(owner: "NVIDIA-NeMo", name: "nemo-platform") {
pullRequest(number: 705) {
reviewThreads(first: 100) {
nodes {
id
isResolved
isOutdated
path
line
comments(first: 10) {
nodes {
author { login }
body
url
}
}
}
}
}
}
}' --jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved==false) | {path, line, isOutdated, comments: [.comments.nodes[] | {author: .author.login, body: .body, url}]}'Length of output: 7338 🏁 Script executed: #!/bin/bash
awk 'NR>=255 && NR<=275{print NR": "$0}' k8s/helm/README.md
echo "---openapi---"
sed -n '8990,9015p' openapi/ga/individual/platform.openapi.yamlLength of output: 5116
models_controller._loop.run_until_complete(deployments_controller.on_shutdown())
models_controller.shutdown()If |
Make deployments_plugin delete non-blocking with deleting_timeout_seconds escalation in both the models backend and plugin reconciler. Remove synchronous delete_wait_seconds polling. File follow-up AIRCORE-925 for docker LoRA multi-container support. Signed-off-by: Tyler Bray <tbray@nvidia.com>
…RE-759 P2) Port tool_call_plugin init containers, multi-LLM FT-model env gating, and k8s_nim_operator_config mapping (tolerations, node selector, resources, startup probe grace) from nimservice_compiler into deployments_plugin. Signed-off-by: Tyler Bray <tbray@nvidia.com>
…RCORE-759 P2) Apply NIM and vLLM default uid/gid from DeploymentsPluginConfig onto plugin K8sDeploymentConfig.security_context, merged with k8s_nim_operator_config. Signed-off-by: Tyler Bray <tbray@nvidia.com>
…AIRCORE-759 P2) Flip get_default_backends_for_runtime and local/helm/e2e values to enable deployments_plugin with local-docker and local-k8s executor wiring. Signed-off-by: Tyler Bray <tbray@nvidia.com>
…E-759 P2) Add controller_with_deployments_plugin fixture running models and plugin controllers against real Docker, mirroring test_docker_deployment_lifecycle. Signed-off-by: Tyler Bray <tbray@nvidia.com>
…E-759 P2) Delete legacy deployment backends and ~8k lines of unit tests; registry and config now only support deployments_plugin. Update integration tests and IGW fixtures to exercise the plugin path; drop docker/kubernetes from nmp-models deps. Signed-off-by: Tyler Bray <tbray@nvidia.com>
Remove the bundled NIM Operator Helm dependency, controller Role rules for NIMService/NIMCache CRs, and stale e2e values overrides. Model deployments now use deployments_plugin with native k8s executors. Signed-off-by: Tyler Bray <tbray@nvidia.com>
Exercise generic CPU model deployment create→READY→delete on a real kind cluster and wire the test into kind-cpu-smoke CI alongside the jobs smoke. Signed-off-by: Tyler Bray <tbray@nvidia.com>
Mark executor_config.k8s_nim_operator_config and override_config as deprecated in schemas/OpenAPI, expand deployments_plugin docstrings, and point persistent-volumes examples at deployments_plugin config. Signed-off-by: Tyler Bray <tbray@nvidia.com>
Remove stale FAILED plugin Deployment entities during models delete so teardown does not hang in DELETING after the reconciler gives up. Signed-off-by: Tyler Bray <tbray@nvidia.com>
Map legacy NIMService Spec override_config fragments onto plugin Container and K8sDeploymentConfig after k8s_nim_operator_config so precedence matches the removed k8s-nim-operator backend. Signed-off-by: Tyler Bray <tbray@nvidia.com>
Fix kind CI action YAML, stop re-firing deleting timeouts, improve NIM compiler parity (tool-call plugins, resource merge, override_config), and tighten delete polling in e2e/integration tests. Signed-off-by: Tyler Bray <tbray@nvidia.com>
Correct deployments_plugin storage-class config path, skip no-op plugin mv on unweighted NIM, sync override_config OpenAPI docs, and fix helm doc example. Signed-off-by: Tyler Bray <tbray@nvidia.com>
- Drop nemo-deployments-plugin as an unconditional models dependency; the registry loads it lazily, matching main's optional-plugin design (#698). Re-sync vendored models-service extra (drop kubernetes/docker) and uv.lock. - Make the deployments_plugin integration test self-contained instead of importing skip_without_docker from a sibling conftest, which collided with another service's conftest during full-repo collection. - Add services/core/models/tests/unit to pytest pythonpath so the override_config test can import its nimservice_spec_types helper. - Revert override_config/k8s_nim_operator_config schema description tweaks and regenerate the OpenAPI spec to keep it in sync with the vendored SDK. Signed-off-by: Tyler Bray <tbray@nvidia.com>
- Reject unsupported override_config keys at NIM compile time and document the supported subset in schemas and OpenAPI. - Guarantee models_controller.shutdown() in deployments_plugin integration fixture teardown via try/finally. - Sync SDK .nmpcontext OpenAPI snapshot for description-only spec change. Signed-off-by: Tyler Bray <tbray@nvidia.com>
tylersbray
force-pushed
the
759-models-plugin-cutover-pr2/tbray
branch
from
July 16, 2026 16:33
464f530 to
d1a6037
Compare
Kind CPU e2e/smoke jobs failed prepulling docker.io/library/python: prepull_kind_images.sh applied the GHCR KIND_IMAGE_PULL_* credentials to every non-nvcr.io image, and auth.docker.io rejects those with a 401. Pull docker.io images anonymously instead (as the kubelet already does for postgres/busybox). IGW integration tests still assumed the removed docker backend: they deleted the ModelDeploymentConfig after a single controller.step(), but the deployments_plugin delete path is non-blocking, so the deployment was not yet DELETED (409 Conflict). They also resolved containers via the legacy md- naming instead of the plugin dep-...-server naming (404 No such container). Add a _wait_for_deployment_deleted helper that steps the reconciler to DELETED before deleting the config, and resolve container/volume names via the plugin naming helpers. Signed-off-by: Tyler Bray <tbray@nvidia.com>
mckornfield
approved these changes
Jul 16, 2026
Generic k8s deployments no longer inherit NIM runAsUser/runAsGroup defaults;
only explicit run_as_* overrides apply, matching the schema ("generic runs as
the image's own user"). Fixes kind smoke/e2e crash loops for python:alpine
http.server probes.
Also fix YAML indentation in persistent-volumes.mdx and cross-reference the
GENERIC_HTTP_* image tag between the kind prepull action and e2e test.
Signed-off-by: Tyler Bray <tbray@nvidia.com>
The kind smoke e2e deploys python:3.12-alpine with additional_args only. On Kubernetes, container args replace the image CMD; python:alpine has no ENTRYPOINT and CMD ["python3"], so args ["-m", "http.server", "8000"] made kubelet exec "-m" directly (exec: "-m": executable file not found in $PATH). Include python3 as argv[0] and clarify compile_generic_args docstring. Signed-off-by: Tyler Bray <tbray@nvidia.com>
benmccown
approved these changes
Jul 16, 2026
benmccown
left a comment
benmccown
left a comment
Contributor
There was a problem hiding this comment.
18k lines of code deleted. Amazing.
Contributor
Author
Manual smoke results (AIRCORE-759 / PR #705)Smoke plan executed on branch
Ben's "proof in the pudding" / e2e coverage
Ops notes for dev-pod smoke
New helper
|
Contributor
Author
GPU smoke update (dev-blue / tbray-dev pod)NGC NIM Llama 3.2 1B (k8s) — PASS
Workarounds required (not in deploy-models.mdx verbatim):
vLLM Qwen3-1.7B (k8s, weighted/HF) — BLOCKED (product gaps)
Recommend follow-up issues for: auto GPU resources from Docker executor (pod dockerd) — PASS
Config notes (
|
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
deployments_pluginexclusively; removes legacydocker,k8s_nim_operator, andnonebackends (~18k lines deleted).nimservice_compiler(tool-call plugin, multi-LLM env,k8s_nim_operator_config,override_config) intodeployments_plugin/compiler.k8s-nim-operatorHelm subchart and NIM CR RBAC; adds required kind smoke e2e (e2e/test_models_deployments_plugin.py) wired intokind-cpu-smokeCI.deleting_timeout_secondson models + plugin reconciler (PR feat(models): deployments_plugin backend (AIRCORE-759 PR1) #663 carryover).Test plan
uv run pytest services/core/models/tests/unit/ -q(834 passed)uv run pytest services/core/models/tests/integration/ -q(236 passed, 7 skipped)uv run pytest services/core/models/tests/unit/controllers/backends/deployments_plugin/ -vuv run pytest plugins/nemo-deployments/tests/unit/reconciler/ -k delete -qhelm template+kind.yaml)kind-cpu-smokeCI (runs on this PR)Enablement
deployments_pluginis now the default backend for docker/k8s/none runtimes. HelmbasePlatformConfigand e2e values configurelocal-k8sexecutor.Notes
deployments_pluginbackend introduction).k8s_nim_operator_configandoverride_configare deprecated in OpenAPI but still honored on k8s NIM paths.Summary by CodeRabbit
New Features
Documentation
Refactor