From 455a58875bf39aa7df8fc588c776089a5740b76c Mon Sep 17 00:00:00 2001 From: Evgeni Golov Date: Fri, 21 Apr 2023 10:51:17 +0200 Subject: [PATCH] BZ#2188239 - better detect already migrated PostgreSQL data Users can (and in some cases have to) manually move the PostgreSQL data to the new location. In the case when they would completely delete the old location, the facts code would previously error out: Traceback (most recent call last): File "/usr/lib64/python2.7/multiprocessing/process.py", line 258, in _bootstrap self.run() File "/usr/lib64/python2.7/multiprocessing/process.py", line 114, in run self._target(*self._args, **self._kwargs) File "/usr/lib/python2.7/site-packages/leapp/repository/actor_definition.py", line 72, in _do_run actor_instance.run(*args, **kwargs) File "/usr/lib/python2.7/site-packages/leapp/actors/init.py", line 289, in run self.process(*args) File "/usr/share/leapp-repository/repositories/system_upgrade/el7toel8/actors/satellite_upgrade_facts/actor.py", line 96, in process scl_psql_stat = os.stat(POSTGRESQL_SCL_DATA_PATH) OSError: [Errno 2] No such file or directory: '/var/opt/rh/rh-postgresql12/lib/pgsql/data/' However, if the user already has migrated the data, we don't have to do it in the actor and thus can skip all that detection code. Also adjust the report to say the data looks migrated (but still needs a reindex). --- .../libraries/satellite_upgrade_check.py | 14 +++++--- .../unit_test_satellite_upgrade_check.py | 23 ++++++++++-- .../satellite_upgrade_data_migration/actor.py | 2 +- .../actors/satellite_upgrade_facts/actor.py | 33 +++++++++-------- .../unit_test_satellite_upgrade_facts.py | 35 +++++++++++++++++++ .../el7toel8/models/satellite.py | 2 ++ 6 files changed, 88 insertions(+), 21 deletions(-) diff --git a/repos/system_upgrade/el7toel8/actors/satellite_upgrade_check/libraries/satellite_upgrade_check.py b/repos/system_upgrade/el7toel8/actors/satellite_upgrade_check/libraries/satellite_upgrade_check.py index 6954dd5001..49d912ba4b 100644 --- a/repos/system_upgrade/el7toel8/actors/satellite_upgrade_check/libraries/satellite_upgrade_check.py +++ b/repos/system_upgrade/el7toel8/actors/satellite_upgrade_check/libraries/satellite_upgrade_check.py @@ -23,12 +23,18 @@ def satellite_upgrade_check(facts): title = "Satellite PostgreSQL data migration" flags = [] severity = reporting.Severity.MEDIUM + intro_msg = "PostgreSQL on RHEL 8 expects its data in /var/lib/pgsql/data." reindex_msg = textwrap.dedent(""" - After the data has been moved to the new location, all databases will require a REINDEX. - This will happen automatically during the first boot of the system. + PostgreSQL on RHEL 8 requires a rebuild of all database indexes, when using data created on RHEL 7. + This REINDEX will happen automatically during the first boot of the system. """).strip() - if facts.postgresql.same_partition: + if not facts.postgresql.scl_pgsql_data: + migration_msg = """ + Your PostgreSQL data seems to be already migrated to the new location. + No further movement will be performed. + """ + elif facts.postgresql.same_partition: migration_msg = "Your PostgreSQL data will be automatically migrated." else: scl_psql_path = '/var/opt/rh/rh-postgresql12/lib/pgsql/data/' @@ -48,7 +54,7 @@ def satellite_upgrade_check(facts): so that the contents of {} are available in /var/lib/pgsql/data. """.format(scl_psql_path, storage_message, scl_psql_path) - summary = "{}\n{}".format(textwrap.dedent(migration_msg).strip(), reindex_msg) + summary = "{}\n{}\n{}".format(intro_msg, textwrap.dedent(migration_msg).strip(), reindex_msg) reporting.create_report([ reporting.Title(title), diff --git a/repos/system_upgrade/el7toel8/actors/satellite_upgrade_check/tests/unit_test_satellite_upgrade_check.py b/repos/system_upgrade/el7toel8/actors/satellite_upgrade_check/tests/unit_test_satellite_upgrade_check.py index 8b75adf7e9..13c160e0e5 100644 --- a/repos/system_upgrade/el7toel8/actors/satellite_upgrade_check/tests/unit_test_satellite_upgrade_check.py +++ b/repos/system_upgrade/el7toel8/actors/satellite_upgrade_check/tests/unit_test_satellite_upgrade_check.py @@ -28,8 +28,27 @@ def test_no_old_data(monkeypatch): assert reporting.create_report.called == 1 expected_title = 'Satellite PostgreSQL data migration' + expected_intro = 'PostgreSQL on RHEL 8 expects its data in /var/lib/pgsql/data.' assert expected_title == reporting.create_report.report_fields['title'] + assert expected_intro in reporting.create_report.report_fields['summary'] + + +def test_migrated_data(monkeypatch): + monkeypatch.setattr(reporting, 'create_report', create_report_mocked()) + + satellite_upgrade_check(SatelliteFacts(has_foreman=True, + postgresql=SatellitePostgresqlFacts(local_postgresql=True, scl_pgsql_data=False))) + + assert reporting.create_report.called == 1 + + expected_title = 'Satellite PostgreSQL data migration' + expected_summary = 'Your PostgreSQL data seems to be already migrated to the new location.' + expected_reindex = 'PostgreSQL on RHEL 8 requires a rebuild of all database indexes' + + assert expected_title == reporting.create_report.report_fields['title'] + assert expected_summary in reporting.create_report.report_fields['summary'] + assert expected_reindex in reporting.create_report.report_fields['summary'] def test_same_disk(monkeypatch): @@ -42,7 +61,7 @@ def test_same_disk(monkeypatch): expected_title = 'Satellite PostgreSQL data migration' expected_summary = 'Your PostgreSQL data will be automatically migrated.' - expected_reindex = 'all databases will require a REINDEX' + expected_reindex = 'PostgreSQL on RHEL 8 requires a rebuild of all database indexes' assert expected_title == reporting.create_report.report_fields['title'] assert expected_summary in reporting.create_report.report_fields['summary'] @@ -60,7 +79,7 @@ def test_different_disk_sufficient_storage(monkeypatch): expected_title = 'Satellite PostgreSQL data migration' expected_summary = 'You currently have enough free storage to move the data' - expected_reindex = 'all databases will require a REINDEX' + expected_reindex = 'PostgreSQL on RHEL 8 requires a rebuild of all database indexes' assert expected_title == reporting.create_report.report_fields['title'] assert expected_summary in reporting.create_report.report_fields['summary'] diff --git a/repos/system_upgrade/el7toel8/actors/satellite_upgrade_data_migration/actor.py b/repos/system_upgrade/el7toel8/actors/satellite_upgrade_data_migration/actor.py index 0cf6697032..e96eee251b 100644 --- a/repos/system_upgrade/el7toel8/actors/satellite_upgrade_data_migration/actor.py +++ b/repos/system_upgrade/el7toel8/actors/satellite_upgrade_data_migration/actor.py @@ -40,7 +40,7 @@ def process(self): except Exception as e: # pylint: disable=broad-except self.log.warning('Failed disabling service {}: {}'.format(service, e)) - if facts.postgresql.local_postgresql and os.path.exists(POSTGRESQL_SCL_DATA_PATH): + if facts.postgresql.local_postgresql and facts.postgresql.scl_pgsql_data: # we can assume POSTGRESQL_DATA_PATH exists and is empty # move PostgreSQL data to the new home for item in glob.glob(os.path.join(POSTGRESQL_SCL_DATA_PATH, '*')): diff --git a/repos/system_upgrade/el7toel8/actors/satellite_upgrade_facts/actor.py b/repos/system_upgrade/el7toel8/actors/satellite_upgrade_facts/actor.py index 3cd9d9daaf..4d8cede488 100644 --- a/repos/system_upgrade/el7toel8/actors/satellite_upgrade_facts/actor.py +++ b/repos/system_upgrade/el7toel8/actors/satellite_upgrade_facts/actor.py @@ -80,6 +80,7 @@ def process(self): bytes_required = None bytes_available = None old_pgsql_data = False + scl_pgsql_data = True if local_postgresql: """ @@ -99,20 +100,23 @@ def process(self): old_pgsql_data = bool(os.path.exists('/var/lib/pgsql/data/') and os.listdir('/var/lib/pgsql/data/') and os.path.exists(POSTGRESQL_SCL_DATA_PATH) and os.listdir(POSTGRESQL_SCL_DATA_PATH)) - scl_psql_stat = os.stat(POSTGRESQL_SCL_DATA_PATH) - for nonscl_path in ['/var/lib/pgsql/data/', '/var/lib/pgsql/', '/var/lib/', '/']: - if os.path.exists(nonscl_path): - nonscl_psql_stat = os.stat(nonscl_path) - break - - if scl_psql_stat.st_dev != nonscl_psql_stat.st_dev: - on_same_partition = False - # get the current disk usage of the PostgreSQL data - scl_du_call = run(['du', '--block-size=1', '--summarize', POSTGRESQL_SCL_DATA_PATH]) - bytes_required = int(scl_du_call['stdout'].split()[0]) - # get the current free space on the target partition - nonscl_stat = os.statvfs(nonscl_path) - bytes_available = nonscl_stat.f_bavail * nonscl_stat.f_frsize + if os.path.exists(POSTGRESQL_SCL_DATA_PATH): + scl_psql_stat = os.stat(POSTGRESQL_SCL_DATA_PATH) + for nonscl_path in ['/var/lib/pgsql/data/', '/var/lib/pgsql/', '/var/lib/', '/']: + if os.path.exists(nonscl_path): + nonscl_psql_stat = os.stat(nonscl_path) + break + + if scl_psql_stat.st_dev != nonscl_psql_stat.st_dev: + on_same_partition = False + # get the current disk usage of the PostgreSQL data + scl_du_call = run(['du', '--block-size=1', '--summarize', POSTGRESQL_SCL_DATA_PATH]) + bytes_required = int(scl_du_call['stdout'].split()[0]) + # get the current free space on the target partition + nonscl_stat = os.statvfs(nonscl_path) + bytes_available = nonscl_stat.f_bavail * nonscl_stat.f_frsize + else: + scl_pgsql_data = False modules_to_enable.append(Module(name='postgresql', stream='12')) to_remove.append('rh-postgresql12-runtime') @@ -130,6 +134,7 @@ def process(self): postgresql=SatellitePostgresqlFacts( local_postgresql=local_postgresql, old_var_lib_pgsql_data=old_pgsql_data, + scl_pgsql_data=scl_pgsql_data, same_partition=on_same_partition, space_required=bytes_required, space_available=bytes_available, diff --git a/repos/system_upgrade/el7toel8/actors/satellite_upgrade_facts/tests/unit_test_satellite_upgrade_facts.py b/repos/system_upgrade/el7toel8/actors/satellite_upgrade_facts/tests/unit_test_satellite_upgrade_facts.py index 2fb8a3ba5d..b489689bd7 100644 --- a/repos/system_upgrade/el7toel8/actors/satellite_upgrade_facts/tests/unit_test_satellite_upgrade_facts.py +++ b/repos/system_upgrade/el7toel8/actors/satellite_upgrade_facts/tests/unit_test_satellite_upgrade_facts.py @@ -129,6 +129,40 @@ def mocked_stat(path): return mocked_stat monkeypatch.setattr("os.stat", mock_stat()) + def mock_path_exists(): + orig_path_exists = os.path.exists + + def mocked_path_exists(path): + if path == '/var/opt/rh/rh-postgresql12/lib/pgsql/data/': + return True + return orig_path_exists(path) + return mocked_path_exists + monkeypatch.setattr("os.path.exists", mock_path_exists()) + + current_actor_context.feed(InstalledRPM(items=[FOREMAN_RPM, POSTGRESQL_RPM])) + current_actor_context.run(config_model=mock_configs.CONFIG) + + rpmmessage = current_actor_context.consume(RpmTransactionTasks)[0] + assert Module(name='postgresql', stream='12') in rpmmessage.modules_to_enable + + satellitemsg = current_actor_context.consume(SatelliteFacts)[0] + assert satellitemsg.postgresql.local_postgresql + assert satellitemsg.postgresql.scl_pgsql_data + + assert current_actor_context.consume(DNFWorkaround) + + +def test_detects_migrated_postgresql(monkeypatch, current_actor_context): + def mock_path_exists(): + orig_path_exists = os.path.exists + + def mocked_path_exists(path): + if path == '/var/opt/rh/rh-postgresql12/lib/pgsql/data/': + return False + return orig_path_exists(path) + return mocked_path_exists + monkeypatch.setattr("os.path.exists", mock_path_exists()) + current_actor_context.feed(InstalledRPM(items=[FOREMAN_RPM, POSTGRESQL_RPM])) current_actor_context.run(config_model=mock_configs.CONFIG) @@ -137,6 +171,7 @@ def mocked_stat(path): satellitemsg = current_actor_context.consume(SatelliteFacts)[0] assert satellitemsg.postgresql.local_postgresql + assert not satellitemsg.postgresql.scl_pgsql_data assert current_actor_context.consume(DNFWorkaround) diff --git a/repos/system_upgrade/el7toel8/models/satellite.py b/repos/system_upgrade/el7toel8/models/satellite.py index b4282790bf..813195bb45 100644 --- a/repos/system_upgrade/el7toel8/models/satellite.py +++ b/repos/system_upgrade/el7toel8/models/satellite.py @@ -9,6 +9,8 @@ class SatellitePostgresqlFacts(Model): """ Whether or not PostgreSQL is installed on the same system """ old_var_lib_pgsql_data = fields.Boolean(default=False) """ Whether or not there is old PostgreSQL data in /var/lib/pgsql/data """ + scl_pgsql_data = fields.Boolean(default=True) + """ Whether or not there is data in the SCL PostgreSQL path """ same_partition = fields.Boolean(default=True) """ Whether or not target and source postgresql data will stay on the same partition """ space_required = fields.Nullable(fields.Integer())