Make logs collection a hardware manager call

This allows hardware managers to collect additional logs.

Change-Id: If082b921d4bf71c4cc41a5a72db6995b08637374

2 files changed, 74 insertions, 94 deletions

diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index ab4f9cc0..fdb5c8df 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import binascii
+import json
 import os
 import shutil
 import stat
@@ -5916,3 +5917,53 @@ class TestProtectedDiskSafetyChecks(base.IronicAgentTest):
                               hardware.safety_check_block_device,
                               {}, '/dev/foo')
             self.assertEqual(1, mock_execute.call_count)
+
+
+@mock.patch.object(il_utils, 'execute', autospec=True)
+class TestCollectSystemLogs(base.IronicAgentTest):
+
+    def setUp(self):
+        super().setUp()
+        self.hardware = hardware.GenericHardwareManager()
+
+    @mock.patch('pyudev.Context', lambda: mock.sentinel.context)
+    @mock.patch('pyudev.Devices.from_device_file', autospec=True)
+    def test__collect_udev(self, mock_from_dev, mock_execute):
+        mock_execute.return_value = """
+            fake0
+            fake1
+            fake42
+        """, ""
+        mock_from_dev.side_effect = [
+            mock.Mock(properties={'ID_UUID': '0'}),
+            RuntimeError('nope'),
+            {'ID_UUID': '42'}
+        ]
+
+        result = {}
+        hardware._collect_udev(result)
+        self.assertEqual({'udev/fake0', 'udev/fake42'}, set(result))
+        for i in ('0', '42'):
+            buf = result[f'udev/fake{i}']
+            # Avoiding getvalue on purpose - checking that the IO is not closed
+            val = json.loads(buf.read().decode('utf-8'))
+            self.assertEqual({'ID_UUID': i}, val)
+
+    @mock.patch.object(hardware, '_collect_udev', autospec=True)
+    def test_collect_system_logs(self, mock_udev, mock_execute):
+        commands = set()
+        expected = {'df', 'dmesg', 'iptables', 'ip', 'lsblk',
+                    'lshw', 'cat', 'mount', 'multipath', 'parted', 'ps'}
+
+        def fake_execute(cmd, *args, **kwargs):
+            commands.add(cmd)
+            return cmd.encode(), ''
+
+        mock_execute.side_effect = fake_execute
+
+        io_dict = {}
+        file_list = []
+        self.hardware.collect_system_logs(io_dict, file_list)
+
+        self.assertEqual(commands, expected)
+        self.assertGreaterEqual(len(io_dict), len(expected))
diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py
index 9382a6b4..fc683d22 100644
--- a/ironic_python_agent/tests/unit/test_utils.py
+++ b/ironic_python_agent/tests/unit/test_utils.py
@@ -17,7 +17,6 @@ import base64
 import errno
 import glob
 import io
-import json
 import os
 import shutil
 import subprocess
@@ -411,14 +410,12 @@ class TestUtils(ironic_agent_base.IronicAgentTest):
         mock_call.side_effect = os_error
         self.assertFalse(utils.is_journalctl_present())
 
-    @mock.patch.object(utils, '_collect_udev', autospec=True)
     @mock.patch.object(utils, 'gzip_and_b64encode', autospec=True)
+    @mock.patch.object(hardware, 'dispatch_to_all_managers', autospec=True)
     @mock.patch.object(utils, 'is_journalctl_present', autospec=True)
-    @mock.patch.object(utils, 'get_command_output', autospec=True)
     @mock.patch.object(utils, 'get_journalctl_output', autospec=True)
     def test_collect_system_logs_journald(
-            self, mock_logs, mock_outputs, mock_journalctl, mock_gzip_b64,
-            mock_udev):
+            self, mock_logs, mock_journalctl, mock_dispatch, mock_gzip_b64):
         mock_journalctl.return_value = True
         ret = 'Patrick Star'
         mock_gzip_b64.return_value = ret
@@ -426,28 +423,17 @@ class TestUtils(ironic_agent_base.IronicAgentTest):
         logs_string = utils.collect_system_logs()
         self.assertEqual(ret, logs_string)
         mock_logs.assert_called_once_with(lines=None)
