From 4cdf6a793089013085f8e29f23c98a19edcaf6e9 Mon Sep 17 00:00:00 2001
From: licanwei <li.canwei2@zte.com.cn>
Date: Mon, 4 Mar 2019 15:09:39 +0800
Subject: [PATCH]     improve _collect_aggregates

    There are id,name and hosts fields in the response of list aggregates.
    So don't need invoke get_aggregate_detail again.
    https://developer.openstack.org/api-ref/compute/?expanded=#list-aggregates

Change-Id: I94b768c2d3e0471ca619ce43387889ec74769ce2
---
 watcher/decision_engine/scope/compute.py      |  8 ++---
 .../decision_engine/scope/test_compute.py     | 31 +++++--------------
 2 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/watcher/decision_engine/scope/compute.py b/watcher/decision_engine/scope/compute.py
index def63e877..a6d252623 100644
--- a/watcher/decision_engine/scope/compute.py
+++ b/watcher/decision_engine/scope/compute.py
@@ -61,12 +61,10 @@ class ComputeScope(base.BaseScope):
                                 for field in (aggregate_ids, aggregate_names))
 
         for aggregate in aggregate_list:
-            detailed_aggregate = self.wrapper.get_aggregate_detail(
-                aggregate.id)
-            if (detailed_aggregate.id in aggregate_ids or
-                detailed_aggregate.name in aggregate_names or
+            if (aggregate.id in aggregate_ids or
+                aggregate.name in aggregate_names or
                     include_all_nodes):
-                compute_nodes.extend(detailed_aggregate.hosts)
+                compute_nodes.extend(aggregate.hosts)
 
     def _collect_zones(self, availability_zones, allowed_nodes):
         service_list = self.wrapper.get_service_list()
diff --git a/watcher/tests/decision_engine/scope/test_compute.py b/watcher/tests/decision_engine/scope/test_compute.py
index 9d77a1815..609c902bc 100644
--- a/watcher/tests/decision_engine/scope/test_compute.py
+++ b/watcher/tests/decision_engine/scope/test_compute.py
@@ -71,25 +71,20 @@ class TestComputeScope(base.TestCase):
         ]
         self.assertEqual(sorted(expected_edges), sorted(model.edges()))
 
-    @mock.patch.object(nova_helper.NovaHelper, 'get_aggregate_detail')
     @mock.patch.object(nova_helper.NovaHelper, 'get_aggregate_list')
-    def test_collect_aggregates(self, mock_aggregate, mock_detailed_aggregate):
+    def test_collect_aggregates(self, mock_aggregate):
         allowed_nodes = []
-        mock_aggregate.return_value = [mock.Mock(id=i) for i in range(2)]
-        mock_detailed_aggregate.side_effect = [
+        mock_aggregate.return_value = [
             mock.Mock(id=i, hosts=['Node_{0}'.format(i)]) for i in range(2)]
         compute.ComputeScope([{'host_aggregates': [{'id': 1}, {'id': 2}]}],
                              mock.Mock(), osc=mock.Mock())._collect_aggregates(
             [{'id': 1}, {'id': 2}], allowed_nodes)
         self.assertEqual(['Node_1'], allowed_nodes)
 
-    @mock.patch.object(nova_helper.NovaHelper, 'get_aggregate_detail')
     @mock.patch.object(nova_helper.NovaHelper, 'get_aggregate_list')
-    def test_aggregates_wildcard_is_used(self, mock_aggregate,
-                                         mock_detailed_aggregate):
+    def test_aggregates_wildcard_is_used(self, mock_aggregate):
         allowed_nodes = []
-        mock_aggregate.return_value = [mock.Mock(id=i) for i in range(2)]
-        mock_detailed_aggregate.side_effect = [
+        mock_aggregate.return_value = [
             mock.Mock(id=i, hosts=['Node_{0}'.format(i)]) for i in range(2)]
         compute.ComputeScope([{'host_aggregates': [{'id': '*'}]}],
                              mock.Mock(), osc=mock.Mock())._collect_aggregates(
@@ -108,20 +103,15 @@ class TestComputeScope(base.TestCase):
                           [{'id': '*'}, {'id': 1}],
                           allowed_nodes)
 
-    @mock.patch.object(nova_helper.NovaHelper, 'get_aggregate_detail')
     @mock.patch.object(nova_helper.NovaHelper, 'get_aggregate_list')
-    def test_aggregates_with_names_and_ids(self, mock_aggregate,
-                                           mock_detailed_aggregate):
+    def test_aggregates_with_names_and_ids(self, mock_aggregate):
         allowed_nodes = []
-        mock_aggregate.return_value = [mock.Mock(id=i,
-                                                 name="HA_{0}".format(i))
-                                       for i in range(2)]
         mock_collection = [mock.Mock(id=i, hosts=['Node_{0}'.format(i)])
                            for i in range(2)]
         mock_collection[0].name = 'HA_0'
         mock_collection[1].name = 'HA_1'
 
-        mock_detailed_aggregate.side_effect = mock_collection
+        mock_aggregate.return_value = mock_collection
 
         compute.ComputeScope([{'host_aggregates': [{'name': 'HA_1'},
                                                    {'id': 0}]}],
@@ -178,18 +168,13 @@ class TestComputeScope(base.TestCase):
             audit_template.AuditTemplatePostType._build_schema()
             ).validate(test_scope)
 
-    @mock.patch.object(nova_helper.NovaHelper, 'get_aggregate_detail')
     @mock.patch.object(nova_helper.NovaHelper, 'get_aggregate_list')
-    def test_exclude_resource(
-            self, mock_aggregate, mock_detailed_aggregate):
-        mock_aggregate.return_value = [mock.Mock(id=i,
-                                                 name="HA_{0}".format(i))
-                                       for i in range(2)]
+    def test_exclude_resource(self, mock_aggregate):
         mock_collection = [mock.Mock(id=i, hosts=['Node_{0}'.format(i)])
                            for i in range(2)]
         mock_collection[0].name = 'HA_0'
         mock_collection[1].name = 'HA_1'
-        mock_detailed_aggregate.side_effect = mock_collection
+        mock_aggregate.return_value = mock_collection
 
         resources_to_exclude = [{'host_aggregates': [{'name': 'HA_1'},
                                                      {'id': 0}]},