Merge branch 'master' into dependabot/pip/cryptography-3.2

9 files changed, 103 insertions, 22 deletions

diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
index 37f55d26..4314b9da 100644
--- a/.github/PULL_REQUEST_TEMPLATE.md
+++ b/.github/PULL_REQUEST_TEMPLATE.md
@@ -6,7 +6,7 @@
 
 ## Test Steps
 <!-- Please include any steps necessary to verify (and reproduce if
-this is a bug fix) this change on an actual Ubuntu Server image,
+this is a bug fix) this change on a live deployed system,
 including any necessary configuration files, user-data,
 setup, and teardown. Scripts used may be attached directly to this PR. -->
 
diff --git a/cloudinit/tests/test_util.py b/cloudinit/tests/test_util.py
index 77714928..b7a302f1 100644
--- a/cloudinit/tests/test_util.py
+++ b/cloudinit/tests/test_util.py
@@ -730,6 +730,41 @@ class TestMountCb:
         """already_mounted_device_and_mountdict, but return only the device"""
         return already_mounted_device_and_mountdict[0]
 
+    @pytest.mark.parametrize(
+        "mtype,expected",
+        [
+            # While the filesystem is called iso9660, the mount type is cd9660
+            ("iso9660", "cd9660"),
+            # vfat is generally called "msdos" on BSD
+            ("vfat", "msdos"),
+            # judging from man pages, only FreeBSD has this alias
+            ("msdosfs", "msdos"),
+            # Test happy path
+            ("ufs", "ufs")
+        ],
+    )
+    @mock.patch("cloudinit.util.is_Linux", autospec=True)
+    @mock.patch("cloudinit.util.is_BSD", autospec=True)
+    @mock.patch("cloudinit.util.subp.subp")
+    @mock.patch("cloudinit.temp_utils.tempdir", autospec=True)
+    def test_normalize_mtype_on_bsd(
+        self, m_tmpdir, m_subp, m_is_BSD, m_is_Linux, mtype, expected
+    ):
+        m_is_BSD.return_value = True
+        m_is_Linux.return_value = False
+        m_tmpdir.return_value.__enter__ = mock.Mock(
+            autospec=True, return_value="/tmp/fake"
+        )
+        m_tmpdir.return_value.__exit__ = mock.Mock(
+            autospec=True, return_value=True
+        )
+        callback = mock.Mock(autospec=True)
+
+        util.mount_cb('/dev/fake0', callback, mtype=mtype)
+        assert mock.call(
+            ["mount", "-o", "ro", "-t", expected, "/dev/fake0", "/tmp/fake"],
+            update_env=None) in m_subp.call_args_list
+
     @pytest.mark.parametrize("invalid_mtype", [int(0), float(0.0), dict()])
     def test_typeerror_raised_for_invalid_mtype(self, invalid_mtype):
         with pytest.raises(TypeError):
diff --git a/cloudinit/util.py b/cloudinit/util.py
index 83727544..b8856af1 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -418,6 +418,11 @@ def multi_log(text, console=True, stderr=True,
 
 
 @lru_cache()
+def is_Linux():
+    return 'Linux' in platform.system()
+
+
+@lru_cache()
 def is_BSD():
     return 'BSD' in platform.system()
 
@@ -1661,16 +1666,17 @@ def mount_cb(device, callback, data=None, mtype=None,
                 _type=type(mtype)))
 
     # clean up 'mtype' input a bit based on platform.
-    platsys = platform.system().lower()
-    if platsys == "linux":
+    if is_Linux():
         if mtypes is None:
             mtypes = ["auto"]
-    elif platsys.endswith("bsd"):
+    elif is_BSD():
         if mtypes is None:
-            mtypes = ['ufs', 'cd9660', 'vfat']
+            mtypes = ['ufs', 'cd9660', 'msdos']
         for index, mtype in enumerate(mtypes):
             if mtype == "iso9660":
                 mtypes[index] = "cd9660"
+            if mtype in ["vfat", "msdosfs"]:
+                mtypes[index] = "msdos"
     else:
         # we cannot do a smart "auto", so just call 'mount' once with no -t
         mtypes = ['']
