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

6 files changed, 65 insertions, 24 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 0bf6fdfec5..f59fd82d10 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -1473,7 +1473,7 @@ class ComputeManager(manager.Manager):
             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 "
@@ -1481,7 +1481,8 @@ class ComputeManager(manager.Manager):
                 "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.
@@ -1489,9 +1490,18 @@ class ComputeManager(manager.Manager):
                         '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):
diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py
index c923b0e967..c5fed20377 100644
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -1162,24 +1162,48 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
             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 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
 
     @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 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
 
         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')
diff --git a/nova/virt/driver.py b/nova/virt/driver.py
index b6297bb785..5d42a392d8 100644
--- a/nova/virt/driver.py
+++ b/nova/virt/driver.py
@@ -1596,8 +1596,10 @@ class ComputeDriver(object):
         """
         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."""
diff --git a/nova/virt/fake.py b/nova/virt/fake.py
index f86db8fcb2..2234bd068e 100644
--- a/nova/virt/fake.py
+++ b/nova/virt/fake.py
@@ -510,7 +510,7 @@ class FakeDriver(driver.ComputeDriver):
         # 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 @@ class FakeDriver(driver.ComputeDriver):
     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
diff --git a/nova/virt/ironic/driver.py b/nova/virt/ironic/driver.py
index b437b4a959..77fefb81ea 100644
--- a/nova/virt/ironic/driver.py
+++ b/nova/virt/ironic/driver.py
@@ -839,8 +839,12 @@ class IronicDriver(virt_driver.ComputeDriver):
 
         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
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 4b34759bb7..542383cbad 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -11332,8 +11332,8 @@ class LibvirtDriver(driver.ComputeDriver):
     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."""