From b59d0e5a42ee6fee4728b8e8f662e5647d40b8cd Mon Sep 17 00:00:00 2001
From: Tetsuro Nakamura <nakamura.tetsuro@lab.ntt.co.jp>
Date: Fri, 25 Jan 2019 12:23:02 +0000
Subject: [PATCH] Retry on inventory update conflict

Placement raises 409 conflict if the provided generation is
inconsistent with the registered in placement DB. For Blazar's use
case, The generation inconsistency can occur in cases where another
thread concurrently updates the same reservation provider for a
different reservation.

This patch changes update_inventory() to retry for such cases.

Change-Id: I1b57210d623791a49ad5ac19f791c3b17b871ed3
---
 .../tests/utils/openstack/test_placement.py   | 19 +++++++++++++++++
 blazar/utils/openstack/exceptions.py          |  6 ++++++
 blazar/utils/openstack/placement.py           | 21 ++++++++++++++++++-
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/blazar/tests/utils/openstack/test_placement.py b/blazar/tests/utils/openstack/test_placement.py
index f3ec7666..1b5741b5 100644
--- a/blazar/tests/utils/openstack/test_placement.py
+++ b/blazar/tests/utils/openstack/test_placement.py
@@ -574,6 +574,25 @@ class TestPlacementClient(tests.TestCase):
                                           json=expected_data)
         self.assertEqual(mock_put_json, result)
 
+        kss_req.reset_mock()
+
+        # Test retrying on 409 conflict
+        mock_json_data = {
+            "errors": [
+                {"status": 409,
+                 "code": "placement.concurrent_update",
+                 "title": "Conflict"}
+            ]
+        }
+
+        kss_req.return_value = fake_requests.FakeResponse(
+            409, content=jsonutils.dump_as_bytes(mock_json_data))
+        self.assertRaises(
+            exceptions.InventoryConflict,
+            self.client.update_reservation_inventory, host_name, 'add', 3)
+        self.assertEqual(5, kss_req.call_count)
+        kss_req.reset_mock()
+
     @mock.patch('blazar.utils.openstack.placement.'
                 'BlazarPlacementClient.get_resource_provider')
     @mock.patch('keystoneauth1.session.Session.request')
diff --git a/blazar/utils/openstack/exceptions.py b/blazar/utils/openstack/exceptions.py
index db0d3540..46eb54f6 100644
--- a/blazar/utils/openstack/exceptions.py
+++ b/blazar/utils/openstack/exceptions.py
@@ -49,6 +49,12 @@ class InventoryUpdateFailed(exceptions.BlazarException):
                 "%(resource_provider)s")
 
 
+class InventoryConflict(exceptions.BlazarException):
+    code = 409
+    msg_fmt = _("Conflict on updating inventory on resource provider "
+                "%(resource_provider)s")
+
+
 class FloatingIPNetworkNotFound(exceptions.InvalidInput):
     msg_fmt = _("Failed to find network %(network)s")
 
diff --git a/blazar/utils/openstack/placement.py b/blazar/utils/openstack/placement.py
index 4549e7e7..b0ab4849 100644
--- a/blazar/utils/openstack/placement.py
+++ b/blazar/utils/openstack/placement.py
@@ -11,6 +11,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import retrying
+
 from keystoneauth1 import adapter
 from keystoneauth1.identity import v3
 from keystoneauth1 import session
@@ -294,6 +296,9 @@ class BlazarPlacementClient(object):
             return resp.json()
         raise exceptions.ResourceProviderNotFound(resource_provider=rp_uuid)
 
+    @retrying.retry(stop_max_attempt_number=5,
+                    retry_on_exception=lambda e: isinstance(
+                        e, exceptions.InventoryConflict))
     def update_inventory(self, rp_uuid, rc_name, num, additional):
         """Update the inventory for the resource provider.
 
@@ -334,7 +339,21 @@ class BlazarPlacementClient(object):
         if resp:
             return resp.json()
 
-        # TODO(tetsuro): Try again on 409 conflict errors
+        if resp.status_code == 409:
+            err = resp.json()['errors'][0]
+            if err['code'] == 'placement.concurrent_update':
+                # NOTE(tetsuro): Another thread updated the inventory of the
+                # same rp during the get_inventory() and the put(). We simply
+                # retry it for this case.
+                msg = ("Conflict on updating inventory in placement. "
+                       "Got %(status_code)d: %(err_text)s. ")
+                args = {
+                    'status_code': resp.status_code,
+                    'err_text': resp.text,
+                }
+                LOG.error(msg, args)
+                raise exceptions.InventoryConflict(resource_provider=rp_uuid)
+
         raise exceptions.InventoryUpdateFailed(resource_provider=rp_uuid)
 
     def delete_inventory(self, rp_uuid, rc_name):