Optimize hypervisor API calls

The nova CDM builder code and notification handling
code had some inefficiencies when it came to looking
up a hypevisor to get details. The general pattern
used before was:

1. get the minimal hypervisor information by hypervisor_hostname
2. make another query to get the hypervisor details by id

In the notifications case, it was actually three calls because
the first is listing hyprvisors to filter client-side by service
host.

This change collapses 1 and 2 above into a single API call
to get the hypervisor by hypervisor_hostname with details
which will include the service (compute) host information
which is what get_compute_node_by_id() was being used for.
Now that nothing is using get_compute_node_by_id it is removed.

There is more work we could do in get_compute_node_by_hostname
if the compute API allowed filtering hypervisors by service
host so a TODO is left for that.

One final thing: the TODO in get_compute_node_by_hostname about
there being more than one hypervisor per compute service host
for vmware vcenter is not accurate - nova's vcenter driver
hasn't supported a host:node 1:M topology like that since the
Liberty release [1]. The only in-tree driver in nova that supports
1:M is the ironic baremetal driver, so the comment is updated.

[1] Ifc17c5049e3ed29c8dd130339207907b00433960

Depends-On: https://review.opendev.org/661785/
Change-Id: I5e0e88d7b2dd1a69117ab03e0e66851c687606da

lower-constraints.txt

@@ -117,7 +117,7 @@ python-keystoneclient==3.15.0
 python-mimeparse==1.6.0
 python-monascaclient==1.12.0
 python-neutronclient==6.7.0
-python-novaclient==10.1.0
+python-novaclient==14.1.0
 python-openstackclient==3.14.0
 python-subunit==1.2.0
 pytz==2018.3

requirements.txt

@@ -36,7 +36,7 @@ python-glanceclient>=2.9.1 # Apache-2.0
 python-keystoneclient>=3.15.0 # Apache-2.0
 python-monascaclient>=1.12.0 # Apache-2.0
 python-neutronclient>=6.7.0 # Apache-2.0
-python-novaclient>=10.1.0 # Apache-2.0
+python-novaclient>=14.1.0 # Apache-2.0
 python-openstackclient>=3.14.0 # Apache-2.0
 python-ironicclient>=2.3.0 # Apache-2.0
 six>=1.11.0 # MIT

watcher/common/nova_helper.py

@@ -27,7 +27,6 @@ import novaclient.exceptions as nvexceptions
 
 from watcher.common import clients
 from watcher.common import exception
-from watcher.common import utils
 from watcher import conf
 
 LOG = log.getLogger(__name__)
@@ -48,30 +47,37 @@ class NovaHelper(object):
     def get_compute_node_list(self):
         return self.nova.hypervisors.list()
 
-    def get_compute_node_by_id(self, node_id):
+    def get_compute_node_by_name(self, node_name, servers=False,
-        """Get compute node by ID (*not* UUID)"""
+                                 detailed=False):
-        # We need to pass an object with an 'id' attribute to make it work
+        """Search for a hypervisor (compute node) by hypervisor_hostname
-        return self.nova.hypervisors.get(utils.Struct(id=node_id))
 
-    def get_compute_node_by_name(self, node_name, servers=False):
+        :param node_name: The hypervisor_hostname to search
-        return self.nova.hypervisors.search(node_name, servers)
+        :param servers: If true, include information about servers per
+            hypervisor
+        :param detailed: If true, include information about the compute service
+            per hypervisor (requires microversion 2.53)
+        """
+        return self.nova.hypervisors.search(node_name, servers=servers,
+                                            detailed=detailed)
 
     def get_compute_node_by_hostname(self, node_hostname):
         """Get compute node by hostname"""
+        # TODO(mriedem): This method could be optimized if
+        # GET /os-hypervisors/detail had a host filter parameter.
         try:
             hypervisors = [hv for hv in self.get_compute_node_list()
                            if hv.service['host'] == node_hostname]
             if len(hypervisors) != 1:
                 # TODO(hidekazu)
