openstack
/
nova
Code Issues Proposed changes

Ironic: remove backwards compatibility code 

This patch removes backwards compatibility with old versions
of Ironic (Kilo and below) that won't clean up the instance
association during tear down. Starting with Liberty release
Ironic does cleaning of "instance_uuid" in tear down.
Also remove unneeded "context" parameter from _cleanup_deploy().

Change-Id: Ic0af1296d84edb9507df043c969a6dd6e91e4e9c
This commit is contained in:
Yuriy Zveryanskyy 2016-02-01 14:47:20 +02:00
parent 38c3f883db
commit 52061f59af
4 changed files with 9 additions and 116 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

70
nova/tests/unit/virt/ironic/test_driver.py
View File

@ -951,62 +951,6 @@ class IronicDriverTestCase(test.NoDBTestCase):
self.driver._add_driver_fields,
node, instance, image_meta, flavor)
@mock.patch.object(FAKE_CLIENT.node, 'update')
def test__cleanup_deploy_good_with_flavor(self, mock_update):
node = ironic_utils.get_test_node(driver='fake',
instance_uuid=self.instance_uuid)
instance = fake_instance.fake_instance_obj(self.ctx,
node=node.uuid)
flavor = ironic_utils.get_test_flavor(extra_specs={})
self.driver._cleanup_deploy(self.ctx, node, instance, None,
flavor=flavor)
expected_patch = [{'path': '/instance_uuid', 'op': 'remove'}]
mock_update.assert_called_once_with(node.uuid, expected_patch)
@mock.patch.object(ironic_driver.IronicDriver,
'_validate_instance_and_node')
@mock.patch.object(FAKE_CLIENT.node, 'update')
def test__cleanup_deploy_instance_already_removed(self, mock_update,
mock_validate):
mock_validate.side_effect = exception.InstanceNotFound(
instance_id='fake-instance')
node = ironic_utils.get_test_node(driver='fake',
instance_uuid=self.instance_uuid)
instance = fake_instance.fake_instance_obj(self.ctx,
node=node.uuid)
flavor = ironic_utils.get_test_flavor(extra_specs={})
self.driver._cleanup_deploy(self.ctx, node, instance, None,
flavor=flavor)
# assert node.update is not called
self.assertFalse(mock_update.called)
mock_validate.assert_called_once_with(instance)
@mock.patch.object(FAKE_CLIENT.node, 'update')
def test__cleanup_deploy_without_flavor(self, mock_update):
node = ironic_utils.get_test_node(driver='fake',
instance_uuid=self.instance_uuid)
flavor = ironic_utils.get_test_flavor(extra_specs={})
instance = fake_instance.fake_instance_obj(self.ctx,
node=node.uuid)
instance.flavor = flavor
self.driver._cleanup_deploy(self.ctx, node, instance, None)
expected_patch = [{'path': '/instance_uuid', 'op': 'remove'}]
mock_update.assert_called_once_with(node.uuid, expected_patch)
@mock.patch.object(FAKE_CLIENT.node, 'update')
def test__cleanup_deploy_fail(self, mock_update):
mock_update.side_effect = ironic_exception.BadRequest()
node = ironic_utils.get_test_node(driver='fake',
instance_uuid=self.instance_uuid)
flavor = ironic_utils.get_test_flavor(extra_specs={})
instance = fake_instance.fake_instance_obj(self.ctx,
node=node.uuid)
instance.flavor = flavor
self.assertRaises(exception.InstanceTerminationFailure,
self.driver._cleanup_deploy,
self.ctx, node, instance, None)
@mock.patch.object(configdrive, 'required_by')
@mock.patch.object(FAKE_CLIENT, 'node')
def test_spawn_node_driver_validation_fail(self, mock_node,
@ -1057,8 +1001,7 @@ class IronicDriverTestCase(test.NoDBTestCase):
mock_node.get.assert_called_once_with(
node_uuid, fields=ironic_driver._NODE_FIELDS)
mock_node.validate.assert_called_once_with(node_uuid)
mock_cleanup_deploy.assert_called_with(self.ctx, node, instance, None,
flavor=flavor)
mock_cleanup_deploy.assert_called_with(node, instance, None)
@mock.patch.object(configdrive, 'required_by')
@mock.patch.object(objects.Instance, 'save')
@ -1120,9 +1063,7 @@ class IronicDriverTestCase(test.NoDBTestCase):
mock_node.get.assert_called_once_with(
node_uuid, fields=ironic_driver._NODE_FIELDS)
mock_node.validate.assert_called_once_with(node_uuid)
mock_cleanup_deploy.assert_called_once_with(self.ctx, node,
instance, None,
flavor=flavor)
mock_cleanup_deploy.assert_called_once_with(node, instance, None)
@mock.patch.object(configdrive, 'required_by')
@mock.patch.object(FAKE_CLIENT, 'node')
@ -1150,9 +1091,7 @@ class IronicDriverTestCase(test.NoDBTestCase):
mock_node.get.assert_called_once_with(
node_uuid, fields=ironic_driver._NODE_FIELDS)
mock_node.validate.assert_called_once_with(node_uuid)
mock_cleanup_deploy.assert_called_once_with(self.ctx, node,
instance, None,
flavor=flavor)
mock_cleanup_deploy.assert_called_once_with(node, instance, None)
@mock.patch.object(configdrive, 'required_by')
@mock.patch.object(loopingcall, 'FixedIntervalLoopingCall')
@ -1229,8 +1168,7 @@ class IronicDriverTestCase(test.NoDBTestCase):
mock_node.get_by_instance_uuid.assert_called_with(
instance.uuid, fields=ironic_driver._NODE_FIELDS)
mock_cleanup_deploy.assert_called_with(self.ctx, node,
instance, network_info)
mock_cleanup_deploy.assert_called_with(node, instance, network_info)
# For states that makes sense check if set_provision_state has
# been called

7
nova/tests/unit/virt/ironic/test_patcher.py
View File

@ -57,7 +57,6 @@ class IronicDriverFieldsTestCase(test.NoDBTestCase):
'value': str(self.node.properties.get('local_gb', 0)),
'op': 'add'}
]
self._expected_cleanup_patch = []
def test_create_generic(self):
node = ironic_utils.get_test_node(driver='pxe_fake')
@ -129,9 +128,3 @@ class IronicDriverFieldsTestCase(test.NoDBTestCase):
'value': str(preserve), 'op': 'add', }]
expected += self._expected_deploy_patch
self.assertEqual(sorted(expected), sorted(patch))
def test_generic_get_cleanup_patch(self):
node = ironic_utils.get_test_node(driver='fake')
patch = patcher.create(node).get_cleanup_patch(self.instance, None,
self.flavor)
self.assertEqual(self._expected_cleanup_patch, patch)

