openstack/ironic
Code Issues Proposed changes

Update nova rebuild to account for new image

Browse Source 
Update the Nova driver's rebuild() to account for changes in the instance image
during rebuild.  Currently, rebuild ignores user requests to rebuild with a
new image, making it impossible to do image-based upgrades.  This adds logic
to the patcher to account for the preserve_ephemeral flag and updates rebuild()
to use the patcher to re-patch all driver_info during rebuild.

This also adds missing test coverage of the rebuild code.

Change-Id: I6c8aacd36e0095d0590a1dc552730f2d52036225
Closes-bug: #1334905.
This commit is contained in:
Adam Gandelman 2014-06-27 12:43:28 -07:00 committed by Ghe Rivero
parent 23f4736057
commit ed2eb6370d
4 changed files with 139 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -28,6 +28,7 @@ from ironic.nova.virt.ironic import driver as ironic_driver
from ironic.nova.virt.ironic import ironic_states
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
@ -933,3 +934,96 @@ class IronicDriverTestCase(test.NoDBTestCase):
fake_group = 'fake-security-group-members'
self.driver.refresh_instance_security_rules(fake_group)
mock_risr.assert_called_once_with(fake_group)
@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(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_fg_bid, mock_set_pstate, mock_looping,
mock_wait_active, preserve=False):
node_uuid = uuidutils.generate_uuid()
instance_uuid = uuidutils.generate_uuid()
node = ironic_utils.get_test_node(uuid=node_uuid,
instance_uuid=instance_uuid,
instance_type_id=5)
mock_get.return_value = node
image_meta = ironic_utils.get_test_image_meta()
flavor_id = 5
flavor = {'id': flavor_id, 'name': 'baremetal'}
mock_fg_bid.return_value = flavor
instance = fake_instance.fake_instance_obj(self.ctx,
uuid=instance_uuid,
node=node_uuid,
instance_type_id=flavor_id)
fake_looping_call = FakeLoopingCall()
mock_looping.return_value = fake_looping_call
self.driver.rebuild(
context=self.ctx, instance=instance, image_meta=image_meta,
injected_files=None, admin_password=None, bdms=None,
detach_block_devices=None, attach_block_devices=None,
preserve_ephemeral=preserve)
mock_save.assert_called_once_with(
expected_task_state=[task_states.REBUILDING])
mock_driver_fields.assert_called_once_with(node, instance, image_meta,
flavor, preserve)
mock_set_pstate.assert_called_once_with(node_uuid,
ironic_states.REBUILD)
mock_looping.assert_called_once_with(mock_wait_active,
FAKE_CLIENT_WRAPPER,
instance)
fake_looping_call.start.assert_called_once_with(
interval=CONF.ironic.api_retry_interval)
fake_looping_call.wait.assert_called_once()
def test_rebuild_preserve_ephemeral(self):
self._test_rebuild(preserve=True)
def test_rebuild_no_preserve_ephemeral(self):
self._test_rebuild(preserve=False)
@mock.patch.object(FAKE_CLIENT.node, 'set_provision_state')
@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_fg_bid, mock_set_pstate):
node_uuid = uuidutils.generate_uuid()
instance_uuid = uuidutils.generate_uuid()
node = ironic_utils.get_test_node(uuid=node_uuid,
instance_uuid=instance_uuid,
instance_type_id=5)
mock_get.return_value = node
image_meta = ironic_utils.get_test_image_meta()
flavor_id = 5
flavor = {'id': flavor_id, 'name': 'baremetal'}
mock_fg_bid.return_value = flavor
instance = fake_instance.fake_instance_obj(self.ctx,
uuid=instance_uuid,
node=node_uuid,
instance_type_id=flavor_id)
exceptions = [
exception.NovaException(),
ironic_exception.BadRequest(),
ironic_exception.InternalServerError(),
]
for e in exceptions:
mock_set_pstate.side_effect = e
self.assertRaises(exception.InstanceDeployFailure,
self.driver.rebuild,
context=self.ctx, instance=instance, image_meta=image_meta,
injected_files=None, admin_password=None, bdms=None,
detach_block_devices=None, attach_block_devices=None)

23
ironic/nova/tests/virt/ironic/test_patcher.py
View File

