Merge "Only pull associated *sharing* providers" into stable/queens

3 files changed, 54 insertions, 40 deletions

diff --git a/nova/scheduler/client/report.py b/nova/scheduler/client/report.py
index 726cb5bb4e..82a6936a24 100644
--- a/nova/scheduler/client/report.py
+++ b/nova/scheduler/client/report.py
@@ -19,6 +19,7 @@ import re
 import time
 
 from keystoneauth1 import exceptions as ks_exc
+import os_traits
 from oslo_log import log as logging
 from oslo_middleware import request_id
 from oslo_utils import versionutils
@@ -458,9 +459,10 @@ class SchedulerReportClient(object):
             raise exception.ResourceProviderRetrievalFailed(uuid=uuid)
 
     @safe_connect
-    def _get_providers_in_aggregates(self, agg_uuids):
+    def _get_sharing_providers(self, agg_uuids):
         """Queries the placement API for a list of the resource providers
-        associated with any of the specified aggregates.
+        associated with any of the specified aggregates and possessing the
+        MISC_SHARES_VIA_AGGREGATE trait.
 
         :param agg_uuids: Iterable of string UUIDs of aggregates to filter on.
         :return: A list of dicts of resource provider information, which may be
@@ -471,10 +473,16 @@ class SchedulerReportClient(object):
             return []
 
         qpval = ','.join(agg_uuids)
+        # TODO(efried): Need a ?having_traits=[...] on this API!
         resp = self.get("/resource_providers?member_of=in:" + qpval,
                         version='1.3')
         if resp.status_code == 200:
-            return resp.json()['resource_providers']
+            rps = []
+            for rp in resp.json()['resource_providers']:
+                traits = self._get_provider_traits(rp['uuid'])
+                if os_traits.MISC_SHARES_VIA_AGGREGATE in traits:
+                    rps.append(rp)
+            return rps
 
         # Some unexpected error
         placement_req_id = get_placement_request_id(resp)
@@ -769,7 +777,7 @@ class SchedulerReportClient(object):
 
             if refresh_sharing:
                 # Refresh providers associated by aggregate
-                for rp in self._get_providers_in_aggregates(aggs):
+                for rp in self._get_sharing_providers(aggs):
                     if not self._provider_tree.exists(rp['uuid']):
                         # NOTE(efried): Right now sharing providers are always
                         # treated as roots. This is deliberate. From the
diff --git a/nova/tests/functional/api/openstack/placement/test_report_client.py b/nova/tests/functional/api/openstack/placement/test_report_client.py
index 93b925c79a..4d0ebb47c7 100644
--- a/nova/tests/functional/api/openstack/placement/test_report_client.py
+++ b/nova/tests/functional/api/openstack/placement/test_report_client.py
@@ -128,7 +128,7 @@ class SchedulerReportClientTests(test.TestCase):
             # We should also have empty sets of aggregate and trait
             # associations
             self.assertEqual(
-                [], self.client._get_providers_in_aggregates([uuids.agg]))
+                [], self.client._get_sharing_providers([uuids.agg]))
             self.assertFalse(
                 self.client._provider_tree.have_aggregates_changed(
                     self.compute_uuid, []))
diff --git a/nova/tests/unit/scheduler/client/test_report.py b/nova/tests/unit/scheduler/client/test_report.py
index acf1fc2f8e..f625eb823e 100644
--- a/nova/tests/unit/scheduler/client/test_report.py
+++ b/nova/tests/unit/scheduler/client/test_report.py
@@ -1193,11 +1193,11 @@ class TestProviderOperations(SchedulerReportClientTestCase):
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_get_provider_traits')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_aggregates')
+                '_get_sharing_providers')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_get_providers_in_tree')
     def test_ensure_resource_provider_exists_in_cache(self, get_rpt_mock,
-            get_pia_mock, get_trait_mock, get_agg_mock, create_rp_mock):
+            get_shr_mock, get_trait_mock, get_agg_mock, create_rp_mock):
         # Override the client object's cache to contain a resource provider
         # object for the compute host and check that
         # _ensure_resource_provider() doesn't call _get_resource_provider() or
@@ -1219,7 +1219,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
             set(),
             set(['CUSTOM_BRONZE'])
         ]
-        get_pia_mock.return_value = [
+        get_shr_mock.return_value = [
             {
                 'uuid': uuids.shr1,
                 'name': 'sharing1',
@@ -1232,7 +1232,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
             },
         ]
         self.client._ensure_resource_provider(self.context, cn.uuid)
-        get_pia_mock.assert_called_once_with(set([uuids.agg1, uuids.agg2]))
+        get_shr_mock.assert_called_once_with(set([uuids.agg1, uuids.agg2]))
         self.assertTrue(self.client._provider_tree.exists(uuids.shr1))
         self.assertTrue(self.client._provider_tree.exists(uuids.shr2))
         # _get_provider_aggregates and _traits were called thrice: one for the
@@ -1270,11 +1270,11 @@ class TestProviderOperations(SchedulerReportClientTestCase):
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_get_provider_traits')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_aggregates')
+                '_get_sharing_providers')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_get_providers_in_tree')
-    def test_ensure_resource_provider_get(self, get_rpt_mock, get_pia_mock,
-            get_trait_mock, get_agg_mock, create_rp_mock):
+    def test_ensure_resource_provider_get(self, get_rpt_mock, get_shr_mock,
+              get_trait_mock, get_agg_mock, create_rp_mock):
         # No resource provider exists in the client's cache, so validate that
         # if we get the resource provider from the placement API that we don't
         # try to create the resource provider.
@@ -1286,7 +1286,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
 
         get_agg_mock.return_value = set([uuids.agg1])
         get_trait_mock.return_value = set(['CUSTOM_GOLD'])
-        get_pia_mock.return_value = []
+        get_shr_mock.return_value = []
 
         self.client._ensure_resource_provider(self.context, uuids.compute_node)
 
@@ -1306,7 +1306,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
         self.assertFalse(
             self.client._provider_tree.has_traits(uuids.compute_node,
                                                   ['CUSTOM_SILVER']))
-        get_pia_mock.assert_called_once_with(set([uuids.agg1]))
+        get_shr_mock.assert_called_once_with(set([uuids.agg1]))
         self.assertTrue(self.client._provider_tree.exists(uuids.compute_node))
         self.assertFalse(create_rp_mock.called)
 
@@ -1673,8 +1673,10 @@ class TestProviderOperations(SchedulerReportClientTestCase):
         self.assertEqual(uuids.request_id,
                          logging_mock.call_args[0][1]['placement_req_id'])
 
-    def test_get_providers_in_aggregates(self):
-        # Ensure _get_providers_in_aggregates() returns a list of resource
+    @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
+                '_get_provider_traits')
+    def test_get_sharing_providers(self, mock_get_traits):
+        # Ensure _get_sharing_providers() returns a list of resource
         # provider dicts if it finds resource provider records from the
         # placement API
         resp_mock = mock.Mock(status_code=200)
@@ -1699,22 +1701,26 @@ class TestProviderOperations(SchedulerReportClientTestCase):
         resp_mock.json.return_value = {'resource_providers': rpjson}
         self.ks_adap_mock.get.return_value = resp_mock
 
-        result = self.client._get_providers_in_aggregates([uuids.agg1,
-                                                           uuids.agg2])
+        mock_get_traits.side_effect = [
+            set(['MISC_SHARES_VIA_AGGREGATE', 'CUSTOM_FOO']),
+            set(['CUSTOM_BAR']),
+        ]
+        result = self.client._get_sharing_providers([uuids.agg1, uuids.agg2])
 
         expected_url = ('/resource_providers?member_of=in:' +
                         ','.join((uuids.agg1, uuids.agg2)))
         self.ks_adap_mock.get.assert_called_once_with(
             expected_url, raise_exc=False, microversion='1.3')
-        self.assertEqual(rpjson, result)
+        self.assertEqual(rpjson[:1], result)
 
-    def test_get_providers_in_aggregates_emptylist(self):
-        self.assertEqual([], self.client._get_providers_in_aggregates([]))
+    def test_get_sharing_providers_emptylist(self):
+        self.assertEqual(
+            [], self.client._get_sharing_providers([]))
         self.ks_adap_mock.get.assert_not_called()
 
     @mock.patch.object(report.LOG, 'error')
-    def test_get_providers_in_aggregates_error(self, logging_mock):
-        # Ensure _get_providers_in_aggregates() logs an error and raises if the
+    def test_get_sharing_providers_error(self, logging_mock):
+        # Ensure _get_sharing_providers() 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
@@ -1723,7 +1729,7 @@ class TestProviderOperations(SchedulerReportClientTestCase):
 
         uuid = uuids.agg
         self.assertRaises(exception.ResourceProviderRetrievalFailed,
-                          self.client._get_providers_in_aggregates, [uuid])
+                          self.client._get_sharing_providers, [uuid])
 
         expected_url = '/resource_providers?member_of=in:' + uuid
         self.ks_adap_mock.get.assert_called_once_with(
@@ -2224,8 +2230,8 @@ class TestAssociations(SchedulerReportClientTestCase):
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_get_provider_traits')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_aggregates')
-    def test_refresh_associations_no_last(self, mock_pia_get, mock_trait_get,
+                '_get_sharing_providers')
+    def test_refresh_associations_no_last(self, mock_shr_get, mock_trait_get,
                                           mock_agg_get):
         """Test that associations are refreshed when stale."""
         uuid = uuids.compute_node
@@ -2236,7 +2242,7 @@ class TestAssociations(SchedulerReportClientTestCase):
         self.client._refresh_associations(uuid)
         mock_agg_get.assert_called_once_with(uuid)
         mock_trait_get.assert_called_once_with(uuid)
-        mock_pia_get.assert_called_once_with(mock_agg_get.return_value)
+        mock_shr_get.assert_called_once_with(mock_agg_get.return_value)
         self.assertIn(uuid, self.client.association_refresh_time)
         self.assertTrue(
             self.client._provider_tree.in_aggregates(uuid, [uuids.agg1]))
@@ -2252,8 +2258,8 @@ class TestAssociations(SchedulerReportClientTestCase):
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_get_provider_traits')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_aggregates')
-    def test_refresh_associations_no_refresh_sharing(self, mock_pia_get,
+                '_get_sharing_providers')
+    def test_refresh_associations_no_refresh_sharing(self, mock_shr_get,
                                                      mock_trait_get,
                                                      mock_agg_get):
         """Test refresh_sharing=False."""
@@ -2265,7 +2271,7 @@ class TestAssociations(SchedulerReportClientTestCase):
         self.client._refresh_associations(uuid, refresh_sharing=False)
         mock_agg_get.assert_called_once_with(uuid)
         mock_trait_get.assert_called_once_with(uuid)
-        mock_pia_get.assert_not_called()
+        mock_shr_get.assert_not_called()
         self.assertIn(uuid, self.client.association_refresh_time)
         self.assertTrue(
             self.client._provider_tree.in_aggregates(uuid, [uuids.agg1]))
@@ -2281,10 +2287,10 @@ class TestAssociations(SchedulerReportClientTestCase):
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_get_provider_traits')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_aggregates')
+                '_get_sharing_providers')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_associations_stale')
-    def test_refresh_associations_not_stale(self, mock_stale, mock_pia_get,
+    def test_refresh_associations_not_stale(self, mock_stale, mock_shr_get,
                                             mock_trait_get, mock_agg_get):
         """Test that refresh associations is not called when the map is
         not stale.
