feat(bundler): structured deploy.sh output with color and step headers by mohityadav8 · Pull Request #1393 · NVIDIA/aicr

mohityadav8 · 2026-06-17T13:48:38Z

Add NO_COLOR-aware color helpers (_ok, _fail, _warn_line)
Add _step_header/ok/fail/retry for per-component section framing
Show [N/M] progress counter and manual copy-paste command per step
Color-code pre-flight pass/fail and final deployment summary

- Add NO_COLOR-aware color helpers (_ok, _fail, _warn_line) - Add _step_header/ok/fail/retry for per-component section framing - Show [N/M] progress counter and manual copy-paste command per step - Color-code pre-flight pass/fail and final deployment summary Closes NVIDIA#1362

copy-pr-bot · 2026-06-17T13:48:43Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

github-actions · 2026-06-17T13:49:00Z

Welcome to AICR, @mohityadav8! Thanks for your first pull request.

Before review, please ensure:

All commits are signed off per the DCO
CI checks pass (tests, lint, security scan)
The PR description explains the why behind your changes

A maintainer will review this soon.

coderabbitai · 2026-06-17T13:54:10Z

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

The deploy script template (deploy.sh.tmpl) and all six rendered testdata instances receive identical changes: seven new color/TTY-aware helper functions (_ok, _fail, _warn_line, _step_header, _step_ok, _step_fail, _step_retry) are added. These helpers respect NO_COLOR environment variable and TTY detection, replacing plain echo-based status output throughout the script. The helm_failed best-effort warning switches to _warn_line. Pre-flight checks gain a formatted section header and use _fail/_ok for failure/pass outcomes. The component install loop pre-counts directories for [N/M] progress display and calls _step_header per component. The retry loop uses _step_ok on successful install, _step_fail on max-retry exhaustion (followed by helm_failed), and _step_retry with exponentially-computed backoff on intermediate failures. The final summary uses _fail/_ok based on whether FAILED_COMPONENTS accumulated any failures.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Linked Issues check	❓ Inconclusive	The PR partially addresses `#1362`'s core goal of making deploy.sh copy-pasteable by adding structured output with step headers and progress indicators, but the changes focus on formatting/presentation rather than fundamentally restructuring the script as a top-level series of manually-copyable commands.	Verify whether the structured output with step headers and copy-paste command hints in _step_header sufficiently addresses the requirement to make deploy.sh commands 'easily executable even manually' or if further refactoring of the script's core logic is needed.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the main change: adding structured, color-aware output with step headers to deploy.sh for improved user experience.
Description check	✅ Passed	The description is directly related to the changeset, clearly listing the new helper functions, progress counters, and color-coding improvements introduced in the pull request.
Out of Scope Changes check	✅ Passed	All changes are focused on output formatting and helper functions for deploy.sh; no unrelated or out-of-scope modifications were introduced.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

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 `@pkg/bundler/deployer/helm/templates/deploy.sh.tmpl`:
- Around line 70-73: In the _step_header function, the second printf statement
prints the directory path without quotes in the manual execution instruction.
Add quotes around the path parameter in the printf format string so that when
the bundle path is substituted, it will be properly escaped for shell copy-paste
commands. This ensures paths containing spaces or special characters will not
break the displayed command.

🪄 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: ASSERTIVE

Plan: Enterprise

Run ID: f957c905-4a38-4094-9c8a-ca0384a6bcf5

📥 Commits

Reviewing files that changed from the base of the PR and between eea2a81 and 4eb36f6.

📒 Files selected for processing (7)

pkg/bundler/deployer/helm/templates/deploy.sh.tmpl
pkg/bundler/deployer/helm/testdata/kai_scheduler_present/deploy.sh
pkg/bundler/deployer/helm/testdata/manifest_only/deploy.sh
pkg/bundler/deployer/helm/testdata/mixed_gpu_operator/deploy.sh
pkg/bundler/deployer/helm/testdata/mixed_with_pre/deploy.sh
pkg/bundler/deployer/helm/testdata/nodewright_present/deploy.sh
pkg/bundler/deployer/helm/testdata/upstream_helm_only/deploy.sh

Signed-off-by: Mohit Yadav <[email protected]>

mchmarny

Welcome to AICR, @mohityadav8 — and thanks for picking this up! This is a genuinely nice quality-of-life improvement to deploy.sh: the NO_COLOR + non-TTY detection is done correctly, the helpers are clean, and the per-step [N/M] framing makes a long install far easier to follow. Regenerating all 6 testdata goldens alongside the template is exactly the right discipline, too.

