openstack
/
ironic
Code Issues Proposed changes

Logic for skipping deployment with BFV 

In order to boot from volume, the deploy driver needs to
know when not to actually deploy. This change wires in the
checks to skip deployment activities if it appears that we
have a valid root volume target defined.

Co-Authored-By: Hironori Shiina <shiina.hironori@jp.fujitsu.com>
Partial-Bug: #1559691
Change-Id: I62e622a2b053f685c2da42ca5106bdb9bdd22dc6
This commit is contained in:
Julia Kreger 2017-04-06 15:48:54 +00:00
parent 6883674b3c
commit 152a3654ce
8 changed files with 265 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
ironic/drivers/modules/agent.py
View File

@ -330,10 +330,17 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
task.driver.boot.validate(task)
node = task.node
# Validate node capabilities
deploy_utils.validate_capabilities(node)
if not task.driver.storage.should_write_image(task):
# NOTE(TheJulia): There is no reason to validate
# image properties if we will not be writing an image
# in a boot from volume case. As such, return to the caller.
return
params = {}
# TODO(jtaryma): Skip validation of image_source if
# task.driver.storage.should_write_image()
# returns False.
image_source = node.instance_info.get('image_source')
params['instance_info.image_source'] = image_source
error_msg = _('Node %s failed to validate deploy image info. Some '
@ -358,9 +365,6 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
'%(node)s. Error: %(error)s') % {'node': node.uuid,
'error': e})
# Validate node capabilities
deploy_utils.validate_capabilities(node)
validate_image_proxies(node)
@METRICS.timer('AgentDeploy.deploy')
@ -376,8 +380,21 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
:param task: a TaskManager instance.
:returns: status of the deploy. One of ironic.common.states.
"""
manager_utils.node_power_action(task, states.REBOOT)
return states.DEPLOYWAIT
if task.driver.storage.should_write_image(task):
manager_utils.node_power_action(task, states.REBOOT)
return states.DEPLOYWAIT
else:
# TODO(TheJulia): At some point, we should de-dupe this code
# as it is nearly identical to the iscsi deploy interface.
# This is not being done now as it is expected to be
# refactored in the near future.
manager_utils.node_power_action(task, states.POWER_OFF)
task.driver.network.remove_provisioning_network(task)
task.driver.network.configure_tenant_networks(task)
task.driver.boot.prepare_instance(task)
manager_utils.node_power_action(task, states.POWER_ON)
LOG.info('Deployment to node %s done', task.node.uuid)
return states.DEPLOYDONE
@METRICS.timer('AgentDeploy.tear_down')
@task_manager.require_exclusive_lock
@ -425,14 +442,20 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
# Adding the node to provisioning network so that the dhcp
# options get added for the provisioning port.
manager_utils.node_power_action(task, states.POWER_OFF)
# NOTE(vdrok): in case of rebuild, we have tenant network already
# configured, unbind tenant ports if present
task.driver.network.unconfigure_tenant_networks(task)
task.driver.network.add_provisioning_network(task)
if task.driver.storage.should_write_image(task):
# NOTE(vdrok): in case of rebuild, we have tenant network
# already configured, unbind tenant ports if present
task.driver.network.unconfigure_tenant_networks(task)
task.driver.network.add_provisioning_network(task)
# Signal to storage driver to attach volumes
task.driver.storage.attach_volumes(task)
if node.provision_state == states.ACTIVE:
# Call is due to conductor takeover
task.driver.boot.prepare_instance(task)
elif not task.driver.storage.should_write_image(task):
# We have nothing else to do as this is handled in the
# backend storage system.
return
elif node.provision_state != states.ADOPTING:
node.instance_info = deploy_utils.build_instance_info_for_deploy(
task)

27
ironic/drivers/modules/iscsi_deploy.py
View File

@ -424,6 +424,11 @@ class ISCSIDeploy(AgentDeployMixin, base.DeployInterface):
# Check the boot_mode, boot_option and disk_label capabilities values.
deploy_utils.validate_capabilities(node)
# Edit early if we are not writing a volume as the validate
# tasks evaluate root device hints.
if not task.driver.storage.should_write_image(task):
return
# TODO(rameshg87): iscsi_ilo driver uses this method. Remove
# and copy-paste it's contents here once iscsi_ilo deploy driver
# broken down into separate boot and deploy implementations.
@ -443,12 +448,24 @@ class ISCSIDeploy(AgentDeployMixin, base.DeployInterface):
:returns: deploy state DEPLOYWAIT.
"""
node = task.node
cache_instance_image(task.context, node)
check_image_size(task)
if task.driver.storage.should_write_image(task):
cache_instance_image(task.context, node)
check_image_size(task)
manager_utils.node_power_action(task, states.REBOOT)
manager_utils.node_power_action(task, states.REBOOT)
return states.DEPLOYWAIT
return states.DEPLOYWAIT
else:
# TODO(TheJulia): At some point, we should de-dupe this code
# as it is nearly identical to the agent deploy interface.
# This is not being done now as it is expected to be
# refactored in the near future.
manager_utils.node_power_action(task, states.POWER_OFF)
task.driver.network.remove_provisioning_network(task)
task.driver.network.configure_tenant_networks(task)
manager_utils.node_power_action(task, states.POWER_ON)
task.process_event('done')
LOG.info('Deployment to node %s done', node.uuid)
return states.DEPLOYDONE
@METRICS.timer('ISCSIDeploy.tear_down')
@task_manager.require_exclusive_lock

