tests: Disallow use of util.subp except for where needed.

In many cases, cloud-init uses 'util.subp' to run a subprocess.
This is not really desirable in our unit tests as it makes the tests
dependent upon existance of those utilities.

The change here is to modify the base test case class (CiTestCase) to
raise exception any time subp is called.  Then, fix all callers.
For cases where subp is necessary or actually desired, we can use it
via
  a.) context hander CiTestCase.allow_subp(value)
  b.) class level self.allowed_subp = value

Both cases the value is a list of acceptable executable names that
will be called (essentially argv[0]).

Some cleanups in AltCloud were done as the code was being updated.

5 files changed, 43 insertions, 16 deletions

diff --git a/tests/unittests/test_handler/test_handler_apt_source_v3.py b/tests/unittests/test_handler/test_handler_apt_source_v3.py
index 7a64c230..a81c67c7 100644
--- a/tests/unittests/test_handler/test_handler_apt_source_v3.py
+++ b/tests/unittests/test_handler/test_handler_apt_source_v3.py
@@ -48,6 +48,10 @@ ADD_APT_REPO_MATCH = r"^[\w-]+:\w"
 
 TARGET = None
 
+MOCK_LSB_RELEASE_DATA = {
+    'id': 'Ubuntu', 'description': 'Ubuntu 18.04.1 LTS',
+    'release': '18.04', 'codename': 'bionic'}
+
 
 class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
     """TestAptSourceConfig
@@ -64,6 +68,9 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
         self.aptlistfile3 = os.path.join(self.tmp, "single-deb3.list")
         self.join = os.path.join
         self.matcher = re.compile(ADD_APT_REPO_MATCH).search
+        self.add_patch(
+            'cloudinit.config.cc_apt_configure.util.lsb_release',
+            'm_lsb_release', return_value=MOCK_LSB_RELEASE_DATA.copy())
 
     @staticmethod
     def _add_apt_sources(*args, **kwargs):
@@ -76,7 +83,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
         Get the most basic default mrror and release info to be used in tests
         """
         params = {}
-        params['RELEASE'] = util.lsb_release()['codename']
+        params['RELEASE'] = MOCK_LSB_RELEASE_DATA['release']
         arch = 'amd64'
         params['MIRROR'] = cc_apt_configure.\
             get_default_mirrors(arch)["PRIMARY"]
@@ -464,7 +471,7 @@ class TestAptSourceConfig(t_help.FilesystemMockingTestCase):
                              'uri':
                              'http://testsec.ubuntu.com/%s/' % component}]}
         post = ("%s_dists_%s-updates_InRelease" %
-                (component, util.lsb_release()['codename']))
+                (component, MOCK_LSB_RELEASE_DATA['codename']))
         fromfn = ("%s/%s_%s" % (pre, archive, post))
         tofn = ("%s/test.ubuntu.com_%s" % (pre, post))
 
diff --git a/tests/unittests/test_handler/test_handler_bootcmd.py b/tests/unittests/test_handler/test_handler_bootcmd.py
index b1375269..a76760fa 100644
--- a/tests/unittests/test_handler/test_handler_bootcmd.py
+++ b/tests/unittests/test_handler/test_handler_bootcmd.py
@@ -118,7 +118,8 @@ class TestBootcmd(CiTestCase):
             'echo {0} $INSTANCE_ID > {1}'.format(my_id, out_file)]}
 
         with mock.patch(self._etmpfile_path, FakeExtendedTempFile):
-            handle('cc_bootcmd', valid_config, cc, LOG, [])
+            with self.allow_subp(['/bin/sh']):
+                handle('cc_bootcmd', valid_config, cc, LOG, [])
         self.assertEqual(my_id + ' iid-datasource-none\n',
                          util.load_file(out_file))
 
@@ -128,12 +129,13 @@ class TestBootcmd(CiTestCase):
         valid_config = {'bootcmd': ['exit 1']}  # Script with error
 
         with mock.patch(self._etmpfile_path, FakeExtendedTempFile):
-            with self.assertRaises(util.ProcessExecutionError) as ctxt_manager:
-                handle('does-not-matter', valid_config, cc, LOG, [])
+            with self.allow_subp(['/bin/sh']):
+                with self.assertRaises(util.ProcessExecutionError) as ctxt:
+                    handle('does-not-matter', valid_config, cc, LOG, [])
         self.assertIn(
             'Unexpected error while running command.\n'
             "Command: ['/bin/sh',",
-            str(ctxt_manager.exception))
+            str(ctxt.exception))
         self.assertIn(
             'Failed to run bootcmd module does-not-matter',
             self.logs.getvalue())
diff --git a/tests/unittests/test_handler/test_handler_chef.py b/tests/unittests/test_handler/test_handler_chef.py
index f4bbd66d..b16532ea 100644
--- a/tests/unittests/test_handler/test_handler_chef.py
+++ b/tests/unittests/test_handler/test_handler_chef.py
@@ -36,13 +36,21 @@ class TestInstallChefOmnibus(HttprettyTestCase):
 
     @mock.patch("cloudinit.config.cc_chef.OMNIBUS_URL", OMNIBUS_URL_HTTP)
     def test_install_chef_from_omnibus_runs_chef_url_content(self):