-        calls = [mock.call(['ps', 'au']), mock.call(['df', '-a']),
-                 mock.call(['iptables', '-L']), mock.call(['ip', 'addr']),
-                 mock.call(['lshw', '-quiet', '-json'])]
-        mock_outputs.assert_has_calls(calls, any_order=True)
         mock_gzip_b64.assert_called_once_with(
-            io_dict={'journal': mock.ANY, 'ps': mock.ANY, 'df': mock.ANY,
-                     'iptables': mock.ANY, 'ip_addr': mock.ANY,
-                     'lshw': mock.ANY, 'lsblk': mock.ANY,
-                     'lsblk-full': mock.ANY, 'mdstat': mock.ANY,
-                     'mount': mock.ANY, 'parted': mock.ANY,
-                     'multipath': mock.ANY},
-            file_list=[])
-        mock_udev.assert_called_once_with(mock.ANY)
-
-    @mock.patch.object(utils, '_collect_udev', autospec=True)
+            io_dict=mock.ANY, file_list=[])
+        mock_dispatch.assert_called_once_with('collect_system_logs',
+                                              mock.ANY, [])
+
     @mock.patch.object(utils, 'gzip_and_b64encode', autospec=True)
+    @mock.patch.object(hardware, 'dispatch_to_all_managers', autospec=True)
     @mock.patch.object(utils, 'is_journalctl_present', autospec=True)
-    @mock.patch.object(utils, 'get_command_output', autospec=True)
     @mock.patch.object(utils, 'get_journalctl_output', autospec=True)
     def test_collect_system_logs_journald_with_logfile(
-            self, mock_logs, mock_outputs, mock_journalctl, mock_gzip_b64,
-            mock_udev):
+            self, mock_logs, mock_journalctl, mock_dispatch, mock_gzip_b64):
         tmp = tempfile.NamedTemporaryFile()
         self.addCleanup(lambda: tmp.close())
 
@@ -459,55 +445,32 @@ class TestUtils(ironic_agent_base.IronicAgentTest):
         logs_string = utils.collect_system_logs()
         self.assertEqual(ret, logs_string)
         mock_logs.assert_called_once_with(lines=None)
-        calls = [mock.call(['ps', 'au']), mock.call(['df', '-a']),
-                 mock.call(['iptables', '-L']), mock.call(['ip', 'addr']),
-                 mock.call(['lshw', '-quiet', '-json'])]
-        mock_outputs.assert_has_calls(calls, any_order=True)
         mock_gzip_b64.assert_called_once_with(
-            io_dict={'journal': mock.ANY, 'ps': mock.ANY, 'df': mock.ANY,
-                     'iptables': mock.ANY, 'ip_addr': mock.ANY,
-                     'lshw': mock.ANY, 'lsblk': mock.ANY,
-                     'lsblk-full': mock.ANY, 'mdstat': mock.ANY,
-                     'mount': mock.ANY, 'parted': mock.ANY,
-                     'multipath': mock.ANY},
-            file_list=[tmp.name])
-        mock_udev.assert_called_once_with(mock.ANY)
-
-    @mock.patch.object(utils, '_collect_udev', autospec=True)
+            io_dict=mock.ANY, file_list=[tmp.name])
+        mock_dispatch.assert_called_once_with('collect_system_logs',
+                                              mock.ANY, [tmp.name])
+
     @mock.patch.object(utils, 'gzip_and_b64encode', autospec=True)
+    @mock.patch.object(hardware, 'dispatch_to_all_managers', autospec=True)
     @mock.patch.object(utils, 'is_journalctl_present', autospec=True)
-    @mock.patch.object(utils, 'get_command_output', autospec=True)
     def test_collect_system_logs_non_journald(
-            self, mock_outputs, mock_journalctl, mock_gzip_b64,
-            mock_udev):
+            self, mock_journalctl, mock_dispatch, mock_gzip_b64):
         mock_journalctl.return_value = False
         ret = 'SpongeBob SquarePants'
         mock_gzip_b64.return_value = ret
 
         logs_string = utils.collect_system_logs()
         self.assertEqual(ret, logs_string)
