sar_facts - refact module (#154) (#155)

* sar_facts - refact module

* sar_facts - add changelogs fragment

* sar_facts - fix changelogs fragment

(cherry picked from commit 5303f96)

Co-authored-by: Nocchia <133043574+NomakCooper@users.noreply.github.com>

Author: patchback[bot]

changelogs/fragments/154-sar_facts-refactor.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - sar_facts - Updated to follow the Ansible standard, it now uses ``module.run_command()`` and ``module.get_bin_path()`` (https://github.com/3A2DEV/ans2dev.general/pull/154).

plugins/modules/sar_facts.py

@@ -15,6 +15,7 @@
   - Retrieves SAR data using the C(sar) command from system logs.
   - Supports filtering by date range, time range, and partition details.
   - Returns performance metrics such as CPU utilization, memory usage, disk activity, and network statistics.
+  - ans2dev.general.sar_facts only supports sar data with a V(12H) time format and automatically converts format to V(24H).
 version_added: "0.1.0"
 options:
   date_start:
@@ -139,12 +140,10 @@
 '''
 
 from ansible.module_utils.basic import AnsibleModule
-import os
-import subprocess
 from datetime import datetime, timedelta
+import os
 
 SAR_LOG_PATHS = ["/var/log/sa/", "/var/log/sysstat/"]
-SAR_BIN_PATHS = ["/usr/bin/sar", "/usr/sbin/sar", "/bin/sar"]
 
 SAR_FACT_MAPPING = {
     "cpu": "sar_cpu",
@@ -156,33 +155,20 @@
 }
 
 
-def locate_sar():
-    """Finds the SAR binary in the system."""
-    for path in SAR_BIN_PATHS:
-        if os.path.exists(path):
-            return path
-    return None
-
-
 def find_sar_file(date_str):
-    """Finds the SAR log file for a given date."""
     try:
         date_obj = datetime.strptime(date_str, "%Y-%m-%d")
         day = date_obj.strftime("%d")
-
         for path in SAR_LOG_PATHS:
             file_path = os.path.join(path, f"sa{day}")
             if os.path.exists(file_path):
                 return file_path
-
     except ValueError:
         return None
-
     return None
 
 
 def run_sar_command(module, sar_bin, sar_file, sar_type, time_start, time_end, partition, average, date_str):
-    """Executes the SAR command and returns parsed results."""
     command = [sar_bin, "-f", sar_file]
 
     sar_flags = {
@@ -202,21 +188,17 @@ def run_sar_command(module, sar_bin, sar_file, sar_type, time_start, time_end, p
     if time_end:
         command.extend(["-e", time_end])
 
-    try:
-        result = subprocess.run(command, capture_output=True, text=True, check=True)
-        return parse_sar_output(result.stdout, sar_type, average, date_str)
-    except subprocess.CalledProcessError as e:
-        module.fail_json(msg=f"Failed to execute SAR command: {str(e)}")
+    rc, stdout, stderr = module.run_command(command)
+    if rc != 0:
+        module.fail_json(msg=f"Failed to execute SAR command: {stderr}")
+    return parse_sar_output(stdout, sar_type, average, date_str)
 
 
 def convert_to_24h(time_str, am_pm):
     return datetime.strptime(f"{time_str} {am_pm}", "%I:%M:%S %p").strftime("%H:%M:%S")
 
 
 def parse_sar_output(output, sar_type, average, date_str):
-    """Parses SAR output by finding the header line and converting the first two columns (TIME, AM/PM)
-    into 24H format. Il campo AM/PM viene preservato nel dizionario finale.
-    Vengono anche filtrate le righe che contengono 'Linux' o 'restart'."""
     import re
     parsed_data = []
     header = None
@@ -225,31 +207,26 @@ def is_valid_time(token):
         return re.match(r'^\d{1,2}:\d{2}:\d{2}$', token)
 
     for line in output.splitlines():
-        # Filtra le righe che contengono "Linux" o "restart"
         if re.search(r'\b(Linux|restart)\b', line, flags=re.IGNORECASE):
             continue
 
         parts = line.split()
         if not parts:
             continue
 
-        # Gestione del flag "Average:"
         if parts[0] == "Average:":
             parts = parts[1:]
             if not average:
                 continue
 
-        # Se l'header non è definito, prendi la riga completa se inizia con orario valido
         if header is None:
             if len(parts) >= 2 and is_valid_time(parts[0]) and parts[1] in ["AM", "PM"]:
-                header = parts  # conserva l'intero header
+                header = parts
             continue
 
-        # Processa le righe dati: controlla se i primi due token sono orario e AM/PM
         if len(parts) >= 2 and is_valid_time(parts[0]) and parts[1] in ["AM", "PM"]:
             converted = convert_to_24h(parts[0], parts[1])
             data_entry = {"date": date_str, "time": converted}
-            # Mappa i dati a partire dall'indice 1 del header (per preservare la colonna AM/PM)
             for idx in range(1, min(len(header), len(parts))):
                 data_entry[header[idx]] = parts[idx]
             parsed_data.append(data_entry)
@@ -296,15 +273,28 @@ def main():
         date_list = [date_start] if date_start else []
 
     collected_data = []
+
+    sar_bin = module.get_bin_path("sar", required=True)
+
     for date in date_list:
         sar_file = find_sar_file(date)
         if sar_file:
-            collected_data.extend(run_sar_command(module, locate_sar(), sar_file, sar_type, time_start, time_end, partition, average, date))
+            collected_data.extend(
+                run_sar_command(
+                    module,
+                    sar_bin,
+                    sar_file,
+                    sar_type,
+                    time_start,
+                    time_end,
+                    partition,
+                    average,
+                    date
+                )
+            )
 
     fact_name = SAR_FACT_MAPPING.get(sar_type, f"sar_{sar_type}")
-
     result = {'ansible_facts': {fact_name: collected_data}}
-
     module.exit_json(**result)
 
 

tests/unit/plugins/modules/test_sar_facts.py

@@ -7,9 +7,7 @@
 
 import unittest
 from unittest.mock import patch, MagicMock
-import subprocess
 
-# Import the sar_facts module from the collection.
 from ansible_collections.ans2dev.general.plugins.modules import sar_facts  # type: ignore
 
 
@@ -22,15 +20,31 @@ def fail_json(*args, **kwargs):
 
 
 class TestSarFactsModule(unittest.TestCase):
+    def run_sar_fact_test(self, test_params, fake_sar_output, expected_fact_key, expected_sample_value):
+        with patch.object(sar_facts, 'AnsibleModule') as mock_AnsibleModule:
+            fake_module = MagicMock()
+            fake_module.params = test_params
+            fake_module.exit_json.side_effect = exit_json
+            fake_module.fail_json.side_effect = fail_json
+            fake_module.run_command.return_value = (0, fake_sar_output, "")
+            fake_module.get_bin_path.return_value = "/usr/bin/sar"
+            mock_AnsibleModule.return_value = fake_module
+
+            with patch.object(sar_facts, 'find_sar_file', return_value="/dummy/sa01"):
+                with self.assertRaises(Exception) as context:
+                    sar_facts.main()
+                result_str = str(context.exception)
+
+                self.assertIn(expected_fact_key, result_str)
+                self.assertIn(expected_sample_value, result_str)
+                self.assertIn("2025-05-01", result_str)
+
     def test_sar_cpu(self):
-        # Simulated output for the 'sar' command.
         fake_sar_output = (
             "08:00:00 AM %user %system %idle\n"
             "08:00:00 AM 5.00 2.00 93.00\n"
             "08:10:00 AM 6.00 3.00 91.00\n"
         )
-
-        # Parameters for a SAR facts call (CPU type).
         test_params = {
             'date_start': '2025-05-01',
             'date_end': '2025-05-01',
@@ -40,36 +54,92 @@ def test_sar_cpu(self):
             'average': False,
             'partition': False
         }
+        self.run_sar_fact_test(test_params, fake_sar_output, "sar_cpu", "5.00")
 
-        # Patch AnsibleModule in sar_facts.
-        with patch.object(sar_facts, 'AnsibleModule') as mock_AnsibleModule:
-            fake_module = MagicMock()
-            fake_module.params = test_params
-            fake_module.exit_json.side_effect = exit_json
-            fake_module.fail_json.side_effect = fail_json
-            fake_module.run_command.return_value = (0, fake_sar_output, "")
-            mock_AnsibleModule.return_value = fake_module
+    def test_sar_memory(self):
+        fake_sar_output = (
+            "08:00:00 AM %memused %commit\n"
+            "08:00:00 AM 75.00 60.00\n"
+            "08:10:00 AM 76.00 59.50\n"
+        )
+        test_params = {
+            'date_start': '2025-05-01',
+            'date_end': '2025-05-01',
+            'time_start': '08:00:00',
+            'time_end': '10:00:00',
+            'type': 'memory',
+            'average': False,
+            'partition': False
+        }
+        self.run_sar_fact_test(test_params, fake_sar_output, "sar_mem", "75.00")
 
-            # Patch file-check and external command functions.
-            with patch.object(sar_facts, 'find_sar_file', return_value="/dummy/sa01"), \
-                 patch.object(sar_facts, 'subprocess') as mock_subprocess:
+    def test_sar_swap(self):
+        fake_sar_output = (
+            "08:00:00 AM %swpused %swpcad\n"
+            "08:00:00 AM 10.00 0.50\n"
+            "08:10:00 AM 11.00 0.60\n"
+        )
+        test_params = {
+            'date_start': '2025-05-01',
+            'date_end': '2025-05-01',
+            'time_start': '08:00:00',
+            'time_end': '10:00:00',
+            'type': 'swap',
+            'average': False,
+            'partition': False
+        }
+        self.run_sar_fact_test(test_params, fake_sar_output, "sar_swap", "10.00")
 
-                # Simulate a successful subprocess.run call.
-                mock_subprocess.run.return_value = subprocess.CompletedProcess(
-                    args=["/usr/bin/sar", "-f", "/dummy/sa01"],
-                    returncode=0,
-                    stdout=fake_sar_output
-                )
+    def test_sar_network(self):
+        fake_sar_output = (
+            "08:00:00 AM IFACE rxpck/s txpck/s %ifutil\n"
+            "08:00:00 AM eth0 100.00 200.00 0.50\n"
+            "08:10:00 AM eth0 110.00 210.00 0.55\n"
+        )
+        test_params = {
+            'date_start': '2025-05-01',
+            'date_end': '2025-05-01',
+            'time_start': '08:00:00',
+            'time_end': '10:00:00',
+            'type': 'network',
+            'average': False,
+            'partition': False
+        }
+        self.run_sar_fact_test(test_params, fake_sar_output, "sar_net", "100.00")
 
-                with self.assertRaises(Exception) as context:
-                    sar_facts.main()
+    def test_sar_disk(self):
+        fake_sar_output = (
+            "08:00:00 AM DEV %util await rkB/s wkB/s\n"
+            "08:00:00 AM sda 90.00 5.00 100.00 200.00\n"
+            "08:10:00 AM sda 91.00 5.10 101.00 201.00\n"
+        )
+        test_params = {
+            'date_start': '2025-05-01',
+            'date_end': '2025-05-01',
+            'time_start': '08:00:00',
+            'time_end': '10:00:00',
+            'type': 'disk',
+            'average': False,
+            'partition': False
+        }
+        self.run_sar_fact_test(test_params, fake_sar_output, "sar_disk", "90.00")
 
-                result_str = str(context.exception)
-                # Verify that exit_json output contains expected SAR fact keys and values.
-                self.assertIn("sar_cpu", result_str)
-                self.assertIn("5.00", result_str)
-                self.assertIn("2025-05-01", result_str)
-                self.assertIn("exit_json called", result_str)
+    def test_sar_load(self):
+        fake_sar_output = (
+            "08:00:00 AM ldavg-1 ldavg-5 ldavg-15\n"
+            "08:00:00 AM 0.50 0.60 0.70\n"
+            "08:10:00 AM 0.55 0.65 0.75\n"
+        )
+        test_params = {
+            'date_start': '2025-05-01',
+            'date_end': '2025-05-01',
+            'time_start': '08:00:00',
+            'time_end': '10:00:00',
+            'type': 'load',
+            'average': False,
+            'partition': False
+        }
+        self.run_sar_fact_test(test_params, fake_sar_output, "sar_load", "0.50")
 
 
 if __name__ == '__main__':