Always include the oslo_log log file in ramdisk logs

Even if journald is present, there is no guarantee that IPA logs there
(this is the case in container-based ramdisks).

Change-Id: Iceeab0010827728711e19e5b031ccac55fe1efde
(cherry picked from commit 2cedaa53c2ad306c4aeb2c2dc76c051db7408dbd)

5 files changed, 72 insertions, 3 deletions

diff --git a/ironic_python_agent/tests/unit/base.py b/ironic_python_agent/tests/unit/base.py
index 7d14821f..59f1be4a 100644
--- a/ironic_python_agent/tests/unit/base.py
+++ b/ironic_python_agent/tests/unit/base.py
@@ -21,6 +21,8 @@ import ironic_lib
 from oslo_concurrency import processutils
 from oslo_config import cfg
 from oslo_config import fixture as config_fixture
+from oslo_log import log
+from oslo_service import sslutils
 from oslotest import base as test_base
 
 from ironic_python_agent.extensions import base as ext_base
@@ -65,6 +67,9 @@ class IronicAgentTest(test_base.BaseTestCase):
 
     def _set_config(self):
         self.cfg_fixture = self.useFixture(config_fixture.Config(CONF))
+        # Register options from libraries that are explicitly used in the code
+        log.register_options(CONF)
+        sslutils.register_opts(CONF)
 
     def config(self, **kw):
         """Override config options for a test."""
diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py
index 9b95e695..0c568028 100644
--- a/ironic_python_agent/tests/unit/test_agent.py
+++ b/ironic_python_agent/tests/unit/test_agent.py
@@ -20,7 +20,6 @@ from ironic_lib import exception as lib_exc
 from oslo_concurrency import processutils
 from oslo_config import cfg
 from oslo_serialization import jsonutils
-from oslo_service import sslutils
 import pkg_resources
 from stevedore import extension
 
@@ -865,7 +864,6 @@ class TestAgentStandalone(ironic_agent_base.IronicAgentTest):
 
     def setUp(self):
         super(TestAgentStandalone, self).setUp()
-        sslutils.register_opts(CONF)
         self.agent = agent.IronicPythonAgent('https://fake_api.example.'
                                              'org:8081/',
                                              agent.Host(hostname='203.0.113.1',
diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py
index bfad67f0..2cf2a20b 100644
--- a/ironic_python_agent/tests/unit/test_utils.py
+++ b/ironic_python_agent/tests/unit/test_utils.py
@@ -291,7 +291,7 @@ class TestFailures(testtools.TestCase):
         self.assertRaisesRegex(FakeException, 'foo', f.raise_if_needed)
 
 
-class TestUtils(testtools.TestCase):
+class TestUtils(ironic_agent_base.IronicAgentTest):
 
     def _get_journalctl_output(self, mock_execute, lines=None, units=None):
         contents = b'Krusty Krab'
@@ -449,6 +449,33 @@ class TestUtils(testtools.TestCase):
     @mock.patch.object(utils, 'gzip_and_b64encode', 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):
+        tmp = tempfile.NamedTemporaryFile()
+        self.addCleanup(lambda: tmp.close())
+
+        self.config(log_file=tmp.name)
+        mock_journalctl.return_value = True
+        ret = 'Patrick Star'
+        mock_gzip_b64.return_value = ret
+
+        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(
+            file_list=[tmp.name],
+            io_dict={'journal': mock.ANY, 'ip_addr': mock.ANY, 'ps': mock.ANY,
+                     'df': mock.ANY, 'iptables': mock.ANY, 'lshw': mock.ANY,
+                     'lsblk': mock.ANY, 'mdstat': mock.ANY})
+
+    @mock.patch.object(utils, 'gzip_and_b64encode', 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_journalctl.return_value = False
@@ -468,6 +495,32 @@ class TestUtils(testtools.TestCase):
                      'dmesg': mock.ANY, 'df': mock.ANY, 'lshw': mock.ANY,
                      'lsblk': mock.ANY, 'mdstat': mock.ANY})
 
+    @mock.patch.object(utils, 'gzip_and_b64encode', 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):
+        tmp = tempfile.NamedTemporaryFile()
+        self.addCleanup(lambda: tmp.close())
+
+        self.config(log_file=tmp.name)
+        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(
+            file_list=['/var/log', tmp.name],
+            io_dict={'iptables': mock.ANY, 'ip_addr': mock.ANY, 'ps': mock.ANY,
+                     'dmesg': mock.ANY, 'df': mock.ANY, 'lshw': mock.ANY,
+                     'lsblk': mock.ANY, 'mdstat': mock.ANY})
+
     def test_get_ssl_client_options(self):
         # defaults
         conf = mock.Mock(insecure=False, cafile=None,
diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py
index 74ac328b..e34aa9b1 100644
--- a/ironic_python_agent/utils.py
+++ b/ironic_python_agent/utils.py
@@ -550,11 +550,19 @@ def collect_system_logs(journald_max_lines=None):
 
     io_dict = {}
     file_list = []
+    log_locations = [CONF.log_file, CONF.log_dir]
     if is_journalctl_present():
         io_dict['journal'] = get_journalctl_output(lines=journald_max_lines)
+        for log_loc in log_locations:
+            if log_loc and os.path.exists(log_loc):
+                file_list.append(log_loc)
     else:
         try_get_command_output(io_dict, 'dmesg', ['dmesg'])
         file_list.append('/var/log')
+        for log_loc in log_locations:
+            if (log_loc and os.path.exists(log_loc)
+                    and not log_loc.startswith('/var/log')):
+                file_list.append(log_loc)
 
     for name, cmd in COLLECT_LOGS_COMMANDS.items():
         try_get_command_output(io_dict, name, cmd)
diff --git a/releasenotes/notes/log-file-7aaaf31693ddc617.yaml b/releasenotes/notes/log-file-7aaaf31693ddc617.yaml
new file mode 100644
index 00000000..805073f9
--- /dev/null
+++ b/releasenotes/notes/log-file-7aaaf31693ddc617.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    The configured log file and/or log directory is now always explicitly
+    included in the ramdisk logs.