PMM 15014 Upgrade internal PostgreSQL version from 14 to 18 - #5295
Open
talhabinrizwan wants to merge 21 commits into
Open
PMM 15014 Upgrade internal PostgreSQL version from 14 to 18#5295talhabinrizwan wants to merge 21 commits into
talhabinrizwan wants to merge 21 commits into
Conversation
PostgreSQL 15+ revoked the default CREATE privilege on the public schema from all users. Explicit grants are now required in the build-time Ansible role, the initialization role, and the pg14→pg18 migration script.
PostgreSQL 15+ revoked the default CREATE privilege on the public schema from all users. Add an explicit grant in initWithRoot() after provisioning the database and role.
PostgreSQL 15+ revoked the default CREATE privilege on the public schema from all users. Add ensureSchemaGrant() called unconditionally in SetupDB before migrateDB, covering both fresh installs and existing containers where the role was provisioned without the grant. Also fix a pre-existing bug in initWithRoot where GRANT ALL PRIVILEGES used $1/$2 placeholders, which are not supported for identifiers in PostgreSQL.
Add back the scram-sha-256 authentication option alongside trust in the dev container's pg_hba.conf configuration, preserving the comments explaining the dual-auth setup for convenience in dev environments.
talhabinrizwan
requested review from
JiriCtvrtka and
maxkondr
and removed request for
a team
April 27, 2026 08:06
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5295 +/- ##
==========================================
+ Coverage 43.59% 45.43% +1.84%
==========================================
Files 415 418 +3
Lines 43134 43332 +198
==========================================
+ Hits 18804 19690 +886
+ Misses 22454 21701 -753
- Partials 1876 1941 +65
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
@talhabinrizwan It still needs some changes in managed to pass tests there properly. |
The Go template for the PostgreSQL supervisord config had an extra -c max_connections=2000 flag that was not present in the original PG14 template and does not match the test fixture (pmm-db_enabled.ini), causing TestSavePMMConfig to fail. Removed to restore parity with prior behavior.
maxkondr
reviewed
Apr 29, 2026
| fi | ||
| } | ||
|
|
||
| upgrade_pg14_to_pg18 |
Contributor
There was a problem hiding this comment.
this func uses /srv/.pgpassword file but is called before ensure_postgres_password that creates this file in case it is absent
Conflict resolutions: - .devcontainer/setup.py: deleted in favour of main's setup.sh; the PG18 data-dir rename ported over to it. - build/ansible/roles/grafana/files/datasources.yml: took main's removal of the PostgreSQL datasource, dropping the postgresVersion bump. - build/ansible/roles/postgres/tasks/main.yml: kept the PG18 rename and the public-schema grant, dropped the grafana data-migration task removed on main. - managed/models/database.go: kept the ensureSchemaGrant call, adapted to main's non-inline error handling. - managed/testdata/supervisord.d/pmm-ch_low_memory.ini: new fixture on main, renamed to PG18 to match the supervisord template.
Add a section describing the automatic pg14 to pg18 upgrade performed by build/ansible/roles/postgres/files/postgres-migration on the first start of a PMM Server that ships PostgreSQL 18: the trigger conditions, the dump and restore steps, the role and schema grants required by PostgreSQL 15+, and the rollback path. Also scope the existing section to the v11 to v14 upgrade it describes and fix a few typos in it.
catalinaadam
temporarily deployed
to
PMM-15014-upgrade-to-pg18 - pmm-doc-3.9.0 PR #5295
July 30, 2026 14:22 — with 
Render
Destroyed
Render
Destroyed
Since PostgreSQL 15 the public schema belongs to pg_database_owner, and GRANT ALL PRIVILEGES ON DATABASE does not confer CREATE on it. The branch worked around that by granting CREATE on public in five separate places: in Go on every pmm-managed start, in three ansible tasks and inline in the upgrade script. Create each database with its role as the owner instead, which confers the privilege implicitly and leaves one mechanism: - initWithRoot creates the role before the database and passes OWNER; the now-redundant GRANT ALL PRIVILEGES and the whole ensureSchemaGrant function are gone, so SetupDB no longer reads the superuser password and opens a second connection pool on every start. - The postgresql_privs tasks become owner: on the existing postgresql_db tasks, and priv: ALL on the user tasks is dropped as redundant. - The upgrade script already created the databases with OWNER, so its two GRANT statements are dropped. Also thread ctx through initWithRoot to use the Context query variants, collapse the duplicated grafana restore block into a loop over both databases, drop a redundant PGPASSWORD unset/re-read cycle, and hoist the repeated PostgreSQL bin path into PG_BIN. The external PostgreSQL setup instructions had the same problem and would leave PMM with a database it cannot create tables in, so they now use OWNER too.
The entrypoint created the PostgreSQL cluster on a fresh installation and the migration script created one for the pg14 upgrade, so the data dir creation, the initdb flag set and the pg_stat_statements call existed twice. A change to either had to be mirrored, and a divergence would only show up as an auth failure on one installation flavour. Give the script sole ownership of the embedded cluster. It gains init_postgres_cluster for the fresh case, and both paths now share initdb_cluster, create_pg_stat_statements and store_postgres_password. The entrypoint just invokes the script, which it already did. init_postgres_cluster runs after upgrade_pg14_to_pg18, so an upgradable data directory can no longer be replaced by an empty one; previously that ordering rested on /srv/pmm-distribution being present. Creation also happens after /run/postgresql is created rather than before it.
update_pg_hba_auth was added alongside the switch to scram (f2d3c3a) to retrofit clusters created earlier with initdb --auth=trust. Those clusters are PostgreSQL 14 ones, and every PostgreSQL 18 data directory is created by initdb_cluster or the ansible role with --auth-host=scram-sha-256, so the condition it looks for can no longer arise: pg_hba.conf is not carried across the dump and restore, and upgrading an --auth=trust cluster already yields a scram-only one. Its remaining effect was to revert the trust rule the dev container adds, on every container start.
Member
Note on
|
minPGVersion |
PG 14.22 | PG 16.14 | PG 18.3 |
|---|---|---|---|
| 18 | rejected | rejected | 118/118 |
| 15 | rejected | 118/118 | 118/118 |
| 14 | 118/118 | 118/118 | 118/118 |
Nothing in the schema requires 15+, so the floor is purely a support-policy choice. 15 is a defensible line because it is where public stopped being writable by PUBLIC and became owned by pg_database_owner — the ownership model this PR adopts for provisioning. PostgreSQL 14 reaches end of life this autumn, so 14 is not a viable final value either way.
The unit test job runs against a prebuilt dev container image that is built from main and still embeds PostgreSQL 14.23, so a floor of 18 makes checkVersion reject it on the first database-backed test and every later test cascades on the leftover connection. Lower the floor so the suite can run. This must be restored before merging; the constant is marked and the pull request description says so.
Same reason as the minPGVersion floor: the unit test job runs against a prebuilt dev container built from main, whose supervisord still writes /srv/logs/postgresql14.log. Restore before merging.
/srv/logs/postgresql18.log becomes /srv/logs/postgresql.log, so the name survives future major upgrades instead of changing with each one. This also matches what the troubleshooting documentation already describes. Covers the supervisord template, the ansible supervisord config and log file task, both supervisord test fixtures and the api-tests expectation. The unit test in managed/services/server/logs_test.go still expects the old name because it runs against the prebuilt dev container; its TODO now points at postgresql.log. Also corrects the minPGVersion marker to name 15 as the pre-merge target.
countDatabases and countRoles become dbCount and roleCount.
CREATE USER, ALTER USER and CREATE DATABASE were built with fmt.Sprintf,
interpolating params.Username and params.Password into a double-quoted
identifier and a single-quoted literal. Either value could close its quote
and append further statements. initWithRoot connects as the postgres
superuser, and these calls pass no bind arguments, so lib/pq uses the simple
query protocol, which accepts several statements in one Exec: a username of
inj" LOGIN PASSWORD 'x'; CREATE ROLE pwned SUPERUSER; --
creates a superuser role, and the equivalent payload in the password does so
while initWithRoot still returns nil. gosec does not catch this because G201
only matches DML keywords, not CREATE.
These statements take no bind parameters, so quote the values with
pq.QuoteIdentifier and pq.QuoteLiteral. This also fixes ordinary passwords
containing a quote, which previously failed with a syntax error.
The values come from PMM_POSTGRES_USERNAME and PMM_POSTGRES_DBPASSWORD, so
this is not remotely reachable; the CREATE USER site predates the PostgreSQL
18 work, while the ALTER USER one was added by it.
WalkthroughThis change upgrades embedded PostgreSQL from version 14 to 18. It adds staged migration and fresh-cluster initialization, updates runtime paths and ownership provisioning, and revises documentation and log expectations. ChangesPostgreSQL 18 upgrade
Sequence Diagram(s)sequenceDiagram
participant ServerEntrypoint
participant PostgresMigration
participant PostgreSQL14
participant PostgreSQL18
ServerEntrypoint->>PostgresMigration: run embedded database setup
PostgresMigration->>PostgreSQL14: dump available databases
PostgresMigration->>PostgreSQL18: initialize staged cluster
PostgresMigration->>PostgreSQL18: recreate roles, restore dumps, enable pg_stat_statements
PostgresMigration->>ServerEntrypoint: publish PostgreSQL 18 cluster
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
build/docker/server/entrypoint.sh (1)
9-9: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
POSTGRES_DATA_DIRis hardcoded here but overridable inpostgres-migration.
build/ansible/roles/postgres/files/postgres-migrationdeclares this as"${POSTGRES_DATA_DIR:-/srv/postgres18}"(override-friendly), while this file hardcodes/srv/postgres18. If anything (e.g. tests) relies on overriding this path, the two scripts would silently disagree on the effective data directory.♻️ Proposed fix
-declare POSTGRES_DATA_DIR="/srv/postgres18" +declare POSTGRES_DATA_DIR="${POSTGRES_DATA_DIR:-/srv/postgres18}"🤖 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 `@build/docker/server/entrypoint.sh` at line 9, Update the POSTGRES_DATA_DIR declaration in the entrypoint script to use the existing environment value when provided, falling back to /srv/postgres18 otherwise, matching the override-friendly behavior of postgres-migration.build/docs/MIGRATION.md (2)
65-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a language tag to this fenced code block.
Static analysis (MD040) flags this fence as missing a language identifier.
📝 Proposed fix
-3. Move the database directory to /srv/backup/postgres14 -``` +3. Move the database directory to /srv/backup/postgres14 +```sh mv /srv/postgres14 /srv/backup/</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@build/docs/MIGRATION.mdaround lines 65 - 68, Update the fenced code block
under “Move the database directory to /srv/backup/postgres14” by adding the sh
language tag to its opening fence, leaving the command content unchanged.</details> <!-- cr-comment:v1:5a875c40fe6d38f0d3c89043 --> _Source: Linters/SAST tools_ --- `99-152`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _⚡ Quick win_ **New v14→v18 migration section is accurate against the script; only missing code-fence languages.** The described trigger conditions, dump/restore mechanics, role/database recreation, and rollback steps all match `postgres-migration` and `entrypoint.sh` as reviewed. Static analysis (MD040) flags four fences in this section (lines 111, 124, 131, 141) missing a language identifier — worth adding `sh`/`bash`/`sql` tags for consistency with the rest of the doc. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@build/docs/MIGRATION.mdaround lines 99 - 152, Add language identifiers to
the four code fences in the PostgreSQL v14-to-v18 migration section: use shell
syntax tags for command blocks and the appropriate SQL tag for SQL statements,
matching the existing documentation convention and satisfying MD040.</details> <!-- cr-comment:v1:d556a1a4c4b77d3e9ad54555 --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>build/ansible/roles/postgres/files/postgres-migration (3)</summary><blockquote> `11-14`: _🔒 Security & Privacy_ | _🔵 Trivial_ | _⚡ Quick win_ **Brief permission-gap window when writing the password file.** `echo -n ... > file` creates the file with default (umask-derived) permissions before `chmod 600` narrows them, leaving a short window where the password file is world/group-readable. Consider writing under a restrictive `umask` or using `install -m 600 /dev/null "$POSTGRES_PASSWORD_FILE"` first. <details> <summary>🔒️ Proposed fix</summary> ```diff store_postgres_password() { - echo -n "$1" > "$POSTGRES_PASSWORD_FILE" - chmod 600 "$POSTGRES_PASSWORD_FILE" + install -m 600 /dev/null "$POSTGRES_PASSWORD_FILE" + echo -n "$1" > "$POSTGRES_PASSWORD_FILE" }🤖 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 `@build/ansible/roles/postgres/files/postgres-migration` around lines 11 - 14, Update store_postgres_password so the password file is created with restrictive permissions before its contents are written, using a temporary umask or pre-creating it with mode 600 via install. Preserve the existing password value and final file location.
57-95: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRestrict permissions on the backup directory holding plain-text SQL dumps.
mkdir -p "$BACKUP_DIR"andpg_dump -f ...don't set restrictive permissions; the resulting dumps ofpmm-managed/grafanamay contain credentials or other sensitive data and end up readable per the container's default umask.🔒️ Suggested fix
- mkdir -p "$BACKUP_DIR" + install -d -m 700 "$BACKUP_DIR"🤖 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 `@build/ansible/roles/postgres/files/postgres-migration` around lines 57 - 95, Update the backup-directory setup around mkdir and the pg_dump calls to ensure the directory and generated plain-text SQL dumps are restricted to the intended owner, independent of the container umask. Apply restrictive permissions when creating or preparing BACKUP_DIR and ensure each dump file is created with owner-only access before restoration.
148-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent binary path:
/usr/bin/psqlinstead of$PG_BIN/psql.Every other database connection in this file now goes through
"$PG_BIN/..."(the earlier reviewer's suggestion to centralize binary paths), but line 153 still hardcodes/usr/bin/psql. If that path resolves to a different major-version client than the pg18 server, this is the one place the refactor was missed.♻️ Proposed fix
- PGPASSWORD="$POSTGRES_PASSWORD" /usr/bin/psql -U postgres -h /run/postgresql -d postgres \ + PGPASSWORD="$POSTGRES_PASSWORD" "$PG_BIN/psql" -U postgres -h /run/postgresql -d postgres \ -c "ALTER USER postgres WITH PASSWORD '${POSTGRES_PASSWORD}'"🤖 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 `@build/ansible/roles/postgres/files/postgres-migration` around lines 148 - 161, Update the psql invocation in the migration function to use the centralized "$PG_BIN/psql" path instead of hardcoded /usr/bin/psql, while preserving its existing arguments and password-setting behavior.
🤖 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 `@build/ansible/roles/postgres/files/postgres-migration`:
- Around line 38-51: Update upgrade_pg14_to_pg18 to validate that
POSTGRES_PASSWORD_FILE exists and is readable before reading it or invoking
initdb_cluster. If it is missing, emit a clear migration-specific diagnostic and
return or fail explicitly, preventing initdb --pwfile from receiving an invalid
path; keep the existing password-loading flow for valid files.
In `@build/docker/server/entrypoint.sh`:
- Line 9: Update POSTGRES_DATA_DIR in build/docker/server/entrypoint.sh to use
the "${POSTGRES_DATA_DIR:-/srv/postgres18}" defaulting pattern, matching the
reference in build/ansible/roles/postgres/files/postgres-migration (lines 5-8),
which requires no change.
In `@documentation/docs/reference/third-party/postgresql.md`:
- Line 13: Update the PostgreSQL Docker examples in the PMM configuration guide
to use a PostgreSQL 18+ image tag instead of postgres:14, and change the pg_data
volume mount to the PostgreSQL 18 data directory layout. Apply both changes
together in every affected snippet.
In `@managed/services/supervisord/pmm_config.go`:
- Line 135: Update the PostgreSQL log filename expectation in logs_test.go from
postgresql14.log to postgresql.log to match the supervisord template, and
restore the temporary minPGVersion value in the database model as required by
the PR objective.
---
Nitpick comments:
In `@build/ansible/roles/postgres/files/postgres-migration`:
- Around line 11-14: Update store_postgres_password so the password file is
created with restrictive permissions before its contents are written, using a
temporary umask or pre-creating it with mode 600 via install. Preserve the
existing password value and final file location.
- Around line 57-95: Update the backup-directory setup around mkdir and the
pg_dump calls to ensure the directory and generated plain-text SQL dumps are
restricted to the intended owner, independent of the container umask. Apply
restrictive permissions when creating or preparing BACKUP_DIR and ensure each
dump file is created with owner-only access before restoration.
- Around line 148-161: Update the psql invocation in the migration function to
use the centralized "$PG_BIN/psql" path instead of hardcoded /usr/bin/psql,
while preserving its existing arguments and password-setting behavior.
In `@build/docker/server/entrypoint.sh`:
- Line 9: Update the POSTGRES_DATA_DIR declaration in the entrypoint script to
use the existing environment value when provided, falling back to
/srv/postgres18 otherwise, matching the override-friendly behavior of
postgres-migration.
In `@build/docs/MIGRATION.md`:
- Around line 65-68: Update the fenced code block under “Move the database
directory to /srv/backup/postgres14” by adding the sh language tag to its
opening fence, leaving the command content unchanged.
- Around line 99-152: Add language identifiers to the four code fences in the
PostgreSQL v14-to-v18 migration section: use shell syntax tags for command
blocks and the appropriate SQL tag for SQL statements, matching the existing
documentation convention and satisfying MD040.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 12dce150-fd4c-4375-b68d-348d09a0c4ee
📒 Files selected for processing (17)
.devcontainer/setup.shapi-tests/server/logs_test.gobuild/ansible/roles/initialization/tasks/main.ymlbuild/ansible/roles/pmm-images/tasks/main.ymlbuild/ansible/roles/postgres/files/postgres-migrationbuild/ansible/roles/postgres/tasks/backup.ymlbuild/ansible/roles/postgres/tasks/main.ymlbuild/ansible/roles/postgres/tasks/restore.ymlbuild/ansible/roles/supervisord/files/pmm.inibuild/docker/server/entrypoint.shbuild/docs/MIGRATION.mddocumentation/docs/reference/third-party/postgresql.mdmanaged/models/database.gomanaged/services/server/logs_test.gomanaged/services/supervisord/pmm_config.gomanaged/testdata/supervisord.d/pmm-ch_low_memory.inimanaged/testdata/supervisord.d/pmm-db_enabled.ini
|
|
||
| ## Prerequisites | ||
| Before configuring PMM with an external PostgreSQL database, ensure you have a PostgreSQL 14+ server accessible from your PMM Server. | ||
| Before configuring PMM with an external PostgreSQL database, ensure you have a PostgreSQL 18+ server accessible from your PMM Server. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'postgres:14|/var/lib/postgresql/data' documentation/docs/reference/third-party/postgresql.mdRepository: percona/pmm
Length of output: 320
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' documentation/docs/reference/third-party/postgresql.md | cat -nRepository: percona/pmm
Length of output: 14943
🌐 Web query:
PostgreSQL 18 official Docker image default data directory PGDATA /var/lib/postgresql/18/data
💡 Result:
For the official PostgreSQL 18 Docker image, the default PGDATA environment variable is set to /var/lib/postgresql/18/docker [1][2]. This represents a change from previous versions of the official image. Starting with PostgreSQL 18, the image's PGDATA path was updated to be version-specific to match the pg_ctlcluster standard directory structure [1][3]. Additionally, the volume defined in the Dockerfile for PostgreSQL 18 and later is /var/lib/postgresql [1][2], rather than /var/lib/postgresql/data used in earlier releases [1][4]. Users are advised to target their mounts and volumes at /var/lib/postgresql to facilitate easier upgrades between PostgreSQL major releases using features like pg_upgrade --link [1][5]. For users who prefer to maintain the older directory structure or who are migrating existing data, PGDATA can be set explicitly via environment variables [1][3].
Citations:
- 1: https://hub.docker.com/_/postgres?ordering=last_updated&page=1&tab=description
- 2: https://github.com/docker-library/postgres/blob/master/18/bookworm/Dockerfile
- 3: Change
PGDATAin 18+ to/var/lib/postgresql/MAJOR/dockerdocker-library/postgres#1259 - 4: https://hub.docker.com/_/postgres
- 5: https://docs.docker.com/guides/postgresql/advanced-configuration-and-initialization/
Update the PostgreSQL example to match the 18+ requirement The Docker snippets still use postgres:14 and -v pg_data:/var/lib/postgresql/data, so the guide currently tells users to provision an unsupported version and the wrong volume layout for PostgreSQL 18. Update the image tag and mount path together.
🤖 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 `@documentation/docs/reference/third-party/postgresql.md` at line 13, Update
the PostgreSQL Docker examples in the PMM configuration guide to use a
PostgreSQL 18+ image tag instead of postgres:14, and change the pg_data volume
mount to the PostgreSQL 18 data directory layout. Apply both changes together in
every affected snippet.
The Helm chart test that installs the last released PMM 2, upgrades to v3
and uninstalls left the pmm container in CrashLoopBackOff:
PostgreSQL 14 data found at /srv/postgres14. Migrating to PostgreSQL 18...
cat: /srv/.postgres_password: No such file or directory
/srv/.postgres_password only arrived in PMM 3.7, so a data directory from
PMM 2 does not have one, and reading it unconditionally under errexit killed
the script and, with it, the entrypoint. Generate a password when the file is
missing, since initdb seeds the new cluster's postgres role from it; an
existing file is left alone so the superuser password is preserved. The dump
itself needs no password because it connects over the socket, which pg_hba
matches with a local trust rule.
Dumping also assumed both databases exist. A PMM 2 installation need not have
kept Grafana in PostgreSQL, and pg_dump of a missing database aborted the
upgrade the same way, so dump only the databases that are present. This
mirrors the restore loop, which already skips absent dumps.
Verified against real clusters for a PMM 2 directory with both databases, one
without grafana, a PMM 3.7+ directory with a password file, and an empty
/srv: all complete, restore the data with the right owners, produce a
scram-only pg_hba, and are no-ops on re-run.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
build/ansible/roles/postgres/files/postgres-migration (2)
89-93: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftMake failed migrations retryable.
initdb_clustercreates/srv/postgres18before restoration completes. Any failure or interruption before Line 120 leaves that directory behind; the next run returns at Line 43 and can start an incomplete pg18 cluster while pg14 remains. Stage the new cluster in a temporary directory and rename it into place only after a successful restore, or clean up the newly-created pg18 directory on failure.🤖 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 `@build/ansible/roles/postgres/files/postgres-migration` around lines 89 - 93, Update the migration flow around initdb_cluster and the pg_ctl startup/restore sequence so a failed or interrupted restoration cannot leave /srv/postgres18 as a reusable partial cluster. Stage initialization in a temporary directory and atomically rename it into the final location only after restoration succeeds, or ensure failure cleanup removes only the newly created cluster while preserving existing pg14 data.
109-109: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAbort on the first SQL restore error.
psql -fcan continue after SQL errors unlessON_ERROR_STOPis set, which risks a partial restore before the pg14 directory is retired. Add-v ON_ERROR_STOP=1.Proposed fix
- "$PG_BIN/psql" -h /run/postgresql -U postgres -d "$db" -f "$dump" + "$PG_BIN/psql" -v ON_ERROR_STOP=1 -h /run/postgresql -U postgres -d "$db" -f "$dump"🤖 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 `@build/ansible/roles/postgres/files/postgres-migration` at line 109, Update the psql restore invocation to pass the ON_ERROR_STOP variable set to 1, ensuring the dump process aborts on the first SQL error while preserving the existing host, user, database, and dump-file arguments.build/docs/MIGRATION.md (1)
88-89: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore these SQL dumps with
psql.pg_dumpwrites plain-text.sqlfiles here, sopg_restore --file=...will not load them. Usepsql -X --dbname=pmm-managed -f /srv/backup/pmm-managed.sqlandpsql -X --dbname=grafana -f /srv/backup/grafana.sqlinstead.🤖 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 `@build/docs/MIGRATION.md` around lines 88 - 89, Replace the two pg_restore commands in the migration instructions with psql commands using -X, the respective pmm-managed and grafana databases via --dbname, and the matching SQL dump via -f; preserve the existing PGPASSWORD and connection context.
🤖 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 `@build/docs/MIGRATION.md`:
- Around line 122-123: Update the migration instructions so the missing
/srv/.postgres_password generation condition is performed before Step 1, before
the pg14 startup and dump commands read it. Keep the existing-password path
unchanged and remove the duplicated or later-only generation step from Step 2,
ensuring pre-PMM 3.7 volumes follow the documented sequence.
---
Outside diff comments:
In `@build/ansible/roles/postgres/files/postgres-migration`:
- Around line 89-93: Update the migration flow around initdb_cluster and the
pg_ctl startup/restore sequence so a failed or interrupted restoration cannot
leave /srv/postgres18 as a reusable partial cluster. Stage initialization in a
temporary directory and atomically rename it into the final location only after
restoration succeeds, or ensure failure cleanup removes only the newly created
cluster while preserving existing pg14 data.
- Line 109: Update the psql restore invocation to pass the ON_ERROR_STOP
variable set to 1, ensuring the dump process aborts on the first SQL error while
preserving the existing host, user, database, and dump-file arguments.
In `@build/docs/MIGRATION.md`:
- Around line 88-89: Replace the two pg_restore commands in the migration
instructions with psql commands using -X, the respective pmm-managed and grafana
databases via --dbname, and the matching SQL dump via -f; preserve the existing
PGPASSWORD and connection context.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dbb68baa-6c36-4248-a37a-4e70e5649abe
📒 Files selected for processing (2)
build/ansible/roles/postgres/files/postgres-migrationbuild/docs/MIGRATION.md
The renamed /srv/postgres14.old is the rollback artifact and is a complete copy of the old cluster, so keeping the plain-text dumps as well leaves a second copy of every database sitting in /srv for no benefit. Delete them after the rename. Only the pg18-upgrade-*.sql files written by this script are removed, since /srv/backup also holds user backups taken through the backup playbook. The rename is the last step before this, and errexit aborts earlier on any failure, so a failed upgrade still leaves its dumps behind to inspect.
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 `@build/ansible/roles/postgres/files/postgres-migration`:
- Around line 125-126: Restrict the cleanup in the postgres-migration upgrade
flow to dump files created by the current invocation instead of using the
unscoped "${BACKUP_DIR}"/pg18-upgrade-*.sql wildcard. Track each exact dump path
or place upgrade artifacts in a dedicated temporary directory, then remove only
those paths after the rename while leaving unrelated user backups untouched.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c570f345-b1bc-4570-b1da-85dceebf3052
📒 Files selected for processing (2)
build/ansible/roles/postgres/files/postgres-migrationbuild/docs/MIGRATION.md
🚧 Files skipped from review as they are similar to previous changes (1)
- build/docs/MIGRATION.md
Both cluster-creation paths wrote straight into /srv/postgres18, so an interrupted attempt left a half-built directory behind. Since the upgrade is guarded on that directory being absent and the fresh install on it being present, neither would run again: the next start came up on an empty cluster while the PostgreSQL 14 data sat untouched, with no retry and no complaint. Create the cluster in /srv/postgres18.new and move it into place once it is finished, which makes the directory's existence mean "completed" rather than "attempted". The move is a rename within /srv, so it is atomic, and a stale staging directory from an earlier attempt is discarded before retrying. initdb_cluster now takes the target directory, and publish_cluster performs the move. Verified by failing the upgrade just after initdb: nothing is published, and the next run completes and restores the data.
ademidoff
approved these changes
Jul 31, 2026
Member
|
@coderabbitai configuration |
|
Configuration used: Organization UI CodeRabbit configuration# Source: Organization UI (base)
language: en-US
# Source: Organization UI (base)
tone_instructions: ''
# Source: Organization UI (base)
early_access: false
# Source: Organization UI (base)
enable_free_tier: true
# Source: defaults
inheritance: false
reviews:
# Source: Organization UI (base)
profile: chill
# Source: Organization UI (base)
request_changes_workflow: false
# Source: Organization UI (base)
high_level_summary: true
# Source: defaults
high_level_summary_instructions: ''
# Source: Organization UI (base)
high_level_summary_placeholder: '`@coderabbitai` summary'
# Source: Organization UI (base)
high_level_summary_in_walkthrough: false
# Source: Organization UI (base)
auto_title_placeholder: '`@coderabbitai`'
# Source: Organization UI (base)
auto_title_instructions: ''
# Source: Organization UI (base)
review_status: true
# Source: defaults
review_details: false
# Source: defaults
review_progress: true
# Source: Organization UI (base)
commit_status: true
# Source: Organization UI (base)
fail_commit_status: false
# Source: Organization UI (base)
collapse_walkthrough: false
# Source: Organization UI (base)
changed_files_summary: true
# Source: Organization UI (base)
sequence_diagrams: true
# Source: defaults
estimate_code_review_effort: true
# Source: Organization UI (base)
assess_linked_issues: true
# Source: Organization UI (base)
related_issues: true
# Source: Organization UI (base)
related_prs: true
# Source: Organization UI (base)
suggested_labels: true
# Source: defaults
labeling_instructions: []
# Source: defaults
mutually_exclusive_groups: {}
# Source: Organization UI (base)
auto_apply_labels: false
# Source: Organization UI (base)
suggested_reviewers: true
# Source: Organization UI (base)
auto_assign_reviewers: false
# Source: defaults
suggested_reviewers_instructions: []
# Source: defaults
in_progress_fortune: true
# Source: Organization UI (base)
poem: true
# Source: defaults
enable_prompt_for_ai_agents: true
# Source: defaults
path_filters: []
# Source: defaults
path_instructions: []
# Source: Organization UI (base)
abort_on_close: true
# Source: Organization UI (base)
disable_cache: false
slop_detection:
# Source: defaults
enabled: true
auto_review:
# Source: Organization UI (base)
enabled: true
# Source: defaults
description_keyword: ''
# Source: Organization UI (base)
auto_incremental_review: true
# Source: defaults
auto_pause_after_reviewed_commits: 5
# Source: defaults
ignore_title_keywords: []
# Source: defaults
labels: []
# Source: Organization UI (base)
drafts: false
# Source: defaults
base_branches: []
# Source: defaults
ignore_usernames: []
finishing_touches:
docstrings:
# Source: Organization UI (base)
enabled: true
unit_tests:
# Source: Organization UI (base)
enabled: true
simplify:
# Source: defaults
enabled: false
autofix:
# Source: defaults
enabled: true
fix_ci:
# Source: defaults
enabled: true
resolve_merge_conflict:
# Source: defaults
enabled: true
# Source: defaults
custom: []
pre_merge_checks:
# Source: defaults
override_requested_reviewers_only: false
docstrings:
# Source: defaults
mode: warning
# Source: defaults
threshold: 80
title:
# Source: defaults
mode: warning
# Source: defaults
requirements: ''
description:
# Source: defaults
mode: warning
issue_assessment:
# Source: defaults
mode: warning
# Source: defaults
custom_checks: []
# Source: defaults
post_merge_actions: []
tools:
ast-grep:
# Source: defaults
rule_dirs: []
# Source: defaults
util_dirs: []
# Source: Organization UI (base)
essential_rules: true
# Source: defaults
packages: []
shellcheck:
# Source: Organization UI (base)
enabled: true
ruff:
# Source: Organization UI (base)
enabled: true
markdownlint:
# Source: Organization UI (base)
enabled: true
github-checks:
# Source: Organization UI (base)
enabled: true
# Source: Organization UI (base)
timeout_ms: 90000
languagetool:
# Source: Organization UI (base)
enabled: true
# Source: defaults
enabled_rules: []
# Source: defaults
disabled_rules: []
# Source: defaults
enabled_categories: []
# Source: defaults
disabled_categories: []
# Source: Organization UI (base)
enabled_only: false
# Source: Organization UI (base)
level: default
biome:
# Source: Organization UI (base)
enabled: true
hadolint:
# Source: Organization UI (base)
enabled: true
swiftlint:
# Source: Organization UI (base)
enabled: true
phpstan:
# Source: Organization UI (base)
enabled: true
# Source: Organization UI (base)
level: default
phpmd:
# Source: defaults
enabled: true
phpcs:
# Source: defaults
enabled: true
golangci-lint:
# Source: Organization UI (base)
enabled: true
yamllint:
# Source: Organization UI (base)
enabled: true
gitleaks:
# Source: Organization UI (base)
enabled: true
trufflehog:
# Source: defaults
enabled: true
checkov:
# Source: Organization UI (base)
enabled: true
tflint:
# Source: defaults
enabled: true
detekt:
# Source: Organization UI (base)
enabled: true
eslint:
# Source: Organization UI (base)
enabled: true
e18e:
# Source: defaults
enabled: true
flake8:
# Source: defaults
enabled: true
fbinfer:
# Source: defaults
enabled: true
# Source: defaults
enable_java: false
fortitudeLint:
# Source: defaults
enabled: true
rubocop:
# Source: Organization UI (base)
enabled: true
buf:
# Source: Organization UI (base)
enabled: true
regal:
# Source: Organization UI (base)
enabled: true
actionlint:
# Source: Organization UI (base)
enabled: true
zizmor:
# Source: defaults
enabled: true
pmd:
# Source: Organization UI (base)
enabled: true
clang:
# Source: defaults
enabled: true
cppcheck:
# Source: Organization UI (base)
enabled: true
opengrep:
# Source: defaults
enabled: true
semgrep:
# Source: Organization UI (base)
enabled: true
circleci:
# Source: Organization UI (base)
enabled: true
clippy:
# Source: Organization UI (base)
enabled: true
sqlfluff:
# Source: Organization UI (base)
enabled: true
squawk:
# Source: defaults
enabled: true
trivy:
# Source: defaults
enabled: true
prismaLint:
# Source: Organization UI (base)
enabled: true
pylint:
# Source: Organization UI (base)
enabled: true
oxc:
# Source: Organization UI (base)
enabled: true
shopifyThemeCheck:
# Source: Organization UI (base)
enabled: true
luacheck:
# Source: Organization UI (base)
enabled: true
brakeman:
# Source: Organization UI (base)
enabled: true
dotenvLint:
# Source: defaults
enabled: true
htmlhint:
# Source: defaults
enabled: true
stylelint:
# Source: defaults
enabled: true
checkmake:
# Source: defaults
enabled: true
osvScanner:
# Source: defaults
enabled: true
oasdiff:
# Source: defaults
enabled: true
reactDoctor:
# Source: defaults
enabled: true
presidio:
# Source: defaults
enabled: true
blinter:
# Source: defaults
enabled: true
smartyLint:
# Source: defaults
enabled: true
emberTemplateLint:
# Source: defaults
enabled: true
skillspector:
# Source: defaults
enabled: true
psscriptanalyzer:
# Source: defaults
enabled: true
chat:
# Source: defaults
art: true
# Source: defaults
allow_non_org_members: true
# Source: Organization UI (base)
auto_reply: true
integrations:
jira:
# Source: Organization UI (base)
usage: auto
linear:
# Source: Organization UI (base)
usage: auto
knowledge_base:
# Source: Organization UI (base)
opt_out: false
web_search:
# Source: Organization UI (base)
enabled: true
code_guidelines:
# Source: defaults
enabled: true
# Source: defaults
filePatterns: []
learnings:
# Source: Organization UI (base)
scope: auto
# Source: defaults
approval_delay: 0
issues:
# Source: Organization UI (base)
scope: auto
jira:
# Source: Organization UI (base)
usage: auto
# Source: defaults
project_keys: []
# Source: defaults
excluded_project_keys: []
linear:
# Source: Organization UI (base)
usage: auto
# Source: defaults
team_keys: []
pull_requests:
# Source: Organization UI (base)
scope: auto
mcp:
# Source: defaults
usage: auto
# Source: defaults
disabled_servers: []
# Source: defaults
automatic_repository_linking: false
# Source: defaults
linked_repositories: []
code_generation:
docstrings:
# Source: Organization UI (base)
language: en-US
# Source: defaults
path_instructions: []
unit_tests:
# Source: defaults
path_instructions: []
issue_enrichment:
auto_enrich:
# Source: defaults
enabled: false
planning:
# Source: defaults
enabled: true
auto_planning:
# Source: defaults
enabled: true
# Source: defaults
labels: []
labeling:
# Source: defaults
labeling_instructions: []
# Source: defaults
auto_apply_labels: false
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
build/ansible/roles/postgres/files/postgres-migration (2)
85-98: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not reuse retained dumps on a retry.
When
grafanais absent or uses an external database, the dump loop skips the current file. The restore loop still consumes any existingpg18-upgrade-grafana.sql. A failed earlier attempt can therefore restore stale data into the new cluster. Keep dumps in an invocation-specific directory, or restore only files created by the current invocation. This must be corrected before merge.Also applies to: 115-125
🤖 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 `@build/ansible/roles/postgres/files/postgres-migration` around lines 85 - 98, The migration script’s dump and restore flows can reuse stale retained database dumps across retries, especially for the skipped grafana database. Update the dump/restore logic around the database loop and restore section to use an invocation-specific backup directory or track only files created during the current invocation, ensuring skipped databases are never restored from prior attempts.
121-125: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winMake the migration fail closed on SQL errors.
Without
ON_ERROR_STOP,psql -fcontinues after SQL errors and does not return the script-error status required byerrexit. The migration can then publish a partial staged cluster atpostgres-migration#L135.
- Add
-v ON_ERROR_STOP=1to the role/database setup and dump restore commands.- Apply the same option to both
psqlcommands inbuild/docs/MIGRATION.md#L133-L136.🤖 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 `@build/ansible/roles/postgres/files/postgres-migration` around lines 121 - 125, Add -v ON_ERROR_STOP=1 to both psql commands in build/ansible/roles/postgres/files/postgres-migration, covering role/database setup and dump restoration. Apply the same option to both corresponding psql commands in build/docs/MIGRATION.md. Ensure all four commands stop and propagate SQL errors.build/docs/MIGRATION.md (1)
88-89: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore the plain-text dumps with
psql.
pg_dumpwithout-Fcreates plain-text SQL files.pg_restorecannot read them. Usepsql --dbname=<database> --file=<dump> --set=ON_ERROR_STOP=onfor both databases. Make it so.🤖 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 `@build/docs/MIGRATION.md` around lines 88 - 89, Replace both pg_restore commands in the migration instructions with psql commands, targeting the postgres database and grafana database respectively. Use the existing dump files as input via --file, preserve the PostgreSQL connection settings and PGPASSWORD environment variable, and enable --set=ON_ERROR_STOP=on for each restore.
🤖 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 `@build/docs/MIGRATION.md`:
- Line 105: Update the migration retry description near the one-time upgrade
behavior to state that an interrupted attempt can leave both the staging
directory and previously written dumps under /srv/backup, rather than saying it
leaves only a staging directory. Preserve the existing retry and cleanup
behavior described elsewhere.
- Around line 124-127: Update the opening fences for all three command blocks in
MIGRATION.md, including the blocks around the shown install/initdb commands and
the sections at the referenced locations, from untyped fences to ```bash so
markdownlint MD040 passes.
---
Outside diff comments:
In `@build/ansible/roles/postgres/files/postgres-migration`:
- Around line 85-98: The migration script’s dump and restore flows can reuse
stale retained database dumps across retries, especially for the skipped grafana
database. Update the dump/restore logic around the database loop and restore
section to use an invocation-specific backup directory or track only files
created during the current invocation, ensuring skipped databases are never
restored from prior attempts.
- Around line 121-125: Add -v ON_ERROR_STOP=1 to both psql commands in
build/ansible/roles/postgres/files/postgres-migration, covering role/database
setup and dump restoration. Apply the same option to both corresponding psql
commands in build/docs/MIGRATION.md. Ensure all four commands stop and propagate
SQL errors.
In `@build/docs/MIGRATION.md`:
- Around line 88-89: Replace both pg_restore commands in the migration
instructions with psql commands, targeting the postgres database and grafana
database respectively. Use the existing dump files as input via --file, preserve
the PostgreSQL connection settings and PGPASSWORD environment variable, and
enable --set=ON_ERROR_STOP=on for each restore.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 88aeeb40-a701-4db0-b2c7-5d541ffdee7f
📒 Files selected for processing (2)
build/ansible/roles/postgres/files/postgres-migrationbuild/docs/MIGRATION.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
percona/pmm-qa(manual)percona/pmm(manual)
|
|
||
| The upgrade is performed automatically on the first start of a PMM Server that ships PostgreSQL 18, so no manual intervention is required. It is driven by `build/docker/server/entrypoint.sh`, which runs `build/ansible/roles/postgres/files/postgres-migration` before supervisord starts. The steps below document what that script does. | ||
|
|
||
| The upgrade runs only when `/srv/postgres14` exists and `/srv/postgres18` does not, which makes it a one-time operation. The new cluster is built in `/srv/postgres18.new` and moved into place only once it is fully restored, so `/srv/postgres18` existing always means a finished cluster. An interrupted attempt leaves nothing but a staging directory, which the next start discards before retrying. It is skipped entirely when the embedded PostgreSQL is not in use, i.e. when `PMM_HA_ENABLE` or `PMM_DISABLE_BUILTIN_POSTGRES` is enabled. In those cases the external database has to be upgraded by its owner. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe retained dumps in the retry behavior.
An interrupted run can leave the staging directory and dumps already written under /srv/backup. Line 152 confirms that failed dumps remain. Replace “nothing but a staging directory” with wording that names both retained artifacts.
🤖 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 `@build/docs/MIGRATION.md` at line 105, Update the migration retry description
near the one-time upgrade behavior to state that an interrupted attempt can
leave both the staging directory and previously written dumps under /srv/backup,
rather than saying it leaves only a staging directory. Preserve the existing
retry and cleanup behavior described elsewhere.
Comment on lines
+124
to
+127
| ``` | ||
| install -d -m 750 /srv/postgres18.new | ||
| /usr/pgsql-18/bin/initdb -D /srv/postgres18.new --auth-host=scram-sha-256 --auth-local=trust --username=postgres --pwfile=/srv/.postgres_password | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Specify the language for each new fenced command block.
markdownlint-cli2 reports MD040 at Lines 124, 131, and 148. Change each opening fence to ```bash.
Also applies to: 131-137, 148-150
🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 124-124: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@build/docs/MIGRATION.md` around lines 124 - 127, Update the opening fences
for all three command blocks in MIGRATION.md, including the blocks around the
shown install/initdb commands and the sections at the referenced locations, from
untyped fences to ```bash so markdownlint MD040 passes.
Source: Linters/SAST tools
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
5 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.
PMM-15014
Link to the Feature Build: SUBMODULES-4324
Important
Two values are temporarily set to keep CI green and must be changed before merging.
minPGVersion,managed/models/database.go1415commonExpectedFiles,managed/services/server/logs_test.gopostgresql14.logpostgresql.logBoth are marked with a
TODOin the code.Why. The
Unit testsjob runs against the prebuiltghcr.io/percona/pmm:3-dev-containerimage, which is built frommainand still ships PostgreSQL 14.23 with a supervisord config that writes/srv/logs/postgresql14.log. With the real values in place,checkVersionrejects the server on the first database-backed test and every later test cascades on the leftover connection, andTestFilescompares against a log name the old container does not produce. The image only picks up PostgreSQL 18 once this branch's server image is published as3-dev-latest, which is circular until merge.15rather than18for the floor: the bundled server is 18 either way, so standalone is unaffected, while a floor of 18 needlessly rejects externally hosted PostgreSQL 15/16/17 for HA and BYO-database setups. PostgreSQL 14 reaches end of life this autumn, so 14 is not viable as a final value. See the comment below for the measured compatibility data.Summary by CodeRabbit