BZ#2188239 - better detect already migrated PostgreSQL data

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),

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']

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, '*')):

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,

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)
 

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())