@@ -2294,7 +2300,7 @@ class TestAssociations(SchedulerReportClientTestCase):
         self.client._refresh_associations(uuid)
         mock_agg_get.assert_not_called()
         mock_trait_get.assert_not_called()
-        mock_pia_get.assert_not_called()
+        mock_shr_get.assert_not_called()
         self.assertFalse(self.client.association_refresh_time)
 
     @mock.patch.object(report.LOG, 'debug')
@@ -2303,8 +2309,8 @@ class TestAssociations(SchedulerReportClientTestCase):
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_get_provider_traits')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_get_providers_in_aggregates')
-    def test_refresh_associations_time(self, mock_pia_get, mock_trait_get,
+                '_get_sharing_providers')
+    def test_refresh_associations_time(self, mock_shr_get, mock_trait_get,
                                        mock_agg_get, log_mock):
         """Test that refresh associations is called when the map is stale."""
         uuid = uuids.compute_node
@@ -2312,14 +2318,14 @@ class TestAssociations(SchedulerReportClientTestCase):
         self.client._provider_tree.new_root('compute', uuid, 1)
         mock_agg_get.return_value = set([])
         mock_trait_get.return_value = set([])
-        mock_pia_get.return_value = []
+        mock_shr_get.return_value = []
 
         # Called a first time because association_refresh_time is empty.
         now = time.time()
         self.client._refresh_associations(uuid)
         mock_agg_get.assert_called_once_with(uuid)
         mock_trait_get.assert_called_once_with(uuid)
