fix: Enhance spec "satellite_settings" and parser to support satellit…

…e 6.14 (#3952) * fix: Enhance spec "satellite_settings" and parser to support satellite 6.14 - Since satellite 6.14, the column "default" has been deleted from satellite_settings table. Signed-off-by: Huanhuan Li <huali@redhat.com> * Exclude satellite 5.x * Add parenthese to make statement more clear Signed-off-by: Huanhuan Li <huali@redhat.com> (cherry picked from commit 50b8ce9)
RedHatInsights · Nov 16, 2023 · 6422a8d · 6422a8d
1 parent c67e462
commit 6422a8d
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 17 deletions.
diff --git a/insights/collect.py b/insights/collect.py
@@ -210,6 +210,10 @@
           enabled: true
         - name: insights.components.satellite.IsSatellite
           enabled: true
+        - name: insights.components.satellite.IsSatellite614AndLater
+          enabled: true
+        - name: insights.components.satellite.IsSatelliteLessThan614
+          enabled: true
 
     # needed for the 'pre-check' of the 'is_satellite_capsule' spec
         - name: insights.combiners.satellite_version.CapsuleVersion

diff --git a/insights/components/satellite.py b/insights/components/satellite.py
@@ -91,3 +91,34 @@ class IsSatellite611(IsSatellite):
     """
     def __init__(self, sat):
         super(IsSatellite611, self).__init__(sat, 6, 11)
+
+
+@component(SatelliteVersion)
+class IsSatellite614AndLater(object):
+    """
+    This component uses ``SatelliteVersion`` combiner
+    to determine the Satellite version. It checks if the Satellite version is 6.14 and later,
+    and raises ``SkipComponent`` when it isn't.
+
+    Raises:
+        SkipComponent: When the Satellite version is earlier than 6.14.
+    """
+    def __init__(self, sat):
+        if (sat.major < 6 or (sat.major == 6 and sat.minor < 14)):
+            raise SkipComponent
+
+
+@component(SatelliteVersion)
+class IsSatelliteLessThan614(object):
+    """
+    This component uses ``SatelliteVersion`` combiner
+    to determine the Satellite version. It checks if the Satellite version is 6.x and less than 6.14,
+    and raises ``SkipComponent`` when it isn't.
+
+    Raises:
+        SkipComponent: When the Satellite version isn't 6.x or it's 6.14 and later.
+    """
+    def __init__(self, sat):
+        if sat.major == 6 and sat.minor < 14:
+            return
+        raise SkipComponent
diff --git a/insights/parsers/satellite_postgresql_query.py b/insights/parsers/satellite_postgresql_query.py
@@ -97,8 +97,9 @@ def parse_content(self, content):
             valid_lines = content[start_index:]
         reader = DictReader(os.linesep.join(valid_lines).splitlines(True))
         for row in reader:
-            for name in self.columns_in_yaml:
-                row[name] = self._parse_yaml(row[name])
+            for column in row:
+                if column in self.columns_in_yaml:
+                    row[column] = self._parse_yaml(row[column])
             self.append(row)
         if not self:
             raise SkipComponent("There is no data in the table.")
@@ -124,6 +125,7 @@ def search(self, **kwargs):
 class SatelliteAdminSettings(SatellitePostgreSQLQuery):
     """
     Parse the output of the command ``psql -d foreman -c '"select name, value, "default" from settings where name in ('destroy_vm_on_host_delete', 'unregister_delete_host') --csv"``.
+    Since satellite 6.14, the default column is deleted, the default value is "No" for columns "destroy_vm_on_host_delete" and "unregister_delete_host".
 
     Sample output::
 
@@ -134,6 +136,12 @@ class SatelliteAdminSettings(SatellitePostgreSQLQuery):
         destroy_vm_on_host_delete,,"--- true
         ..."
 
+        or
+
+        name,value
+        destroy_vm_on_host_delete,
+        unregister_delete_host,
+
     Examples:
         >>> type(table)
         <class 'insights.parsers.satellite_postgresql_query.SatelliteAdminSettings'>
@@ -142,7 +150,7 @@ class SatelliteAdminSettings(SatellitePostgreSQLQuery):
         >>> table.get_setting('destroy_vm_on_host_delete')
         True
     """
-    columns = ['name', 'value', 'default']
+    columns = ['name', 'value']
     columns_in_yaml = ['value', 'default']
 
     def get_setting(self, setting_name):
@@ -161,7 +169,10 @@ def get_setting(self, setting_name):
         rows = self.search(name=setting_name)
         if rows:
             value = rows[0].get('value')
-            return rows[0].get('default') if value == '' else value
+            if 'default' in rows[0]:
+                return rows[0].get('default') if value == '' else value
+            else:
+                return value if value else False
 
 
 @parser(Specs.satellite_compute_resources)

diff --git a/insights/specs/default.py b/insights/specs/default.py
@@ -14,7 +14,7 @@
 from insights.components.ceph import IsCephMonitor
 from insights.components.cloud_provider import IsAzure, IsGCP
 from insights.components.rhel_version import IsRhel6
-from insights.components.satellite import IsSatellite611, IsSatellite
+from insights.components.satellite import IsSatellite611, IsSatellite, IsSatellite614AndLater, IsSatelliteLessThan614
 from insights.components.virtualization import IsBareMetal
 from insights.core.context import HostContext
 from insights.core.spec_factory import (
@@ -570,9 +570,15 @@ class DefaultSpecs(Specs):
         "/usr/bin/sudo -iu postgres /usr/bin/psql -d foreman -c \"select count(*) from hosts where \"compute_resource_id\" in (select id from compute_resources where type='Foreman::Model::Ovirt')\" --csv",
         deps=[IsSatellite]
     )
-    satellite_settings = simple_command(
-        "/usr/bin/sudo -iu postgres /usr/bin/psql -d foreman -c \"select name, value, \\\"default\\\" from settings where name in ('destroy_vm_on_host_delete', 'unregister_delete_host')\" --csv",
-        deps=[IsSatellite]
+    satellite_settings = first_of(
+        [
+            simple_command(
+                "/usr/bin/sudo -iu postgres /usr/bin/psql -d foreman -c \"select name, value, \\\"default\\\" from settings where name in ('destroy_vm_on_host_delete', 'unregister_delete_host')\" --csv",
+                deps=[IsSatelliteLessThan614]),
+            simple_command(
+                "/usr/bin/sudo -iu postgres /usr/bin/psql -d foreman -c \"select name, value from settings where name in ('destroy_vm_on_host_delete', 'unregister_delete_host')\" --csv",
+                deps=[IsSatellite614AndLater]),
+        ]
     )
     satellite_version_rb = simple_file("/usr/share/foreman/lib/satellite/version.rb")
     satellite_yaml = simple_file("/etc/foreman-installer/scenarios.d/satellite.yaml")

diff --git a/insights/tests/parsers/test_satellite_postgresql_query.py b/insights/tests/parsers/test_satellite_postgresql_query.py
@@ -25,6 +25,10 @@
 name,default,value
 '''.strip()
 
+SATELLITE_POSTGRESQL_WRONG_6 = '''
+name,value
+'''.strip()
+
 SATELLITE_SETTINGS_1 = '''
 name,value,default
 unregister_delete_host,"--- true
@@ -52,6 +56,20 @@
 ..."
 '''
 
+SATELLITE_SETTINGS_4 = '''
+name,value
+destroy_vm_on_host_delete,
+unregister_delete_host,
+'''
+
+SATELLITE_SETTINGS_5 = '''
+name,value
+destroy_vm_on_host_delete,"--- true
+..."
+unregister_delete_host,"--- true
+..."
+'''
+
 SATELLITE_SETTINGS_WITH_DIFFERENT_TYPES = '''
 name,value,default
 http_proxy_except_list,,--- []
@@ -89,13 +107,6 @@
 ..."
 '''
 
-SATELLITE_SETTINGS_BAD_1 = '''
-name,value
-unregister_delete_host,"--- true
-..."
-destroy_vm_on_host_delete,
-'''
-
 SATELLITE_SETTINGS_BAD_2 = '''
 name,value,default
 unregister_delete_host,"--- true:: def
@@ -259,6 +270,8 @@ def test_basic_output_with_satellite_admin_setting():
         satellite_postgresql_query.SatelliteAdminSettings(context_wrap(SATELLITE_POSTGRESQL_WRONG_4))
     with pytest.raises(SkipComponent):
         satellite_postgresql_query.SatelliteAdminSettings(context_wrap(SATELLITE_POSTGRESQL_WRONG_5))
+    with pytest.raises(SkipComponent):
+        satellite_postgresql_query.SatelliteAdminSettings(context_wrap(SATELLITE_POSTGRESQL_WRONG_6))
 
 
 def test_satellite_admin_settings():
@@ -273,6 +286,17 @@ def test_satellite_admin_settings():
     assert not settings.get_setting('destroy_vm_on_host_delete')
     assert settings.get_setting('non_exist_column') is None
 
+    settings = satellite_postgresql_query.SatelliteAdminSettings(context_wrap(SATELLITE_SETTINGS_4))
+    assert (len(settings)) == 2
+    assert not settings.get_setting('unregister_delete_host')
+    assert not settings.get_setting('destroy_vm_on_host_delete')
+    assert settings.get_setting('non_exist_column') is None
+
+    settings = satellite_postgresql_query.SatelliteAdminSettings(context_wrap(SATELLITE_SETTINGS_5))
+    assert (len(settings)) == 2
+    assert settings.get_setting('unregister_delete_host')
+    assert settings.get_setting('destroy_vm_on_host_delete')
+
     table = satellite_postgresql_query.SatelliteAdminSettings(context_wrap(SATELLITE_SETTINGS_WITH_DIFFERENT_TYPES))
     setting_value = table.get_setting('ignored_interface_identifiers')
     assert isinstance(setting_value, list)
@@ -289,8 +313,6 @@ def test_satellite_admin_settings():
 
 
 def test_satellite_admin_settings_exception():
-    with pytest.raises(ValueError):
-        satellite_postgresql_query.SatelliteAdminSettings(context_wrap(SATELLITE_SETTINGS_BAD_1))
     with pytest.raises(ParseException):
         satellite_postgresql_query.SatelliteAdminSettings(context_wrap(SATELLITE_SETTINGS_BAD_2))