Allow comments in runcmd and report failed commands correctly (#1049)

Allow comments in runcmd and report failed commands correctly

A `runcmd` script may fail to parse properly, but does not mark
`runcmd` as failed when that occurs. Additionally `shellify()` fails
to correctly parse scripts that contain a comment line.

Rectify both issues and add unit tests to verify correct behavior.

LP: #1853146

1 files changed, 21 insertions, 6 deletions

diff --git a/tests/unittests/test_handler/test_handler_runcmd.py b/tests/unittests/test_handler/test_handler_runcmd.py
index 73237d68..c03efa67 100644
--- a/tests/unittests/test_handler/test_handler_runcmd.py
+++ b/tests/unittests/test_handler/test_handler_runcmd.py
@@ -7,6 +7,7 @@ from cloudinit.tests.helpers import (
     CiTestCase, FilesystemMockingTestCase, SchemaTestCaseMixin,
     skipUnlessJsonSchema)
 
+from unittest.mock import patch
 import logging
 import os
 import stat
@@ -41,15 +42,27 @@ class TestRuncmd(FilesystemMockingTestCase):
             "Skipping module named notimportant, no 'runcmd' key",
             self.logs.getvalue())
 
+    @patch('cloudinit.util.shellify')
+    def test_runcmd_shellify_fails(self, cls):
+        """When shellify fails throw exception"""
+        cls.side_effect = TypeError("patched shellify")
+        valid_config = {'runcmd': ['echo 42']}
+        cc = self._get_cloud('ubuntu')
+        with self.assertRaises(TypeError) as cm:
+            with self.allow_subp(['/bin/sh']):
+                handle('cc_runcmd', valid_config, cc, LOG, None)
+        self.assertIn("Failed to shellify", str(cm.exception))
+
     def test_handler_invalid_command_set(self):
         """Commands which can't be converted to shell will raise errors."""
         invalid_config = {'runcmd': 1}
         cc = self._get_cloud('ubuntu')
-        handle('cc_runcmd', invalid_config, cc, LOG, [])
+        with self.assertRaises(TypeError) as cm:
+            handle('cc_runcmd', invalid_config, cc, LOG, [])
         self.assertIn(
             'Failed to shellify 1 into file'
             ' /var/lib/cloud/instances/iid-datasource-none/scripts/runcmd',
-            self.logs.getvalue())
+            str(cm.exception))
 
     @skipUnlessJsonSchema()
     def test_handler_schema_validation_warns_non_array_type(self):
@@ -60,11 +73,12 @@ class TestRuncmd(FilesystemMockingTestCase):
         """
         invalid_config = {'runcmd': 1}
         cc = self._get_cloud('ubuntu')
-        handle('cc_runcmd', invalid_config, cc, LOG, [])
+        with self.assertRaises(TypeError) as cm:
+            handle('cc_runcmd', invalid_config, cc, LOG, [])
         self.assertIn(
             'Invalid config:\nruncmd: 1 is not of type \'array\'',
             self.logs.getvalue())
-        self.assertIn('Failed to shellify', self.logs.getvalue())
+        self.assertIn('Failed to shellify', str(cm.exception))
 
     @skipUnlessJsonSchema()
     def test_handler_schema_validation_warns_non_array_item_type(self):
@@ -76,7 +90,8 @@ class TestRuncmd(FilesystemMockingTestCase):
         invalid_config = {
             'runcmd': ['ls /', 20, ['wget', 'http://stuff/blah'], {'a': 'n'}]}
         cc = self._get_cloud('ubuntu')
-        handle('cc_runcmd', invalid_config, cc, LOG, [])
+        with self.assertRaises(TypeError) as cm:
+            handle('cc_runcmd', invalid_config, cc, LOG, [])
         expected_warnings = [
             'runcmd.1: 20 is not valid under any of the given schemas',
             'runcmd.3: {\'a\': \'n\'} is not valid under any of the given'
@@ -85,7 +100,7 @@ class TestRuncmd(FilesystemMockingTestCase):
         logs = self.logs.getvalue()
         for warning in expected_warnings:
             self.assertIn(warning, logs)
-        self.assertIn('Failed to shellify', logs)
+        self.assertIn('Failed to shellify', str(cm.exception))
 
     def test_handler_write_valid_runcmd_schema_to_file(self):
         """Valid runcmd schema is written to a runcmd shell script."""