-        mock_pia_get.assert_called_once_with(set())
+        mock_shr_get.assert_called_once_with(set())
         log_mock.assert_has_calls([
             mock.call('Refreshing aggregate associations for resource '
                       'provider %s, aggregates: %s', uuid, 'None'),
@@ -2331,7 +2337,7 @@ class TestAssociations(SchedulerReportClientTestCase):
         # Clear call count.
         mock_agg_get.reset_mock()
         mock_trait_get.reset_mock()
-        mock_pia_get.reset_mock()
+        mock_shr_get.reset_mock()
 
         with mock.patch('time.time') as mock_future:
             # Not called a second time because not enough time has passed.
@@ -2339,14 +2345,14 @@ class TestAssociations(SchedulerReportClientTestCase):
             self.client._refresh_associations(uuid)
             mock_agg_get.assert_not_called()
             mock_trait_get.assert_not_called()
-            mock_pia_get.assert_not_called()
+            mock_shr_get.assert_not_called()
 
             # Called because time has passed.
             mock_future.return_value = now + report.ASSOCIATION_REFRESH + 1
             self.client._refresh_associations(uuid)
             mock_agg_get.assert_called_once_with(uuid)
             mock_trait_get.assert_called_once_with(uuid)
-            mock_pia_get.assert_called_once_with(set())
+            mock_shr_get.assert_called_once_with(set())
 
 
 class TestComputeNodeToInventoryDict(test.NoDBTestCase):