Merge "Fix incorrect logging format"

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")

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):

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,

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):

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'.

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()