From 498413074d1f11688123b6b592d5c204dc7b5ef2 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Tue, 16 Oct 2018 16:23:54 -0400 Subject: [PATCH] Ignore uuid if already set in ComputeNode.update_from_virt_driver Change Ia69fabce8e7fd7de101e291fe133c6f5f5f7056a sets the ComputeNode.uuid to whatever the virt driver reports if the virt driver reports a uuid, like in the case of ironic. However, that breaks upgrades for any pre-existing compute node records which have a random uuid since ComputeNode.uuid is a read-only field once set. This change simply ignores the uuid from the virt driver resources dict if the ComputeNode.uuid is already set. The bug actually shows up in the ironic grenade CI job logs in stable/rocky but didn't fail the nova-compute startup because ComputeManager._update_available_resource_for_node() catches and just logs the error, but it doesn't kill the service. Change-Id: Id02f501feefca358d36f39b24d426537685e425c Closes-Bug: #1798172 --- nova/objects/compute_node.py | 5 +++++ nova/tests/unit/objects/test_compute_node.py | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/nova/objects/compute_node.py b/nova/objects/compute_node.py index 702081cfc82b..aa01b5e79fe1 100644 --- a/nova/objects/compute_node.py +++ b/nova/objects/compute_node.py @@ -351,6 +351,11 @@ class ComputeNode(base.NovaPersistentObject, base.NovaObject): "disk_available_least", "host_ip", "uuid"] for key in keys: if key in resources: + # The uuid field is read-only so it should only be set when + # creating the compute node record for the first time. Ignore + # it otherwise. + if key == 'uuid' and 'uuid' in self: + continue setattr(self, key, resources[key]) # supported_instances has a different name in compute_node diff --git a/nova/tests/unit/objects/test_compute_node.py b/nova/tests/unit/objects/test_compute_node.py index ba9543dbed1b..9aa761a5241f 100644 --- a/nova/tests/unit/objects/test_compute_node.py +++ b/nova/tests/unit/objects/test_compute_node.py @@ -496,6 +496,20 @@ class _TestComputeNodeObject(object): expected.uuid = uuidsentinel.node_uuid self.assertTrue(base.obj_equal_prims(expected, compute)) + def test_update_from_virt_driver_uuid_already_set(self): + """Tests update_from_virt_driver where the compute node object already + has a uuid value so the uuid from the virt driver is ignored. + """ + # copy in case the update has a side effect + resources = copy.deepcopy(fake_resources) + # Emulate the ironic driver which adds a uuid field. + resources['uuid'] = uuidsentinel.node_uuid + compute = compute_node.ComputeNode(uuid=uuidsentinel.something_else) + compute.update_from_virt_driver(resources) + expected = fake_compute_with_resources.obj_clone() + expected.uuid = uuidsentinel.something_else + self.assertTrue(base.obj_equal_prims(expected, compute)) + def test_update_from_virt_driver_missing_field(self): # NOTE(pmurray): update_from_virt_driver does not require # all fields to be present in resources. Validation of the