Un-safe_connect and publicize get_providers_in_tree

In the continuing saga to wipe @safe_connect from the annals of history,
and in preparation, for its use outside of SchedulerReportClient, this
commit does two things to _get_providers_in_tree:

- Removes @safe_connect from it. Callers now need to be aware that they
  can get ClientExceptionZ from ksa. (The two existing callers were
  vetted and needed no additional handling - it's way more appropriate
  for them to raise ClientException than a mysterious NoneType error
  somewhere down the line as they would have been doing previously.)
- Renames it to get_providers_in_tree.

Change-Id: I2b284d69d345d15287f04a7ca4cd422155768525

Author: Eric Fried

nova/scheduler/client/report.py

@@ -491,8 +491,7 @@ def _get_sharing_providers(self, context, agg_uuids):
         LOG.error(msg, args)
         raise exception.ResourceProviderRetrievalFailed(message=msg % args)
 
-    @safe_connect
-    def _get_providers_in_tree(self, context, uuid):
+    def get_providers_in_tree(self, context, uuid):
         """Queries the placement API for a list of the resource providers in
         the tree associated with the specified UUID.
 
@@ -501,6 +500,8 @@ def _get_providers_in_tree(self, context, uuid):
         :return: A list of dicts of resource provider information, which may be
                  empty if no provider exists with the specified UUID.
         :raise: ResourceProviderRetrievalFailed on error.
+        :raise: keystoneauth1.exceptions.ClientException if placement API
+                communication fails.
         """
         resp = self.get("/resource_providers?in_tree=%s" % uuid,
                         version=NESTED_PROVIDER_API_VERSION,
@@ -628,6 +629,10 @@ def _ensure_resource_provider(self, context, uuid, name=None,
                      value
         :param parent_provider_uuid: Optional UUID of the immediate parent,
                                      which must have been previously _ensured.
+        :raise ResourceProviderCreationFailed: If we expected to be creating
+                providers, but couldn't.
+        :raise: keystoneauth1.exceptions.ClientException if placement API
+                communication fails.
         """
         # NOTE(efried): We currently have no code path where we need to set the
         # parent_provider_uuid on a previously-parent-less provider - so we do
@@ -654,7 +659,7 @@ def _ensure_resource_provider(self, context, uuid, name=None,
         else:
             # We either don't have it locally or it's stale. Pull or create it.
             created_rp = None
-            rps_to_refresh = self._get_providers_in_tree(context, uuid)
+            rps_to_refresh = self.get_providers_in_tree(context, uuid)
             if not rps_to_refresh:
                 created_rp = self._create_resource_provider(
                     context, uuid, name or uuid,
@@ -1801,7 +1806,7 @@ def remove_provider_tree_from_instance_allocation(self, context,
             # do anything with the return value except log
             return False
 
-        rps = self._get_providers_in_tree(context, root_rp_uuid)
+        rps = self.get_providers_in_tree(context, root_rp_uuid)
         rp_uuids = [rp['uuid'] for rp in rps]
 
         # go through the current allocations and remove every RP from it that

nova/tests/functional/test_report_client.py

@@ -188,8 +188,8 @@ def test_client_report_smoke(self):
             rp = self.client._get_resource_provider(self.context,
                                                     self.compute_uuid)
             self.assertIsNone(rp)
-            rps = self.client._get_providers_in_tree(self.context,
-                                                     self.compute_uuid)
+            rps = self.client.get_providers_in_tree(self.context,
+                                                    self.compute_uuid)
             self.assertEqual([], rps)
             # But get_provider_tree_and_ensure_root creates one (via
             # _ensure_resource_provider)
@@ -209,8 +209,8 @@ def test_client_report_smoke(self):
             rp = self.client._get_resource_provider(self.context,
                                                     self.compute_uuid)
             self.assertIsNotNone(rp)
-            rps = self.client._get_providers_in_tree(self.context,
-                                                     self.compute_uuid)
+            rps = self.client.get_providers_in_tree(self.context,
+                                                    self.compute_uuid)
             self.assertEqual(1, len(rps))
 
             # We should also have empty sets of aggregate and trait

nova/tests/unit/scheduler/client/test_report.py

@@ -1758,7 +1758,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_get_sharing_providers')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_tree')
+                'get_providers_in_tree')
     def test_ensure_resource_provider_get(self, get_rpt_mock, get_shr_mock,
             get_trait_mock, get_agg_mock, get_inv_mock, create_rp_mock):
         # No resource provider exists in the client's cache, so validate that
@@ -1825,7 +1825,7 @@ def assert_cache_contents():
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_refresh_associations')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_tree')
+                'get_providers_in_tree')
     def test_ensure_resource_provider_create_fail(self, get_rpt_mock,
             refresh_mock, create_rp_mock):
         # No resource provider exists in the client's cache, and
@@ -1858,7 +1858,7 @@ def test_ensure_resource_provider_create_fail(self, get_rpt_mock,
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_refresh_associations')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_tree')
+                'get_providers_in_tree')
     def test_ensure_resource_provider_create_no_placement(self, get_rpt_mock,
             refresh_mock, create_rp_mock):
         # No resource provider exists in the client's cache, and
@@ -1892,7 +1892,7 @@ def test_ensure_resource_provider_create_no_placement(self, get_rpt_mock,
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_refresh_associations')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_tree')
+                'get_providers_in_tree')
     def test_ensure_resource_provider_create(self, get_rpt_mock,
                                              refresh_inv_mock,
                                              refresh_assoc_mock,
@@ -1939,7 +1939,7 @@ def test_ensure_resource_provider_create(self, get_rpt_mock,
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_create_resource_provider')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_tree')
+                'get_providers_in_tree')
     def test_ensure_resource_provider_tree(self, get_rpt_mock, create_rp_mock,
                                            refresh_mock):
         """Test _ensure_resource_provider with a tree of providers."""
@@ -2011,7 +2011,7 @@ def mocked_refresh(context, rp_uuid, **kwargs):
         refresh_mock.assert_not_called()
 
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_tree')
+                'get_providers_in_tree')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_refresh_associations')
     def test_ensure_resource_provider_refresh_fetch(self, mock_ref_assoc,
@@ -2033,7 +2033,7 @@ def test_ensure_resource_provider_refresh_fetch(self, mock_ref_assoc,
                          set(self.client._provider_tree.get_provider_uuids()))
 
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_tree')
+                'get_providers_in_tree')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_create_resource_provider')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
@@ -2319,7 +2319,7 @@ def test_get_sharing_providers_error(self, logging_mock):
                          logging_mock.call_args[0][1]['placement_req_id'])
 
     def test_get_providers_in_tree(self):
-        # Ensure _get_providers_in_tree() returns a list of resource
+        # Ensure get_providers_in_tree() returns a list of resource
         # provider dicts if it finds a resource provider record from the
         # placement API
         root = uuids.compute_node
@@ -2341,7 +2341,7 @@ def test_get_providers_in_tree(self):
         resp_mock.json.return_value = {'resource_providers': rpjson}
         self.ks_adap_mock.get.return_value = resp_mock
 
-        result = self.client._get_providers_in_tree(self.context, root)
+        result = self.client.get_providers_in_tree(self.context, root)
 
         expected_url = '/resource_providers?in_tree=' + root
         self.ks_adap_mock.get.assert_called_once_with(
@@ -2351,7 +2351,7 @@ def test_get_providers_in_tree(self):
 
     @mock.patch.object(report.LOG, 'error')
     def test_get_providers_in_tree_error(self, logging_mock):
-        # Ensure _get_providers_in_tree() logs an error and raises if the
+        # Ensure get_providers_in_tree() logs an error and raises if the
         # placement API call doesn't respond 200
         resp_mock = mock.Mock(status_code=503)
         self.ks_adap_mock.get.return_value = resp_mock
@@ -2360,7 +2360,7 @@ def test_get_providers_in_tree_error(self, logging_mock):
 
         uuid = uuids.compute_node
         self.assertRaises(exception.ResourceProviderRetrievalFailed,
-                          self.client._get_providers_in_tree, self.context,
+                          self.client.get_providers_in_tree, self.context,
                           uuid)
 
         expected_url = '/resource_providers?in_tree=' + uuid
@@ -2373,6 +2373,12 @@ def test_get_providers_in_tree_error(self, logging_mock):
         self.assertEqual('req-' + uuids.request_id,
                          logging_mock.call_args[0][1]['placement_req_id'])
 
+    def test_get_providers_in_tree_ksa_exc(self):
+        self.ks_adap_mock.get.side_effect = ks_exc.EndpointNotFound()
+        self.assertRaises(
+            ks_exc.ClientException,
+            self.client.get_providers_in_tree, self.context, uuids.whatever)
+
     def test_create_resource_provider(self):
         """Test that _create_resource_provider() sends a dict of resource
         provider information without a parent provider UUID.