docs(quick-start): replace nicocli site create with site verification#3111
Open
shayan1995 wants to merge 1 commit into
Open
docs(quick-start): replace nicocli site create with site verification#3111shayan1995 wants to merge 1 commit into
shayan1995 wants to merge 1 commit into
Conversation
setup.sh Phase 7g bootstraps and registers the site automatically via the nico-rest-site-agent bootstrap Job — operators must not run nicocli site create after setup as it would create a second site that the already-deployed site-agent cannot use. - Replace "Create your First Site" step with a verification step explaining why site create must not be run and how to confirm the site was registered by setup.sh - Update section header from "Create your First Site" to "Verify your Site" - Fix step 3f: update DB reference from "dev postgres instance" to nico-pg-cluster (Zalando-managed), matching the current setup - Fix component list: postgres StatefulSet hosts temporal + keycloak databases only; NICo REST DB runs on nico-pg-cluster
2 tasks
Contributor
Summary by CodeRabbit
WalkthroughThis PR updates the Quick Start documentation to clarify database configuration defaults for the site-agent, revise the postgres StatefulSet description in setup phases, and replace "create your first site" instructions with site verification guidance using ChangesQuick Start Documentation Update
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔍 Container Scan SummaryNo Grype artifacts were found to aggregate. |
![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: 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 `@docs/getting-started/quick-start.md`:
- Around line 189-193: This section is out of sync with the site-agent defaults
and currently points to the wrong database configuration. Update the quick-start
text to match the actual `helm-prereqs/values/nico-site-agent.yaml` settings
used by the site agent, and make sure the references to `DB_ADDR` and
`DB_DATABASE` align with the `nico-site-agent` defaults instead of the stale
`nico-pg-cluster`/`nico_rest` values.
🪄 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: f2043458-314f-4f66-845d-6c7e6f41bed7
📒 Files selected for processing (1)
docs/getting-started/quick-start.md
Comment on lines
+189
to
+193
| The defaults in `helm-prereqs/values/nico-site-agent.yaml` point at the Zalando-managed `nico-pg-cluster` (`DB_ADDR: nico-pg-cluster.postgres.svc.cluster.local`, `DB_DATABASE: nico_rest`), which is the same cluster used by `nico-rest-api`. No changes are needed for a standard deployment. | ||
|
|
||
| `DB_USER` and `DB_PASSWORD` are injected at runtime from the `db-creds` Kubernetes Secret (created by the `nico-rest-common` sub-chart during Phase 7g). The Secret is referenced via `secrets.dbCreds` in the site-agent values. | ||
|
|
||
| For production or a different database, override the Secret name and connection config: | ||
| For a non-standard database, override the connection config: |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== quick-start excerpt ==\n'
sed -n '180,200p' docs/getting-started/quick-start.md
printf '\n== site-agent values excerpt ==\n'
sed -n '1,120p' helm-prereqs/values/nico-site-agent.yamlRepository: NVIDIA/infra-controller
Length of output: 4565
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'db-creds|Phase 7g|secrets\.dbCreds|DB_ADDR|DB_DATABASE' docs/getting-started/quick-start.md helm-prereqs/values/nico-site-agent.yamlRepository: NVIDIA/infra-controller
Length of output: 4278
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
paths = [
Path('docs/getting-started/quick-start.md'),
Path('helm-prereqs/values/nico-site-agent.yaml'),
]
for path in paths:
print(f"\n== {path} ==")
text = path.read_text()
for needle in ['DB_ADDR', 'DB_DATABASE', 'db-creds', 'Phase 7g', 'secrets.dbCreds', 'postgres.postgres.svc.cluster.local', 'nico-pg-cluster.postgres.svc.cluster.local', 'elektratest', 'nico_rest']:
if needle in text:
print(f"contains: {needle}")
PYRepository: NVIDIA/infra-controller
Length of output: 582
Sync this section with the site-agent defaults. helm-prereqs/values/nico-site-agent.yaml still uses the dev PostgreSQL endpoint (postgres.postgres.svc.cluster.local / elektratest), so the nico-pg-cluster / nico_rest values here are stale and will point operators at the wrong database.
🤖 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/getting-started/quick-start.md` around lines 189 - 193, This section is
out of sync with the site-agent defaults and currently points to the wrong
database configuration. Update the quick-start text to match the actual
`helm-prereqs/values/nico-site-agent.yaml` settings used by the site agent, and
make sure the references to `DB_ADDR` and `DB_DATABASE` align with the
`nico-site-agent` defaults instead of the stale `nico-pg-cluster`/`nico_rest`
values.
Contributor
|
@shayan1995 For whatever reason, the Fern preview build doesn't seem to have happened. Have you tested this locally to make sure it builds, and to make sure you didn't break any links when you renamed headings? |
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.
Fix incorrect instruction in the quick-start guide that tells operators to run
nicocli site createaftersetup.sh. The site is already bootstrapped and registered bysetup.shPhase 7g — runningnicocli site createcreates a second site the already-deployed site-agent cannot use, causing the site to be stuck in Pending.Related issues
Closes #2245 - #2245
Type of Change
Breaking Changes
No breaking changes. This is a documentation correction — no code, chart, or config is modified.
Testing
Validated end-to-end on dev6:
setup.shPhase 7g bootstraps the site automatically,nicocli site listconfirms exactly one site registered matchingNICO_SITE_UUID, and the site-agent logs show successful bootstrap — nonicocli site createcall required.Additional Notes