fix(jobs): resolve default task image from platform config on K8s#302
Merged
matthewgrossman merged 11 commits intoJun 12, 2026
Conversation
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…-k8s-task-pod-tries-to-pull Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
matthewgrossman
changed the title
Contributor
|
mckornfield
approved these changes
Jun 12, 2026
mckornfield
reviewed
Jun 12, 2026
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…-k8s-task-pod-tries-to-pull Signed-off-by: Matthew Grossman <mgrossman@nvidia.com> # Conflicts: # sdk/python/nemo-platform/api.md # sdk/python/nemo-platform/src/nemo_platform/resources/files/filesets.py # sdk/python/nemo-platform/src/nemo_platform/types/files/fileset_metadata_param.py # sdk/python/nemo-platform/src/nemo_platform/types/files/fileset_update_params.py # sdk/python/nemo-platform/src/nemo_platform/types/shared_params/fileset_metadata.py # sdk/python/nemo-platform/src/nemo_platform/types/shared_params/fileset_metadata_param.py
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
matthewgrossman
marked this pull request as ready for review
June 12, 2026 18:04
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 ignored due to path filters (2)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughContainer image is now optional on job steps. Execution profiles provide a default task image fallback. When both are unset, the platform CPU tasks image is used. Schemas, backend implementations, tests, and docs were updated. ChangesContainer Image Resolution Fallback
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 5
🧹 Nitpick comments (3)
docs/k8s-dev-guide.md (3)
178-221: ⚡ Quick winUse dropdowns for known issues and debugging details.
The Known Issues section (lines 178–192) lists failures with explanations. The Observability section (lines 194–221) contains detailed kubectl and profiling commands. Both are dense reference material that many readers won't need immediately.
Per guidelines, use dropdowns for:
- Advanced sections (e.g.,
<details>with<summary>)- Optional/advanced content
- Troubleshooting details
- Long examples
- Content most users will skip
Restructure:
- Known Issues: Keep summary line (e.g., "2 passed, 5 skipped, 52 failed on nmp.dev as of 2026-06-10"), collapse each issue into a
<details>block.- Observability: Keep summary ("kubectl commands for pod inspection…"), collapse detailed commands into
<details>blocks labeled by task (e.g., "Check pod logs", "Attach debug container", "Profile with py-spy").🤖 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 `@docs/k8s-dev-guide.md` around lines 178 - 221, Collapse verbose reference material in the "Known issues" and "Observability and debugging" sections by converting the long lists and command blocks into HTML details/summary dropdowns: keep the short summary lines (e.g., the overall test-count summaries) visible and wrap each individual known-issue bullet (RBAC blocks entity operations, Health/Studio tests, Mock inference provider, etc.) inside its own <details> with a one-line <summary>, and move the long kubectl and profiling command blocks under the "Observability and debugging" heading into labeled <details> blocks ("Check pod logs", "Attach debug container", "Profile with py-spy"); update the docs/k8s-dev-guide.md headings "Known issues" and "Observability and debugging" accordingly so readers see the summaries but must expand to view commands or per-issue details.Source: Coding guidelines
123-135: ⚡ Quick winUse tab sets for parallel auth alternatives.
Both the SDK section (lines 123–135) and e2e test section (lines 156–166) present two alternatives side-by-side:
- SDK: CLI context vs. explicit token
- E2E: explicit token vs. CLI context
Use markdown tab sets (or HTML equivalent) to reduce repetition and improve scannability. This follows the guideline: "Use tab sets for parallel alternatives or variants (platform/language variants, before/after comparisons, consecutive code blocks showing alternatives)."
Example (Markdown syntax):
=== "Option 1: CLI context" nemo config set --context tot --base-url ... === "Option 2: Explicit token" export NMP_ACCESS_TOKEN=$(nemo auth token)Also applies to: 156-166
🤖 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 `@docs/k8s-dev-guide.md` around lines 123 - 135, Replace the two parallel code alternatives with Markdown tab sets: wrap the SDK examples (the two NeMoPlatform snippets under the “SDK” section) into two tabs titled e.g. "Option 1: CLI context" and "Option 2: Explicit token", moving each code block into its respective tab; do the same for the e2e test section (the explicit-token and CLI-context snippets) so the two alternatives appear as parallel tabs, keep the same code blocks and descriptions, and ensure tab titles and ordering are consistent across both sections for scannability.Source: Coding guidelines
254-310: ⚡ Quick winUse dropdowns to layer minikube setup details.
The minikube setup (lines 254–310) spans 56 lines with nested prerequisites, image configuration options, build steps, and Helm values. This is detailed reference material that needs structure.
Per progressive disclosure and dropout guidelines, layer this section:
- Top level (always visible): Brief overview ("Start a local K8s cluster, build images, deploy via Helm").
- Prerequisites dropdown: Docker/minikube/kubectl/helm versions, NGC_API_KEY, GitHub CLI auth.
- Image configuration dropdown: Explain Options A vs B (local build vs GHCR pre-built) with trade-offs.
- Setup steps: Keep numbered, but move long explanatory comments (e.g., lines 312–316) into a dropdown "Why these settings?"
- Values files dropdown: Table at lines 326–336 is reference material — collapse it.
🤖 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 `@docs/k8s-dev-guide.md` around lines 254 - 310, Replace the long flat "#### Setup" block with a layered version using collapsible sections: keep a one-line visible overview ("Start a local K8s cluster, build images, deploy via Helm"), then add a "Prerequisites" dropdown listing Docker/minikube/kubectl/helm versions, NGC_API_KEY and GH CLI auth; add an "Image configuration" dropdown that explains NMP_REGISTRY, NMP_TAG, NMP_PULL_POLICY and clearly compares Option A (local build) vs Option B (GHCR) and trade-offs; keep the numbered setup steps (the minikube start, docker-env, gh auth, buildx bake using BUILD_ARCH, helm repo/add and helm upgrade commands) but move long explanatory comments into a "Why these settings?" collapsible pane; finally collapse the Helm values table into a "Values files" dropdown — preserve all existing variables (NMP_IMAGE, BUILD_ARCH, BAKE_REGISTRY_IMAGE, etc.) and exact commands so the commands remain runnable.Source: Coding guidelines
🤖 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 `@docs/k8s-dev-guide.md`:
- Around line 196-202: Replace informal and vague phrases like "Phil's team has
observability flowing for both environments" and the "Dashboards: TBD" / "Logs:
TBD" lines with concrete, actionable directives: update the "Phil's team..."
sentence to "Observability is available for both nmp.dev and minikube via
<Grafana dashboard link>" and provide explicit access steps or links for logs
(e.g., "To access pod logs use kubectl logs -n <namespace> <pod> or view via
<Loki/Grafana link> with instructions to request access"), and either populate
or remove the "Dashboards: TBD" and "Logs: TBD" entries; also change "Deployment
notifications: ... may be surfaced to devs in the future" to a clear current
state (e.g., where deployment status is posted and whom to contact to change
notifications) so every item has a concrete link, command, or owner.
- Around line 1-20: Update the opening paragraph to lead with: "This guide helps
you set up and test NeMo Platform against local or shared Kubernetes
environments." Immediately follow with a one-line audience/when-to-read sentence
(e.g., "Use this if you're debugging K8s-specific failures, testing on minikube,
or validating changes against nmp.dev"). Remove mixed Diataxis content from this
file by either (A) splitting into three pages—K8s Developer HOW-TO (this file:
setup, run tests, debug), CLI Reference (commands/auth/context), and Environment
EXPLANATION (purpose/when-to-use each URL)—or (B) convert this file into a pure
HOW-TO by reorganizing into a task flow (set up → authenticate → run tests),
collapsing the detailed environment/command reference into expandable sections,
and adding cross-links to the new CLI Reference and Environment EXPLANATION
pages; update the "Environments" heading and any inline command examples (e.g.,
the /cluster-info curl example) to link to the CLI Reference instead of
containing full reference syntax here.
- Line 198: The document contains four incomplete sections ("Dashboards", "Logs
access", "Docker backend setup", "Dev-blue box setup") that must not be
published as TBDs; either remove those sections entirely or replace each TBD
with concrete, actionable setup/access instructions (e.g., Grafana URL and auth
steps for "Dashboards", Loki/Grafana or kubectl steps for "Logs access", exact
Docker backend install/config commands for "Docker backend setup", and
step-by-step dev-blue box provisioning and access for "Dev-blue box setup"); if
the features are not yet supported, mark the file as draft/hidden from
navigation instead of leaving TODO text. Ensure each updated section includes
explicit credentials/roles required, links to team contacts (if external), and
any CLI commands or UI navigation paths so reviewers can verify before merging.
In `@openapi/ga/openapi.yaml`:
- Around line 8599-8601: The schema currently allows the image property to be an
empty string (field "image" with type: string) which bypasses the unset→fallback
contract; update each "image" schema instance (the ones around lines referenced)
to forbid empty strings by adding a constraint such as minLength: 1 (or an
equivalent pattern like pattern: ".+") so that "" is invalid; ensure you apply
this change to every occurrence (the image property blocks at the noted
locations) so runtime image resolution cannot receive an empty string.
In
`@services/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.py`:
- Around line 1106-1108: Update the stale comment describing task image fallback
to reflect current precedence: note that resolve_task_image(container.image,
config.default_task_image) now uses container.image first, then
config.default_task_image (profile default), and if neither is set it falls back
to the platform CPU tasks image (not an error); update the comment above the
call to resolve_task_image to describe this exact order and behavior.
---
Nitpick comments:
In `@docs/k8s-dev-guide.md`:
- Around line 178-221: Collapse verbose reference material in the "Known issues"
and "Observability and debugging" sections by converting the long lists and
command blocks into HTML details/summary dropdowns: keep the short summary lines
(e.g., the overall test-count summaries) visible and wrap each individual
known-issue bullet (RBAC blocks entity operations, Health/Studio tests, Mock
inference provider, etc.) inside its own <details> with a one-line <summary>,
and move the long kubectl and profiling command blocks under the "Observability
and debugging" heading into labeled <details> blocks ("Check pod logs", "Attach
debug container", "Profile with py-spy"); update the docs/k8s-dev-guide.md
headings "Known issues" and "Observability and debugging" accordingly so readers
see the summaries but must expand to view commands or per-issue details.
- Around line 123-135: Replace the two parallel code alternatives with Markdown
tab sets: wrap the SDK examples (the two NeMoPlatform snippets under the “SDK”
section) into two tabs titled e.g. "Option 1: CLI context" and "Option 2:
Explicit token", moving each code block into its respective tab; do the same for
the e2e test section (the explicit-token and CLI-context snippets) so the two
alternatives appear as parallel tabs, keep the same code blocks and
descriptions, and ensure tab titles and ordering are consistent across both
sections for scannability.
- Around line 254-310: Replace the long flat "#### Setup" block with a layered
version using collapsible sections: keep a one-line visible overview ("Start a
local K8s cluster, build images, deploy via Helm"), then add a "Prerequisites"
dropdown listing Docker/minikube/kubectl/helm versions, NGC_API_KEY and GH CLI
auth; add an "Image configuration" dropdown that explains NMP_REGISTRY, NMP_TAG,
NMP_PULL_POLICY and clearly compares Option A (local build) vs Option B (GHCR)
and trade-offs; keep the numbered setup steps (the minikube start, docker-env,
gh auth, buildx bake using BUILD_ARCH, helm repo/add and helm upgrade commands)
but move long explanatory comments into a "Why these settings?" collapsible
pane; finally collapse the Helm values table into a "Values files" dropdown —
preserve all existing variables (NMP_IMAGE, BUILD_ARCH, BAKE_REGISTRY_IMAGE,
etc.) and exact commands so the commands remain runnable.
🪄 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: fecf3ec3-1aef-424b-b63e-be084e6a3300
⛔ Files ignored due to path filters (9)
sdk/python/nemo-platform/.nmpcontext/openapi.yamlis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/container_spec.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/container_spec_param.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/docker_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/kubernetes_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/subprocess_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/types/jobs/volcano_job_execution_profile_config.pyis excluded by!sdk/**sdk/python/nemo-platform/tests/api_resources/test_jobs.pyis excluded by!sdk/**
📒 Files selected for processing (13)
docs/k8s-dev-guide.mddocs/set-up/config-reference.mdxe2e/test_jobs.pyopenapi/ga/individual/platform.openapi.yamlopenapi/ga/openapi.yamlopenapi/openapi.yamlservices/core/jobs/src/nmp/core/jobs/app/providers.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/base.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/docker.pyservices/core/jobs/src/nmp/core/jobs/controllers/backends/kubernetes/common.pyservices/core/jobs/tests/controllers/test_base.pyservices/core/models/tests/integration/test_model_entity_service_integration.pyservices/core/models/tests/unit/test_model_entity_service_unit.py
matthewgrossman
enabled auto-merge
June 12, 2026 19:22
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
Fixes AIRCORE-790: job e2e tests fail on K8s because task pods try to pull
placeholder:unused.ContainerSpec.imageoptional — jobs can omit it when they just need to run a commanddefault_task_imageto execution profile config as an explicit override{platform.image_registry}/nmp-cpu-tasks:{platform.image_tag}(same pattern as automodel/unsloth task images)resolve_task_image()placeholder:unused— they omit the image and let the platform resolve itdocs/k8s-dev-guide.mdwith updated build/deploy instructions using nemo-platform'sdocker-bake.hclTest plan
services/core/jobs/tests/)services/core/jobs/tests/integration/)minikube delete— full setup, build, deploy, test cyclemy-registry/nmp-cpu-tasks:local(confirmed via pod events)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests