Skip to content

chore(v2.0): Consolidate REST service DBs into shared nico-pg-cluster#3182

Merged
nv-dmendoza merged 1 commit into
NVIDIA:release/v2.0from
shayan1995:cherry-pick-3081-release-v2.0
Jul 7, 2026
Merged

chore(v2.0): Consolidate REST service DBs into shared nico-pg-cluster#3182
nv-dmendoza merged 1 commit into
NVIDIA:release/v2.0from
shayan1995:cherry-pick-3081-release-v2.0

Conversation

@shayan1995

@shayan1995 shayan1995 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Cherry-pick of #3081 (12ef832) onto release/v2.0, following the same process as #3181

Move the NICo REST API database from the standalone postgres.postgres StatefulSet onto the Zalando-managed nico-pg-cluster, eliminating the separate REST database instance.

Changes

  • postgresql.yaml: add nico-rest.nico user and nico_rest database, gated on rest.enabled (default: true)
  • eso-external-secrets.yaml: add nico-rest-db-eso ClusterExternalSecret syncing Zalando credentials to the nico-rest namespace as nico-rest-pg-creds — named to avoid collision with the db-creds hook managed by nico-rest-common (hook-delete-policy: before-hook-creation causes a Helm/ESO race if ESO targets db-creds directly)
  • values.yaml: add rest.enabled / rest.namespace toggles with documentation
  • setup.sh:
  • Wait (up to 120 s) for nico-rest-pg-creds to be synced by ESO; extract Zalando credentials and write to a chmod 600 temp file; pass via -f to helm upgrade --install to populate
    nico-rest-common.secrets.dbCreds.* at install time (avoids --set credential exposure and Helm special-char escaping issues)
  • Install pg_trgm extension on nico_rest after the cluster reaches Running state, with a 120 s bounded retry loop
  • nico-rest-db/values.yaml: update db.host / db.name / db.user defaults to nico-pg-cluster.postgres.svc.cluster.local / nico_rest / nico-rest.nico
  • nico-rest-api/values.yaml: same db defaults plus secrets.dbCreds: db-creds to enable the secret-based password path for live rotation

Related Issues

N/A

Type of Change

Change - Changes in existing functionality

Breaking Changes

This PR contains breaking changes.

Testing

Manual end-to-end validation performed on dev6 (rg-forge-dev6, 3-node cluster).

<!-- Describe what this PR does -->
Move the NICo REST API database from the standalone `postgres.postgres`
StatefulSet onto the Zalando-managed `nico-pg-cluster`, eliminating the
separate REST database instance.

## Changes
- **`postgresql.yaml`**: add `nico-rest.nico` user and `nico_rest`
database, gated on `rest.enabled` (default: `true`)
- **`eso-external-secrets.yaml`**: add `nico-rest-db-eso`
ClusterExternalSecret syncing Zalando credentials to the `nico-rest`
namespace as `nico-rest-pg-creds` — named to avoid collision with the
`db-creds` hook managed by `nico-rest-common` (`hook-delete-policy:
before-hook-creation` causes a Helm/ESO race if ESO targets `db-creds`
directly)
- **`values.yaml`**: add `rest.enabled` / `rest.namespace` toggles with
documentation
- **`setup.sh`**:
- Wait (up to 120 s) for `nico-rest-pg-creds` to be synced by ESO;
extract Zalando credentials and write to a `chmod 600` temp file; pass
via `-f` to `helm upgrade --install` to populate
`nico-rest-common.secrets.dbCreds.*` at install time (avoids `--set`
credential exposure and Helm special-char escaping issues)
- Install `pg_trgm` extension on `nico_rest` after the cluster reaches
`Running` state, with a 120 s bounded retry loop
- **`nico-rest-db/values.yaml`**: update `db.host` / `db.name` /
`db.user` defaults to `nico-pg-cluster.postgres.svc.cluster.local` /
`nico_rest` / `nico-rest.nico`
- **`nico-rest-api/values.yaml`**: same db defaults plus
`secrets.dbCreds: db-creds` to enable the secret-based password path for
live rotation

## Related Issues
N/A

## Type of Change
Change - Changes in existing functionality

## Breaking Changes
**This PR contains breaking changes.**

`rest.enabled` defaults to `true`. On upgrade, `nico-prereqs` will
create a `nico-rest.nico` Postgres user, a `nico_rest` database on
`nico-pg-cluster`, and a `nico-rest-db-eso` ClusterExternalSecret in
every site that does not explicitly set `rest.enabled: false`. Sites
that do **not** run `nico-rest` should add `rest.enabled: false` to
their nico-prereqs site values before upgrading to prevent these
resources from being created.

