Update instance node on rebuild only when it is recreate
When using ironic virt driver, if scheduled_node is not specified in rebuild_instance compute manager method (as it happens in case of instance rebuild), the first ironic node is selected: computes = ComputeNodeList.get_all_by_host(context, host, use_slave) return computes[0] After the first rebuild, instance.node is updated to be this first ironic node, which causes subsequent rebuilds to fail, as virt driver tries to set instance_uuid on a newly selected ironic node and fails. Closes-bug: #1564921 Change-Id: I2fe6e439135ba6aa4120735d030ced31081ef202
This commit is contained in:
parent
414df1e56e
commit
54b122caec
|
@ -2657,12 +2657,15 @@ class ComputeManager(manager.Manager):
|
||||||
# the instance's host and node properties to reflect it's
|
# the instance's host and node properties to reflect it's
|
||||||
# destination node for the recreate.
|
# destination node for the recreate.
|
||||||
if not scheduled_node:
|
if not scheduled_node:
|
||||||
try:
|
if recreate:
|
||||||
compute_node = self._get_compute_info(context, self.host)
|
try:
|
||||||
scheduled_node = compute_node.hypervisor_hostname
|
compute_node = self._get_compute_info(context, self.host)
|
||||||
except exception.ComputeHostNotFound:
|
scheduled_node = compute_node.hypervisor_hostname
|
||||||
LOG.exception(_LE('Failed to get compute_info for %s'),
|
except exception.ComputeHostNotFound:
|
||||||
self.host)
|
LOG.exception(_LE('Failed to get compute_info for %s'),
|
||||||
|
self.host)
|
||||||
|
else:
|
||||||
|
scheduled_node = instance.node
|
||||||
|
|
||||||
with self._error_out_instance_on_exception(context, instance):
|
with self._error_out_instance_on_exception(context, instance):
|
||||||
try:
|
try:
|
||||||
|
|
|
@ -2936,16 +2936,50 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
|
||||||
'rebuild.error', fault=ex)
|
'rebuild.error', fault=ex)
|
||||||
|
|
||||||
def test_rebuild_deleting(self):
|
def test_rebuild_deleting(self):
|
||||||
instance = objects.Instance(uuid=uuids.instance)
|
instance = fake_instance.fake_instance_obj(self.context)
|
||||||
ex = exception.UnexpectedDeletingTaskStateError(
|
ex = exception.UnexpectedDeletingTaskStateError(
|
||||||
instance_uuid=instance.uuid, expected='expected', actual='actual')
|
instance_uuid=instance.uuid, expected='expected', actual='actual')
|
||||||
self._test_rebuild_ex(instance, ex)
|
self._test_rebuild_ex(instance, ex)
|
||||||
|
|
||||||
def test_rebuild_notfound(self):
|
def test_rebuild_notfound(self):
|
||||||
instance = objects.Instance(uuid=uuids.instance)
|
instance = fake_instance.fake_instance_obj(self.context)
|
||||||
ex = exception.InstanceNotFound(instance_id=instance.uuid)
|
ex = exception.InstanceNotFound(instance_id=instance.uuid)
|
||||||
self._test_rebuild_ex(instance, ex)
|
self._test_rebuild_ex(instance, ex)
|
||||||
|
|
||||||
|
def test_rebuild_node_not_updated_if_not_recreate(self):
|
||||||
|
node = uuidutils.generate_uuid() # ironic node uuid
|
||||||
|
instance = fake_instance.fake_instance_obj(self.context, node=node)
|
||||||
|
instance.migration_context = None
|
||||||
|
with test.nested(
|
||||||
|
mock.patch.object(self.compute, '_get_compute_info'),
|
||||||
|
mock.patch.object(self.compute, '_do_rebuild_instance_with_claim'),
|
||||||
|
mock.patch.object(objects.Instance, 'save'),
|
||||||
|
mock.patch.object(self.compute, '_set_migration_status'),
|
||||||
|
) as (mock_get, mock_rebuild, mock_save, mock_set):
|
||||||
|
self.compute.rebuild_instance(self.context, instance, None, None,
|
||||||
|
None, None, None, None, False)
|
||||||
|
self.assertFalse(mock_get.called)
|
||||||
|
self.assertEqual(node, instance.node)
|
||||||
|
mock_set.assert_called_once_with(None, 'done')
|
||||||
|
|
||||||
|
def test_rebuild_node_updated_if_recreate(self):
|
||||||
|
dead_node = uuidutils.generate_uuid()
|
||||||
|
instance = fake_instance.fake_instance_obj(self.context,
|
||||||
|
node=dead_node)
|
||||||
|
instance.migration_context = None
|
||||||
|
with test.nested(
|
||||||
|
mock.patch.object(self.compute, '_get_compute_info'),
|
||||||
|
mock.patch.object(self.compute, '_do_rebuild_instance_with_claim'),
|
||||||
|
mock.patch.object(objects.Instance, 'save'),
|
||||||
|
mock.patch.object(self.compute, '_set_migration_status'),
|
||||||
|
) as (mock_get, mock_rebuild, mock_save, mock_set):
|
||||||
|
mock_get.return_value.hypervisor_hostname = 'new-node'
|
||||||
|
self.compute.rebuild_instance(self.context, instance, None, None,
|
||||||
|
None, None, None, None, True)
|
||||||
|
mock_get.assert_called_once_with(mock.ANY, self.compute.host)
|
||||||
|
self.assertEqual('new-node', instance.node)
|
||||||
|
mock_set.assert_called_once_with(None, 'done')
|
||||||
|
|
||||||
def test_rebuild_default_impl(self):
|
def test_rebuild_default_impl(self):
|
||||||
def _detach(context, bdms):
|
def _detach(context, bdms):
|
||||||
# NOTE(rpodolyaka): check that instance has been powered off by
|
# NOTE(rpodolyaka): check that instance has been powered off by
|
||||||
|
|
Loading…
Reference in New Issue