-        """install_chef_from_omnibus runs downloaded OMNIBUS_URL as script."""
-        chef_outfile = self.tmp_path('chef.out', self.new_root)
-        response = '#!/bin/bash\necho "Hi Mom" > {0}'.format(chef_outfile)
+        """install_chef_from_omnibus calls subp_blob_in_tempfile."""
+        response = b'#!/bin/bash\necho "Hi Mom"'
         httpretty.register_uri(
             httpretty.GET, cc_chef.OMNIBUS_URL, body=response, status=200)
-        cc_chef.install_chef_from_omnibus()
-        self.assertEqual('Hi Mom\n', util.load_file(chef_outfile))
+        ret = (None, None)  # stdout, stderr but capture=False
+
+        with mock.patch("cloudinit.config.cc_chef.util.subp_blob_in_tempfile",
+                        return_value=ret) as m_subp_blob:
+            cc_chef.install_chef_from_omnibus()
+        # admittedly whitebox, but assuming subp_blob_in_tempfile works
+        # this should be fine.
+        self.assertEqual(
+            [mock.call(blob=response, args=[], basename='chef-omnibus-install',
+                       capture=False)],
+            m_subp_blob.call_args_list)
 
     @mock.patch('cloudinit.config.cc_chef.url_helper.readurl')
     @mock.patch('cloudinit.config.cc_chef.util.subp_blob_in_tempfile')
diff --git a/tests/unittests/test_handler/test_handler_resizefs.py b/tests/unittests/test_handler/test_handler_resizefs.py
index f92175fd..feca56c2 100644
--- a/tests/unittests/test_handler/test_handler_resizefs.py
+++ b/tests/unittests/test_handler/test_handler_resizefs.py
@@ -150,10 +150,12 @@ class TestResizefs(CiTestCase):
         self.assertEqual(('growfs', '-y', devpth),
                          _resize_ufs(mount_point, devpth))
 
+    @mock.patch('cloudinit.util.is_container', return_value=False)
     @mock.patch('cloudinit.util.get_mount_info')
     @mock.patch('cloudinit.util.get_device_info_from_zpool')
     @mock.patch('cloudinit.util.parse_mount')
-    def test_handle_zfs_root(self, mount_info, zpool_info, parse_mount):
+    def test_handle_zfs_root(self, mount_info, zpool_info, parse_mount,
+                             is_container):
         devpth = 'vmzroot/ROOT/freebsd'
         disk = 'gpt/system'
         fs_type = 'zfs'
@@ -354,8 +356,10 @@ class TestMaybeGetDevicePathAsWritableBlock(CiTestCase):
             ('btrfs', 'filesystem', 'resize', 'max', '/'),
             _resize_btrfs("/", "/dev/sda1"))
 
+    @mock.patch('cloudinit.util.is_container', return_value=True)
     @mock.patch('cloudinit.util.is_FreeBSD')
-    def test_maybe_get_writable_device_path_zfs_freebsd(self, freebsd):
+    def test_maybe_get_writable_device_path_zfs_freebsd(self, freebsd,
+                                                        m_is_container):
         freebsd.return_value = True
         info = 'dev=gpt/system mnt_point=/ path=/'
         devpth = maybe_get_writable_device_path('gpt/system', info, LOG)
diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py
index fb266faf..1bad07f6 100644
--- a/tests/unittests/test_handler/test_schema.py
+++ b/tests/unittests/test_handler/test_schema.py
@@ -4,7 +4,7 @@ from cloudinit.config.schema import (
     CLOUD_CONFIG_HEADER, SchemaValidationError, annotated_cloudconfig_file,
     get_schema_doc, get_schema, validate_cloudconfig_file,
     validate_cloudconfig_schema, main)
-from cloudinit.util import subp, write_file
+from cloudinit.util import write_file
 
 from cloudinit.tests.helpers import CiTestCase, mock, skipUnlessJsonSchema
 
@@ -406,8 +406,14 @@ class CloudTestsIntegrationTest(CiTestCase):
         integration_testdir = os.path.sep.join(
             [testsdir, 'cloud_tests', 'testcases'])
         errors = []
-        out, _ = subp(['find', integration_testdir, '-name', '*yaml'])
-        for filename in out.splitlines():
+
+        yaml_files = []
+        for root, _dirnames, filenames in os.walk(integration_testdir):
+            yaml_files.extend([os.path.join(root, f)
+                               for f in filenames if f.endswith(".yaml")])
+        self.assertTrue(len(yaml_files) > 0)
+
+        for filename in yaml_files:
             test_cfg = safe_load(open(filename))
             cloud_config = test_cfg.get('cloud_config')
             if cloud_config: