feat(postgres): hardcode PostgreSQL 17 for monitoring and add migration#2304
feat(postgres): hardcode PostgreSQL 17 for monitoring and add migration#2304IvanHunters merged 2 commits intomainfrom
Conversation
Add migration 37 to backfill spec.version=v17 for existing PostgreSQL resources without a version set. Hardcode PostgreSQL 17.7 image in monitoring databases (Grafana and Alerta) to ensure compatibility with monitoring queries that expect PostgreSQL 17 features (pg_stat_checkpointer, updated pg_stat_bgwriter). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
IvanHunters
requested review from
androndo,
kvaps,
lexfrei,
lllamnyp and
sircthulhu
as code owners
dosubot
bot
added
size:M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
📝 WalkthroughWalkthroughA new shell migration script is added to backfill Changes
Sequence DiagramsequenceDiagram
participant Migration as Migration Script (migrations/37)
participant K8sAPI as Kubernetes API
participant CRD as postgreses.apps.cozystack.io CRD
participant Resources as postgreses Resources
participant ConfigMap as cozystack-version ConfigMap
Migration->>K8sAPI: Check if `postgreses` CRD exists
alt CRD Not Found
Migration->>ConfigMap: Apply/Update `cozystack-version` -> version=38 (cozy-system)
Migration->>Migration: Exit successfully
else CRD Exists
Migration->>K8sAPI: List all `postgreses` across namespaces
K8sAPI-->>Resources: Return resource list
loop For each postgres resource
Migration->>K8sAPI: Get `.spec.version`
alt `.spec.version` non-empty
Migration->>Migration: Log skip
else `.spec.version` missing
Migration->>K8sAPI: Patch `.spec.version` = v17
Migration->>Migration: Log applied patch
end
end
Migration->>ConfigMap: Apply/Update `cozystack-version` -> version=38 (cozy-system)
Migration->>Migration: Complete
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a migration script to backfill the PostgreSQL version to v17 for existing resources and explicitly pins the PostgreSQL image to version 17.7 for Alerta and Grafana databases. The review feedback identifies critical inconsistencies in the migration versioning logic (where the script refers to version 38 despite being named 37) and a shell compatibility issue regarding the use of 'pipefail' in a POSIX sh script.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/platform/images/migrations/migrations/37 (1)
1-10: Shell compatibility:set -euo pipefailwith#!/bin/shis a bash extension, not POSIX-compliant.This is not unique to migration 37. The entrypoint script (
run-migrations.sh) and 19+ other migrations use the same#!/bin/sh+set -euo pipefailpattern. Migration 36 is exceptional in using onlyset -e. Since this pattern is established throughout the migration system and appears functional, consider this a consistency/hardening opportunity rather than a required fix.Option 1 (preferred): Align with migration 36's conservative approach
- Change to
set -eufor POSIX compliance with#!/bin/shOption 2: Use bash explicitly for migrations needing bash features
- Change shebang to
#!/bin/bash(like migrations 28, 29, 30 do for complex logic)For migration 37, the logic is simple and does not require pipefail. Aligning with migration 36 (
set -eu) is recommended.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/platform/images/migrations/migrations/37` around lines 1 - 10, The script uses a non-POSIX combination (#!/bin/sh) together with `set -euo pipefail`; update migration 37 to be POSIX-safe by replacing `set -euo pipefail` with `set -eu` (aligning with migration 36) so the script remains compatible with /bin/sh while keeping strict error handling; ensure the same change is applied consistently to other migrations using the same pattern or alternatively switch the shebang to `#!/bin/bash` if you intentionally need bash-only features.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core/platform/images/migrations/migrations/37`:
- Around line 1-10: The script uses a non-POSIX combination (#!/bin/sh) together
with `set -euo pipefail`; update migration 37 to be POSIX-safe by replacing `set
-euo pipefail` with `set -eu` (aligning with migration 36) so the script remains
compatible with /bin/sh while keeping strict error handling; ensure the same
change is applied consistently to other migrations using the same pattern or
alternatively switch the shebang to `#!/bin/bash` if you intentionally need
bash-only features.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a6d20069-f895-4c17-90ed-70b253e419dd
📒 Files selected for processing (3)
packages/core/platform/images/migrations/migrations/37packages/system/monitoring/templates/alerta/alerta-db.yamlpackages/system/monitoring/templates/grafana/db.yaml
Add explicit PostgreSQL 17.7 image to Harbor, SeaweedFS, and Keycloak databases to ensure consistent version across all system components. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
Successfully created backport PR for |
3 participants
Summary
This PR ensures PostgreSQL version consistency across the platform by:
Motivation
CloudNativePG operator defaults to PostgreSQL 18.3 when no explicit image is specified. However, monitoring queries are configured for PostgreSQL 17 features (pg_stat_checkpointer, updated pg_stat_bgwriter). This mismatch could cause issues with existing deployments.
Changes
Testing
Summary by CodeRabbit