Merge ~chad.smith/curtin:fix-autocollect-error-logs-makedirs into curtin:master

[Chad Smith]

error logs: Create error_tarfile path if path does not exist

When curtin's configured error_tarfile path does not exist in the
ephermeral environment, create it.

LP: #1746363

see commit message

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:e5ca7e7954c1e694cac62a52f83792bd0aabf59e
https://jenkins.ubuntu.com/server/job/curtin-ci/789/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/789
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/789
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/789
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/789

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/789/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

review: Needs Fixing

a9aad7a... by Chad Smith on 2018-01-30

use util.ensure_dir instead of a call to os.makedirs directly

[Ryan Harper (raharper)]

Looks good

review: Approve

diff --git a/curtin/commands/collect_logs.py b/curtin/commands/collect_logs.py
index 59b66c4..a61d91e 100644
--- a/curtin/commands/collect_logs.py
+++ b/curtin/commands/collect_logs.py
@@ -61,6 +61,9 @@ def create_log_tarfile(tarfile, config):
     """
     if not (isinstance(tarfile, util.string_types) and tarfile):
         raise ValueError("Invalid value '%s' for tarfile" % tarfile)
+    target_dir = os.path.dirname(tarfile)
+    if target_dir and not os.path.exists(target_dir):
+        util.ensure_dir(target_dir)
 
     instcfg = config.get('install', {})
     logfile = instcfg.get('log_file')
diff --git a/tests/unittests/test_commands_collect_logs.py b/tests/unittests/test_commands_collect_logs.py
index 100fbc1..d20cc54 100644
--- a/tests/unittests/test_commands_collect_logs.py
+++ b/tests/unittests/test_commands_collect_logs.py
@@ -226,11 +226,11 @@ class TestCollectLogs(CiTestCase):
             self.assertEqual(f.read(), expected)
 
 
-class TestWBCreateTar(CiTestCase):
-    """Whitebox texting of create_log_tarfile."""
+class TestCreateTar(CiTestCase):
+    """Whitebox testing of create_log_tarfile."""
 
     def setUp(self):
-        super(TestWBCreateTar, self).setUp()
+        super(TestCreateTar, self).setUp()
         self.new_root = self.tmp_dir()
         date = datetime.utcnow().strftime('%Y-%m-%d-%H-%M')
         self.tardir = 'curtin-logs-%s' % date
@@ -256,8 +256,29 @@ class TestWBCreateTar(CiTestCase):
             self.mock_subp.call_args_list)
         self.m_sys_info.assert_called_with(self.tardir, {})
 
+    def test_create_log_tarfile_creates_target_tar_directory_if_absent(self):
+        """create_log_tarfile makes the tarfile's directory if needed."""
+        tarfile = self.tmp_path('my.tar',
+                                _dir=os.path.join(self.new_root, 'dont/exist'))
+        destination_dir = os.path.dirname(tarfile)
+        self.assertFalse(os.path.exists(destination_dir),
+                         'Expected absent directory: %s' % destination_dir)
+        self.add_patch('curtin.util.subp', 'mock_subp')
+        self.mock_subp.return_value = ('', '')
+        with mock.patch('sys.stderr'):
+            with self.assertRaises(SystemExit) as context_manager:
+                collect_logs.create_log_tarfile(tarfile, config={})
+        self.assertEqual('0', str(context_manager.exception))
+        self.assertIn(
+            mock.call(['tar', '-cvf', tarfile, self.tardir],
+                      capture=True),
+            self.mock_subp.call_args_list)
+        self.m_sys_info.assert_called_with(self.tardir, {})
+        self.assertTrue(os.path.exists(destination_dir),
+                        'Expected directory created: %s' % destination_dir)
+
     def test_create_log_tarfile_copies_configured_logs(self):
-        """create_log_tarfile copies configured log_file and post_files if present.
+        """create_log_tarfile copies configured log_file and post_files.
 
         Configured log_file or post_files which don't exist are ignored.
         """
@@ -306,7 +327,7 @@ class TestWBCreateTar(CiTestCase):
 
 
 class TestWBCollectLogs(CiTestCase):
-    """Whitebox texting of _redact_sensitive_information."""
+    """Whitebox testing of _redact_sensitive_information."""
 
     def test_wb_redact_sensitive_information(self):
         """_redact_sensitive_information replaces redact_values in any file."""