From efa22cd98556d2692193cb4a52cc1b80af53f425 Mon Sep 17 00:00:00 2001
From: Eric Fried <openstack@fried.cc>
Date: Mon, 11 Feb 2019 15:48:07 -0600
Subject: [PATCH] Use placement.inventory.inuse in report client

Since I9a833aa35d474caa35e640bbad6c436a3b16ac5e we've had the framework
for placement to return specific error codes allowing us to
differentiate among error conditions for oft-repeated status codes.
That change also included as its proof-of-concept a specific code for
the placement side of InventoryInUse - i.e. an attempt to delete an
inventory record for which there are existing allocations.

SchedulerReportClient was previously identifying this error condition by
parsing the text of the 409 response. With this change, it instead uses
the provided error code.

Change-Id: Ic621adcadf10cc607455eba48c4cb1882bde23fa
---
 nova/exception.py                           |  5 +---
 nova/scheduler/client/report.py             | 28 +++++----------------
 nova/tests/functional/test_report_client.py | 16 ++++++------
 3 files changed, 15 insertions(+), 34 deletions(-)

diff --git a/nova/exception.py b/nova/exception.py
index ca46075b04..45872f3e7c 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -2248,10 +2248,7 @@ class InvalidInventory(Invalid):
 # An exception with this name is used on both sides of the placement/
 # nova interaction.
 class InventoryInUse(InvalidInventory):
-    # NOTE(mriedem): This message cannot change without impacting the
-    # nova.scheduler.client.report._RE_INV_IN_USE regex.
-    msg_fmt = _("Inventory for '%(resource_classes)s' on "
-                "resource provider '%(resource_provider)s' in use.")
+    pass
 
 
 class UnsupportedPointerModelRequested(Invalid):
diff --git a/nova/scheduler/client/report.py b/nova/scheduler/client/report.py
index 2476e23e90..4f8d9e97c8 100644
--- a/nova/scheduler/client/report.py
+++ b/nova/scheduler/client/report.py
@@ -18,7 +18,6 @@ import contextlib
 import copy
 import functools
 import random
-import re
 import time
 
 from keystoneauth1 import exceptions as ks_exc
@@ -41,8 +40,6 @@ from nova import utils
 
 CONF = nova.conf.CONF
 LOG = logging.getLogger(__name__)
-_RE_INV_IN_USE = re.compile("Inventory for (.+) on resource provider "
-                            "(.+) in use")
 WARN_EVERY = 10
 RESHAPER_VERSION = '1.30'
 CONSUMER_GENERATION_VERSION = '1.28'
@@ -188,19 +185,6 @@ def _move_operation_alloc_request(source_allocs, dest_alloc_req):
     return new_alloc_req
 
 
-def _extract_inventory_in_use(body):
-    """Given an HTTP response body, extract the resource classes that were
-    still in use when we tried to delete inventory.
-
-    :returns: String of resource classes or None if there was no InventoryInUse
-              error in the response body.
-    """
-    match = _RE_INV_IN_USE.search(body)
-    if match:
-        return match.group(1)
-    return None
-
-
 def get_placement_request_id(response):
     if response is not None:
         return response.headers.get(request_id.HTTP_RESP_HEADER_REQUEST_ID)
@@ -961,12 +945,12 @@ class SchedulerReportClient(object):
         if resp.status_code == 409:
             # If a conflict attempting to remove inventory in a resource class
             # with active allocations, raise InventoryInUse
-            rc = _extract_inventory_in_use(resp.text)
-            if rc is not None:
-                raise exception.InventoryInUse(
-                    resource_classes=rc,
-                    resource_provider=rp_uuid,
-                )
+            err = resp.json()['errors'][0]
+            # TODO(efried): If there's ever a lib exporting symbols for error
+            # codes, use it.
+            if err['code'] == 'placement.inventory.inuse':
+                # The error detail includes the resource class and provider.
+                raise exception.InventoryInUse(err['detail'])
             # Other conflicts are generation mismatch: raise conflict exception
             raise exception.ResourceProviderUpdateConflict(
                 uuid=rp_uuid, generation=generation, error=resp.text)
diff --git a/nova/tests/functional/test_report_client.py b/nova/tests/functional/test_report_client.py
index 70e18d8327..245553a211 100644
--- a/nova/tests/functional/test_report_client.py
+++ b/nova/tests/functional/test_report_client.py
@@ -702,10 +702,11 @@ class SchedulerReportClientTests(SchedulerReportClientTestBase):
             }
             # Allocation bumped the generation, so refresh to get the latest
             self.client._refresh_and_get_inventory(self.context, uuids.cn)
-            self.assertRaises(
-                exception.InventoryInUse,
-                self.client.set_inventory_for_provider,
-                self.context, uuids.cn, bad_inv)
+            msgre = (".*update conflict: Inventory for 'SRIOV_NET_VF' on "
+                     "resource provider '%s' in use..*" % uuids.cn)
+            with self.assertRaisesRegex(exception.InventoryInUse, msgre):
+                self.client.set_inventory_for_provider(self.context, uuids.cn,
+                                                       bad_inv)
             self.assertEqual(
                 inv,
                 self.client._get_inventory(
@@ -713,10 +714,9 @@ class SchedulerReportClientTests(SchedulerReportClientTestBase):
 
             # Same result if we try to clear all the inventory
             bad_inv = {}
-            self.assertRaises(
-                exception.InventoryInUse,
-                self.client.set_inventory_for_provider,
-                self.context, uuids.cn, bad_inv)
+            with self.assertRaisesRegex(exception.InventoryInUse, msgre):
+                self.client.set_inventory_for_provider(self.context, uuids.cn,
+                                                       bad_inv)
             self.assertEqual(
                 inv,
                 self.client._get_inventory(