@ -72,6 +72,29 @@ class IronicDriverFieldsTestCase(test.NoDBTestCase):
self.assertIn(expected1, patch)
self.assertIn(expected2, patch)
def test_pxe_get_deploy_patch_preserve_ephemeral(self):
node = ironic_utils.get_test_node(driver='pxe_fake')
instance = fake_instance.fake_instance_obj(
self.ctx, node=node.uuid, ephemeral_gb=10)
for preserve in [True, False]:
patch = patcher.create(node).get_deploy_patch(
instance, self.image_meta, self.flavor,
preserve_ephemeral=preserve)
expected = {'path': '/instance_info/preserve_ephemeral',
'value': str(preserve), 'op': 'add', }
self.assertIn(expected, patch)
def test_pxe_get_deploy_patch_no_preserve_ephemeral(self):
node = ironic_utils.get_test_node(driver='pxe_fake')
instance = fake_instance.fake_instance_obj(
self.ctx, node=node.uuid, ephemeral_gb=10)
patch = patcher.create(node).get_deploy_patch(
instance, self.image_meta, self.flavor)
for preserve in [True, False]:
unexpected = {'path': '/instance_info/preserve_ephemeral',
'value': str(preserve), 'op': 'add', }
self.assertNotIn(unexpected, patch)
def test_pxe_get_deploy_patch_no_flavor_kernel_ramdisk_ids(self):
self.flavor = ironic_utils.get_test_flavor(extra_specs={})
node = ironic_utils.get_test_node(driver='pxe_fake')

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

@ -242,11 +242,13 @@ class IronicDriver(virt_driver.ComputeDriver):
def _stop_firewall(self, instance, network_info):
self.firewall_driver.unfilter_instance(instance, network_info)
def _add_driver_fields(self, node, instance, image_meta, flavor):
def _add_driver_fields(self, node, instance, image_meta, flavor,
preserve_ephemeral=None):
icli = client_wrapper.IronicClientWrapper()
patch = patcher.create(node).get_deploy_patch(instance,
image_meta,
flavor)
flavor,
preserve_ephemeral)
# Associate the node with an instance
patch.append({'path': '/instance_uuid', 'op': 'add',
@ -726,21 +728,14 @@ class IronicDriver(virt_driver.ComputeDriver):
instance.task_state = task_states.REBUILD_SPAWNING
instance.save(expected_task_state=[task_states.REBUILDING])
node_uuid = instance.get('node')
node_uuid = instance['node']
icli = client_wrapper.IronicClientWrapper()
node = icli.call("node.get", node_uuid)
flavor = flavor_obj.Flavor.get_by_id(context,
instance['instance_type_id'])
# Update instance_info for the ephemeral preservation value.
patch = []
patch.append({'path': '/instance_info/preserve_ephemeral',
'op': 'add', 'value': str(preserve_ephemeral)})
try:
icli.call('node.update', node_uuid, patch)
except ironic_exception.BadRequest:
msg = (_("Failed to add deploy parameters on node %(node)s "
"when rebuilding the instance %(instance)s")
% {'node': node_uuid, 'instance': instance['uuid']})
raise exception.InstanceDeployFailure(msg)
self._add_driver_fields(node, instance, image_meta, flavor,
preserve_ephemeral)
# Trigger the node rebuild/redeploy.
try:

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

@ -47,7 +47,8 @@ class GenericDriverFields(object):
def __init__(self, node):
self.node = node
def get_deploy_patch(self, instance, image_meta, flavor):
def get_deploy_patch(self, instance, image_meta, flavor,
preserve_ephemeral=None):
return []
def get_cleanup_patch(self, instance, network_info, flavor):
@ -74,7 +75,8 @@ class PXEDriverFields(GenericDriverFields):
deploy_ids['pxe_deploy_ramdisk'] = deploy_ramdisk
return deploy_ids
def get_deploy_patch(self, instance, image_meta, flavor):
def get_deploy_patch(self, instance, image_meta, flavor,
preserve_ephemeral=None):
"""Build a patch to add the required fields to deploy a node.
Build a json-patch to add the required fields to deploy a node
@ -82,8 +84,10 @@ class PXEDriverFields(GenericDriverFields):
:param instance: the instance object.
:param image_meta: the metadata associated with the instance
image.
image.
:param flavor: the flavor object.
:param preserve_ephemeral: preserve_ephemeral status (bool) to be
specified during rebuild.
:returns: a json-patch with the fields that needs to be updated.
"""
@ -108,6 +112,11 @@ class PXEDriverFields(GenericDriverFields):
patch.append({'path': '/instance_info/ephemeral_format',
'op': 'add',
'value': CONF.default_ephemeral_format})
if preserve_ephemeral is not None:
patch.append({'path': '/instance_info/preserve_ephemeral',
'op': 'add', 'value': str(preserve_ephemeral)})
return patch
def get_cleanup_patch(self, instance, network_info, flavor):