One thing blocks merge: the goldens are out of sync with the template. The _step_header quoting fix (printf -v manual_dir '%q') landed in deploy.sh.tmpl but the committed testdata/*/deploy.sh files still have the pre-fix version, so the byte-comparing TestBundleGolden_* tests will fail. A quick go test ./pkg/bundler/deployer/helm/... -run TestBundleGolden -update (then commit) fixes all six at once — details inline. The other two are small nits (loop indentation, and the Manual: hint not reflecting the env the real install uses).

Heads up: the PR is behind main, so rebasing on origin/main before re-pushing will also let the real CI gates run. Nice first contribution — this is close.

Signed-off-by: Mohit Yadav <[email protected]>

- Re-indent attempt/while/done block inside for-loop body - Add env var caveat to Manual: step hint with printf -v quoting - Regenerate all 6 testdata goldens

mohityadav8 · 2026-06-22T22:46:12Z

@mchmarny done all the suggested changes

mohityadav8 requested a review from a team as a code owner June 17, 2026 13:48

github-actions Bot added the area/bundler label Jun 17, 2026

github-actions Bot added the size/XL label Jun 17, 2026

coderabbitai Bot reviewed Jun 17, 2026

View reviewed changes

Comment thread pkg/bundler/deployer/helm/templates/deploy.sh.tmpl

mchmarny assigned mohityadav8 Jun 22, 2026

mchmarny and others added 3 commits June 22, 2026 07:43

Merge branch 'main' into fix-1362-deploy-script-ux

b44f764

Update deploy.sh.tmpl

7d4ff8e

Signed-off-by: Mohit Yadav <[email protected]>

Merge branch 'main' into fix-1362-deploy-script-ux

3bc089e

mchmarny requested changes Jun 22, 2026

View reviewed changes

Comment thread pkg/bundler/deployer/helm/testdata/manifest_only/deploy.sh Outdated

Comment thread pkg/bundler/deployer/helm/templates/deploy.sh.tmpl Outdated

Comment thread pkg/bundler/deployer/helm/templates/deploy.sh.tmpl Outdated

mchmarny and others added 2 commits June 22, 2026 12:28

Merge branch 'main' into fix-1362-deploy-script-ux

90f6927

Update deploy.sh.tmpl

81d0873

Signed-off-by: Mohit Yadav <[email protected]>

mohityadav8 force-pushed the fix-1362-deploy-script-ux branch from 81d0873 to de7354e Compare June 22, 2026 22:37

mohityadav8 added 2 commits June 23, 2026 04:08

fix: address review feedback on deploy.sh UX

c6d2eef

- Re-indent attempt/while/done block inside for-loop body - Add env var caveat to Manual: step hint with printf -v quoting - Regenerate all 6 testdata goldens

fix: regenerate goldens to remove duplicate old/new lines

fbd9f83

mohityadav8 force-pushed the fix-1362-deploy-script-ux branch from 8e7401c to fbd9f83 Compare June 22, 2026 22:41

mohityadav8 requested a review from mchmarny June 22, 2026 22:45

mohityadav8 and others added 2 commits June 23, 2026 12:15

Merge branch 'main' into fix-1362-deploy-script-ux

a14017b

Merge branch 'main' into fix-1362-deploy-script-ux

589ac11

mchmarny approved these changes Jun 23, 2026

View reviewed changes

mchmarny merged commit e9aba0d into NVIDIA:main Jun 23, 2026
25 checks passed

coderabbitai Bot mentioned this pull request Jun 25, 2026

feat(bundler): support dynamic toleration injection for pre-built OCI bundles #1471

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(bundler): structured deploy.sh output with color and step headers#1393

feat(bundler): structured deploy.sh output with color and step headers#1393
mchmarny merged 10 commits into
NVIDIA:mainfrom
mohityadav8:fix-1362-deploy-script-ux

mohityadav8 commented Jun 17, 2026

Uh oh!

copy-pr-bot Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading

Reviews paused

Walkthrough

Estimated code review effort

❌ Failed checks (1 inconclusive)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

mchmarny left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mohityadav8 commented Jun 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mohityadav8 commented Jun 17, 2026

Uh oh!

copy-pr-bot Bot commented Jun 17, 2026

Uh oh!

github-actions Bot commented Jun 17, 2026

Uh oh!

coderabbitai Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Estimated code review effort

❌ Failed checks (1 inconclusive)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mchmarny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mohityadav8 commented Jun 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jun 17, 2026 •

edited

Loading