Merge "Reduce the query time of the instances when call get_instance_list()"

watcher/common/nova_helper.py

@@ -82,19 +82,25 @@ class NovaHelper(object):
             LOG.exception(exc)
             raise exception.ComputeNodeNotFound(name=node_hostname)
 
-    def get_instance_list(self, filters=None):
+    def get_instance_list(self, filters=None, limit=-1):
         """List servers for all tenants with details.
 
         This always gets servers with the all_tenants=True filter.
 
-        :param filters: dict of additional filters
+        :param filters: dict of additional filters (optional).
+        :param limit: Maximum number of servers to return (optional).
+                      If limit == -1, all servers will be returned,
+                      note that limit == -1 will have a performance
+                      penalty. For details, please see:
+                      https://bugs.launchpad.net/watcher/+bug/1834679
         :returns: list of novaclient Server objects
         """
         search_opts = {'all_tenants': True}
         if filters:
             search_opts.update(filters)
+        # TODO(chenker) Add marker param to list Server objects.
         return self.nova.servers.list(search_opts=search_opts,
-                                      limit=-1)
+                                      limit=limit)
 
     def get_instance_by_uuid(self, instance_uuid):
         return [instance for instance in

watcher/decision_engine/model/collector/nova.py

@@ -327,8 +327,18 @@ class ModelBuilder(object):
             return
         host = node.service["host"]
         compute_node = self.model.get_node_by_uuid(host)
+        filters = {'host': host}
+        limit = len(instances) if len(instances) <= 1000 else -1
         # Get all servers on this compute host.
-        instances = self.nova_helper.get_instance_list({'host': host})
+        # Note that the advantage of passing the limit parameter is
+        # that it can speed up the call time of novaclient. 1000 is
+        # the default maximum number of return servers provided by
+        # compute API. If we need to request more than 1000 servers,
+        # we can set limit=-1. For details, please see:
+        # https://bugs.launchpad.net/watcher/+bug/1834679
+        instances = self.nova_helper.get_instance_list(
+            filters=filters,
+            limit=limit)
         for inst in instances:
             # Add Node
             instance = self._build_instance_node(inst)

watcher/tests/decision_engine/cluster/test_nova_cdmc.py

@@ -115,4 +115,26 @@ class TestNovaClusterDataModelCollector(base.TestCase):
         m_nova_helper.get_compute_node_by_name.assert_called_once_with(
             minimal_node['hypervisor_hostname'], servers=True, detailed=True)
         m_nova_helper.get_instance_list.assert_called_once_with(
-            {'host': fake_compute_node.service['host']})
+            filters={'host': fake_compute_node.service['host']}, limit=1)
+
+
+class TestModelBuilder(base.TestCase):
+
+    @mock.patch.object(nova_helper, 'NovaHelper', mock.MagicMock())
+    def test_add_instance_node(self):
+        model_builder = nova.ModelBuilder(osc=mock.MagicMock())
+        model_builder.model = mock.MagicMock()
+        mock_node = mock.MagicMock()
+        mock_host = mock_node.service["host"]
+        mock_instances = [mock.MagicMock()]
+        model_builder.add_instance_node(mock_node, mock_instances)
+        # verify that when len(instances) <= 1000, limit == len(instance).
+        model_builder.nova_helper.get_instance_list.assert_called_once_with(
+            filters={'host': mock_host}, limit=1)
+
+        # verify that when len(instances) > 1000, limit == -1.
+        mock_instance = mock.Mock()
+        mock_instances = [mock_instance] * 1001
+        model_builder.add_instance_node(mock_node, mock_instances)
+        model_builder.nova_helper.get_instance_list.assert_called_with(
+            filters={'host': mock_host}, limit=-1)