Check our nodes for hypervisor_hostname changes

When we are loading our ComputeNode objects by UUID according to what
the virt driver reported, we can sanity check the DB records against
the virt driver's hostname. This covers the case where our CONF.host
has not changed but the hostname reported by the virt driver has,
assuming we can find the ComputeNode object(s) that match our
persistent node uuid.

Related to blueprint stable-compute-uuid
Change-Id: I41635210d7d6f46b437b06d2570a26a80ed8676a

Author: kk7ds

nova/compute/manager.py

@@ -1473,25 +1473,35 @@ def _get_nodes(self, context):
             node.
         """
         try:
-            node_uuids = self.driver.get_available_node_uuids()
+            node_ids = self.driver.get_nodenames_by_uuid()
         except exception.VirtDriverNotReady:
             LOG.warning(
                 "Virt driver is not ready. If this is the first time this "
                 "service is starting on this host, then you can ignore "
                 "this warning.")
             return {}
 
-        nodes = objects.ComputeNodeList.get_all_by_uuids(context, node_uuids)
+        nodes = objects.ComputeNodeList.get_all_by_uuids(context,
+                                                         list(node_ids.keys()))
         if not nodes:
             # NOTE(danms): This should only happen if the compute_id is
             # pre-provisioned on a host that has never started.
             LOG.warning('Compute nodes %s for host %s were not found in the '
                         'database. If this is the first time this service is '
                         'starting on this host, then you can ignore this '
                         'warning.',
-                        node_uuids, self.host)
+                        list(node_ids.keys()), self.host)
             return {}
 
+        for node in nodes:
+            if node.hypervisor_hostname != node_ids.get(node.uuid):
+                raise exception.InvalidConfiguration(
+                    ('My compute node %s has hypervisor_hostname %s '
+                     'but virt driver reports it should be %s. Possible '
+                     'rename detected, refusing to start!') % (
+                         node.uuid, node.hypervisor_hostname,
+                         node_ids.get(node.uuid)))
+
         return {n.uuid: n for n in nodes}
 
     def _ensure_existing_node_identity(self, service_ref):

nova/tests/unit/compute/test_compute_mgr.py

@@ -1162,24 +1162,48 @@ def test_init_host_with_in_progress_evacuations(self, mock_destroy_evac,
             mock_get_nodes.return_value.keys())
 
     @mock.patch.object(objects.ComputeNodeList, 'get_all_by_uuids')
-    @mock.patch.object(fake_driver.FakeDriver, 'get_available_nodes')
+    @mock.patch.object(fake_driver.FakeDriver, 'get_nodenames_by_uuid')
     def test_get_nodes(self, mock_driver_get_nodes, mock_get_by_uuid):
-        mock_driver_get_nodes.return_value = ['fake_node1', 'fake_node2']
+        mock_driver_get_nodes.return_value = {uuids.node_fake_node1: 'host',
+                                              uuids.node_fake_node2: 'host'}
         # NOTE(danms): The fake driver, by default, uses
-        # uuidsentinel.$node_name, so we can predict the uuids it will
+        # uuidsentinel.node_$node_name, so we can predict the uuids it will
         # return here.
-        cn1 = objects.ComputeNode(uuid=uuids.fake_node1)
-        cn2 = objects.ComputeNode(uuid=uuids.fake_node2)
+        cn1 = objects.ComputeNode(uuid=uuids.node_fake_node1,
+                                  hypervisor_hostname='host')
+        cn2 = objects.ComputeNode(uuid=uuids.node_fake_node2,
+                                  hypervisor_hostname='host')
         mock_get_by_uuid.return_value = [cn1, cn2]
 
         nodes = self.compute._get_nodes(self.context)
 
-        self.assertEqual({uuids.fake_node1: cn1, uuids.fake_node2: cn2}, nodes)
+        self.assertEqual({uuids.node_fake_node1: cn1,
+                          uuids.node_fake_node2: cn2}, nodes)
 
         mock_driver_get_nodes.assert_called_once_with()
         mock_get_by_uuid.assert_called_once_with(self.context,
-                                                 [uuids.fake_node1,
-                                                  uuids.fake_node2])
+                                                 [uuids.node_fake_node1,
+                                                  uuids.node_fake_node2])
+
+    @mock.patch.object(objects.ComputeNodeList, 'get_all_by_uuids')
+    @mock.patch.object(fake_driver.FakeDriver, 'get_nodenames_by_uuid')
+    def test_get_nodes_mismatch(self, mock_driver_get_nodes, mock_get_by_uuid):
+        # Virt driver reports a (hypervisor_) hostname of 'host1'
+        mock_driver_get_nodes.return_value = {uuids.node_fake_node1: 'host1',
+                                              uuids.node_fake_node2: 'host1'}
+
+        # The database records for our compute nodes (by UUID) show a
+        # hypervisor_hostname of 'host2'
+        cn1 = objects.ComputeNode(uuid=uuids.node_fake_node1,
+                                  hypervisor_hostname='host2')
+        cn2 = objects.ComputeNode(uuid=uuids.node_fake_node2,
+                                  hypervisor_hostname='host2')
+        mock_get_by_uuid.return_value = [cn1, cn2]
+
+        # Possible hostname (as reported by the virt driver) rename,
+        # which should abort our startup
+        self.assertRaises(exception.InvalidConfiguration,
+                          self.compute._get_nodes, self.context)
 
     @mock.patch.object(manager.LOG, 'warning')
     @mock.patch.object(
@@ -1202,11 +1226,11 @@ def test_get_nodes_driver_not_ready(
 
     @mock.patch.object(manager.LOG, 'warning')
     @mock.patch.object(objects.ComputeNodeList, 'get_all_by_uuids')
-    @mock.patch.object(fake_driver.FakeDriver, 'get_available_node_uuids')
+    @mock.patch.object(fake_driver.FakeDriver, 'get_nodenames_by_uuid')
     def test_get_nodes_node_not_found(
             self, mock_driver_get_nodes, mock_get_all_by_uuids,
             mock_log_warning):
-        mock_driver_get_nodes.return_value = ['fake-node1']
+        mock_driver_get_nodes.return_value = {uuids.node_1: 'fake-node1'}
         mock_get_all_by_uuids.return_value = []
 
         nodes = self.compute._get_nodes(self.context)
@@ -1215,11 +1239,11 @@ def test_get_nodes_node_not_found(
 
         mock_driver_get_nodes.assert_called_once_with()
         mock_get_all_by_uuids.assert_called_once_with(self.context,
-                                                      ['fake-node1'])
+                                                      [uuids.node_1])
         mock_log_warning.assert_called_once_with(
             "Compute nodes %s for host %s were not found in the database. "
             "If this is the first time this service is starting on this host, "
-            "then you can ignore this warning.", ['fake-node1'], 'fake-mini')
+            "then you can ignore this warning.", [uuids.node_1], 'fake-mini')
 
     def test_init_host_disk_devices_configuration_failure(self):
         self.flags(max_disk_devices_to_attach=0, group='compute')

nova/virt/driver.py

@@ -1596,8 +1596,10 @@ def get_available_nodes(self, refresh=False):
         """
         raise NotImplementedError()
 