43
ironic/drivers/modules/pxe.py
View File

@ -419,6 +419,11 @@ class PXEBoot(base.BootInterface):
validate_boot_parameters_for_trusted_boot(node)
_parse_driver_info(node)
# NOTE(TheJulia): If we're not writing an image, we can skip
# the remainder of this method.
if not task.driver.storage.should_write_image(task):
return
d_info = deploy_utils.get_image_instance_info(node)
if (node.driver_internal_info.get('is_whole_disk_image') or
deploy_utils.get_boot_option(node) == 'local'):
@ -530,12 +535,34 @@ class PXEBoot(base.BootInterface):
boot_option = deploy_utils.get_boot_option(node)
boot_device = None
if boot_option != "local":
# Make sure that the instance kernel/ramdisk is cached.
# This is for the takeover scenario for active nodes.
instance_image_info = _get_instance_image_info(
task.node, task.context)
_cache_ramdisk_kernel(task.context, task.node, instance_image_info)
if deploy_utils.is_iscsi_boot(task):
dhcp_opts = pxe_utils.dhcp_options_for_instance(task)
provider = dhcp_factory.DHCPFactory()
provider.update_dhcp(task, dhcp_opts)
# configure iPXE for iscsi boot
pxe_config_path = pxe_utils.get_pxe_config_file_path(
task.node.uuid)
if not os.path.isfile(pxe_config_path):
pxe_options = _build_pxe_config_options(task, {})
pxe_config_template = (
deploy_utils.get_pxe_config_template(node))
pxe_utils.create_pxe_config(
task, pxe_options, pxe_config_template)
deploy_utils.switch_pxe_config(
pxe_config_path, None,
deploy_utils.get_boot_mode_for_deploy(node), False,
iscsi_boot=True)
boot_device = boot_devices.PXE
elif boot_option != "local":
if task.driver.storage.should_write_image(task):
# Make sure that the instance kernel/ramdisk is cached.
# This is for the takeover scenario for active nodes.
instance_image_info = _get_instance_image_info(
task.node, task.context)
_cache_ramdisk_kernel(task.context, task.node,
instance_image_info)
# If it's going to PXE boot we need to update the DHCP server
dhcp_opts = pxe_utils.dhcp_options_for_instance(task)
@ -548,7 +575,9 @@ class PXEBoot(base.BootInterface):
'root_uuid_or_disk_id'
]
except KeyError:
if not iwdi:
if not task.driver.storage.should_write_image(task):
pass
elif not iwdi:
LOG.warning("The UUID for the root partition can't be "
"found, unable to switch the pxe config from "
"deployment mode to service (boot) mode for "

15
ironic/drivers/modules/storage/cinder.py
View File

@ -352,15 +352,20 @@ class CinderStorage(base.StorageInterface):
def should_write_image(self, task):
"""Determines if deploy should perform the image write-out.
NOTE: This method will be written as part of the changes to the
boot logic, and for now should always return false to indicate
that the deployment image write-out process should be skipped.
:param task: The task object.
:returns: True if the deployment write-out process should be
executed.
"""
return False
# NOTE(TheJulia): There is no reason to check if a root volume
# exists here because if the validation has already been passed
# then we know that there should be a volume. If there is an
# image_source, then we should expect to write it out.
instance_info = task.node.instance_info
if 'image_source' not in instance_info:
for volume in task.volume_targets:
if volume['boot_index'] == 0:
return False
return True
def _generate_connector(self, task):
"""Generate cinder connector value based upon the node.

16
ironic/tests/unit/drivers/modules/storage/test_cinder.py
View File

@ -590,3 +590,19 @@ class CinderInterfaceTestCase(db_base.DbTestCase):
self.assertEqual(1, mock_log.error.call_count)
# CONF.cinder.action_retries + 1, number of retries is set to 3.
self.assertEqual(4, mock_detach.call_count)
def test_should_write_image(self):
self.node = object_utils.create_test_node(self.context,
storage_interface='cinder')
object_utils.create_test_volume_target(
self.context, node_id=self.node.id, volume_type='iscsi',
boot_index=0, volume_id='1234')
with task_manager.acquire(self.context, self.node.id) as task:
self.assertFalse(self.interface.should_write_image(task))
self.node.instance_info = {'image_source': 'fake-value'}
self.node.save()
with task_manager.acquire(self.context, self.node.id) as task:
self.assertTrue(self.interface.should_write_image(task))

36
ironic/tests/unit/drivers/modules/test_agent.py
View File

@ -256,13 +256,47 @@ class TestAgentDeploy(db_base.DbTestCase):
task.driver.boot, task)
show_mock.assert_called_once_with(self.context, 'fake-image')
@mock.patch.object(pxe.PXEBoot, 'validate', autospec=True)
@mock.patch.object(deploy_utils, 'check_for_missing_params',
autospec=True)
@mock.patch.object(deploy_utils, 'validate_capabilities', autospec=True)
@mock.patch.object(noop_storage.NoopStorage, 'should_write_image',
autospec=True)
def test_validate_storage_should_write_image_false(self, mock_write,
mock_capabilities,
mock_params,
mock_pxe_validate):
mock_write.return_value = False
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
self.driver.validate(task)
mock_capabilities.assert_called_once_with(task.node)
self.assertFalse(mock_params.called)
@mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True)
@mock.patch('ironic.conductor.utils.node_power_action', autospec=True)
def test_deploy(self, power_mock):
def test_deploy(self, power_mock, mock_pxe_instance):
with task_manager.acquire(
self.context, self.node['uuid'], shared=False) as task:
driver_return = self.driver.deploy(task)
self.assertEqual(driver_return, states.DEPLOYWAIT)
power_mock.assert_called_once_with(task, states.REBOOT)
self.assertFalse(mock_pxe_instance.called)
@mock.patch.object(pxe.PXEBoot, 'prepare_instance', autospec=True)
@mock.patch.object(noop_storage.NoopStorage, 'should_write_image',
autospec=True)
def test_deploy_storage_should_write_image_false(self, mock_write,
mock_pxe_instance):
mock_write.return_value = False
self.node.provision_state = states.DEPLOYING
self.node.save()
with task_manager.acquire(
self.context, self.node['uuid'], shared=False) as task:
driver_return = self.driver.deploy(task)
self.assertEqual(driver_return, states.DEPLOYDONE)
self.assertTrue(mock_pxe_instance.called)
@mock.patch.object(noop_storage.NoopStorage, 'detach_volumes',
autospec=True)

50
ironic/tests/unit/drivers/modules/test_iscsi_deploy.py
View File

@ -568,6 +568,27 @@ class ISCSIDeployTestCase(db_base.DbTestCase):
validate_capabilities_mock.assert_called_once_with(task.node)
validate_mock.assert_called_once_with(task)
@mock.patch.object(noop_storage.NoopStorage, 'should_write_image',
autospec=True)
@mock.patch.object(iscsi_deploy, 'validate', autospec=True)
@mock.patch.object(deploy_utils, 'validate_capabilities', autospec=True)
@mock.patch.object(pxe.PXEBoot, 'validate', autospec=True)
def test_validate_storage_should_write_image_false(
self, pxe_validate_mock,
validate_capabilities_mock, validate_mock,
should_write_image_mock):
should_write_image_mock.return_value = False
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
task.driver.deploy.validate(task)
pxe_validate_mock.assert_called_once_with(task.driver.boot, task)
validate_capabilities_mock.assert_called_once_with(task.node)
self.assertFalse(validate_mock.called)
should_write_image_mock.assert_called_once_with(
task.driver.storage, task)
@mock.patch.object(noop_storage.NoopStorage, 'attach_volumes',
autospec=True)
@mock.patch.object(deploy_utils, 'populate_storage_driver_internal_info',
@ -647,6 +668,35 @@ class ISCSIDeployTestCase(db_base.DbTestCase):
mock_check_image_size.assert_called_once_with(task)
mock_node_power_action.assert_called_once_with(task, states.REBOOT)
@mock.patch.object(noop_storage.NoopStorage, 'should_write_image',
autospec=True)
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.'
'configure_tenant_networks', spec_set=True, autospec=True)
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.'
'remove_provisioning_network', spec_set=True, autospec=True)
@mock.patch.object(manager_utils, 'node_power_action', autospec=True)
@mock.patch.object(iscsi_deploy, 'check_image_size', autospec=True)
@mock.patch.object(iscsi_deploy, 'cache_instance_image', autospec=True)
def test_deploy_storage_check_write_image_false(self,
mock_cache_instance_image,
mock_check_image_size,
mock_node_power_action,
mock_remove_network,
mock_tenant_network,
mock_write):
mock_write.return_value = False
self.node.provision_state = states.DEPLOYING
self.node.save()
with task_manager.acquire(self.context,
self.node.uuid, shared=False) as task:
state = task.driver.deploy.deploy(task)
self.assertEqual(state, states.DEPLOYDONE)
self.assertFalse(mock_cache_instance_image.called)
self.assertFalse(mock_check_image_size.called)
mock_remove_network.assert_called_once_with(mock.ANY, task)
mock_tenant_network.assert_called_once_with(mock.ANY, task)
self.assertEqual(2, mock_node_power_action.call_count)
@mock.patch.object(noop_storage.NoopStorage, 'detach_volumes',
autospec=True)
@mock.patch('ironic.drivers.modules.network.flat.FlatNetwork.'

61
ironic/tests/unit/drivers/modules/test_pxe.py
View File

@ -35,6 +35,7 @@ from ironic.conductor import task_manager
from ironic.drivers.modules import agent_base_vendor
from ironic.drivers.modules import deploy_utils
from ironic.drivers.modules import pxe
from ironic.drivers.modules.storage import noop as noop_storage
from ironic.tests.unit.conductor import mgr_utils
from ironic.tests.unit.db import base as db_base
from ironic.tests.unit.db import utils as db_utils
@ -697,6 +698,18 @@ class PXEBootTestCase(db_base.DbTestCase):
task.node.driver_internal_info['is_whole_disk_image'] = True
task.driver.boot.validate(task)
@mock.patch.object(base_image_service.BaseImageService, '_show',
autospec=True)
@mock.patch.object(noop_storage.NoopStorage, 'should_write_image',
autospec=True)
def test_validate_skip_check_write_image_false(self, mock_write,
mock_glance):
mock_write.return_value = False
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
task.driver.boot.validate(task)
self.assertFalse(mock_glance.called)
def test_validate_fail_missing_deploy_kernel(self):
with task_manager.acquire(self.context, self.node.uuid,
shared=True) as task:
@ -1077,6 +1090,54 @@ class PXEBootTestCase(db_base.DbTestCase):
self.assertFalse(switch_pxe_config_mock.called)
self.assertFalse(set_boot_device_mock.called)
@mock.patch('os.path.isfile', return_value=False, autospec=True)
@mock.patch.object(pxe_utils, 'create_pxe_config', autospec=True)
@mock.patch.object(deploy_utils, 'is_iscsi_boot', autospec=True)
@mock.patch.object(noop_storage.NoopStorage, 'should_write_image',
autospec=True)
@mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True)
@mock.patch.object(deploy_utils, 'switch_pxe_config', autospec=True)
@mock.patch.object(dhcp_factory, 'DHCPFactory', autospec=True)
@mock.patch.object(pxe, '_cache_ramdisk_kernel', autospec=True)
@mock.patch.object(pxe, '_get_instance_image_info', autospec=True)
def test_prepare_instance_netboot_iscsi(
self, get_image_info_mock, cache_mock,
dhcp_factory_mock, switch_pxe_config_mock,
set_boot_device_mock, should_write_image_mock,
is_iscsi_boot_mock, create_pxe_config_mock, isfile_mock):
http_url = 'http://192.1.2.3:1234'
self.config(ipxe_enabled=True, group='pxe')
self.config(http_url=http_url, group='deploy')
is_iscsi_boot_mock.return_value = True
should_write_image_mock.return_valule = False
provider_mock = mock.MagicMock()
dhcp_factory_mock.return_value = provider_mock
vol_id = uuidutils.generate_uuid()
obj_utils.create_test_volume_target(
self.context, node_id=self.node.id, volume_type='iscsi',
boot_index=0, volume_id='1234', uuid=vol_id,
properties={'target_lun': 0,
'target_portal': 'fake_host:3260',
'target_iqn': 'fake_iqn',
'auth_username': 'fake_username',
'auth_password': 'fake_password'})
with task_manager.acquire(self.context, self.node.uuid) as task:
task.node.driver_internal_info = {
'boot_from_volume': vol_id}
dhcp_opts = pxe_utils.dhcp_options_for_instance(task)
pxe_config_path = pxe_utils.get_pxe_config_file_path(
task.node.uuid)
task.driver.boot.prepare_instance(task)
self.assertFalse(get_image_info_mock.called)
self.assertFalse(cache_mock.called)
provider_mock.update_dhcp.assert_called_once_with(task, dhcp_opts)
create_pxe_config_mock.assert_called_once_with(
task, mock.ANY, CONF.pxe.pxe_config_template)
switch_pxe_config_mock.assert_called_once_with(
pxe_config_path, None, None, False, iscsi_boot=True)
set_boot_device_mock.assert_called_once_with(task,
boot_devices.PXE)
@mock.patch.object(deploy_utils, 'try_set_boot_device', autospec=True)
@mock.patch.object(pxe_utils, 'clean_up_pxe_config', autospec=True)
def test_prepare_instance_localboot(self, clean_up_pxe_config_mock,