diff --git a/watcher/common/cinder_helper.py b/watcher/common/cinder_helper.py index 1790613f3..a686cd67d 100644 --- a/watcher/common/cinder_helper.py +++ b/watcher/common/cinder_helper.py @@ -179,8 +179,9 @@ class CinderHelper(object): LOG.error(error_msg) return False LOG.debug( - "Volume migration succeeded : volume %s is now on host '%s'.", ( - volume.id, host_name)) + "Volume migration succeeded : " + "volume %(volume)s is now on host '%(host)s'.", + {'volume': volume.id, 'host': host_name}) return True def migrate(self, volume, dest_node): @@ -193,8 +194,8 @@ class CinderHelper(object): message=(_("Volume type must be same for migrating"))) source_node = getattr(volume, 'os-vol-host-attr:host') - LOG.debug("Volume %s found on host '%s'.", - (volume.id, source_node)) + LOG.debug("Volume %(volume)s found on host '%(host)s'.", + {'volume': volume.id, 'host': source_node}) self.cinder.volumes.migrate_volume( volume, dest_node, False, True) @@ -210,8 +211,8 @@ class CinderHelper(object): source_node = getattr(volume, 'os-vol-host-attr:host') LOG.debug( - "Volume %s found on host '%s'.", - (volume.id, source_node)) + "Volume %(volume)s found on host '%(host)s'.", + {'volume': volume.id, 'host': source_node}) self.cinder.volumes.retype( volume, dest_type, "on-demand") diff --git a/watcher/common/nova_helper.py b/watcher/common/nova_helper.py index 7417bcf2c..e0635dcb8 100644 --- a/watcher/common/nova_helper.py +++ b/watcher/common/nova_helper.py @@ -731,8 +731,8 @@ class NovaHelper(object): host_name = getattr(new_volume, "os-vol-host-attr:host") LOG.debug( "Volume update succeeded : " - "Volume %s is now on host '%s'.", - (new_volume.id, host_name)) + "Volume %(volume)s is now on host '%(host)s'.", + {'volume': new_volume.id, 'host': host_name}) return True def _check_nova_api_version(self, client, version): diff --git a/watcher/common/placement_helper.py b/watcher/common/placement_helper.py index c6cf14705..c4ecdc174 100644 --- a/watcher/common/placement_helper.py +++ b/watcher/common/placement_helper.py @@ -122,7 +122,7 @@ class PlacementHelper(object): if resp.status_code == HTTPStatus.OK: json = resp.json() return json['allocations'] - msg = ("Failed to get allocations for consumer %(c_uuid). " + msg = ("Failed to get allocations for consumer %(c_uuid)s. " "Got %(status_code)d: %(err_text)s.") args = { 'c_uuid': consumer_uuid, diff --git a/watcher/tests/common/test_cinder_helper.py b/watcher/tests/common/test_cinder_helper.py index b07bd0fb0..5334d08f2 100644 --- a/watcher/tests/common/test_cinder_helper.py +++ b/watcher/tests/common/test_cinder_helper.py @@ -154,13 +154,8 @@ class TestCinderHelper(base.TestCase): volume_type = self.fake_volume_type() cinder_util.cinder.volume_types.list.return_value = [volume_type] - # the logging in this function has a bug, temporarily changing the - # assert to catch the exception - # https://review.opendev.org/c/openstack/watcher/+/822559 merges - self.assertRaises(TypeError, cinder_util.migrate, - volume, 'host@backend#pool') - # result = cinder_util.migrate(volume, 'host@backend#pool') - # self.assertTrue(result) + result = cinder_util.migrate(volume, 'host@backend#pool') + self.assertTrue(result) @mock.patch.object(time, 'sleep', mock.Mock()) def test_migrate_fail(self, mock_cinder): @@ -187,14 +182,8 @@ class TestCinderHelper(base.TestCase): volume_type = self.fake_volume_type() cinder_util.cinder.volume_types.list.return_value = [volume_type] - # the logging in this function has a bug, temporarily changing the - # assert to catch the exception - # https://review.opendev.org/c/openstack/watcher/+/822559 merges - self.assertRaises(TypeError, cinder_util.migrate, - volume, 'host@backend#pool') - - # result = cinder_util.migrate(volume, 'host@backend#pool') - # self.assertFalse(result) + result = cinder_util.migrate(volume, 'host@backend#pool') + self.assertFalse(result) @mock.patch.object(time, 'sleep', mock.Mock()) def test_retype_success(self, mock_cinder): @@ -205,13 +194,8 @@ class TestCinderHelper(base.TestCase): setattr(volume, 'migration_status', 'success') cinder_util.cinder.volumes.get.return_value = volume - # the logging in this function has a bug, temporarily changing the - # assert to catch the exception - # https://review.opendev.org/c/openstack/watcher/+/822559 merges - self.assertRaises(TypeError, cinder_util.retype, - volume, 'notfake_type') - # result = cinder_util.retype(volume, 'notfake_type') - # self.assertTrue(result) + result = cinder_util.retype(volume, 'notfake_type') + self.assertTrue(result) @mock.patch.object(time, 'sleep', mock.Mock()) def test_retype_fail(self, mock_cinder): @@ -232,13 +216,8 @@ class TestCinderHelper(base.TestCase): setattr(volume, 'migration_status', 'error') cinder_util.cinder.volumes.get.return_value = volume - # the logging in this function has a bug, temporarily changing the - # assert to catch the exception - # https://review.opendev.org/c/openstack/watcher/+/822559 merges - self.assertRaises(TypeError, cinder_util.retype, - volume, 'notfake_type') - # result = cinder_util.retype(volume, 'notfake_type') - # self.assertFalse(result) + result = cinder_util.retype(volume, 'notfake_type') + self.assertFalse(result) @mock.patch.object(time, 'sleep', mock.Mock()) def test_create_volume_success(self, mock_cinder): @@ -403,12 +382,8 @@ class TestCinderHelper(base.TestCase): setattr(volume, 'os-vol-host-attr:host', 'host@backend#pool') cinder_util.cinder.volumes.get.return_value = volume cinder_util.check_volume_deleted = mock.MagicMock(return_value=True) - # the logging in this function has a bug, temporarily changing the - # assert to catch the exception - # https://review.opendev.org/c/openstack/watcher/+/822559 merges - self.assertRaises(TypeError, cinder_util.check_migrated, volume) - # result = cinder_util.check_migrated(volume) - # self.assertTrue(result) + result = cinder_util.check_migrated(volume) + self.assertTrue(result) @mock.patch.object(time, 'sleep', mock.Mock()) def test_check_migrated_fail(self, mock_cinder): diff --git a/watcher/tests/common/test_nova_helper.py b/watcher/tests/common/test_nova_helper.py index c8b9a24e4..5e6401516 100644 --- a/watcher/tests/common/test_nova_helper.py +++ b/watcher/tests/common/test_nova_helper.py @@ -576,13 +576,8 @@ class TestNovaHelper(base.TestCase): new_volume = self.fake_volume( id=utils.generate_uuid(), status='in-use') - # the logging in this function has a bug, temporarily changing the - # assert to catch the exception - # https://review.opendev.org/c/openstack/watcher/+/822559/7 merges - self.assertRaises(TypeError, nova_util.swap_volume, - old_volume, new_volume) - # result = nova_util.swap_volume(old_volume, new_volume) - # self.assertTrue(result) + result = nova_util.swap_volume(old_volume, new_volume) + self.assertTrue(result) # verify that the method will return False when the status of # new_volume is 'fake-use'. diff --git a/watcher/tests/common/test_placement_helper.py b/watcher/tests/common/test_placement_helper.py index 6a2b1667b..5b7a18e2b 100644 --- a/watcher/tests/common/test_placement_helper.py +++ b/watcher/tests/common/test_placement_helper.py @@ -233,13 +233,8 @@ class TestPlacementHelper(base.TestCase): kss_req.return_value = fake_requests.FakeResponse( HTTPStatus.NOT_FOUND, content=jsonutils.dump_as_bytes(self.fake_err_msg)) - # the logging in this function has a bug, temporarily changing the - # assert to catch the exception - # https://review.opendev.org/c/openstack/watcher/+/822559 merges - self.assertRaises(ValueError, self.client.get_allocations_for_consumer, - c_uuid) - # result = self.client.get_allocations_for_consumer(c_uuid) - # self.assertIsNone(result) + result = self.client.get_allocations_for_consumer(c_uuid) + self.assertIsNone(result) def test_get_usages_for_resource_provider_OK(self, kss_req): rp_uuid = uuidutils.generate_uuid()