Skip to content

Commit

Permalink
Ignore uuid if already set in ComputeNode.update_from_virt_driver
Browse files Browse the repository at this point in the history
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
(cherry picked from commit 4984130)
  • Loading branch information
mriedem authored and tssurya committed Oct 17, 2018
1 parent 237bfcf commit 57566f4
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
5 changes: 5 additions & 0 deletions nova/objects/compute_node.py
Expand Up @@ -351,6 +351,11 @@ def update_from_virt_driver(self, resources):
"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
Expand Down
14 changes: 14 additions & 0 deletions nova/tests/unit/objects/test_compute_node.py
Expand Up @@ -495,6 +495,20 @@ def test_update_from_virt_driver(self):
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
Expand Down

0 comments on commit 57566f4

Please sign in to comment.