pal-e-docs

Review: Critical: Re-establish orphaned CNPG cluster manifest (re-review)

review-423-2026-03-26-v2 Review

review ready
Project

Verdict: READY

Re-review of board item #423 after refinement (comment #7931 on forgejo_admin/pal-e-services#33). Previous review: review-423-2026-03-26 (NEEDS_REFINEMENT).

Template Completeness

  • [x] Type — Bug
  • [x] Lineage — standalone, discovered during #187 review
  • [x] Repo — forgejo_admin/pal-e-services
  • [x] What Broke — clear description of orphaned manifest with impact list
  • [x] Repro Steps — 4 concrete steps, all verified
  • [x] Expected Behavior — clear target state
  • [x] Environment — cluster, version, storage, backup, databases all documented
  • [x] Acceptance Criteria — 6 criteria (updated in refinement comment #7931)
  • [x] Related — 3 blocked issues + removed commit reference
  • [x] Architecture diagram — current vs target state visualization
  • [x] Cluster Spec — reconstructed from removed commit, verified against live state
  • [x] File Targets — present
  • [x] Constraints — critical safety notes about live cluster

Traceability

  • [ ] story:X label — missing, but this is foundational infrastructure repair. Acceptable.
  • [x] arch:postgres label — present on board item #423
  • [x] Forgejo issue — forgejo_admin/pal-e-services#33, open

File Targets

  • [x] ~/pal-e-services/ — verified: no pal-e-postgres references exist (grep confirms orphaned state)
  • [x] ~/pal-e-platform/terraform/main.tf — verified: only connection string references remain (line 2206: DATABASE_URL, line 2465: backup verification). Woodpecker ScheduledBackup exists at line 1656 but no pal-e-postgres ScheduledBackup. No pal-e-postgres Cluster resource.
  • [x] Live cluster — verified: kubectl get clusters.postgresql.cnpg.io -n postgres shows pal-e-postgres healthy
  • [x] Live ScheduledBackup — verified: kubectl get scheduledbackups.postgresql.cnpg.io -n postgres shows pal-e-postgres-daily, last backup 22h ago, next at 02:00 UTC
  • [x] Reconstructed Cluster spec — verified against live state: resources (cpu 100m/mem 256Mi request, 512Mi limit), storage (5Gi local-path), backup config (barman/MinIO/gzip/7d retention), bootstrap (paledocs/paledocs) all match
  • [x] ScheduledBackup spec — verified against live state: schedule 0 0 2 * * *, method barmanObjectStore, backupOwnerReference cluster

Repo Placement

Correct. Issue filed on pal-e-services where the manifest should land. Platform-level resources (namespace, S3 creds, MinIO bucket, CNPG operator) stay in pal-e-platform. App-level resources (CNPG clusters) go to pal-e-services. Woodpecker CNPG cluster stays in pal-e-platform as the exception (it IS a platform resource).

Dependencies

  • Blocks #417 (board item) — Critical: Migrate basketball-api Postgres to CNPG (pal-e-platform#187, open)
  • Blocks #419 (board item) — Critical: Migrate mcd-tracker Postgres to CNPG (pal-e-platform#189, open)
  • Blocks #420 (board item) — Critical: Migrate pal-e-mail Postgres to CNPG (pal-e-platform#190, open)
  • No blockers — CNPG operator, namespace, S3 creds, MinIO bucket all exist and are healthy
  • Dependencies are well-documented in the issue body

Acceptance Criteria

Using the updated AC from refinement comment #7931 (supersedes body AC):
  • [x] AC1: "CNPG Cluster manifest exists in pal-e-services repo" — clear, verifiable
  • [x] AC2: "CNPG ScheduledBackup manifest exists in pal-e-services repo" — clear, verifiable. Addresses Fix 1 from previous review.
  • [x] AC3: "Both manifests match running state (verified via kubectl diff)" — clear, verifiable
  • [x] AC4: "kubectl apply is idempotent" — clear, verifiable
  • [x] AC5: "Process for adding databases documented (SQL-based on running cluster)" — clear, verifiable. Addresses Fix 2 from previous review. No longer contradicts Constraints section.
  • [x] AC6: "Documentation updated: which repo owns which CNPG resource" — clear, verifiable

Blast Radius

  • ScheduledBackup now in scope — confirmed orphaned via grep (only woodpecker ScheduledBackup exists in IaC at pal-e-platform main.tf:1656). Live pal-e-postgres-daily is healthy with recent backups.
  • Backup verification CronJob — pal-e-platform main.tf (line 2465) references pal-e-postgres in its backup verification loop. Checks MinIO objects, not the manifest. No impact.
  • pal-e-docs DATABASE_URL — pal-e-platform main.tf (line 2206) references pal-e-postgres-rw.postgres.svc.cluster.local. Connection string remains valid. No impact.
  • No similar orphaned patterns found in other repos.

Process Note

The refinement was applied as a comment addendum (comment #7931) rather than updating the issue body. The issue body still shows the original 5 AC (including the problematic AC4 wording). An implementing agent must read both the body AND the refinement comment. This is acceptable for this ticket since the comment explicitly labels itself as superseding the body's AC section, but future refinements should update the body directly to avoid ambiguity.

Recommendation

No blocking action needed. Scope is complete and correct when reading body + refinement comment together. Both original review findings have been addressed:
  • ScheduledBackup added to scope — AC2 now requires the ScheduledBackup manifest
  • AC4 reworded — now correctly describes SQL-based database creation, no longer contradicts Constraints
Ticket is ready for execution.