Repository navigation
OSMO 6.3 prerelease deploy: pin CLI to OSMO_CLI_REF (no sudo), chart-version discovery, agent prompts for GPU/region/Azure inputs - #1031
Merged
Conversation
When OSMO_CLI_REF is set to a non-"main" release tag (e.g.
6.3.0-prerelease-rc9), the previous flow piped install.sh from that ref —
but install.sh always resolves to releases/latest/download/version.txt
regardless of which ref fetched it. Deploying a prerelease left users
with the latest-GA CLI. Also the install.sh path requires sudo to write
to /usr/local/bin and hard-fails in non-interactive shells without
NOPASSWD, blocking the deploy script's backend-operator setup.
New flow when OSMO_CLI_REF is set to a release tag:
- Resolve platform-specific installer URL from
github.com/NVIDIA/OSMO/releases/download/<ref>/
- Linux: download self-extracting .sh, locate __ARCHIVE_BELOW__,
extract tarball, copy bin/osmo to OSMO_CLI_TARGET
(default $HOME/.local/bin — no sudo).
- macOS .pkg falls back to sudo since it has no archive marker.
- Hard-fail (don't fall through to ./installer) when the marker is
missing — silent regression to sudo defeats the purpose.
- Warn if OSMO_CLI_TARGET isn't on PATH in any shell rc.
Default ref ("main") path is unchanged.
Reported in nvbug 6199725 issue 2.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tions
Two provider-less actions that exit before --provider validation, used
by agent-driven setup flows (see skills/osmo-deploy/SKILL.md):
--list-chart-versions
Runs `helm search repo <chart> --versions --devel` for every chart
in OSMO_CHART_NAMES (default: "service backend-operator"). The
--devel flag is what surfaces prerelease tags — without it, an
agent running `helm search` only sees GA versions and incorrectly
assumes prereleases live in a different NGC org.
--find-gpu-region SKU COUNT
Prints the first Azure region (from TF_REGION_CANDIDATES,
env-overridable) with enough quota for COUNT x SKU. Exits non-zero
if none qualify. Sources azure/terraform.sh and delegates to
azure_find_region_with_gpu_quota. Used by the osmo-deploy skill's
region prompt when the user answers "idk".
show_help cleanup:
- Document OSMO_CLI_REF + OSMO_CLI_TARGET env vars (paired with the
common.sh CLI install fix).
- Update OSMO_CHART_VERSION docs to mention --devel handling.
- Document the new provider-less actions.
Also: fix the `auth_args=()` empty-array expansion in list_chart_versions
under set -u (same bash3 idiom the storage/common.sh fix used —
`${arr[@]+"${arr[@]}"}`). The original "${auth_args[@]}" form errored
"unbound variable" whenever NGC_API_KEY was unset (the common case).
Reported in nvbug 6199725 issues 1 + 3.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New helpers used by deploy-osmo-minimal.sh's --find-gpu-region action
(which agent-driven flows call when the user answers "idk" to the
region prompt):
azure_describe_vm_sku <sku>
Returns "<family>\t<vcpus>" via one `az vm list-skus` call. Object
JMESPath projection ("[0].{f:family, v:capabilities[?name=='vCPUs'].value | [0]}")
keeps both fields on one tab-separated line; array form ("[0].[a,b]")
renders each element on its own line under `-o tsv` which would
break `read -r family vcpus`.
azure_find_region_with_gpu_quota <sku> <count> <subscription>
Iterates TF_REGION_CANDIDATES (env-overridable, defaults to
H100-likely Azure regions) and prints the first region with
sufficient quota for COUNT x SKU. No hardcoded SKU→family or
vCPU-per-node tables — `az` is the single source of truth. Same
object-projection fix for the list-usage query.
Also: tfvars now writes gpu_vm_size / gpu_min / gpu_max from
TF_GPU_VM_SIZE / TF_GPU_COUNT (set by the agent before invoking the
script). TF_GPU_NODE_POOL_ENABLED + TF_GPU_COUNT + TF_GPU_VM_SIZE +
TF_REGION_CANDIDATES added as env-overridable defaults near the top.
The interactive flow itself is unchanged — GPU/region prompts live in
skills/osmo-deploy/SKILL.md (agent-facing), not in the bash script.
Reported in nvbug 6199725 (agent-UX request).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…/Azure
Two additions to the agent-facing SKILL.md:
1. "Required user inputs" section — explicit prompt list the agent should
run BEFORE invoking deploy-osmo-minimal.sh:
- Do you need GPUs? (Y/N)
- How many GPUs?
- What kind of GPU? (informal name → SKU translation table)
- What region do you have availability? (idk → calls --find-gpu-region)
- Azure subscription ID (if not in env)
- Resource group name (if not in env; must already exist)
The agent maps answers to env vars (TF_GPU_*, TF_REGION, etc.) and
invokes the script in --non-interactive mode. This replaces the bash
interactive flow's GPU prompts that were never added.
2. "Picking chart, image, and CLI versions" section + frontmatter +
"When to Use" callouts that make the 6.3+ requirement mandatory:
- The default behavior (no env vars set) resolves to the latest GA,
which is older than 6.3 and incompatible with this skill (chart
attempts `osmo config update` against ConfigMap-mode service →
HTTP 409).
- All three pins (OSMO_CHART_VERSION + OSMO_IMAGE_TAG + OSMO_CLI_REF)
must be set to a 6.3.x release before invoking the script.
- --list-chart-versions is the authoritative discovery action.
- Why each pin matters explained in generic terms (not tied to the
specific incident bug).
Both changes reported in nvbug 6199725.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds optional Azure GPU AKS node-pool provisioning and quota-based region selection, pins/downloads OSMO CLI release assets to a configurable target, enables provider-less discovery (--list-chart-versions, --find-gpu-region) in the deploy script, and expands deployment docs with interactive GPU prompts and version-pinning guidance. ChangesOSMO GPU Deployment & CLI Pinning
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 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 `@deployments/scripts/deploy-osmo-minimal.sh`:
- Around line 345-351: The --find-gpu-region case reads positional args into
FIND_GPU_REGION_SKU and FIND_GPU_REGION_COUNT without checking argument count,
which breaks under set -u; before assigning $2/$3 (in the --find-gpu-region
branch) validate there are at least two additional positional arguments (e.g.
test remaining $# or inspect "$2" and "$3") and if not print a clear usage/error
and exit non-zero; only then assign FIND_GPU_REGION_SKU="$2" and
FIND_GPU_REGION_COUNT="$3" and shift 3 so malformed invocations fail with a
controlled message instead of an unbound variable error.
In `@skills/osmo-deploy/SKILL.md`:
- Line 83: Update the guidance to reflect that the deploy preflight can
auto-create the resource group when TF_RESOURCE_GROUP is provided: change the
sentence that currently states "The group must already exist" to indicate that
if the group does not exist and TF_RESOURCE_GROUP is set (or --resource-group is
supplied), the preflight will attempt to create it (using az group create -n
<rg> -l <region>); otherwise, instruct the operator to create the group manually
with az group create -n <rg> -l <region> before continuing. Reference
TF_RESOURCE_GROUP and the --resource-group flag, and keep the az group show / az
group create commands as examples.
🪄 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: 29c559c5-5cea-45aa-8abc-90aa45a20c5a
📒 Files selected for processing (4)
deployments/scripts/azure/terraform.shdeployments/scripts/common.shdeployments/scripts/deploy-osmo-minimal.shskills/osmo-deploy/SKILL.md
vvnpn-nv
added a commit
that referenced
this pull request
May 21, 2026
…s, fix RG guidance Three review-driven fixes: 1. `deployments/scripts/deploy-osmo-minimal.sh` — `--find-gpu-region` case now checks `$# >= 3` before reading `$2`/`$3`. Malformed invocations like `--find-gpu-region` or `--find-gpu-region foo` now print a clear usage line and exit 2 instead of tripping `set -u` with an "unbound variable" error. (CodeRabbit finding 1.) 2. Strip all hardcoded release tags (6.3.x, 6.3.0-prerelease-rc9, 1.3.x, 1.2.1, etc.) from `common.sh`, `deploy-osmo-minimal.sh` show_help, and `skills/osmo-deploy/SKILL.md`. Replace the "OSMO >= 6.3" framing with "ConfigMap mode" (the actual technical requirement — earlier CLI-write mode is what's incompatible, and the version cutoff between them is incidental). The skill now consistently points users at `--list-chart-versions` for current values. (Per-session directive: PR text must be version-tag-free so the doc doesn't rot as new release pairs ship.) 3. `skills/osmo-deploy/SKILL.md` — resource-group guidance now matches actual script behavior. azure/terraform.sh:529 auto-creates the group (tagged `osmo-deploy-managed=true`) when `TF_RESOURCE_GROUP` is set and the group doesn't pre-exist; the skill was saying the group must pre-exist, which conflicted. Updated to: pass the name and let the script create it; create manually only if you want the group to outlive `terraform destroy`. (CodeRabbit finding 2.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tags, fix RG guidance Three review-driven fixes: 1. `deployments/scripts/deploy-osmo-minimal.sh` — `--find-gpu-region` case now checks `$# >= 3` before reading `$2`/`$3`. Malformed invocations like `--find-gpu-region` or `--find-gpu-region foo` now print a clear usage line and exit 2 instead of tripping `set -u` with an "unbound variable" error. (CodeRabbit finding 1.) 2. Strip hardcoded prerelease RC tags (`6.3.0-prerelease-rc9`, `1.3.0-prerelease-rc9`) from `common.sh`, `deploy-osmo-minimal.sh` show_help, and `skills/osmo-deploy/SKILL.md`. Keep the `>= 6.3` / `6.3.x` / `6.2` framing — those are stable minor-version references that don't rot when new RCs ship. The skill now consistently points users at `--list-chart-versions` for the actual current tags. 3. `skills/osmo-deploy/SKILL.md` — resource-group guidance now matches actual script behavior. azure/terraform.sh:529 auto-creates the group (tagged `osmo-deploy-managed=true`) when `TF_RESOURCE_GROUP` is set and the group doesn't pre-exist; the skill was saying the group must pre-exist, which conflicted. Updated to: pass the name and let the script create it; create manually only if you want the group to outlive `terraform destroy`. (CodeRabbit finding 2.) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vvnpn-nv
force-pushed
the
vivianp/fix-cli-version-pin-and-ux
branch
from
May 21, 2026 18:35
d3aa2be to
cc04d57
Compare
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 `@deployments/scripts/deploy-osmo-minimal.sh`:
- Around line 203-211: Update the doc text for OSMO_CLI_REF to clarify that
--list-chart-versions returns chart versions (not CLI release tags) and instruct
the user to first discover the chart version with --list-chart-versions and then
pick the corresponding GitHub release tag to use for OSMO_CLI_REF; keep
references to OSMO_CHART_VERSION/OSMO_IMAGE_TAG and OSMO_CLI_TARGET
($HOME/.local/bin) intact so users understand when to pin the CLI ref versus the
chart/image tags.
🪄 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: f4e747a9-4e25-4d57-bb39-3673be99b7f9
📒 Files selected for processing (3)
deployments/scripts/common.shdeployments/scripts/deploy-osmo-minimal.shskills/osmo-deploy/SKILL.md
RyaliNvidia
approved these changes
May 21, 2026
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
deployments/scripts/common.sh (1)
197-208:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHonor
OSMO_CLI_REFeven whenosmois already on PATH.This early return makes the pinning fix a no-op on any machine that already has
osmoinstalled. A prerelease deploy can still run with a mismatched GA CLI, which is the exact compatibility problem this change is meant to prevent. IfOSMO_CLI_REF != main, either verify the installed CLI matches that ref and reinstall when it does not, or fail loudly instead of silently succeeding.Possible minimal safeguard
install_osmo_cli_if_missing() { - if command -v osmo &>/dev/null; then - return 0 - fi - log_info "Installing osmo CLI from GitHub" + local osmo_cli_ref="${OSMO_CLI_REF:-main}" + if command -v osmo &>/dev/null; then + if [[ "$osmo_cli_ref" == "main" ]]; then + return 0 + fi + log_error "Found an existing osmo on PATH, but OSMO_CLI_REF=$osmo_cli_ref requires a matching CLI." + log_error "Remove the existing binary or extend this helper to verify/reinstall the requested ref." + return 1 + fi + log_info "Installing osmo CLI from GitHub" if ! command -v curl &>/dev/null; then log_error "curl is required to install the osmo CLI" return 1 fi - - local osmo_cli_ref="${OSMO_CLI_REF:-main}" local osmo_cli_target="${OSMO_CLI_TARGET:-$HOME/.local/bin}"🤖 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 `@deployments/scripts/common.sh` around lines 197 - 208, The early return in install_osmo_cli_if_missing skips honoring OSMO_CLI_REF when osmo is on PATH; change the function to check OSMO_CLI_REF when osmo exists: if OSMO_CLI_REF is unset or "main" keep the current fast-return behavior, otherwise invoke a verification step (e.g., run osmo --version or a command that outputs its git ref or commit) and compare to OSMO_CLI_REF; if it matches return success, if it does not match either reinstall from GitHub using OSMO_CLI_REF or log an error and exit non-zero. Update references to OSMO_CLI_REF and the verification logic inside install_osmo_cli_if_missing so the function enforces the pinned ref even when osmo is already present.
♻️ Duplicate comments (1)
deployments/scripts/deploy-osmo-minimal.sh (1)
203-211:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon't tell users
--list-chart-versionsdiscovers CLI tags.That command lists Helm chart versions, not GitHub release tags. The current wording can send users to an invalid
OSMO_CLI_REFif the tag naming ever diverges. Reword this to say: use--list-chart-versionsto pickOSMO_CHART_VERSION, then setOSMO_CLI_REFto the matching GitHub release tag.Suggested wording
OSMO_CLI_REF Pin osmo CLI to a release tag from github.com/NVIDIA/OSMO/releases. Required when deploying a channel that doesn't match the latest GA. Default "main" pipes install.sh which always resolves to the latest GA — pin this when you pin OSMO_CHART_VERSION/OSMO_IMAGE_TAG. Pinned ref installs without sudo to OSMO_CLI_TARGET ($HOME/.local/bin by default). Discover available - tags with --list-chart-versions. + chart versions with --list-chart-versions, then set + OSMO_CLI_REF to the matching GitHub release tag.🤖 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 `@deployments/scripts/deploy-osmo-minimal.sh` around lines 203 - 211, Update the help text for OSMO_CLI_REF to stop saying "--list-chart-versions" discovers CLI tags: clarify that "--list-chart-versions" lists Helm chart versions (used to pick OSMO_CHART_VERSION) and instruct users to set OSMO_CLI_REF to the matching GitHub release tag for the CLI (OSMO_CLI_REF), so they first use --list-chart-versions to choose OSMO_CHART_VERSION and then set OSMO_CLI_REF to the corresponding GitHub release tag.
🤖 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.
Outside diff comments:
In `@deployments/scripts/common.sh`:
- Around line 197-208: The early return in install_osmo_cli_if_missing skips
honoring OSMO_CLI_REF when osmo is on PATH; change the function to check
OSMO_CLI_REF when osmo exists: if OSMO_CLI_REF is unset or "main" keep the
current fast-return behavior, otherwise invoke a verification step (e.g., run
osmo --version or a command that outputs its git ref or commit) and compare to
OSMO_CLI_REF; if it matches return success, if it does not match either
reinstall from GitHub using OSMO_CLI_REF or log an error and exit non-zero.
Update references to OSMO_CLI_REF and the verification logic inside
install_osmo_cli_if_missing so the function enforces the pinned ref even when
osmo is already present.
---
Duplicate comments:
In `@deployments/scripts/deploy-osmo-minimal.sh`:
- Around line 203-211: Update the help text for OSMO_CLI_REF to stop saying
"--list-chart-versions" discovers CLI tags: clarify that "--list-chart-versions"
lists Helm chart versions (used to pick OSMO_CHART_VERSION) and instruct users
to set OSMO_CLI_REF to the matching GitHub release tag for the CLI
(OSMO_CLI_REF), so they first use --list-chart-versions to choose
OSMO_CHART_VERSION and then set OSMO_CLI_REF to the corresponding GitHub release
tag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e0c29079-45b3-4a02-adfc-7def29d11261
📒 Files selected for processing (3)
deployments/scripts/common.shdeployments/scripts/deploy-osmo-minimal.shskills/osmo-deploy/SKILL.md
✅ Files skipped from review due to trivial changes (1)
- skills/osmo-deploy/SKILL.md
cypres
approved these changes
May 21, 2026
6 of 18 tasks
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.
Description
Three agent-driven OSMO deploy bugs hit on Azure when targeting the 6.3-prerelease channel. All three were single-deploy blockers; addressed here as one PR with four logically-separable commits.
Issue #None
What's in this PR
deployments/scripts/common.sh—install_osmo_cli_if_missingnow honorsOSMO_CLI_REFproperly. When set to a non-"main" release tag, the helper downloads the matching platform installer directly fromreleases/download/<ref>/, extracts the embedded tarball, and copiesbin/osmoto$HOME/.local/bin(orOSMO_CLI_TARGET) — no sudo. Hard-fails on a missing__ARCHIVE_BELOW__marker rather than silently falling through to the sudo path. DefaultOSMO_CLI_REF=mainflow unchanged (still pipesinstall.sh).deployments/scripts/deploy-osmo-minimal.sh— two new provider-less actions:--list-chart-versionsrunshelm search repo <chart> --versions --develperOSMO_CHART_NAMES(default service + backend-operator) so agents can discover prereleases (helm hides them otherwise).--find-gpu-region <sku> <count>iteratesTF_REGION_CANDIDATESand prints the first Azure region with enough quota; used by the SKILL's region prompt when the user answersidk.Also:
show_helpdocumentsOSMO_CLI_REF/OSMO_CLI_TARGETand removes the stale "nvstaging" guidance. Fixes aset -ubash3 empty-array expansion bug introduced by my ownlist_chart_versions(same${arr[@]+\"${arr[@]}\"}idiom used instorage/common.sh).deployments/scripts/azure/terraform.sh—azure_describe_vm_sku+azure_find_region_with_gpu_quotahelpers backing--find-gpu-region. Usesaz vm list-skus/az vm list-usagewith object JMESPath projection (so-o tsvemits one tab-separated line); no hardcoded SKU→family or vCPU-per-node tables.azure_generate_tfvarsnow writesgpu_vm_size/gpu_min/gpu_maxfromTF_GPU_*env vars. No new interactive prompts in the bash flow — those live in the SKILL.md (next item).skills/osmo-deploy/SKILL.md—OSMO_CHART_VERSION+OSMO_IMAGE_TAG+OSMO_CLI_REF) must be set before invoking the script.Checklist
deployments/scripts/have no unit-test home per AGENTS.md; verification is live-deploy. The--list-chart-versionsand--find-gpu-regionactions are exercised by the osmo-deploy skill flow.)skills/osmo-deploy/SKILL.mdupdated in commit 4;show_helptext indeploy-osmo-minimal.shupdated in commit 2.)Test plan
--list-chart-versionsprints both stable and prerelease chart versions when run againstnvidia/osmo.--find-gpu-region Standard_NC40ads_H100_v5 3returns an Azure region with quota or exits non-zero with a clear error (object JMESPath projection regression-tested locally).install_osmo_cli_if_missingwithOSMO_CLI_REF=6.3.0-prerelease-rc9and no sudo: downloads the rc9 installer, extracts to$HOME/.local/bin,osmo --versionprints rc9.install_osmo_cli_if_missinghard-fails (no silent fallback to sudo) when the installer is missing__ARCHIVE_BELOW__.OSMO_CLI_REF=main(default) path unchanged.OSMO_CHART_VERSION=1.3.0-prerelease-rc9 OSMO_IMAGE_TAG=6.3.0-prerelease-rc9 OSMO_CLI_REF=6.3.0-prerelease-rc9— pending in the parent session.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation