fix: NCX to NICo rename QA follow-up - #4825
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
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)
Summary by CodeRabbit
WalkthroughThe PR updates repository URLs, configuration filenames, project metadata, documentation links, Markdown heading structure, code-fence annotations, and Quick Start documentation for observability setup. ChangesRepository and documentation alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This PR updates documentation, generated API outputs, and repository metadata without any supplied evidence of an actionable merge-blocking risk; it is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
thossain-nv
added
the
rest-api
ChatGPT Codex Connector
kfelternv
changed the title
cdcfac1 to
6b5135e
Compare
What changedThis PR corrects stale repository, configuration, anchor, and documentation references. It also updates the compiled admin CLI metadata and the admin UI source links. Scenario and setupI tested head VerificationStep 1: Validate the changed Markdown and Helm chartWhy this step exists: This checks the corrected headings, fences, links, and chart metadata. Runnable command: uvx rumdl check --config docs/.rumdl.toml \
deploy/README.md \
docs/getting-started/installation-options/day0-machine-identity.md \
docs/getting-started/quick-start.md \
docs/provisioning/ip-and-network-configuration.md \
helm-prereqs/README.md \
rest-api/CHANGELOG.md \
rest-api/deploy/INSTALLATION.md
helm lint helm-prereqsObserved result: Why this proves the behavior: The changed reader-facing files and Helm metadata pass their configured validators. Step 2: Validate the generated Go SDK and corrected public linksWhy this step exists: This checks the generated SDK and both corrected repository links used by the OpenAPI and Quick Start documentation. Runnable command: (
cd rest-api/sdk/standard
go test ./...
)
for url in \
https://github.com/NVIDIA/infra-controller/tree/main/rest-api/auth \
https://github.com/NVIDIA/infra-controller/blob/main/helm-prereqs/observability/README.md
do
curl -L -sS -o /dev/null -w '%{http_code} %{url_effective}\n' "$url"
doneObserved result: Why this proves the behavior: The generated client compiles and tests successfully, and both corrected public targets resolve. Step 3: Exercise the compiled admin CLI metadataWhy this step exists: This checks the repository metadata embedded in the deployed admin CLI and confirms the CLI starts normally. Runnable command: kubectl -n nico-system exec deploy/nico-api -- sh -lc \
"grep -ao 'https://github.com/NVIDIA/[A-Za-z0-9_-]*' /opt/carbide/nico-admin-cli | sort -u"
kubectl -n nico-system exec deploy/nico-api -- \
/opt/carbide/nico-admin-cli --help | sed -n '1,3p'Observed result: Why this proves the behavior: The built CLI contains the corrected repository metadata and its real help interface runs successfully. Step 4: Exercise the rendered admin UI source linkWhy this step exists: This checks the changed API-web formatter through a live rendered Machine page. Runnable command: MACHINE_ID="$(
kubectl -n nico-system exec deploy/nico-api -- \
curl -ksS https://localhost:1079/admin/machine.json |
jq -r '.machines[] | select(.state_reason.source_ref != null) | .id' |
sed -n '1p'
)"
kubectl -n nico-system exec deploy/nico-api -- \
curl -ksS "https://localhost:1079/admin/machine/${MACHINE_ID}" |
tr '"' '\n' |
awk '/^https:\/\/github.com\/NVIDIA\// {print}' |
sort -uObserved result: Why this proves the behavior: The deployed admin UI rendered a live state-handler source reference against the consolidated repository. |
|
🌿 Preview your docs: https://nvidia-preview-pull-request-4825.docs.buildwithfern.com/infra-controller |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-08-11 20:38:14 UTC | Commit: 6b5135e |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b5135ee23
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@helm-prereqs/README.md`:
- Line 438: Update the dual-deployment references in helm-prereqs/README.md at
the sections corresponding to lines 438 and 490, linking them to the canonical
helm/README.md “Migrating from Kustomize” and “Upgrading” anchors respectively.
Replace only the unlinked references and preserve the surrounding guidance.
In `@rest-api/CHANGELOG.md`:
- Line 156: Remove the added blank line from the legacy changelog so
rest-api/CHANGELOG.md remains unchanged; current unified release entries belong
in docs/release-notes.md instead.
🪄 Autofix
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: 1b2785cb-94d9-43b4-b7a1-4a6c7dd71e18
⛔ Files ignored due to path filters (1)
rest-api/sdk/standard/model_service_account.gois excluded by!rest-api/sdk/standard/model_*.go
📒 Files selected for processing (12)
crates/admin-cli/src/cfg/cli_options.rscrates/api-web/src/filters.rsdeploy/README.mddocs/getting-started/installation-options/day0-machine-identity.mddocs/getting-started/quick-start.mddocs/provisioning/ip-and-network-configuration.mdhelm-prereqs/Chart.yamlhelm-prereqs/README.mdrest-api/CHANGELOG.mdrest-api/deploy/INSTALLATION.mdrest-api/docs/index.htmlrest-api/openapi/spec.yaml
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
8ef9628 to
78dd3a6
Compare
thossain-nv
changed the title
thossain-nv
left a comment
thossain-nv
left a comment
There was a problem hiding this comment.
Looks good, thanks @kfelternv
polarweasel
left a comment
polarweasel
left a comment
There was a problem hiding this comment.
Super-minor changes, and a request to reflow a file or two...
4 participants
Addresses every actionable item from the #2358 QA review by documenting the supported observability setup option, replacing the remaining stale Core values filename and anchor, correcting the authentication link in the OpenAPI source and generated outputs, clearing the required Markdown lint findings, and updating the remaining repository references used by Helm metadata, API-web source links, and admin CLI metadata.
Related issues
Refs #2358
Type of Change
Breaking Changes
Testing
Additional Notes
OpenAPI SDK and rendered documentation were regenerated from
rest-api/openapi/spec.yaml. The five affected Markdown files passrumdl, all 136 Fern MDX files validate, the Helm chart lints, all 17 migrated changelog URLs and the corrected auth URL return HTTP 200, and the generated Go SDK tests pass. Focused Rust package tests are blocked on Apple Silicon macOS becausetss-esapi-sysdoes not support the target without generated bindings.