Update tests in Azure TestCanDevBeReformatted class (#2771)

Add additional test in test_azure.py to vet the expected behavior of
suppressing error messages from mount_cb.
This is addressing PR #2134

1 files changed, 69 insertions, 32 deletions

diff --git a/tests/unittests/sources/test_azure.py b/tests/unittests/sources/test_azure.py
index ed605ee7..f4aa4927 100644
--- a/tests/unittests/sources/test_azure.py
+++ b/tests/unittests/sources/test_azure.py
@@ -891,7 +891,6 @@ class TestNetworkConfig:
 
 
 class TestAzureDataSource(CiTestCase):
-
     with_logs = True
 
     def setUp(self):
@@ -2221,6 +2220,7 @@ class TestReadAzureOvf(CiTestCase):
 
 
 class TestCanDevBeReformatted(CiTestCase):
+    with_logs = True
     warning_file = "dataloss_warning_readme.txt"
 
     def _domock(self, mockpath, sattr=None):
@@ -2250,23 +2250,12 @@ class TestCanDevBeReformatted(CiTestCase):
             # return sorted by partition number
             return sorted(ret, key=lambda d: d[0])
 
-        def mount_cb(
-            device, callback, mtype, update_env_for_mount, log_error=False
-        ):
-            self.assertEqual("ntfs", mtype)
-            self.assertEqual("C", update_env_for_mount.get("LANG"))
-            p = self.tmp_dir()
-            for f in bypath.get(device).get("files", []):
-                write_file(os.path.join(p, f), content=f)
-            return callback(p)
-
         def has_ntfs_fs(device):
             return bypath.get(device, {}).get("fs") == "ntfs"
 
         p = MOCKPATH
         self._domock(p + "_partitions_on_device", "m_partitions_on_device")
         self._domock(p + "_has_ntfs_filesystem", "m_has_ntfs_filesystem")
-        self._domock(p + "util.mount_cb", "m_mount_cb")
         self._domock(p + "os.path.realpath", "m_realpath")
         self._domock(p + "os.path.exists", "m_exists")
         self._domock(p + "util.SeLinuxGuard", "m_selguard")
@@ -2274,14 +2263,55 @@ class TestCanDevBeReformatted(CiTestCase):
         self.m_exists.side_effect = lambda p: p in bypath
         self.m_realpath.side_effect = realpath
         self.m_has_ntfs_filesystem.side_effect = has_ntfs_fs
-        self.m_mount_cb.side_effect = mount_cb
         self.m_partitions_on_device.side_effect = partitions_on_device
         self.m_selguard.__enter__ = mock.Mock(return_value=False)
         self.m_selguard.__exit__ = mock.Mock()
 
+        return bypath
+
+    M_PATH = "cloudinit.util."
+
+    @mock.patch(M_PATH + "subp.subp")
+    def test_ntfs_mount_logs(self, m_subp):
+        """can_dev_be_reformatted does not log errors in case of
+        unknown filesystem 'ntfs'."""
+        self.patchup(
+            {
+                "/dev/sda": {
+                    "partitions": {
+                        "/dev/fake0": {"num": 1, "fs": "ntfs", "files": []}
+                    }
+                }
+            }
+        )
+
+        log_msg = "Failed to mount device"
+
+        m_subp.side_effect = subp.ProcessExecutionError(
+            "", "unknown filesystem type 'ntfs'"
+        )
+
+        dsaz.can_dev_be_reformatted("/dev/sda", preserve_ntfs=False)
+        self.assertNotIn(log_msg, self.logs.getvalue())
+
+    def _domock_mount_cb(self, bypath):
+        def mount_cb(
+            device, callback, mtype, update_env_for_mount, log_error=False
+        ):
+            self.assertEqual("ntfs", mtype)
+            self.assertEqual("C", update_env_for_mount.get("LANG"))
+            p = self.tmp_dir()
+            for f in bypath.get(device).get("files", []):
+                write_file(os.path.join(p, f), content=f)
+            return callback(p)
+
+        p = MOCKPATH
+        self._domock(p + "util.mount_cb", "m_mount_cb")
+        self.m_mount_cb.side_effect = mount_cb
+
     def test_three_partitions_is_false(self):
         """A disk with 3 partitions can not be formatted."""
-        self.patchup(
+        bypath = self.patchup(
             {
                 "/dev/sda": {
                     "partitions": {
@@ -2292,6 +2322,7 @@ class TestCanDevBeReformatted(CiTestCase):
                 }
             }
         )
+        self._domock_mount_cb(bypath)
         value, msg = dsaz.can_dev_be_reformatted(
             "/dev/sda", preserve_ntfs=False
         )
@@ -2300,7 +2331,8 @@ class TestCanDevBeReformatted(CiTestCase):
 
     def test_no_partitions_is_false(self):
         """A disk with no partitions can not be formatted."""
-        self.patchup({"/dev/sda": {}})
+        bypath = self.patchup({"/dev/sda": {}})
+        self._domock_mount_cb(bypath)
         value, msg = dsaz.can_dev_be_reformatted(
             "/dev/sda", preserve_ntfs=False
         )
@@ -2309,7 +2341,7 @@ class TestCanDevBeReformatted(CiTestCase):
 
     def test_two_partitions_not_ntfs_false(self):
         """2 partitions and 2nd not ntfs can not be formatted."""
-        self.patchup(
+        bypath = self.patchup(
             {
                 "/dev/sda": {
                     "partitions": {
@@ -2319,6 +2351,7 @@ class TestCanDevBeReformatted(CiTestCase):
                 }
             }
         )
+        self._domock_mount_cb(bypath)
         value, msg = dsaz.can_dev_be_reformatted(
             "/dev/sda", preserve_ntfs=False
         )
@@ -2327,7 +2360,7 @@ class TestCanDevBeReformatted(CiTestCase):
 
     def test_two_partitions_ntfs_populated_false(self):
         """2 partitions and populated ntfs fs on 2nd can not be formatted."""
-        self.patchup(
+        bypath = self.patchup(
             {
                 "/dev/sda": {
                     "partitions": {
@@ -2341,6 +2374,7 @@ class TestCanDevBeReformatted(CiTestCase):
                 }
             }
         )
+        self._domock_mount_cb(bypath)
         value, msg = dsaz.can_dev_be_reformatted(
             "/dev/sda", preserve_ntfs=False
         )
@@ -2349,7 +2383,7 @@ class TestCanDevBeReformatted(CiTestCase):
 
     def test_two_partitions_ntfs_empty_is_true(self):
         """2 partitions and empty ntfs fs on 2nd can be formatted."""
-        self.patchup(
+        bypath = self.patchup(
             {
                 "/dev/sda": {
                     "partitions": {
@@ -2359,6 +2393,7 @@ class TestCanDevBeReformatted(CiTestCase):
                 }
             }
         )
+        self._domock_mount_cb(bypath)
         value, msg = dsaz.can_dev_be_reformatted(
             "/dev/sda", preserve_ntfs=False
         )
@@ -2367,7 +2402,7 @@ class TestCanDevBeReformatted(CiTestCase):
 
     def test_one_partition_not_ntfs_false(self):
         """1 partition witih fs other than ntfs can not be formatted."""
-        self.patchup(
+        bypath = self.patchup(
             {
                 "/dev/sda": {
                     "partitions": {
@@ -2376,6 +2411,7 @@ class TestCanDevBeReformatted(CiTestCase):
                 }
             }
         )
+        self._domock_mount_cb(bypath)
         value, msg = dsaz.can_dev_be_reformatted(
             "/dev/sda", preserve_ntfs=False
         )
@@ -2384,7 +2420,7 @@ class TestCanDevBeReformatted(CiTestCase):
 
     def test_one_partition_ntfs_populated_false(self):
         """1 mountable ntfs partition with many files can not be formatted."""
-        self.patchup(
+        bypath = self.patchup(
             {
                 "/dev/sda": {
                     "partitions": {
@@ -2397,6 +2433,7 @@ class TestCanDevBeReformatted(CiTestCase):
                 }
             }
         )
+        self._domock_mount_cb(bypath)
         with mock.patch.object(dsaz.LOG, "warning") as warning:
             value, msg = dsaz.can_dev_be_reformatted(
                 "/dev/sda", preserve_ntfs=False
@@ -2410,7 +2447,7 @@ class TestCanDevBeReformatted(CiTestCase):
 
     def test_one_partition_ntfs_empty_is_true(self):
         """1 mountable ntfs partition and no files can be formatted."""
-        self.patchup(
+        bypath = self.patchup(
             {
                 "/dev/sda": {
                     "partitions": {
@@ -2419,6 +2456,7 @@ class TestCanDevBeReformatted(CiTestCase):
                 }
             }
         )
+        self._domock_mount_cb(bypath)
         value, msg = dsaz.can_dev_be_reformatted(
             "/dev/sda", preserve_ntfs=False
         )
@@ -2427,7 +2465,7 @@ class TestCanDevBeReformatted(CiTestCase):
 
     def test_one_partition_ntfs_empty_with_dataloss_file_is_true(self):
         """1 mountable ntfs partition and only warn file can be formatted."""
-        self.patchup(
+        bypath = self.patchup(
             {
                 "/dev/sda": {
                     "partitions": {
@@ -2440,6 +2478,7 @@ class TestCanDevBeReformatted(CiTestCase):
                 }
             }
         )
+        self._domock_mount_cb(bypath)
         value, msg = dsaz.can_dev_be_reformatted(
             "/dev/sda", preserve_ntfs=False
         )
@@ -2449,7 +2488,7 @@ class TestCanDevBeReformatted(CiTestCase):
     def test_one_partition_through_realpath_is_true(self):
         """A symlink to a device with 1 ntfs partition can be formatted."""
         epath = "/dev/disk/cloud/azure_resource"
-        self.patchup(
+        bypath = self.patchup(
             {
                 epath: {
                     "realpath": "/dev/sdb",
@@ -2465,6 +2504,7 @@ class TestCanDevBeReformatted(CiTestCase):
                 }
             }
         )
+        self._domock_mount_cb(bypath)
         value, msg = dsaz.can_dev_be_reformatted(epath, preserve_ntfs=False)
         self.assertTrue(value)
         self.assertIn("safe for", msg.lower())
@@ -2472,7 +2512,7 @@ class TestCanDevBeReformatted(CiTestCase):
     def test_three_partition_through_realpath_is_false(self):
         """A symlink to a device with 3 partitions can not be formatted."""
         epath = "/dev/disk/cloud/azure_resource"
-        self.patchup(
+        bypath = self.patchup(
             {
                 epath: {
                     "realpath": "/dev/sdb",
@@ -2500,13 +2540,14 @@ class TestCanDevBeReformatted(CiTestCase):
                 }
             }
         )
+        self._domock_mount_cb(bypath)
         value, msg = dsaz.can_dev_be_reformatted(epath, preserve_ntfs=False)
         self.assertFalse(value)
         self.assertIn("3 or more", msg.lower())
 
     def test_ntfs_mount_errors_true(self):
         """can_dev_be_reformatted does not fail if NTFS is unknown fstype."""
-        self.patchup(
+        bypath = self.patchup(
             {
                 "/dev/sda": {
                     "partitions": {
@@ -2515,6 +2556,7 @@ class TestCanDevBeReformatted(CiTestCase):
                 }
             }
         )
+        self._domock_mount_cb(bypath)
 
         error_msgs = [
             "Stderr: mount: unknown filesystem type 'ntfs'",  # RHEL
@@ -2535,7 +2577,7 @@ class TestCanDevBeReformatted(CiTestCase):
 
     def test_never_destroy_ntfs_config_false(self):
         """Normally formattable situation with never_destroy_ntfs set."""
-        self.patchup(
+        bypath = self.patchup(
             {
                 "/dev/sda": {
                     "partitions": {
@@ -2548,6 +2590,7 @@ class TestCanDevBeReformatted(CiTestCase):
                 }
             }
         )
+        self._domock_mount_cb(bypath)
         value, msg = dsaz.can_dev_be_reformatted(
             "/dev/sda", preserve_ntfs=True
         )
@@ -3156,7 +3199,6 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
 
 
 class TestRemoveUbuntuNetworkConfigScripts(CiTestCase):
-
     with_logs = True
 
     def setUp(self):
@@ -4358,7 +4400,6 @@ class TestValidateIMDSMetadata:
     def test_validates_one_nic(
         self, azure_ds, mock_get_interfaces, mock_get_interface_mac
     ):
-
         mock_get_interfaces.return_value = [
             ("dummy0", "9e:65:d6:19:19:01", None, None),
             ("test0", "00:11:22:33:44:55", "hv_netvsc", "0x3"),
@@ -4393,7 +4434,6 @@ class TestValidateIMDSMetadata:
     def test_validates_multiple_nic(
         self, azure_ds, mock_get_interfaces, mock_get_interface_mac
     ):
-
         mock_get_interfaces.return_value = [
             ("dummy0", "9e:65:d6:19:19:01", None, None),
             ("test0", "00:11:22:33:44:55", "hv_netvsc", "0x3"),
@@ -4444,7 +4484,6 @@ class TestValidateIMDSMetadata:
     def test_missing_all(
         self, azure_ds, caplog, mock_get_interfaces, mock_get_interface_mac
     ):
-
         mock_get_interfaces.return_value = [
             ("dummy0", "9e:65:d6:19:19:01", None, None),
             ("test0", "00:11:22:33:44:55", "hv_netvsc", "0x3"),
@@ -4467,7 +4506,6 @@ class TestValidateIMDSMetadata:
     def test_missing_primary(
         self, azure_ds, caplog, mock_get_interfaces, mock_get_interface_mac
     ):
-
         mock_get_interfaces.return_value = [
             ("dummy0", "9e:65:d6:19:19:01", None, None),
             ("test0", "00:11:22:33:44:55", "hv_netvsc", "0x3"),
@@ -4515,7 +4553,6 @@ class TestValidateIMDSMetadata:
     def test_missing_secondary(
         self, azure_ds, mock_get_interfaces, mock_get_interface_mac
     ):
-
         mock_get_interfaces.return_value = [
             ("dummy0", "9e:65:d6:19:19:01", None, None),
             ("test0", "00:11:22:33:44:55", "hv_netvsc", "0x3"),