Additionally, `nico-rest-api` and `nico-rest-db` chart defaults now
point at `nico-pg-cluster` (`host`, `name`, `user`). Any site running
`nico-rest` against the old standalone instance must either migrate to
`nico-pg-cluster` via `setup.sh` or override these values explicitly.

## Testing

Manual end-to-end validation performed on dev6 (rg-forge-dev6, 3-node
cluster).

## Additional Notes
@shayan1995 shayan1995 requested a review from a team July 7, 2026 00:15
@shayan1995 shayan1995 requested a review from a team as a code owner July 7, 2026 00:15
@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 58bd2b86-a3e8-41c2-a2b7-6729bf2c7846

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

🔐 TruffleHog Secret Scan

No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

🕐 Last updated: 2026-07-07 00:19:15 UTC | Commit: 68114bf

@thossain-nv thossain-nv changed the title feat(db): consolidate REST into shared nico-pg-cluster (#3081) chore(v2.0): Consolidate REST service DBs into shared nico-pg-cluster Jul 7, 2026
@thossain-nv thossain-nv requested a review from nv-dmendoza July 7, 2026 00:21
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
boot-artifacts-aarch64 3 0 0 3 0 0
boot-artifacts-x86_64 3 0 0 3 0 0
forge-admin-cli-x86_64 270 5 25 89 7 144
machine-validation-runner 771 25 207 278 40 221
machine_validation 771 25 207 278 40 221
machine_validation-aarch64 771 25 207 278 40 221
nvmetal-carbide 771 25 207 278 40 221
TOTAL 3360 105 853 1207 167 1028

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

@parmani-nv parmani-nv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nv-dmendoza nv-dmendoza left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, needed consolidation of DB that landed after initial RC cutover

@nv-dmendoza nv-dmendoza merged commit 9ef0087 into NVIDIA:release/v2.0 Jul 7, 2026
117 checks passed
nv-dmendoza pushed a commit that referenced this pull request Jul 10, 2026
…elease/v2.0) (#3340)

<!-- Describe what this PR does -->
Cherry-pick of #3284 (`7b35d52`) onto `release/v2.0`, following the same
process as #3182.

Bind the site-agent to a REST-created site and fix the workflow-worker
DB target. Two defects left a fresh site with an empty `nicocli site
list` despite a healthy, handshaked site-agent: the site UUID was minted
without any REST database record behind it (the bootstrap Job's `POST
/v1/site` creates only the Site CR + OTP), and the workflow workers
missed the nico-pg-cluster consolidation (#3081), failing every DB
activity with SQLSTATE 42P01 so no site could ever leave `Pending`.

## Changes
- **`setup.sh`**: resolve the site UUID instead of blindly minting —
explicit `NICO_SITE_UUID` (bind to a pre-existing site), else the prior
install's `CLUSTER_ID` (site-agent ConfigMap), else adopt an existing
REST site with the same name, else mint. Then seed the REST DB directly
(same record shape as forged's per-env `site.sql`): a `default`
`infrastructure_provider` for the org plus a `Pending` site row named
after `siteName`, with the v2 networking capabilities
(`native_networking`, `network_security_group`, `flow`) enabled and the
`status_detail` row the REST create handler writes. All inserts
idempotent (`WHERE NOT EXISTS`); waits up to 120 s for the REST
migrations. IdP-agnostic — no API token needed. The bootstrap Job's
existing `POST /v1/site` flow is untouched and runs after the DB row
exists, under the same UUID.
- **`setup.sh`**: inject workflow-worker DB values at install time via
the existing REST creds temp file — `nico-rest-workflow` now targets
`nico-pg-cluster`/`nico_rest` as `nico-rest.nico` with the `db-creds`
secret, aligned with `nico-rest-api`.
- **`setup.sh`**: delete a `site-registration` secret bound to a stale
UUID so rebinding `CLUSTER_ID` re-bootstraps; input validation (UUID
format, site-name/org charset) before SQL interpolation;
quoting-agnostic `siteName` parsing.
- **Docs** (`quick-start.md`, `helm-prereqs/README.md`,
`configurability.md`): `NICO_SITE_UUID` resolution behavior documented
consistently.

## Related Issues
N/A

## Type of Change
Fix - Bug fixes

## Breaking Changes
None.

## Testing
Manual end-to-end validation on dev6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants