Merge "Abort startup if nodename conflict is detected"

6 files changed, 49 insertions, 2 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index c4537cd9a2..952ab3e199 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -10480,6 +10480,14 @@ class ComputeManager(manager.Manager):
             LOG.exception(
                 "Error updating PCI resources for node %(node)s.",
                 {'node': nodename})
+        except exception.InvalidConfiguration as e:
+            if startup:
+                # If this happens during startup, we need to let it raise to
+                # abort our service startup.
+                raise
+            else:
+                LOG.error("Error updating resources for node %s: %s",
+                          nodename, e)
         except Exception:
             LOG.exception("Error updating resources for node %(node)s.",
                           {'node': nodename})
diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py
index 70c56fd2e3..3f911f3708 100644
--- a/nova/compute/resource_tracker.py
+++ b/nova/compute/resource_tracker.py
@@ -728,7 +728,13 @@ class ResourceTracker(object):
         cn = objects.ComputeNode(context)
         cn.host = self.host
         self._copy_resources(cn, resources, initial=True)
-        cn.create()
+        try:
+            cn.create()
+        except exception.DuplicateRecord:
+            raise exception.InvalidConfiguration(
+                'Duplicate compute node record found for host %s node %s' % (
+                cn.host, cn.hypervisor_hostname))
+
         # Only map the ComputeNode into compute_nodes if create() was OK
         # because if create() fails, on the next run through here nodename
         # would be in compute_nodes and we won't try to create again (because
diff --git a/nova/exception.py b/nova/exception.py
index 20c112b628..f5993e79f8 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -2512,6 +2512,10 @@ class InvalidNodeConfiguration(NovaException):
     msg_fmt = _('Invalid node identity configuration: %(reason)s')
 
 
+class DuplicateRecord(NovaException):
+    msg_fmt = _('Unable to create duplicate record for %(target)s')
+
+
 class NotSupportedComputeForEvacuateV295(NotSupported):
     msg_fmt = _("Starting to microversion 2.95, evacuate API will stop "
                 "instance on destination. To evacuate before upgrades are "
diff --git a/nova/objects/compute_node.py b/nova/objects/compute_node.py
index 528cfc0776..dfc1b2ae28 100644
--- a/nova/objects/compute_node.py
+++ b/nova/objects/compute_node.py
@@ -12,6 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_db import exception as db_exc
 from oslo_serialization import jsonutils
 from oslo_utils import uuidutils
 from oslo_utils import versionutils
@@ -339,7 +340,12 @@ class ComputeNode(base.NovaPersistentObject, base.NovaObject):
         self._convert_supported_instances_to_db_format(updates)
         self._convert_pci_stats_to_db_format(updates)
 
-        db_compute = db.compute_node_create(self._context, updates)
+        try:
+            db_compute = db.compute_node_create(self._context, updates)
+        except db_exc.DBDuplicateEntry:
+            target = 'compute node %s:%s' % (updates['hypervisor_hostname'],
+                                             updates['uuid'])
+            raise exception.DuplicateRecord(target=target)
         self._from_db_object(self._context, self, db_compute)
 
     @base.remotable
diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py
index dfea323a9a..cd36b8987f 100644
--- a/nova/tests/unit/compute/test_resource_tracker.py
+++ b/nova/tests/unit/compute/test_resource_tracker.py
@@ -1552,6 +1552,20 @@ class TestInitComputeNode(BaseTestCase):
         self.assertEqual('fake-host', node.host)
         mock_update.assert_called()
 
+    @mock.patch.object(resource_tracker.ResourceTracker,
+                       '_get_compute_node',
+                       return_value=None)
+    @mock.patch('nova.objects.compute_node.ComputeNode.create')
+    def test_create_failed_conflict(self, mock_create, mock_getcn):
+        self._setup_rt()
+        resources = {'hypervisor_hostname': 'node1',
+                     'uuid': uuids.node1}
+        mock_create.side_effect = exc.DuplicateRecord(target='foo')
+        self.assertRaises(exc.InvalidConfiguration,
+                          self.rt._init_compute_node,
+                          mock.MagicMock,
+                          resources)
+
 
 @ddt.ddt
 class TestUpdateComputeNode(BaseTestCase):
diff --git a/nova/tests/unit/objects/test_compute_node.py b/nova/tests/unit/objects/test_compute_node.py
index 63b070c543..84c4e87785 100644
--- a/nova/tests/unit/objects/test_compute_node.py
+++ b/nova/tests/unit/objects/test_compute_node.py
@@ -16,6 +16,7 @@ import copy
 from unittest import mock
 
 import netaddr
+from oslo_db import exception as db_exc
 from oslo_serialization import jsonutils
 from oslo_utils.fixture import uuidsentinel
 from oslo_utils import timeutils
@@ -341,6 +342,14 @@ class _TestComputeNodeObject(object):
                       'uuid': uuidsentinel.fake_compute_node}
         mock_create.assert_called_once_with(self.context, param_dict)
 
+    @mock.patch('nova.db.main.api.compute_node_create')
+    def test_create_duplicate(self, mock_create):
+        mock_create.side_effect = db_exc.DBDuplicateEntry
+        compute = compute_node.ComputeNode(context=self.context)
+        compute.service_id = 456
+        compute.hypervisor_hostname = 'node1'
+        self.assertRaises(exception.DuplicateRecord, compute.create)
+
     @mock.patch.object(db, 'compute_node_update')
     @mock.patch(
         'nova.db.main.api.compute_node_get', return_value=fake_compute_node)