docs(tasks): close OSS documentation gaps for container and Helm tasks - #640
Open
shelleyshen-0 wants to merge 10 commits into
Open
docs(tasks): close OSS documentation gaps for container and Helm tasks#640shelleyshen-0 wants to merge 10 commits into
shelleyshen-0 wants to merge 10 commits into
Conversation
- Add use-case framing to the intro (fine-tuning, TensorRT, batch pipelines) - Add task lifecycle section with all 8 statuses and descriptions - Expand result handling to explain what the NONE strategy requires: heartbeat cadence, completion signal, and cross-link to progress file
…toring - Add environment variables table (NVCT_TASK_ID, NVCT_TASK_NAME, NVCT_NCA_ID, NVCT_PROGRESS_FILE_PATH, NVCT_RESULTS_DIR) - Fix secrets delivery: secrets arrive as /var/secrets/secrets.json, not as environment variables; add JSON format example and rotation note - Add model/resource artifact mount paths (/config/models, /config/resources) - Add progress file section: JSON schema, field constraints (name length and allowed characters verified against worker-task/progress.go), heartbeat rule, completion semantics, intermediate result semantics, atomic write recommendation - Update result handling section to explain the NONE workflow - Expand monitoring section: explain cancel vs delete semantics, note that task get returns percentComplete and healthInfo - Update JSON and CLI examples to use NONE strategy
…aints - Add prerequisites section: chart version must not contain hyphens, essential values.yaml keys (nvctNcaId, nvctTaskId, nvctTaskName, nvctResultsDir, nvctProgressFilePath) with minimal template, automatic pull secret injection - Add progress updates section: cross-link to progress file schema, Job + backoffLimit:0 + restartPolicy:Never pattern with YAML example, warning that percentComplete must never decrease - Add security constraints section: supported k8s object types, 300-object limit, allowed Pod volume types, hook and CRD behavior - Update JSON example to use NONE strategy and remove GFN-specific backend
|
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:
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)
📝 WalkthroughWalkthroughThe task documentation adds lifecycle statuses, ChangesTask documentation
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
|
🌿 Preview your docs: https://nvidia-preview-docs-tasks-oss-gaps.docs.buildwithfern.com/nvcf |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/user/container-tasks.md`:
- Around line 179-182: Update the intermediate-results guidance to scope the
unique checkpoint names and progress-file write-order rules to the UPLOAD
strategy, since only UPLOAD consumes resultsDir; do not apply these requirements
to NONE unless documenting its separate consumer behavior.
- Around line 167-170: Update the percentComplete documentation in
docs/user/container-tasks.md lines 167-170 and docs/user/helm-tasks.md lines
80-83 to state that values must be non-decreasing, allowing equal percentages
when checkpoints use unique names; retain the existing integer range.
- Around line 135-139: Update the container task documentation around the
`/config/models` and `/config/resources` mount paths to state that each artifact
is stored under its corresponding path using its sanitized name, while
preserving the existing model and resource path descriptions.
In `@docs/user/helm-tasks.md`:
- Around line 100-127: Clarify the Helm policy documentation by removing the
duplicate batch/v1/CronJob entry from the extraKubernetesTypes example, or
explicitly identifying it as redundant. State which supported-object, volume,
object-count, hook, and CRD rules are enforced by Default versus Unrestricted,
and explicitly identify any rules that remain enforced for every policy.
- Around line 8-37: Correct the Helm prerequisites documentation to state that
setInfraValues supplies the five root-level runtime values before rendering, so
charts do not need to declare them in values.yaml and existing non-empty values
are preserved. Replace the claim that pull secrets are injected into every
PodSpec with the actual ServiceAccount behavior, including Helm ServiceAccount
handling being controlled by HelmRBACEnforcement.
🪄 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: cacbf60e-28e1-4155-9f80-3f9b559a1166
📒 Files selected for processing (3)
docs/user/container-tasks.mddocs/user/helm-tasks.mddocs/user/task-creation.md
sbaum1994
approved these changes
Aug 3, 2026
sbaum1994
left a comment
sbaum1994
left a comment
Contributor
There was a problem hiding this comment.
just one comment about making result strategy none more obvious at the top for CLI examples
| --max-runtime PT4H \ | ||
| --result-strategy UPLOAD \ | ||
| --results-location my-org/my-team/my-model | ||
| --result-strategy NONE |
Contributor
There was a problem hiding this comment.
Is there a warning that result strategy none is the only supported result strategy, if not can you add one here (in addition to below)
| ## Result handling | ||
|
|
||
| Note: result upload is not yet supported in this release. | ||
| Note: result upload to a registry is not yet supported in this release. Set |
Contributor
There was a problem hiding this comment.
Can you make this be a warning
- Change "monotonically increasing" to "non-decreasing" in container-tasks.md and helm-tasks.md; the worker accepts equal values and only rejects strictly decreasing ones (progress.go:524) - Scope the intermediate-result write-ordering rule (unique name, do not write to previous result dirs) to the UPLOAD strategy, since the worker only reads resultsDir for UPLOAD; NONE has no write-ordering constraint
- Fix extraKubernetesTypes example: replace CronJob (already in the Default allowlist per serialize.go:178) with DaemonSet, which actually requires an extra type entry - Fix values.yaml prerequisite: setInfraValues injects the five nvct keys before rendering via mergo.Merge; charts do not need to declare them but empty defaults are recommended for local development - Fix pull secret description: secrets are attached to the default ServiceAccount at runtime (prereqs.go:123), not patched into every PodSpec - Add policy scoping to security constraints: object type allowlist applies to all policies; volume types, 300-object limit, hook and CRD rules apply only under Default; Unrestricted bypasses those checks (run.go:376-382)
Add a Warning callout before the CLI examples and convert the plain note in the Result handling section to a Warning, so the UPLOAD-not-supported limitation is more prominent on self-hosted.
- taskId: not required; worker auto-fills it if absent (progress.go:157-159) - name: not required or validated under NONE; isValidProgressName only runs inside the UPLOAD_STRATEGY block (progress.go:193)
shelleyshen-0
enabled auto-merge
August 4, 2026 02:33
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.
TL;DR
Closes gaps between the managed-service Tasks documentation and the OSS self-hosted Tasks documentation, addressing issue #587.
Additional Details
The OSS task docs were missing several pieces that are critical for self-hosted users, particularly around the
NONEresult strategy (the only strategy supported on self-hosted NVCF):/var/secrets/secrets.json)All added content was verified against service source code (
worker-task/internal/progress/progress.go,worker-task/configs/configs.go,cloud-tasks/TaskMapperService.java,nvca/pkg/storage/modelcache.go). One correction was made during verification: thenamefield character pattern was fixed from an incorrect ASCII range notation to an explicit character list matching the code.Managed-only capabilities (UPLOAD strategy, GFN backend limits, NGC registry, SSA-JWT auth, UI creation) are omitted.
For the Reviewer
Three commits, one per file:
docs/user/task-creation.md— lifecycle table, use-case framing, NONE strategy overviewdocs/user/container-tasks.md— env vars, secrets fix, mount paths, progress file, result handling, monitoringdocs/user/helm-tasks.md— prerequisites, progress updates, security constraintsFor QA
Ran
./tools/ci/check-docs— 0 errors.Issues
Closes #587
Checklist
Summary by CodeRabbit
NONEresult handling and container-managed output delivery.