Merge "Optimize NovaClusterDataModelCollector.add_instance_node"

watcher/common/nova_helper.py

@@ -76,8 +76,18 @@ class NovaHelper(object):
             LOG.exception(exc)
             raise exception.ComputeNodeNotFound(name=node_hostname)
 
-    def get_instance_list(self):
+    def get_instance_list(self, filters=None):
-        return self.nova.servers.list(search_opts={'all_tenants': True},
+        """List servers for all tenants with details.
+
+        This always gets servers with the all_tenants=True filter.
+
+        :param filters: dict of additional filters
+        :returns: list of novaclient Server objects
+        """
+        search_opts = {'all_tenants': True}
+        if filters:
+            search_opts.update(filters)
+        return self.nova.servers.list(search_opts=search_opts,
                                       limit=-1)
 
     def get_flavor_list(self):

watcher/decision_engine/model/collector/nova.py

@@ -258,17 +258,25 @@ class ModelBuilder(object):
                 [node.hypervisor_hostname for node in all_nodes])
         LOG.debug("compute nodes: %s", compute_nodes)
         for node_name in compute_nodes:
+            # TODO(mriedem): Change this to list hypervisors with details
+            # so we don't have to call get_compute_node_by_id. It requires
+            # changes to python-novaclient.
             cnode = self.nova_helper.get_compute_node_by_name(node_name,
                                                               servers=True)
             if cnode:
-                self.add_compute_node(cnode[0])
+                # Get the node details (like the service.host).
-                self.add_instance_node(cnode[0])
+                node_info = self.nova_helper.get_compute_node_by_id(
+                    cnode[0].id)
+                self.add_compute_node(node_info)
+                # node.servers is a list of server objects
+                # New in nova version 2.53
+                instances = getattr(cnode[0], "servers", None)
+                self.add_instance_node(node_info, instances)
 
     def add_compute_node(self, node):
         # Build and add base node.
-        node_info = self.nova_helper.get_compute_node_by_id(node.id)
+        LOG.debug("node info: %s", node)
-        LOG.debug("node info: %s", node_info)
+        compute_node = self.build_compute_node(node)
-        compute_node = self.build_compute_node(node_info)
         self.model.add_node(compute_node)
 
         # NOTE(v-francoise): we can encapsulate capabilities of the node
@@ -314,26 +322,18 @@ class ModelBuilder(object):
         #                                 node_attributes)
         return compute_node
 
-    def add_instance_node(self, node):
+    def add_instance_node(self, node, instances):
-        # node.servers is a list of server objects
-        # New in nova version 2.53
-        instances = getattr(node, "servers", None)
         if instances is None:
             # no instances on this node
             return
-        instance_uuids = [s['uuid'] for s in instances]
+        host = node.service["host"]
-        for uuid in instance_uuids:
+        compute_node = self.model.get_node_by_uuid(host)
-            try:
+        # Get all servers on this compute host.
-                inst = self.nova_helper.find_instance(uuid)
+        instances = self.nova_helper.get_instance_list({'host': host})
-            except Exception as exc:
+        for inst in instances:
-                LOG.exception(exc)
-                continue
             # Add Node
             instance = self._build_instance_node(inst)
             self.model.add_instance(instance)
-            cnode_uuid = getattr(inst, "OS-EXT-SRV-ATTR:host")
-            compute_node = self.model.get_node_by_uuid(
-                cnode_uuid)
             # Connect the instance to its compute node
             self.model.map_instance(instance, compute_node)
 

watcher/tests/common/test_nova_helper.py

@@ -146,6 +146,22 @@ class TestNovaHelper(base.TestCase):
             nova_util.get_compute_node_by_hostname,
             hypervisor_name)
 
+    def test_get_instance_list(self, *args):
+        nova_util = nova_helper.NovaHelper()
+        # Call it once with no filters.
+        with mock.patch.object(nova_util, 'nova') as nova_mock:
+            result = nova_util.get_instance_list()
+            nova_mock.servers.list.assert_called_once_with(
+                search_opts={'all_tenants': True}, limit=-1)
+            self.assertIs(result, nova_mock.servers.list.return_value)
+        # Call it again with filters.
+        with mock.patch.object(nova_util, 'nova') as nova_mock:
+            result = nova_util.get_instance_list(filters={'host': 'fake-host'})
+            nova_mock.servers.list.assert_called_once_with(
+                search_opts={'all_tenants': True, 'host': 'fake-host'},
+                limit=-1)
+            self.assertIs(result, nova_mock.servers.list.return_value)
+
     @mock.patch.object(time, 'sleep', mock.Mock())
     def test_stop_instance(self, mock_glance, mock_cinder, mock_neutron,
                            mock_nova):

watcher/tests/decision_engine/cluster/test_nova_cdmc.py

@@ -43,23 +43,30 @@ class TestNovaClusterDataModelCollector(base.TestCase):
             state='up',
             disabled_reason='',
         )
-
+        minimal_node = dict(
-        fake_compute_node = mock.Mock(
             id=1337,
+            hypervisor_hostname='test_hostname',
+            state='TEST_STATE',
+            status='TEST_STATUS',
+        )
+        minimal_node_with_servers = dict(
+            servers=[
+                {'name': 'fake_instance',
+                 'uuid': 'ef500f7e-dac8-470f-960c-169486fce71b'}
+            ],
+            **minimal_node
+        )
+        fake_compute_node = mock.Mock(
             service={'id': 123, 'host': 'test_hostname',
                      'disabled_reason': ''},
-            hypervisor_hostname='test_hostname',
             memory_mb=333,
             free_disk_gb=222,
             local_gb=111,
             vcpus=4,
-            state='TEST_STATE',
+            servers=None,  # Don't let the mock return a value for servers.
-            status='TEST_STATUS',
+            **minimal_node
-            servers=[
-                {'name': 'fake_instance',
-                 'uuid': 'ef500f7e-dac8-470f-960c-169486fce71b'}
-                ]
         )
+        fake_compute_node_with_servers = mock.Mock(**minimal_node_with_servers)
         fake_instance = mock.Mock(
             id='ef500f7e-dac8-470f-960c-169486fce71b',
             human_id='fake_instance',
@@ -68,12 +75,14 @@ class TestNovaClusterDataModelCollector(base.TestCase):
             tenant_id='ff560f7e-dbc8-771f-960c-164482fce21b',
         )
         setattr(fake_instance, 'OS-EXT-STS:vm_state', 'VM_STATE')
-        setattr(fake_instance, 'OS-EXT-SRV-ATTR:host', 'test_hostname')
+        # Returns the hypervisors with details (service) but no servers.
         m_nova_helper.get_compute_node_list.return_value = [fake_compute_node]
+        # Returns the hypervisor with servers but no details (service).
         m_nova_helper.get_compute_node_by_name.return_value = [
-            fake_compute_node]
+            fake_compute_node_with_servers]
+        # Returns the hypervisor with details (service) but no servers.
         m_nova_helper.get_compute_node_by_id.return_value = fake_compute_node
-        m_nova_helper.find_instance.return_value = fake_instance
+        m_nova_helper.get_instance_list.return_value = [fake_instance]
 
         m_config = mock.Mock()
         m_osc = mock.Mock()
@@ -95,3 +104,6 @@ class TestNovaClusterDataModelCollector(base.TestCase):
 
         self.assertEqual(node.uuid, 'test_hostname')
         self.assertEqual(instance.uuid, 'ef500f7e-dac8-470f-960c-169486fce71b')
+
+        m_nova_helper.get_instance_list.assert_called_once_with(
+            {'host': fake_compute_node.service['host']})