From ffdb7a7d5fd78b9f64d2f4d9686236b56fa9503d Mon Sep 17 00:00:00 2001 From: Ksenija Stanojevic Date: Tue, 16 May 2023 10:34:30 -0700 Subject: 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 --- tests/unittests/sources/test_azure.py | 101 +++++++++++++++++++++++----------- 1 file 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"), -- cgit v1.2.1