Optimize NovaHelper.get_compute_node_by_hostname

The get_compute_node_by_hostname method is given a
compute service hostname and then does two queries to
find the matching hypervisor (compute node) with details:

1. List hypervisors with details and find the one that
   matches the given compute service hostname.

2. Using that node, search for hypervisors with the
   matching hypervisor_hostname.

There are two issues here:

1. The first query is inefficient in that it has to list
   all hypervisors in the deployment to try and match the
   one with the compute service hostname client side.

2. The second query is a fuzzy match on the server side [1]
   so even though we have matched on the node we want,
   get_compute_node_by_name can still return more than
   one hypervisor which will result in the helper method
   raising ComputeNodeNotFound. Consider having compute
   hosts with names compute1, compute10, compute11, compute100,
   and so on. The fuzzy match on compute1 would return all of
   those hypervisors.

For non-ironic nodes in nova, the compute service host and
hypervisor should be 1:1, meaning the hypervisor.service['host']
should be the same as hypervisor.hypervisor_hostname. Knowing
this, we can simplify the code to search just on the given
compute service hostname and if we get more than one result, it
is because of the fuzzy match and we can then do our client-side
filtering on the compute service hostname.

[1] https://github.com/openstack/nova/blob/d4f58f5eb/nova/db/sqlalchemy/api.py#L676

Change-Id: I84f387982f665d7cc11bffe8ec390cc7e7ed5278

watcher/common/nova_helper.py

@@ -61,23 +61,23 @@ class NovaHelper(object):
                                             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 ironic driver is used
-                raise exception.ComputeNodeNotFound(name=node_hostname)
-            else:
-                compute_nodes = self.get_compute_node_by_name(
-                    hypervisors[0].hypervisor_hostname, detailed=True)
-                if len(compute_nodes) != 1:
-                    raise exception.ComputeNodeNotFound(name=node_hostname)
+        """Get compute node by hostname
 
-                return compute_nodes[0]
+        :param node_hostname: Compute service hostname
+        :returns: novaclient.v2.hypervisors.Hypervisor object if found
+        :raises: ComputeNodeNotFound if no hypervisor is found for the compute
+            service hostname or there was an error communicating with nova
+        """
+        try:
+            # This is a fuzzy match on hypervisor_hostname so we could get back
+            # more than one compute node. If so, match on the compute service
+            # hostname.
+            compute_nodes = self.get_compute_node_by_name(
+                node_hostname, detailed=True)
+            for cn in compute_nodes:
+                if cn.service['host'] == node_hostname:
+                    return cn
+            raise exception.ComputeNodeNotFound(name=node_hostname)
         except Exception as exc:
             LOG.exception(exc)
             raise exception.ComputeNodeNotFound(name=node_hostname)

watcher/tests/common/test_nova_helper.py

@@ -121,9 +121,6 @@ class TestNovaHelper(base.TestCase):
         hypervisor_id = utils.generate_uuid()
         hypervisor_name = "fake_hypervisor_1"
         hypervisor = self.fake_hypervisor(hypervisor_id, hypervisor_name)
-        self.fake_nova_hypervisor_list(
-            nova_util,
-            fake_list=[hypervisor])
         nova_util.nova.hypervisors.search.return_value = [hypervisor]
         # verify that the compute node can be obtained normally by name
         self.assertEqual(
@@ -145,6 +142,25 @@ class TestNovaHelper(base.TestCase):
             nova_util.get_compute_node_by_hostname,
             hypervisor_name)
 
+    def test_get_compute_node_by_hostname_multiple_matches(self, *mocks):
+        # Tests a scenario where get_compute_node_by_name returns multiple
+        # hypervisors and we have to pick the exact match based on the given
+        # compute service hostname.
+        nova_util = nova_helper.NovaHelper()
+        nodes = []
+        # compute1 is a substring of compute10 to trigger the fuzzy match.
+        for hostname in ('compute1', 'compute10'):
+            node = mock.MagicMock()
+            node.id = utils.generate_uuid()
+            node.hypervisor_hostname = hostname
+            node.service = {'host': hostname}
+            nodes.append(node)
+        # We should get back exact matches based on the service host.
+        nova_util.nova.hypervisors.search.return_value = nodes
+        for index, name in enumerate(['compute1', 'compute10']):
+            result = nova_util.get_compute_node_by_hostname(name)
+            self.assertIs(nodes[index], result)
+
     def test_get_instance_list(self, *args):
         nova_util = nova_helper.NovaHelper()
         # Call it once with no filters.