From 2c31a5c39e8d0c8ab548aea353ad1a809a546cb4 Mon Sep 17 00:00:00 2001 From: Roman Prilipskii Date: Thu, 14 May 2026 23:15:20 -0400 Subject: [PATCH 1/2] CLOS-2132: Improve postgresqlcheck hint with explicit migration steps Replace the generic "follow the docs" hint with explicit post-upgrade steps that walk the user through the manual PostgreSQL data migration: 1. dnf install postgresql-upgrade 2. postgresql-setup --upgrade 3. systemctl start postgresql Also call out that the data stays in the old 9.2 format and that postgresql will refuse to start until the migration is done - this is the proximate confusion the customer described in the ticket (systemd showed "An old version of the database format was found" but the recommended migration command failed with a missing-package error, leaving the customer stuck). Co-Authored-By: Claude Sonnet 4.6 --- .../postgresqlcheck/libraries/postgresqlcheck.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/repos/system_upgrade/el7toel8/actors/postgresqlcheck/libraries/postgresqlcheck.py b/repos/system_upgrade/el7toel8/actors/postgresqlcheck/libraries/postgresqlcheck.py index 575a2798b0..16205df7a6 100644 --- a/repos/system_upgrade/el7toel8/actors/postgresqlcheck/libraries/postgresqlcheck.py +++ b/repos/system_upgrade/el7toel8/actors/postgresqlcheck/libraries/postgresqlcheck.py @@ -12,9 +12,19 @@ ) report_server_inst_hint = ( - 'Back up your data before proceeding with the upgrade' - ' and follow steps in the documentation section "Migrating to a RHEL 8 version of PostgreSQL"' - ' after the upgrade.' + 'Back up your data before proceeding with the upgrade.' + ' The upgrade keeps your PostgreSQL data directory in the old (9.2)' + ' on-disk format - postgresql will refuse to start on the upgraded' + ' system until you migrate the data. After the upgrade completes:\n' + ' 1. Install the postgresql-upgrade package (provides the old' + ' server binaries needed by the migration helper):\n' + ' dnf install postgresql-upgrade\n' + ' 2. Migrate the database files to the new format:\n' + ' postgresql-setup --upgrade\n' + ' 3. Start PostgreSQL:\n' + ' systemctl start postgresql\n' + 'For more details see the documentation section' + ' "Migrating to a RHEL 8 version of PostgreSQL".' ) # Link URL for postgresql-server report From c134cccc2153fd0cd8580bd5fc2e62954f1a9365 Mon Sep 17 00:00:00 2001 From: Roman Prilipskii Date: Thu, 14 May 2026 23:15:57 -0400 Subject: [PATCH 2/2] CLOS-2132: Detect postgresql.conf unix_socket_directories pre-upgrade Add a second report in postgresqlcheck that fires when postgresql-server is installed AND /var/lib/pgsql/data/postgresql.conf contains an active "unix_socket_directories" (plural) setting. Root cause confirmed from 3 customer tickets (ZD-197576, ZD-204621, ZD-228015) and reproduced end-to-end on a CL7 nopanel test VM: CL7's postgresql-server 9.2 ships with a forward-compatible patch that accepts the newer (PG 9.3+) plural parameter name "unix_socket_directories" - so postgres runs fine on CL7 with that setting. RHEL-8's postgresql-upgrade package, which provides the old 9.2 server binary that pg_upgrade launches to read the data-old cluster, ships an UN-patched 9.2 build that rejects the plural form with: LOG: unrecognized configuration parameter "unix_socket_directories" in file "/var/lib/pgsql/data-old/postgresql.conf" FATAL: configuration file "/var/lib/pgsql/data-old/postgresql.conf" contains errors pg_ctl: could not start server The default RHEL-7 postgresql.conf has the plural line commented out by default, but admin edits, cPanel tooling, and config-management commonly uncomment it (it's "the modern form" per current PG docs). Customers hit by this saw their migration fail with a confusing socket-connection error - the actual cause is buried in pg_upgrade_server.log, which customers don't usually know to read. The new check: - Reads /var/lib/pgsql/data/postgresql.conf if it exists. - Matches "^\s*unix_socket_directories\s*=" (active, not commented). - Emits a HIGH-severity report with a concrete sed remediation (rename plural -> singular) for both pre-upgrade and post-upgrade cases. - Treats missing/unreadable files as "not affected" - if the data dir was never initialized (no initdb), the bug can't trigger. Tests cover: the new detection function across plural / commented / singular / whitespace / buried / missing-file cases; both that the new report fires only when postgresql-server is present and that it does not when the package is absent. Co-Authored-By: Claude Sonnet 4.6 --- .../libraries/postgresqlcheck.py | 91 ++++++++++++++++- .../tests/test_postgresqlcheck.py | 98 ++++++++++++++++++- 2 files changed, 187 insertions(+), 2 deletions(-) diff --git a/repos/system_upgrade/el7toel8/actors/postgresqlcheck/libraries/postgresqlcheck.py b/repos/system_upgrade/el7toel8/actors/postgresqlcheck/libraries/postgresqlcheck.py index 16205df7a6..25a933d010 100644 --- a/repos/system_upgrade/el7toel8/actors/postgresqlcheck/libraries/postgresqlcheck.py +++ b/repos/system_upgrade/el7toel8/actors/postgresqlcheck/libraries/postgresqlcheck.py @@ -1,8 +1,18 @@ +import os +import re + from leapp import reporting from leapp.libraries.common.rpms import has_package from leapp.libraries.stdlib import api from leapp.models import DistributionSignedRPM +POSTGRESQL_CONF_PATH = '/var/lib/pgsql/data/postgresql.conf' + +# Matches the start of an active (uncommented) setting line for +# unix_socket_directories. PostgreSQL config syntax allows optional leading +# whitespace; a line is a comment if its first non-whitespace char is '#'. +_UNIX_SOCKET_DIRECTORIES_RE = re.compile(r'^\s*unix_socket_directories\s*=') + # Summary for postgresql-server report report_server_inst_summary = ( 'PostgreSQL server component will be upgraded. Since RHEL-8 includes' @@ -41,6 +51,36 @@ .format(''.join(['\n - {}'.format(i) for i in report_contrib_inst_dropext])) ) +# Summary / remediation for the unix_socket_directories config-incompat report. +# RHEL-7's PG 9.2 ships with a forward-compatible patch that accepts the newer +# (PG 9.3+) plural parameter name unix_socket_directories. RHEL-8's +# postgresql-upgrade package ships an unpatched 9.2 server binary that rejects +# the plural form with a FATAL config error and aborts on startup; that breaks +# postgresql-setup --upgrade post-Elevate. The default CL7 postgresql.conf has +# this line commented out by default, but admin edits, cPanel tooling, and +# config-management commonly uncomment or rewrite it. +report_plural_socket_param_summary = ( + 'PostgreSQL configuration file {conf} contains an active' + ' "unix_socket_directories" setting. RHEL-7 PostgreSQL 9.2 accepts this' + ' plural parameter name (added in PG 9.3 and back-ported by RHEL into 9.2),' + ' but the unpatched 9.2 server binary shipped in RHEL-8\'s postgresql-upgrade' + ' package does not. As a result, "postgresql-setup --upgrade" will fail' + ' post-upgrade with "unrecognized configuration parameter' + ' unix_socket_directories" and PostgreSQL will refuse to start on the' + ' upgraded system until the parameter is renamed.' +).format(conf=POSTGRESQL_CONF_PATH) + +report_plural_socket_param_hint = ( + 'Before running the upgrade, rename the parameter on the affected line of' + ' {conf} from "unix_socket_directories" (plural) to "unix_socket_directory"' + ' (singular). For example:\n' + ' sed -i \'s/^unix_socket_directories/unix_socket_directory/\' {conf}\n' + 'If you have already upgraded and PostgreSQL is failing to start, apply the' + ' same rename to the same file (which post-upgrade may have been renamed to' + ' /var/lib/pgsql/data-old/postgresql.conf by postgresql-setup), then re-run:\n' + ' postgresql-setup --upgrade' +).format(conf=POSTGRESQL_CONF_PATH) + def _report_server_installed(): """ @@ -79,13 +119,60 @@ def _report_contrib_installed(): ]) +def _postgresql_conf_uses_plural_socket_param(path=POSTGRESQL_CONF_PATH): + """ + Return True if `path` contains an active (uncommented) line setting the + plural parameter name `unix_socket_directories`. + + Missing files or unreadable lines are treated as "not affected" - the + caller already knows postgresql-server is installed, but the data dir may + never have been initialized (initdb not yet run), in which case no + postgresql.conf exists. We don't want to scare users with a warning for + a state that can't trigger the bug. + """ + if not os.path.isfile(path): + return False + try: + with open(path, 'r') as fp: + for raw_line in fp: + if _UNIX_SOCKET_DIRECTORIES_RE.match(raw_line): + return True + except (IOError, OSError): + # Permissions or transient I/O issue. Conservatively return False; + # if the file truly contains the bad line, postgresql-setup --upgrade + # will surface it after the upgrade and the generic hint covers it. + return False + return False + + +def _report_plural_socket_param_detected(): + """ + Create report when postgresql.conf contains an active + unix_socket_directories (plural) setting, which will break the post-upgrade + data migration. See module docstring for the full mechanism. + """ + reporting.create_report([ + reporting.Title( + 'PostgreSQL configuration uses "unix_socket_directories"' + ' which will break post-upgrade data migration' + ), + reporting.Summary(report_plural_socket_param_summary), + reporting.Severity(reporting.Severity.HIGH), + reporting.Groups([reporting.Groups.SERVICES]), + reporting.RelatedResource('package', 'postgresql-server'), + reporting.RelatedResource('file', POSTGRESQL_CONF_PATH), + reporting.Remediation(hint=report_plural_socket_param_hint), + ]) + + def report_installed_packages(_context=api): """ Create reports according to detected PostgreSQL packages. Create the report if the postgresql-server rpm (RH signed) is installed. Additionally, create another report if the postgresql-contrib rpm - is installed. + is installed, and another if postgresql.conf contains an active + unix_socket_directories (plural) setting. """ has_server = has_package(DistributionSignedRPM, 'postgresql-server', context=_context) has_contrib = has_package(DistributionSignedRPM, 'postgresql-contrib', context=_context) @@ -96,3 +183,5 @@ def report_installed_packages(_context=api): if has_contrib: # postgresql-contrib _report_contrib_installed() + if _postgresql_conf_uses_plural_socket_param(): + _report_plural_socket_param_detected() diff --git a/repos/system_upgrade/el7toel8/actors/postgresqlcheck/tests/test_postgresqlcheck.py b/repos/system_upgrade/el7toel8/actors/postgresqlcheck/tests/test_postgresqlcheck.py index 559c8f2d04..690c79cbe7 100644 --- a/repos/system_upgrade/el7toel8/actors/postgresqlcheck/tests/test_postgresqlcheck.py +++ b/repos/system_upgrade/el7toel8/actors/postgresqlcheck/tests/test_postgresqlcheck.py @@ -1,7 +1,11 @@ import pytest from leapp import reporting -from leapp.libraries.actor.postgresqlcheck import report_installed_packages +from leapp.libraries.actor import postgresqlcheck +from leapp.libraries.actor.postgresqlcheck import ( + _postgresql_conf_uses_plural_socket_param, + report_installed_packages, +) from leapp.libraries.common.testutils import create_report_mocked, CurrentActorMocked from leapp.libraries.stdlib import api from leapp.models import DistributionSignedRPM, RPM @@ -58,6 +62,10 @@ def test_actor_execution(monkeypatch, has_server, has_contrib): curr_actor_mocked = CurrentActorMocked(msgs=[DistributionSignedRPM(items=rpms)]) monkeypatch.setattr(api, 'current_actor', curr_actor_mocked) monkeypatch.setattr(reporting, "create_report", create_report_mocked()) + # Default: no postgresql.conf scan match (covered separately below). + monkeypatch.setattr( + postgresqlcheck, '_postgresql_conf_uses_plural_socket_param', lambda: False + ) # Executed actor fed with out fake RPMs report_installed_packages(_context=api) @@ -71,3 +79,91 @@ def test_actor_execution(monkeypatch, has_server, has_contrib): else: # Assert for no postgresql packages installed assert not reporting.create_report.called + + +def test_plural_socket_param_report_fires_only_when_server_present(monkeypatch): + """ + When postgresql-server is installed AND postgresql.conf has the active + plural form, we expect TWO reports (server-installed + plural-param). + When postgresql-server is absent, the conf scan must not trigger any + report - the bug is irrelevant without the server. + """ + monkeypatch.setattr(reporting, "create_report", create_report_mocked()) + monkeypatch.setattr( + postgresqlcheck, '_postgresql_conf_uses_plural_socket_param', lambda: True + ) + + # With postgresql-server installed: server report + plural-param report. + rpms_with_server = [ + _generate_rpm_with_name('sed'), + _generate_rpm_with_name('postgresql-server'), + ] + monkeypatch.setattr( + api, 'current_actor', + CurrentActorMocked(msgs=[DistributionSignedRPM(items=rpms_with_server)]), + ) + report_installed_packages(_context=api) + assert reporting.create_report.called == 2 + + # Reset reports between runs. + monkeypatch.setattr(reporting, "create_report", create_report_mocked()) + monkeypatch.setattr( + postgresqlcheck, '_postgresql_conf_uses_plural_socket_param', lambda: True + ) + + # Without postgresql-server, the plural-param report must NOT fire even + # if a file matching the regex somehow existed. + rpms_no_server = [_generate_rpm_with_name('sed')] + monkeypatch.setattr( + api, 'current_actor', + CurrentActorMocked(msgs=[DistributionSignedRPM(items=rpms_no_server)]), + ) + report_installed_packages(_context=api) + assert not reporting.create_report.called + + +# Each entry: (postgresql.conf body, expected match boolean, description) +_CONF_BODY_CASES = [ + # The active uncommented line in plural form must be detected. + ("unix_socket_directories = '/var/run/postgresql, /tmp'\n", True, 'active plural'), + # Leading whitespace is allowed for PG config syntax. + (" unix_socket_directories = '/tmp'\n", True, 'leading whitespace'), + # Commented-out plural is the default and must NOT trigger. + ("#unix_socket_directories = '/var/run/postgresql, /tmp'\n", False, 'commented default'), + # Singular (the form PG 9.2 / postgresql-upgrade actually accepts) - no warning. + ("unix_socket_directory = '/var/run/postgresql'\n", False, 'singular form'), + # Plural appearing inside a comment about the parameter shouldn't trigger. + ("# see unix_socket_directories in the docs\n", False, 'mention in comment'), + # Empty file. + ("", False, 'empty file'), + # Multi-line config with the bad line buried. + ( + "# header\n" + "shared_buffers = 128MB\n" + "unix_socket_directories = '/var/run/postgresql'\n" + "max_connections = 100\n", + True, + 'mixed multi-line', + ), +] + + +@pytest.mark.parametrize('body,expected,desc', _CONF_BODY_CASES) +def test_postgresql_conf_uses_plural_socket_param(tmp_path, body, expected, desc): + """ + The conf-scan helper must: + - return True ONLY for an active (uncommented) "unix_socket_directories" + line, with or without leading whitespace. + - return False for commented references, the singular form, missing + files, and unrelated content. False positives here would inhibit + upgrades unnecessarily. + """ + conf = tmp_path / 'postgresql.conf' + conf.write_text(body) + assert _postgresql_conf_uses_plural_socket_param(str(conf)) is expected, desc + + +def test_postgresql_conf_uses_plural_socket_param_missing_file(tmp_path): + """A non-existent postgresql.conf must NOT raise and must report False.""" + missing = tmp_path / 'does-not-exist' / 'postgresql.conf' + assert _postgresql_conf_uses_plural_socket_param(str(missing)) is False