-                # this may occur if VMware vCenter driver is used
+                # this may occur if ironic driver is used
                 raise exception.ComputeNodeNotFound(name=node_hostname)
             else:
-                compute_nodes = self.nova.hypervisors.search(
+                compute_nodes = self.get_compute_node_by_name(
-                    hypervisors[0].hypervisor_hostname)
+                    hypervisors[0].hypervisor_hostname, detailed=True)
                 if len(compute_nodes) != 1:
                     raise exception.ComputeNodeNotFound(name=node_hostname)
 
-                return self.get_compute_node_by_id(compute_nodes[0].id)
+                return compute_nodes[0]
         except Exception as exc:
             LOG.exception(exc)
             raise exception.ComputeNodeNotFound(name=node_hostname)

watcher/decision_engine/model/collector/nova.py

@@ -258,19 +258,15 @@ 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)
+                                                              servers=True,
+                                                              detailed=True)
             if cnode:
-                # Get the node details (like the service.host).
+                node_info = 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)
+                instances = getattr(node_info, "servers", None)
                 self.add_instance_node(node_info, instances)
 
     def add_compute_node(self, node):

watcher/tests/common/test_nova_helper.py

@@ -123,7 +123,6 @@ class TestNovaHelper(base.TestCase):
         hypervisor = self.fake_hypervisor(hypervisor_id, hypervisor_name)
         self.fake_nova_hypervisor_list(
             nova_util,
-            fake_find=hypervisor,
             fake_list=[hypervisor])
         nova_util.nova.hypervisors.search.return_value = [hypervisor]
         # verify that the compute node can be obtained normally by name

watcher/tests/decision_engine/cluster/test_model_builder.py

@@ -168,8 +168,8 @@ class TestModelBuilder(base.BaseTestCase):
         t_nova_cluster = nova.ModelBuilder(mock.Mock())
         t_nova_cluster.execute(m_scope)
         m_nova.return_value.get_compute_node_by_name.assert_any_call(
-            'hostone', servers=True)
+            'hostone', servers=True, detailed=True)
         m_nova.return_value.get_compute_node_by_name.assert_any_call(
-            'hosttwo', servers=True)
+            'hosttwo', servers=True, detailed=True)
         self.assertEqual(
             m_nova.return_value.get_compute_node_by_name.call_count, 2)

watcher/tests/decision_engine/cluster/test_nova_cdmc.py

@@ -66,7 +66,14 @@ class TestNovaClusterDataModelCollector(base.TestCase):
             servers=None,  # Don't let the mock return a value for servers.
             **minimal_node
         )
-        fake_compute_node_with_servers = mock.Mock(**minimal_node_with_servers)
+        fake_detailed_node = mock.Mock(
+            service={'id': 123, 'host': 'test_hostname',
+                     'disabled_reason': ''},
+            memory_mb=333,
+            free_disk_gb=222,
+            local_gb=111,
+            vcpus=4,
+            **minimal_node_with_servers)
         fake_instance = mock.Mock(
             id='ef500f7e-dac8-470f-960c-169486fce71b',
             human_id='fake_instance',
@@ -77,11 +84,10 @@ class TestNovaClusterDataModelCollector(base.TestCase):
         setattr(fake_instance, 'OS-EXT-STS:vm_state', 'VM_STATE')
         # 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).
+        # Returns the hypervisor with servers and details (service).
         m_nova_helper.get_compute_node_by_name.return_value = [
-            fake_compute_node_with_servers]
+            fake_detailed_node]
         # 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.get_instance_list.return_value = [fake_instance]
 
         m_config = mock.Mock()
@@ -105,5 +111,7 @@ class TestNovaClusterDataModelCollector(base.TestCase):
         self.assertEqual(node.uuid, 'test_hostname')
         self.assertEqual(instance.uuid, 'ef500f7e-dac8-470f-960c-169486fce71b')
 
+        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']})