-    def get_available_node_uuids(self, refresh=False):
-        return [nova.virt.node.get_local_node_uuid()]
+    def get_nodenames_by_uuid(self, refresh=False):
+        """Returns a dict of {uuid: nodename} for all managed nodes."""
+        nodename = self.get_available_nodes()[0]
+        return {nova.virt.node.get_local_node_uuid(): nodename}
 
     def node_is_available(self, nodename):
         """Return whether this compute service manages a particular node."""

nova/virt/fake.py

@@ -510,7 +510,7 @@ def get_available_resource(self, nodename):
         # control our node uuids. Make sure we return a unique and
         # consistent uuid for each node we are responsible for to
         # avoid the persistent local node identity from taking over.
-        host_status['uuid'] = str(getattr(uuids, nodename))
+        host_status['uuid'] = str(getattr(uuids, 'node_%s' % nodename))
         return host_status
 
     def update_provider_tree(self, provider_tree, nodename, allocations=None):
@@ -653,8 +653,9 @@ def get_volume_connector(self, instance):
     def get_available_nodes(self, refresh=False):
         return self._nodes
 
-    def get_available_node_uuids(self, refresh=False):
-        return [str(getattr(uuids, n)) for n in self.get_available_nodes()]
+    def get_nodenames_by_uuid(self, refresh=False):
+        return {str(getattr(uuids, 'node_%s' % n)): n
+                for n in self.get_available_nodes()}
 
     def instance_on_disk(self, instance):
         return False

nova/virt/ironic/driver.py

@@ -839,8 +839,12 @@ def get_available_nodes(self, refresh=False):
 
         return node_uuids
 
-    def get_available_node_uuids(self, refresh=False):
-        return self.get_available_nodes(refresh=refresh)
+    def get_nodenames_by_uuid(self, refresh=False):
+        nodes = self.get_available_nodes(refresh=refresh)
+        # We use the uuid for compute_node.uuid and
+        # compute_node.hypervisor_hostname, so the dict keys and values are
+        # the same.
+        return dict(zip(nodes, nodes))
 
     def update_provider_tree(self, provider_tree, nodename, allocations=None):
         """Update a ProviderTree object with current resource provider and

nova/virt/libvirt/driver.py

@@ -11332,8 +11332,8 @@ def _get_disk_over_committed_size_total(self):
     def get_available_nodes(self, refresh=False):
         return [self._host.get_hostname()]
 
-    def get_available_node_uuids(self, refresh=False):
-        return [self._host.get_node_uuid()]
+    def get_nodenames_by_uuid(self, refresh=False):
+        return {self._host.get_node_uuid(): self._host.get_hostname()}
 
     def get_host_cpu_stats(self):
         """Return the current CPU state of the host."""