diff --git a/conftest.py b/conftest.py
index 459b708a..9e9d9ff8 100644
--- a/conftest.py
+++ b/conftest.py
@@ -70,20 +70,30 @@ def disable_subp_usage(request, fixture_utils):
     """
     Across all (pytest) tests, ensure that subp.subp is not invoked.
 
-    Note that this can only catch invocations where the util module is imported
-    and ``subp.subp(...)`` is called.  ``from cloudinit.subp mport subp``
-    imports happen before the patching here (or the CiTestCase monkey-patching)
-    happens, so are left untouched.
-
-    To allow a particular test method or class to use subp.subp you can mark it
-    as such::
+    Note that this can only catch invocations where the ``subp`` module is
+    imported and ``subp.subp(...)`` is called.  ``from cloudinit.subp import
+    subp`` imports happen before the patching here (or the CiTestCase
+    monkey-patching) happens, so are left untouched.
+
+    While ``disable_subp_usage`` unconditionally patches
+    ``cloudinit.subp.subp``, any test-local patching will override this
+    patching (i.e. the mock created for that patch call will replace the mock
+    created by ``disable_subp_usage``), allowing tests to be written normally.
+    One important exception: if ``autospec=True`` is passed to such an
+    overriding patch call it will fail: autospeccing introspects the object
+    being patched and as ``subp.subp`` will always be a mock when that
+    autospeccing happens, the introspection fails.  (The specific error is:
+    ``TypeError: name must be a str, not a MagicMock``.)
+
+    To allow a particular test method or class to use ``subp.subp`` you can
+    mark it as such::
 
         @pytest.mark.allow_all_subp
         def test_whoami(self):
             subp.subp(["whoami"])
 
-    To instead allow subp.subp usage for a specific command, you can use the
-    ``allow_subp_for`` mark::
+    To instead allow ``subp.subp`` usage for a specific command, you can use
+    the ``allow_subp_for`` mark::
 
         @pytest.mark.allow_subp_for("bash")
         def test_bash(self):
@@ -97,9 +107,9 @@ def disable_subp_usage(request, fixture_utils):
             subp.subp(["whoami"])
 
     This fixture (roughly) mirrors the functionality of
-    CiTestCase.allowed_subp.  N.B. While autouse fixtures do affect non-pytest
-    tests, CiTestCase's allowed_subp does take precedence (and we have
-    TestDisableSubpUsageInTestSubclass to confirm that).
+    ``CiTestCase.allowed_subp``.  N.B. While autouse fixtures do affect
+    non-pytest tests, CiTestCase's ``allowed_subp`` does take precedence (and
+    we have ``TestDisableSubpUsageInTestSubclass`` to confirm that).
     """
     allow_subp_for = fixture_utils.closest_marker_args_or(
         request, "allow_subp_for", None
diff --git a/doc/rtd/topics/instancedata.rst b/doc/rtd/topics/instancedata.rst
index 255245a4..1850982c 100644
--- a/doc/rtd/topics/instancedata.rst
+++ b/doc/rtd/topics/instancedata.rst
@@ -592,6 +592,22 @@ see only redacted values.
  % cloud-init query --format 'cloud: {{ v1.cloud_name }} myregion: {{
  % v1.region }}'
 
+ # Locally test that your template userdata provided to the vm was rendered as
+ # intended.
+ % cloud-init query --format "$(sudo cloud-init query userdata)"
+
+ # The --format command renders jinja templates, this can also be used
+ # to develop and test jinja template constructs
+ % cat > test-templating.yaml <<EOF
+   {% for val in ds.meta_data.keys() %}
+   - {{ val }}
+   {% endfor %}
+   EOF
+ % cloud-init query --format="$( cat test-templating.yaml )"
+ - instance_id
+ - dsmode
+ - local_hostname
+
 .. note::
   To save time designing a user-data template for a specific cloud's
   instance-data.json, use the 'render' cloud-init command on an
diff --git a/tests/integration_tests/bugs/test_lp1886531.py b/tests/integration_tests/bugs/test_lp1886531.py
index b1112185..058ea8bb 100644
--- a/tests/integration_tests/bugs/test_lp1886531.py
+++ b/tests/integration_tests/bugs/test_lp1886531.py
@@ -15,7 +15,7 @@ import pytest
 USER_DATA = """\
 #cloud-config
 bootcmd:
-- rm /etc/fstab
+- rm -f /etc/fstab
 """
 
 
diff --git a/tests/integration_tests/clouds.py b/tests/integration_tests/clouds.py
index 06f1c623..08c86198 100644
--- a/tests/integration_tests/clouds.py
+++ b/tests/integration_tests/clouds.py
@@ -54,13 +54,18 @@ class IntegrationCloud(ABC):
                 self.settings.EXISTING_INSTANCE_ID
             )
             return
-        launch_kwargs = {
+        kwargs = {
             'image_id': self.image_id,
             'user_data': user_data,
             'wait': False,
         }
-        launch_kwargs.update(launch_kwargs)
-        pycloudlib_instance = self.cloud_instance.launch(**launch_kwargs)
+        kwargs.update(launch_kwargs)
+        log.info(
+            "Launching instance with launch_kwargs:\n{}".format(
+                "\n".join("{}={}".format(*item) for item in kwargs.items())
+            )
+        )
+        pycloudlib_instance = self.cloud_instance.launch(**kwargs)
         pycloudlib_instance.wait(raise_on_cloudinit_failure=False)
         log.info('Launched instance: %s', pycloudlib_instance)
         return self.get_instance(pycloudlib_instance, settings)
diff --git a/tests/integration_tests/conftest.py b/tests/integration_tests/conftest.py
index 9163ac66..34e674e9 100644
--- a/tests/integration_tests/conftest.py
+++ b/tests/integration_tests/conftest.py
@@ -111,7 +111,15 @@ def _client(request, fixture_utils, session_cloud):
     """
     user_data = fixture_utils.closest_marker_first_arg_or(
         request, 'user_data', None)
-    with session_cloud.launch(user_data=user_data) as instance:
+    name = fixture_utils.closest_marker_first_arg_or(
+        request, 'instance_name', None
+    )
+    launch_kwargs = {}
+    if name is not None:
+        launch_kwargs = {"name": name}
+    with session_cloud.launch(
+        user_data=user_data, launch_kwargs=launch_kwargs
+    ) as instance:
         yield instance
 
 
diff --git a/tox.ini b/tox.ini
index a8681197..816e6e8e 100644
--- a/tox.ini
+++ b/tox.ini
@@ -158,3 +158,4 @@ markers =
     oci: test will only run on OCI platform
     lxd_container: test will only run in LXD container
     user_data: the user data to be passed to the test instance
+    instance_name: the name to be used for the test instance