-        calls = [mock.call(['dmesg']), mock.call(['ps', 'au']),
-                 mock.call(['df', '-a']), mock.call(['iptables', '-L']),
-                 mock.call(['ip', 'addr']),
-                 mock.call(['lshw', '-quiet', '-json'])]
-        mock_outputs.assert_has_calls(calls, any_order=True)
         mock_gzip_b64.assert_called_once_with(
-            io_dict={'dmesg': mock.ANY, 'ps': mock.ANY, 'df': mock.ANY,
-                     'iptables': mock.ANY, 'ip_addr': mock.ANY,
-                     'lshw': mock.ANY, 'lsblk': mock.ANY,
-                     'lsblk-full': mock.ANY, 'mdstat': mock.ANY,
-                     'mount': mock.ANY, 'parted': mock.ANY,
-                     'multipath': mock.ANY},
-            file_list=['/var/log'])
-        mock_udev.assert_called_once_with(mock.ANY)
-
-    @mock.patch.object(utils, '_collect_udev', autospec=True)
+            io_dict=mock.ANY, file_list=['/var/log'])
+        mock_dispatch.assert_called_once_with('collect_system_logs',
+                                              mock.ANY, ['/var/log'])
+
     @mock.patch.object(utils, 'gzip_and_b64encode', autospec=True)
+    @mock.patch.object(hardware, 'dispatch_to_all_managers', autospec=True)
     @mock.patch.object(utils, 'is_journalctl_present', autospec=True)
-    @mock.patch.object(utils, 'get_command_output', autospec=True)
     def test_collect_system_logs_non_journald_with_logfile(
-            self, mock_outputs, mock_journalctl, mock_gzip_b64,
-            mock_udev):
+            self, mock_journalctl, mock_dispatch, mock_gzip_b64):
         tmp = tempfile.NamedTemporaryFile()
         self.addCleanup(lambda: tmp.close())
 
@@ -518,44 +481,10 @@ class TestUtils(ironic_agent_base.IronicAgentTest):
 
         logs_string = utils.collect_system_logs()
         self.assertEqual(ret, logs_string)
-        calls = [mock.call(['dmesg']), mock.call(['ps', 'au']),
-                 mock.call(['df', '-a']), mock.call(['iptables', '-L']),
-                 mock.call(['ip', 'addr']),
-                 mock.call(['lshw', '-quiet', '-json'])]
-        mock_outputs.assert_has_calls(calls, any_order=True)
         mock_gzip_b64.assert_called_once_with(
-            io_dict={'dmesg': mock.ANY, 'ps': mock.ANY, 'df': mock.ANY,
-                     'iptables': mock.ANY, 'ip_addr': mock.ANY,
-                     'lshw': mock.ANY, 'lsblk': mock.ANY,
-                     'lsblk-full': mock.ANY, 'mdstat': mock.ANY,
-                     'mount': mock.ANY, 'parted': mock.ANY,
-                     'multipath': mock.ANY},
-            file_list=['/var/log', tmp.name])
-        mock_udev.assert_called_once_with(mock.ANY)
-
-    @mock.patch('pyudev.Context', lambda: mock.sentinel.context)
-    @mock.patch('pyudev.Devices.from_device_file', autospec=True)
-    @mock.patch.object(ironic_utils, 'execute', autospec=True)
-    def test_collect_udev(self, mock_execute, mock_from_dev):
-        mock_execute.return_value = """
-            fake0
-            fake1
-            fake42
-        """, ""
-        mock_from_dev.side_effect = [
-            mock.Mock(properties={'ID_UUID': '0'}),
-            RuntimeError('nope'),
-            {'ID_UUID': '42'}
-        ]
-
-        result = {}
-        utils._collect_udev(result)
-        self.assertEqual({'udev/fake0', 'udev/fake42'}, set(result))
-        for i in ('0', '42'):
-            buf = result[f'udev/fake{i}']
-            # Avoiding getvalue on purpose - checking that the IO is not closed
-            val = json.loads(buf.read().decode('utf-8'))
-            self.assertEqual({'ID_UUID': i}, val)
+            io_dict=mock.ANY, file_list=['/var/log', tmp.name])
+        mock_dispatch.assert_called_once_with('collect_system_logs',
+                                              mock.ANY, ['/var/log', tmp.name])
 
     def test_get_ssl_client_options(self):
         # defaults