From 73aaefc4d1adedd32d6ecb0ba012304960e1af72 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 | 4 ++- .../unit_test_satellite_upgrade_check.py | 17 +++++++++ .../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, 77 insertions(+), 16 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..7ab3f2cfd4 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 @@ -28,7 +28,9 @@ def satellite_upgrade_check(facts): This 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." + 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/' 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..b546c9da86 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 @@ -32,6 +32,23 @@ def test_no_old_data(monkeypatch): assert expected_title == reporting.create_report.report_fields['title'] +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.' + expected_reindex = 'all databases will require a REINDEX' + + 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): monkeypatch.setattr(reporting, 'create_report', create_report_mocked()) 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 01e634651e..fae3fb58f9 100644 --- a/repos/system_upgrade/el7toel8/actors/satellite_upgrade_facts/actor.py +++ b/repos/system_upgrade/el7toel8/actors/satellite_upgrade_facts/actor.py @@ -74,6 +74,7 @@ def process(self): bytes_required = None bytes_available = None old_pgsql_data = False + scl_pgsql_data = True if local_postgresql: """ @@ -93,20 +94,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') @@ -124,6 +128,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..47f057e494 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 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) + + 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())