openstack
/
nova
Code Issues Proposed changes

Ironic driver fetches extra_specs when needed 

The Ironic driver needs to access flavor['extra_specs'] in certain
situations. This access was erroneously removed during the large patch
series to land the driver in Nova. The corresponding change did not land
in Ironic, because it fails to pass tempest (as does the current Ironic
driver in Nova).

This patch reverts that change.

Change-Id: I525c611aa0e98076477c2a9a279a0f8a47858aac
Closes-bug: 1367007
This commit is contained in:
Devananda van der Veen 2014-09-08 15:30:40 -07:00 committed by Michael Davies
parent b8a88f1ea0
commit 07f53b002e
2 changed files with 57 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

76
nova/tests/virt/ironic/test_driver.py
View File

@ -23,6 +23,7 @@ from nova.compute import power_state as nova_states
from nova.compute import task_states
from nova import context as nova_context
from nova import exception
from nova.objects import flavor as flavor_obj
from nova.objects import instance as instance_obj
from nova.openstack.common import jsonutils
from nova.openstack.common import loopingcall
@ -486,13 +487,13 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
@mock.patch.object(instance_obj.Instance, 'save')
@mock.patch.object(loopingcall, 'FixedIntervalLoopingCall')
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(instance_obj.Instance, 'get_flavor')
@mock.patch.object(flavor_obj.Flavor, 'get_by_id')
@mock.patch.object(ironic_driver.IronicDriver, '_wait_for_active')
@mock.patch.object(ironic_driver.IronicDriver, '_add_driver_fields')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
def test_spawn(self, mock_sf, mock_pvifs, mock_adf, mock_wait_active,
mock_flavor, mock_node, mock_looping, mock_save):
mock_fg_bid, mock_node, mock_looping, mock_save):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid)
instance = fake_instance.fake_instance_obj(self.ctx, node=node_uuid)
@ -502,7 +503,7 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
mock_node.validate.return_value = ironic_utils.get_test_validation()
mock_node.get_by_instance_uuid.return_value = node
mock_node.set_provision_state.return_value = mock.MagicMock()
mock_flavor.return_value = fake_flavor
mock_fg_bid.return_value = fake_flavor
fake_looping_call = FakeLoopingCall()
mock_looping.return_value = fake_looping_call
@ -511,7 +512,8 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
mock_node.get.assert_called_once_with(node_uuid)
mock_node.validate.assert_called_once_with(node_uuid)
mock_flavor.assert_called_once_with()
mock_fg_bid.assert_called_once_with(self.ctx,
instance['instance_type_id'])
mock_adf.assert_called_once_with(node, instance, None, fake_flavor)
mock_pvifs.assert_called_once_with(node, instance, None)
mock_sf.assert_called_once_with(instance, None)
@ -530,7 +532,7 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
@mock.patch.object(loopingcall, 'FixedIntervalLoopingCall')
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(instance_obj.Instance, 'get_flavor')
@mock.patch.object(flavor_obj.Flavor, 'get_by_id')
@mock.patch.object(ironic_driver.IronicDriver, 'destroy')
@mock.patch.object(ironic_driver.IronicDriver, '_wait_for_active')
@mock.patch.object(ironic_driver.IronicDriver, '_add_driver_fields')
@ -538,7 +540,7 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
def test_spawn_destroyed_after_failure(self, mock_sf, mock_pvifs, mock_adf,
mock_wait_active, mock_destroy,
mock_flavor, mock_node,
mock_fg_bid, mock_node,
mock_looping):
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
node = ironic_utils.get_test_node(driver='fake', uuid=node_uuid)
@ -549,7 +551,7 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
mock_node.validate.return_value = ironic_utils.get_test_validation()
mock_node.get_by_instance_uuid.return_value = node
mock_node.set_provision_state.return_value = mock.MagicMock()
mock_flavor.return_value = fake_flavor
mock_fg_bid.return_value = fake_flavor
fake_looping_call = FakeLoopingCall()
mock_looping.return_value = fake_looping_call
@ -591,7 +593,7 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
self.driver._add_driver_fields,
node, instance, image_meta, flavor)
@mock.patch.object(instance_obj.Instance, 'get_flavor')
@mock.patch.object(flavor_obj.Flavor, 'get_by_id')
@mock.patch.object(FAKE_CLIENT.node, 'update')
def test__cleanup_deploy_good(self, mock_update, mock_flavor):
mock_flavor.return_value = ironic_utils.get_test_flavor(extra_specs={})
@ -599,11 +601,11 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
instance_uuid='fake-id')
instance = fake_instance.fake_instance_obj(self.ctx,
node=node.uuid)
self.driver._cleanup_deploy(node, instance, None)
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(instance_obj.Instance, 'get_flavor')
@mock.patch.object(flavor_obj.Flavor, 'get_by_id')
@mock.patch.object(FAKE_CLIENT.node, 'update')
def test__cleanup_deploy_fail(self, mock_update, mock_flavor):
mock_flavor.return_value = ironic_utils.get_test_flavor(extra_specs={})
@ -614,10 +616,10 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
node=node.uuid)
self.assertRaises(exception.InstanceTerminationFailure,
self.driver._cleanup_deploy,
node, instance, None)
self.ctx, node, instance, None)
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(instance_obj.Instance, 'get_flavor')
@mock.patch.object(flavor_obj.Flavor, 'get_by_id')
def test_spawn_node_driver_validation_fail(self, mock_flavor, mock_node):
mock_flavor.return_value = ironic_utils.get_test_flavor()
node_uuid = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee'
@ -633,10 +635,10 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
self.ctx, instance, image_meta, [], None)
mock_node.get.assert_called_once_with(node_uuid)
mock_node.validate.assert_called_once_with(node_uuid)
mock_flavor.assert_called_with()
mock_flavor.assert_called_with(mock.ANY, instance['instance_type_id'])
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(instance_obj.Instance, 'get_flavor')
@mock.patch.object(flavor_obj.Flavor, 'get_by_id')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_cleanup_deploy')
@ -660,11 +662,12 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
mock_node.get.assert_called_once_with(node_uuid)
mock_node.validate.assert_called_once_with(node_uuid)
mock_flavor.assert_called_once_with()
mock_cleanup_deploy.assert_called_with(node, instance, None)
mock_flavor.assert_called_once_with(self.ctx,
instance['instance_type_id'])
mock_cleanup_deploy.assert_called_with(self.ctx, node, instance, None)
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(instance_obj.Instance, 'get_flavor')
@mock.patch.object(flavor_obj.Flavor, 'get_by_id')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_cleanup_deploy')
@ -686,11 +689,13 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
mock_node.get.assert_called_once_with(node_uuid)
mock_node.validate.assert_called_once_with(node_uuid)
mock_flavor.assert_called_once_with()
mock_cleanup_deploy.assert_called_once_with(node, instance, None)
mock_flavor.assert_called_once_with(self.ctx,
instance['instance_type_id'])
mock_cleanup_deploy.assert_called_once_with(self.ctx, node,
instance, None)
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(instance_obj.Instance, 'get_flavor')
@mock.patch.object(flavor_obj.Flavor, 'get_by_id')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_cleanup_deploy')
@ -712,12 +717,14 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
mock_node.get.assert_called_once_with(node_uuid)
mock_node.validate.assert_called_once_with(node_uuid)
mock_flavor.assert_called_once_with()
mock_cleanup_deploy.assert_called_once_with(node, instance, None)
mock_flavor.assert_called_once_with(self.ctx,
instance['instance_type_id'])
mock_cleanup_deploy.assert_called_once_with(self.ctx, node,
instance, None)
@mock.patch.object(loopingcall, 'FixedIntervalLoopingCall')
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(instance_obj.Instance, 'get_flavor')
@mock.patch.object(flavor_obj.Flavor, 'get_by_id')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, 'destroy')
@ -749,7 +756,7 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
@mock.patch.object(loopingcall, 'FixedIntervalLoopingCall')
@mock.patch.object(instance_obj.Instance, 'save')
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(instance_obj.Instance, 'get_flavor')
@mock.patch.object(flavor_obj.Flavor, 'get_by_id')
@mock.patch.object(ironic_driver.IronicDriver, '_wait_for_active')
@mock.patch.object(ironic_driver.IronicDriver, '_plug_vifs')
@mock.patch.object(ironic_driver.IronicDriver, '_start_firewall')
@ -766,7 +773,8 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
image_meta = ironic_utils.get_test_image_meta()
self.driver.spawn(self.ctx, instance, image_meta, [], None)
mock_flavor.assert_called_once_with()
mock_flavor.assert_called_once_with(self.ctx,
instance['instance_type_id'])
self.assertTrue(mock_save.called)
self.assertEqual('/dev/sda1', instance['default_ephemeral_device'])
@ -789,7 +797,8 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
mock_node.set_provision_state.assert_called_once_with(node_uuid,
'deleted')
mock_node.get_by_instance_uuid.assert_called_with(instance.uuid)
mock_cleanup_deploy.assert_called_with(node, instance, network_info)
mock_cleanup_deploy.assert_called_with(self.ctx, node,
instance, network_info)
@mock.patch.object(FAKE_CLIENT, 'node')
@mock.patch.object(ironic_driver.IronicDriver, '_cleanup_deploy')
@ -806,7 +815,8 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
self.driver.destroy(self.ctx, instance, network_info, None)
self.assertFalse(mock_node.set_provision_state.called)
mock_node.get_by_instance_uuid.assert_called_with(instance.uuid)
mock_cleanup_deploy.assert_called_with(node, instance, network_info)
mock_cleanup_deploy.assert_called_with(self.ctx, node, instance,
network_info)
@mock.patch.object(FAKE_CLIENT.node, 'set_provision_state')
@mock.patch.object(ironic_driver, '_validate_instance_and_node')
@ -1062,12 +1072,12 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
@mock.patch.object(ironic_driver.IronicDriver, '_wait_for_active')
@mock.patch.object(loopingcall, 'FixedIntervalLoopingCall')
@mock.patch.object(FAKE_CLIENT.node, 'set_provision_state')
@mock.patch.object(instance_obj.Instance, 'get_flavor')
@mock.patch.object(flavor_obj.Flavor, 'get_by_id')
@mock.patch.object(ironic_driver.IronicDriver, '_add_driver_fields')
@mock.patch.object(FAKE_CLIENT.node, 'get')
@mock.patch.object(instance_obj.Instance, 'save')
def _test_rebuild(self, mock_save, mock_get, mock_driver_fields,
mock_flavor, mock_set_pstate, mock_looping,
mock_fg_bid, mock_set_pstate, mock_looping,
mock_wait_active, preserve=False):
node_uuid = uuidutils.generate_uuid()
instance_uuid = uuidutils.generate_uuid()
@ -1079,7 +1089,7 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
image_meta = ironic_utils.get_test_image_meta()
flavor_id = 5
flavor = {'id': flavor_id, 'name': 'baremetal'}
mock_flavor.return_value = flavor
mock_fg_bid.return_value = flavor
instance = fake_instance.fake_instance_obj(self.ctx,
uuid=instance_uuid,
@ -1115,12 +1125,12 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
self._test_rebuild(preserve=False)
@mock.patch.object(FAKE_CLIENT.node, 'set_provision_state')
@mock.patch.object(instance_obj.Instance, 'get_flavor')
@mock.patch.object(flavor_obj.Flavor, 'get_by_id')
@mock.patch.object(ironic_driver.IronicDriver, '_add_driver_fields')
@mock.patch.object(FAKE_CLIENT.node, 'get')
@mock.patch.object(instance_obj.Instance, 'save')
def test_rebuild_failures(self, mock_save, mock_get, mock_driver_fields,
mock_flavor, mock_set_pstate):
mock_fg_bid, mock_set_pstate):
node_uuid = uuidutils.generate_uuid()
instance_uuid = uuidutils.generate_uuid()
node = ironic_utils.get_test_node(uuid=node_uuid,
@ -1131,7 +1141,7 @@ class IronicDriverTestCase(test.NoDBTestCase, test_driver.DriverAPITestHelper):
image_meta = ironic_utils.get_test_image_meta()
flavor_id = 5
flavor = {'id': flavor_id, 'name': 'baremetal'}
mock_flavor.return_value = flavor
mock_fg_bid.return_value = flavor
instance = fake_instance.fake_instance_obj(self.ctx,
uuid=instance_uuid,

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

@ -34,6 +34,7 @@ from nova import exception
from nova.i18n import _
from nova.i18n import _LE
from nova.i18n import _LW
from nova.objects import flavor as flavor_obj
from nova.objects import instance as instance_obj
from nova.openstack.common import excutils
from nova.openstack.common import importutils
@ -288,9 +289,12 @@ class IronicDriver(virt_driver.ComputeDriver):
LOG.error(msg)
raise exception.InstanceDeployFailure(msg)
def _cleanup_deploy(self, node, instance, network_info):
def _cleanup_deploy(self, context, node, instance, network_info):
icli = client_wrapper.IronicClientWrapper()
flavor = instance.get_flavor()
# TODO(mrda): It would be better to use instance.get_flavor() here
# but right now that doesn't include extra_specs which are required
flavor = flavor_obj.Flavor.get_by_id(context,
instance['instance_type_id'])
patch = patcher.create(node).get_cleanup_patch(instance, network_info,
flavor)
@ -579,7 +583,8 @@ class IronicDriver(virt_driver.ComputeDriver):
icli = client_wrapper.IronicClientWrapper()
node = icli.call("node.get", node_uuid)
flavor = instance.get_flavor()
flavor = flavor_obj.Flavor.get_by_id(context,
instance['instance_type_id'])
self._add_driver_fields(node, instance, image_meta, flavor)
@ -594,7 +599,7 @@ class IronicDriver(virt_driver.ComputeDriver):
validate_chk = icli.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(node, instance, network_info)
self._cleanup_deploy(context, node, instance, network_info)
raise exception.ValidationError(_(
"Ironic node: %(id)s failed to validate."
" (deploy: %(deploy)s, power: %(power)s)")
@ -612,7 +617,7 @@ class IronicDriver(virt_driver.ComputeDriver):
"%(instance)s on baremetal node %(node)s."),
{'instance': instance['uuid'],
'node': node_uuid})
self._cleanup_deploy(node, instance, network_info)
self._cleanup_deploy(context, node, instance, network_info)
# trigger the node deploy
try:
@ -625,7 +630,7 @@ class IronicDriver(virt_driver.ComputeDriver):
{'inst': instance['uuid'],
'reason': six.text_type(e)})
LOG.error(msg)
self._cleanup_deploy(node, instance, network_info)
self._cleanup_deploy(context, node, instance, network_info)
timer = loopingcall.FixedIntervalLoopingCall(self._wait_for_active,
icli, instance)
@ -712,7 +717,7 @@ class IronicDriver(virt_driver.ComputeDriver):
ironic_states.DEPLOYWAIT):
self._unprovision(icli, instance, node)
self._cleanup_deploy(node, instance, network_info)
self._cleanup_deploy(context, node, instance, network_info)
def reboot(self, context, instance, network_info, reboot_type,
block_device_info=None, bad_volumes_callback=None):
@ -960,7 +965,8 @@ class IronicDriver(virt_driver.ComputeDriver):
node_uuid = instance.node
icli = client_wrapper.IronicClientWrapper()
node = icli.call("node.get", node_uuid)
flavor = instance.get_flavor()
flavor = flavor_obj.Flavor.get_by_id(context,
instance['instance_type_id'])
self._add_driver_fields(node, instance, image_meta, flavor,
preserve_ephemeral)