37
nova/virt/ironic/driver.py
View File

@ -371,31 +371,7 @@ class IronicDriver(virt_driver.ComputeDriver):
LOG.error(msg)
raise exception.InstanceDeployFailure(msg)
def _cleanup_deploy(self, context, node, instance, network_info,
flavor=None):
if flavor is None:
flavor = instance.flavor
patch = patcher.create(node).get_cleanup_patch(instance, network_info,
flavor)
# TODO(lucasagomes): This code is here for backwards compatibility
# with old versions of Ironic that won't clean up the instance
# association as part of the node's tear down. Should be removed
# on the next cycle (M).
patch.append({'op': 'remove', 'path': '/instance_uuid'})
try:
self._validate_instance_and_node(instance)
self.ironicclient.call('node.update', node.uuid, patch)
except exception.InstanceNotFound:
LOG.debug("Instance already removed from Ironic node %s. Skip "
"updating it", node.uuid, instance=instance)
except ironic.exc.BadRequest:
LOG.error(_LE("Failed to clean up the parameters on node %(node)s "
"when unprovisioning the instance %(instance)s"),
{'node': node.uuid, 'instance': instance.uuid})
reason = (_("Fail to clean up node %s parameters") % node.uuid)
raise exception.InstanceTerminationFailure(reason=reason)
def _cleanup_deploy(self, node, instance, network_info):
self._unplug_vifs(node, instance, network_info)
self._stop_firewall(instance, network_info)
@ -732,8 +708,7 @@ class IronicDriver(virt_driver.ComputeDriver):
validate_chk = self.ironicclient.call("node.validate", node_uuid)
if not validate_chk.deploy or not validate_chk.power:
# something is wrong. undo what we have done
self._cleanup_deploy(context, node, instance, network_info,
flavor=flavor)
self._cleanup_deploy(node, instance, network_info)
raise exception.ValidationError(_(
"Ironic node: %(id)s failed to validate."
" (deploy: %(deploy)s, power: %(power)s)")
@ -751,8 +726,7 @@ class IronicDriver(virt_driver.ComputeDriver):
"%(instance)s on baremetal node %(node)s."),
{'instance': instance.uuid,
'node': node_uuid})
self._cleanup_deploy(context, node, instance, network_info,
flavor=flavor)
self._cleanup_deploy(node, instance, network_info)
# Config drive
configdrive_value = None
@ -789,8 +763,7 @@ class IronicDriver(virt_driver.ComputeDriver):
{'inst': instance.uuid,
'reason': six.text_type(e)})
LOG.error(msg)
self._cleanup_deploy(context, node, instance, network_info,
flavor=flavor)
self._cleanup_deploy(node, instance, network_info)
timer = loopingcall.FixedIntervalLoopingCall(self._wait_for_active,
instance)
@ -890,7 +863,7 @@ class IronicDriver(virt_driver.ComputeDriver):
if node.provision_state in _UNPROVISION_STATES:
self._unprovision(instance, node)
self._cleanup_deploy(context, node, instance, network_info)
self._cleanup_deploy(node, instance, network_info)
LOG.info(_LI('Successfully unprovisioned Ironic node %s'),
node.uuid, instance=instance)

11
nova/virt/ironic/patcher.py
View File

@ -106,14 +106,3 @@ class GenericDriverFields(object):
patch.append({'path': '/instance_info/capabilities',
'op': 'add', 'value': jsonutils.dumps(capabilities)})
return patch
def get_cleanup_patch(self, instance, network_info, flavor):
"""Build a patch to clean up the fields.
:param instance: the instance object.
:param network_info: the instance network information.
:param flavor: the flavor object.
:returns: a json-patch with the fields that needs to be updated.
"""
return []