Skip to content

Commit

Permalink
BZ#2188239 - better detect already migrated PostgreSQL data
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
evgeni committed Jan 11, 2024
1 parent c627a0b commit 455a588
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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/'
Expand All @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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']
Expand All @@ -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']
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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, '*')):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ def process(self):
bytes_required = None
bytes_available = None
old_pgsql_data = False
scl_pgsql_data = True

if local_postgresql:
"""
Expand All @@ -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')
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand Down
2 changes: 2 additions & 0 deletions repos/system_upgrade/el7toel8/models/satellite.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down

0 comments on commit 455a588

Please sign in to comment.