diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 61ad724cca..d50dfcf98a 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -352,11 +352,15 @@ # Options defined in ironic.drivers.modules.agent # -# Additional append parameters for baremetal PXE boot. (string -# value) +# DEPRECATED. Additional append parameters for baremetal PXE +# boot. This option is deprecated and will be removed in +# Mitaka release. Please use [pxe]pxe_append_params instead. +# (string value) #agent_pxe_append_params=nofb nomodeset vga=normal -# Template file for PXE configuration. (string value) +# DEPRECATED. Template file for PXE configuration. This option +# is deprecated and will be removed in Mitaka release. Please +# use [pxe]pxe_config_template instead. (string value) #agent_pxe_config_template=$pybasedir/drivers/modules/agent_config.template # Priority to run in-band erase devices via the Ironic Python @@ -369,11 +373,11 @@ # value) #agent_erase_devices_iterations=1 -# Whether Ironic will manage TFTP files for the deploy -# ramdisks. If set to False, you will need to configure your -# own TFTP server that allows booting the deploy ramdisks. -# (boolean value) -#manage_tftp=true +# Whether Ironic will manage booting of the agent ramdisk. If +# set to False, you will need to configure your mechanism to +# allow booting the agent ramdisk. (boolean value) +# Deprecated group/name - [agent]/manage_tftp +#manage_agent_boot=true # diff --git a/ironic/drivers/agent.py b/ironic/drivers/agent.py index f1cc805304..e31eebbe6f 100644 --- a/ironic/drivers/agent.py +++ b/ironic/drivers/agent.py @@ -20,6 +20,7 @@ from ironic.drivers import base from ironic.drivers.modules import agent from ironic.drivers.modules import ipminative from ironic.drivers.modules import ipmitool +from ironic.drivers.modules import pxe from ironic.drivers.modules import ssh from ironic.drivers.modules.ucs import management as ucs_mgmt from ironic.drivers.modules.ucs import power as ucs_power @@ -39,6 +40,7 @@ class AgentAndIPMIToolDriver(base.BaseDriver): def __init__(self): self.power = ipmitool.IPMIPower() + self.boot = pxe.PXEBoot() self.deploy = agent.AgentDeploy() self.management = ipmitool.IPMIManagement() self.console = ipmitool.IPMIShellinaboxConsole() @@ -59,6 +61,7 @@ class AgentAndIPMINativeDriver(base.BaseDriver): def __init__(self): self.power = ipminative.NativeIPMIPower() + self.boot = pxe.PXEBoot() self.deploy = agent.AgentDeploy() self.management = ipminative.NativeIPMIManagement() self.console = ipminative.NativeIPMIShellinaboxConsole() @@ -80,6 +83,7 @@ class AgentAndSSHDriver(base.BaseDriver): def __init__(self): self.power = ssh.SSHPower() + self.boot = pxe.PXEBoot() self.deploy = agent.AgentDeploy() self.management = ssh.SSHManagement() self.vendor = agent.AgentVendorInterface() @@ -104,6 +108,7 @@ class AgentAndVirtualBoxDriver(base.BaseDriver): driver=self.__class__.__name__, reason=_("Unable to import pyremotevbox library")) self.power = virtualbox.VirtualBoxPower() + self.boot = pxe.PXEBoot() self.deploy = agent.AgentDeploy() self.management = virtualbox.VirtualBoxManagement() self.vendor = agent.AgentVendorInterface() @@ -126,6 +131,7 @@ class AgentAndUcsDriver(base.BaseDriver): driver=self.__class__.__name__, reason=_("Unable to import UcsSdk library")) self.power = ucs_power.Power() + self.boot = pxe.PXEBoot() self.deploy = agent.AgentDeploy() self.management = ucs_mgmt.UcsManagement() self.vendor = agent.AgentVendorInterface() diff --git a/ironic/drivers/fake.py b/ironic/drivers/fake.py index f61659614e..5b8514fb83 100644 --- a/ironic/drivers/fake.py +++ b/ironic/drivers/fake.py @@ -135,6 +135,7 @@ class FakeAgentDriver(base.BaseDriver): def __init__(self): self.power = fake.FakePower() + self.boot = pxe.PXEBoot() self.deploy = agent.AgentDeploy() self.vendor = agent.AgentVendorInterface() diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index db61838b96..ed863b4d60 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -12,15 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -import os import time from oslo_config import cfg from oslo_log import log from oslo_utils import excutils -from oslo_utils import fileutils -from ironic.common import boot_devices from ironic.common import dhcp_factory from ironic.common import exception from ironic.common.glance_service import service_utils @@ -30,26 +27,29 @@ from ironic.common.i18n import _LI from ironic.common import image_service from ironic.common import keystone from ironic.common import paths -from ironic.common import pxe_utils from ironic.common import states -from ironic.common import utils from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils from ironic.drivers import base from ironic.drivers.modules import agent_base_vendor from ironic.drivers.modules import agent_client from ironic.drivers.modules import deploy_utils -from ironic.drivers.modules import image_cache agent_opts = [ cfg.StrOpt('agent_pxe_append_params', default='nofb nomodeset vga=normal', - help=_('Additional append parameters for baremetal PXE boot.')), + help=_('DEPRECATED. Additional append parameters for ' + 'baremetal PXE boot. This option is deprecated and ' + 'will be removed in Mitaka release. Please use ' + '[pxe]pxe_append_params instead.')), cfg.StrOpt('agent_pxe_config_template', default=paths.basedir_def( 'drivers/modules/agent_config.template'), - help=_('Template file for PXE configuration.')), + help=_('DEPRECATED. Template file for PXE configuration. ' + 'This option is deprecated and will be removed ' + 'in Mitaka release. Please use [pxe]pxe_config_template ' + 'instead.')), cfg.IntOpt('agent_erase_devices_priority', help=_('Priority to run in-band erase devices via the Ironic ' 'Python Agent ramdisk. If unset, will use the priority ' @@ -59,12 +59,14 @@ agent_opts = [ cfg.IntOpt('agent_erase_devices_iterations', default=1, help=_('Number of iterations to be run for erasing devices.')), - cfg.BoolOpt('manage_tftp', + cfg.BoolOpt('manage_agent_boot', default=True, - help=_('Whether Ironic will manage TFTP files for the deploy ' - 'ramdisks. If set to False, you will need to configure ' - 'your own TFTP server that allows booting the deploy ' - 'ramdisks.')), + deprecated_name='manage_tftp', + deprecated_group='agent', + help=_('Whether Ironic will manage booting of the agent ' + 'ramdisk. If set to False, you will need to configure ' + 'your mechanism to allow booting the agent ' + 'ramdisk.')), ] CONF = cfg.CONF @@ -115,61 +117,6 @@ def build_agent_options(node): return agent_config_opts -def _build_pxe_config_options(node, pxe_info): - """Builds the pxe config options for booting agent. - - This method builds the config options to be replaced on - the agent pxe config template. - - :param node: an ironic node object - :param pxe_info: A dict containing the 'deploy_kernel' and - 'deploy_ramdisk' for the agent pxe config template. - :returns: a dict containing the options to be applied on - the agent pxe config template. - """ - agent_config_opts = { - 'deployment_aki_path': pxe_info['deploy_kernel'][1], - 'deployment_ari_path': pxe_info['deploy_ramdisk'][1], - 'pxe_append_params': CONF.agent.agent_pxe_append_params, - } - agent_opts = build_agent_options(node) - agent_config_opts.update(agent_opts) - return agent_config_opts - - -def _get_tftp_image_info(node): - return pxe_utils.get_deploy_kr_info(node.uuid, node.driver_info) - - -def _driver_uses_pxe(driver): - """A quick hack to check if driver uses pxe.""" - # If driver.deploy says I need deploy_kernel and deploy_ramdisk, - # then it's using PXE boot. - properties = driver.deploy.get_properties() - return (('deploy_kernel' in properties) and - ('deploy_ramdisk' in properties)) - - -@image_cache.cleanup(priority=25) -class AgentTFTPImageCache(image_cache.ImageCache): - def __init__(self): - super(AgentTFTPImageCache, self).__init__( - CONF.pxe.tftp_master_path, - # MiB -> B - CONF.pxe.image_cache_size * 1024 * 1024, - # min -> sec - CONF.pxe.image_cache_ttl * 60) - - -def _cache_tftp_images(ctx, node, pxe_info): - """Fetch the necessary kernels and ramdisks for the instance.""" - fileutils.ensure_tree( - os.path.join(CONF.pxe.tftp_root, node.uuid)) - LOG.debug("Fetching kernel and ramdisk for node %s", - node.uuid) - deploy_utils.fetch_images(ctx, AgentTFTPImageCache(), pxe_info.values()) - - def build_instance_info_for_deploy(task): """Build instance_info necessary for deploying to a node. @@ -209,42 +156,6 @@ def build_instance_info_for_deploy(task): return instance_info -def _prepare_pxe_boot(task): - """Prepare the files required for PXE booting the agent.""" - if CONF.agent.manage_tftp: - pxe_info = _get_tftp_image_info(task.node) - pxe_options = _build_pxe_config_options(task.node, pxe_info) - pxe_utils.create_pxe_config(task, - pxe_options, - CONF.agent.agent_pxe_config_template) - _cache_tftp_images(task.context, task.node, pxe_info) - - -def _do_pxe_boot(task, ports=None): - """Reboot the node into the PXE ramdisk. - - :param task: a TaskManager instance - :param ports: a list of Neutron port dicts to update DHCP options on. If - None, will get the list of ports from the Ironic port objects. - """ - dhcp_opts = pxe_utils.dhcp_options_for_instance(task) - provider = dhcp_factory.DHCPFactory() - provider.update_dhcp(task, dhcp_opts, ports) - manager_utils.node_set_boot_device(task, boot_devices.PXE, persistent=True) - manager_utils.node_power_action(task, states.REBOOT) - - -def _clean_up_pxe(task): - """Clean up left over PXE and DHCP files.""" - if CONF.agent.manage_tftp: - pxe_info = _get_tftp_image_info(task.node) - for label in pxe_info: - path = pxe_info[label][1] - utils.unlink_without_raise(path) - AgentTFTPImageCache().clean_up() - pxe_utils.clean_up_pxe_config(task) - - class AgentDeploy(base.DeployInterface): """Interface for deploy-related actions.""" @@ -265,13 +176,11 @@ class AgentDeploy(base.DeployInterface): :param task: a TaskManager instance :raises: MissingParameterValue """ + if CONF.agent.manage_agent_boot: + task.driver.boot.validate(task) + node = task.node params = {} - if CONF.agent.manage_tftp: - params['driver_info.deploy_kernel'] = node.driver_info.get( - 'deploy_kernel') - params['driver_info.deploy_ramdisk'] = node.driver_info.get( - 'deploy_ramdisk') 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 ' @@ -308,7 +217,7 @@ class AgentDeploy(base.DeployInterface): :param task: a TaskManager instance. :returns: status of the deploy. One of ironic.common.states. """ - _do_pxe_boot(task) + manager_utils.node_power_action(task, states.REBOOT) return states.DEPLOYWAIT @task_manager.require_exclusive_lock @@ -326,11 +235,16 @@ class AgentDeploy(base.DeployInterface): :param task: a TaskManager instance. """ + # Nodes deployed by AgentDeploy always boot from disk now. So there + # is nothing to be done in prepare() when it's called during + # take over. node = task.node - _prepare_pxe_boot(task) - - node.instance_info = build_instance_info_for_deploy(task) - node.save() + if node.provision_state != states.ACTIVE: + node.instance_info = build_instance_info_for_deploy(task) + node.save() + if CONF.agent.manage_agent_boot: + deploy_opts = build_agent_options(node) + task.driver.boot.prepare_ramdisk(task, deploy_opts) def clean_up(self, task): """Clean up the deployment environment for this node. @@ -348,7 +262,8 @@ class AgentDeploy(base.DeployInterface): :param task: a TaskManager instance. """ - _clean_up_pxe(task) + if CONF.agent.manage_agent_boot: + task.driver.boot.clean_up_ramdisk(task) def take_over(self, task): """Take over management of this node from a dead conductor. @@ -403,17 +318,38 @@ class AgentDeploy(base.DeployInterface): provider.provider.delete_cleaning_ports(task) # Create cleaning ports if necessary - ports = None if getattr(provider.provider, 'create_cleaning_ports', None): # Allow to raise if it fails, is caught and handled in conductor ports = provider.provider.create_cleaning_ports(task) + # Add vif_port_id for each of the ports because some boot + # interfaces expects these to prepare for booting ramdisk. + for port in task.ports: + extra_dict = port.extra + try: + extra_dict['vif_port_id'] = ports[port.uuid] + except KeyError: + # This is an internal error in Ironic. All DHCP providers + # implementing create_cleaning_ports are supposed to + # return a VIF port ID for all Ironic ports. But + # that doesn't seem to be true here. + error = (_("When creating cleaning ports, DHCP provider " + "didn't return VIF port ID for %s") % port.uuid) + raise exception.NodeCleaningFailure( + node=task.node.uuid, reason=error) + else: + port.extra = extra_dict + port.save() + # Append required config parameters to node's driver_internal_info # to pass to IPA. deploy_utils.agent_add_clean_params(task) - _prepare_pxe_boot(task) - _do_pxe_boot(task, ports) + if CONF.agent.manage_agent_boot: + ramdisk_opts = build_agent_options(task.node) + task.driver.boot.prepare_ramdisk(task, ramdisk_opts) + manager_utils.node_power_action(task, states.REBOOT) + # Tell the conductor we are waiting for the agent to boot. return states.CLEANWAIT @@ -425,7 +361,8 @@ class AgentDeploy(base.DeployInterface): removed """ manager_utils.node_power_action(task, states.POWER_OFF) - _clean_up_pxe(task) + if CONF.agent.manage_agent_boot: + task.driver.boot.clean_up_ramdisk(task) # If we created cleaning ports, delete them provider = dhcp_factory.DHCPFactory() @@ -433,6 +370,13 @@ class AgentDeploy(base.DeployInterface): # Allow to raise if it fails, is caught and handled in conductor provider.provider.delete_cleaning_ports(task) + for port in task.ports: + if 'vif_port_id' in port.extra: + extra_dict = port.extra + extra_dict.pop('vif_port_id', None) + port.extra = extra_dict + port.save() + class AgentVendorInterface(agent_base_vendor.BaseAgentVendor): @@ -513,12 +457,12 @@ class AgentVendorInterface(agent_base_vendor.BaseAgentVendor): manager_utils.node_set_boot_device(task, 'disk', persistent=True) self.reboot_and_finish_deploy(task) - # NOTE(TheJulia): If we we deployed a whole disk image, we + + # NOTE(TheJulia): If we deployed a whole disk image, we # should expect a whole disk image and clean-up the tftp files # on-disk incase the node is disregarding the boot preference. - # TODO(rameshg87): This shouldn't get called for virtual media deploy - # drivers (iLO and iRMC). This is just a hack, but it will be taken - # care in boot/deploy interface separation. - if (_driver_uses_pxe(task.driver) and - node.driver_internal_info.get('is_whole_disk_image')): - _clean_up_pxe(task) + # TODO(rameshg87): Not all in-tree drivers using reboot_to_instance + # have a boot interface. So include a check for now. Remove this + # check once all in-tree drivers have a boot interface. + if hasattr(task.driver, 'boot'): + task.driver.boot.clean_up_ramdisk(task) diff --git a/ironic/drivers/modules/pxe.py b/ironic/drivers/modules/pxe.py index 2461beac70..eaa21b5f1b 100644 --- a/ironic/drivers/modules/pxe.py +++ b/ironic/drivers/modules/pxe.py @@ -35,6 +35,7 @@ from ironic.common import pxe_utils from ironic.common import states from ironic.common import utils from ironic.drivers import base +from ironic.drivers.modules import agent from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import image_cache from ironic.drivers import utils as driver_utils @@ -95,6 +96,49 @@ REQUIRED_PROPERTIES = { COMMON_PROPERTIES = REQUIRED_PROPERTIES +# TODO(rameshg87): This method is only for allowing time for deployers to +# migrate to CONF.pxe. after the CONF.agent. have been +# deprecated. Remove this in Mitaka release. +def _get_pxe_conf_option(task, opt_name): + """Returns the value of PXEBoot provided CONF option. + + This method returns the value of PXEBoot CONF option after checking + the driver.deploy. If driver.deploy is AgentDeploy and the value of + the CONF option is not it's default value, it returns the value of + CONF.agent.agent_. Otherwise, it returns the value of + CONF.pxe.. There are only 2 such parameters right now - + pxe_config_template and pxe_append_params. Caller + has to make sure that only these 2 options are passed. + + :param task: TaskManager instance. + :param opt_name: The CONF opt whose value is desired. + :returns: The value of the CONF option. + :raises: AttributeError, if such a CONF option doesn't exist. + """ + if isinstance(task.driver.deploy, agent.AgentDeploy): + agent_opt_name = 'agent_' + opt_name + current_value = getattr(CONF.agent, agent_opt_name) + opt_object = [x for x in agent.agent_opts + if x.name == agent_opt_name][0] + default_value = opt_object.default + # Replace $pybasedir which can occur in pxe_config_template + # default value. + default_value = default_value.replace('$pybasedir', + CONF.pybasedir) + + if current_value != default_value: + LOG.warn(_LW("The CONF option [agent]agent_%(opt_name)s is " + "deprecated and will be removed in Mitaka release of " + "Ironic. Please use [pxe]%(opt_name)s instead."), + {'opt_name': opt_name}) + return current_value + + # Either task.driver.deploy is ISCSIDeploy() or the default value hasn't + # been modified. So return the value of corresponding parameter in + # [pxe] group. + return getattr(CONF.pxe, opt_name) + + def _parse_driver_info(node): """Gets the driver specific Node deployment info. @@ -198,7 +242,7 @@ def _get_deploy_image_info(node): return pxe_utils.get_deploy_kr_info(node.uuid, d_info) -def _build_pxe_config_options(node, pxe_info, ctx): +def _build_pxe_config_options(task, pxe_info): """Build the PXE config options for a node This method builds the PXE boot options for a node, @@ -207,12 +251,12 @@ def _build_pxe_config_options(node, pxe_info, ctx): The options should then be passed to pxe_utils.create_pxe_config to create the actual config files. - :param node: a single Node. + :param task: A TaskManager object :param pxe_info: a dict of values to set on the configuration file - :param ctx: security context :returns: A dictionary of pxe options to be used in the pxe bootfile template. """ + node = task.node is_whole_disk_image = node.driver_internal_info.get('is_whole_disk_image') if is_whole_disk_image: # These are dummy values to satisfy elilo. @@ -240,7 +284,7 @@ def _build_pxe_config_options(node, pxe_info, ctx): pxe_options = { 'deployment_aki_path': deploy_kernel, 'deployment_ari_path': deploy_ramdisk, - 'pxe_append_params': CONF.pxe.pxe_append_params, + 'pxe_append_params': _get_pxe_conf_option(task, 'pxe_append_params'), 'tftp_server': CONF.pxe.tftp_server, 'aki_path': kernel, 'ari_path': ramdisk @@ -442,14 +486,14 @@ class PXEBoot(base.BootInterface): if node.provision_state == states.DEPLOYING: pxe_info.update(_get_instance_image_info(node, task.context)) - pxe_options = _build_pxe_config_options(node, pxe_info, - task.context) + pxe_options = _build_pxe_config_options(task, pxe_info) pxe_options.update(ramdisk_params) if deploy_utils.get_boot_mode_for_deploy(node) == 'uefi': pxe_config_template = CONF.pxe.uefi_pxe_config_template else: - pxe_config_template = CONF.pxe.pxe_config_template + pxe_config_template = _get_pxe_conf_option(task, + 'pxe_config_template') pxe_utils.create_pxe_config(task, pxe_options, pxe_config_template) diff --git a/ironic/tests/drivers/test_agent.py b/ironic/tests/drivers/test_agent.py index 76d1972388..59debb634b 100644 --- a/ironic/tests/drivers/test_agent.py +++ b/ironic/tests/drivers/test_agent.py @@ -17,21 +17,16 @@ import types import mock from oslo_config import cfg -from ironic.common import dhcp_factory from ironic.common import exception from ironic.common import image_service from ironic.common import keystone -from ironic.common import pxe_utils from ironic.common import states from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils from ironic.drivers.modules import agent from ironic.drivers.modules import agent_client from ironic.drivers.modules import fake -from ironic.drivers.modules.ilo import power as ilo_power -from ironic.drivers.modules import ipmitool -from ironic.drivers.modules.irmc import deploy as irmc_deploy -from ironic.drivers.modules.irmc import power as irmc_power +from ironic.drivers.modules import pxe from ironic.tests.conductor import utils as mgr_utils from ironic.tests.db import base as db_base from ironic.tests.db import utils as db_utils @@ -155,46 +150,49 @@ class TestAgentDeploy(db_base.DbTestCase): 'driver_internal_info': DRIVER_INTERNAL_INFO, } self.node = object_utils.create_test_node(self.context, **n) - self.ports = [object_utils.create_test_port(self.context, - node_id=self.node.id)] + self.ports = [ + object_utils.create_test_port(self.context, node_id=self.node.id)] def test_get_properties(self): expected = agent.COMMON_PROPERTIES self.assertEqual(expected, self.driver.get_properties()) - def test_validate(self): + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate(self, pxe_boot_validate_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: self.driver.validate(task) + pxe_boot_validate_mock.assert_called_once_with( + task.driver.boot, task) - def test_validate_driver_info_missing_params(self): - self.node.driver_info = {} - self.node.save() - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - e = self.assertRaises(exception.MissingParameterValue, - self.driver.validate, task) - self.assertIn('driver_info.deploy_ramdisk', str(e)) - self.assertIn('driver_info.deploy_kernel', str(e)) - - def test_validate_driver_info_manage_tftp_false(self): - self.config(manage_tftp=False, group='agent') + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate_driver_info_manage_agent_boot_false( + self, pxe_boot_validate_mock): + self.config(manage_agent_boot=False, group='agent') self.node.driver_info = {} self.node.save() with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: self.driver.validate(task) + self.assertFalse(pxe_boot_validate_mock.called) - def test_validate_instance_info_missing_params(self): + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate_instance_info_missing_params( + self, pxe_boot_validate_mock): self.node.instance_info = {} self.node.save() with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: e = self.assertRaises(exception.MissingParameterValue, self.driver.validate, task) + pxe_boot_validate_mock.assert_called_once_with( + task.driver.boot, task) + self.assertIn('instance_info.image_source', str(e)) - def test_validate_nonglance_image_no_checksum(self): + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate_nonglance_image_no_checksum( + self, pxe_boot_validate_mock): i_info = self.node.instance_info i_info['image_source'] = 'http://image-ref' del i_info['image_checksum'] @@ -205,65 +203,38 @@ class TestAgentDeploy(db_base.DbTestCase): self.context, self.node.uuid, shared=False) as task: self.assertRaises(exception.MissingParameterValue, self.driver.validate, task) + pxe_boot_validate_mock.assert_called_once_with( + task.driver.boot, task) - def test_validate_agent_fail_partition_image(self): + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate_agent_fail_partition_image( + self, pxe_boot_validate_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: task.node.driver_internal_info['is_whole_disk_image'] = False self.assertRaises(exception.InvalidParameterValue, self.driver.validate, task) + pxe_boot_validate_mock.assert_called_once_with( + task.driver.boot, task) - def test_validate_invalid_root_device_hints(self): + @mock.patch.object(pxe.PXEBoot, 'validate', autospec=True) + def test_validate_invalid_root_device_hints( + self, pxe_boot_validate_mock): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: task.node.properties['root_device'] = {'size': 'not-int'} self.assertRaises(exception.InvalidParameterValue, task.driver.deploy.validate, task) + pxe_boot_validate_mock.assert_called_once_with( + task.driver.boot, task) - @mock.patch.object(agent, '_cache_tftp_images', autospec=True) - @mock.patch.object(pxe_utils, 'create_pxe_config', autospec=True) - @mock.patch.object(agent, '_build_pxe_config_options', autospec=True) - @mock.patch.object(agent, '_get_tftp_image_info', autospec=True) - def test__prepare_pxe_boot(self, pxe_info_mock, options_mock, - create_mock, cache_mock): - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - agent._prepare_pxe_boot(task) - pxe_info_mock.assert_called_once_with(task.node) - options_mock.assert_called_once_with(task.node, mock.ANY) - create_mock.assert_called_once_with( - task, mock.ANY, CONF.agent.agent_pxe_config_template) - cache_mock.assert_called_once_with(task.context, task.node, - mock.ANY) - - @mock.patch.object(agent, '_cache_tftp_images', autospec=True) - @mock.patch.object(pxe_utils, 'create_pxe_config', autospec=True) - @mock.patch.object(agent, '_build_pxe_config_options', autospec=True) - @mock.patch.object(agent, '_get_tftp_image_info', autospec=True) - def test__prepare_pxe_boot_manage_tftp_false( - self, pxe_info_mock, options_mock, create_mock, cache_mock): - self.config(manage_tftp=False, group='agent') - with task_manager.acquire( - self.context, self.node['uuid'], shared=False) as task: - agent._prepare_pxe_boot(task) - self.assertFalse(pxe_info_mock.called) - self.assertFalse(options_mock.called) - self.assertFalse(create_mock.called) - self.assertFalse(cache_mock.called) - - @mock.patch.object(dhcp_factory.DHCPFactory, 'update_dhcp', autospec=True) - @mock.patch('ironic.conductor.utils.node_set_boot_device', autospec=True) @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) - def test_deploy(self, power_mock, bootdev_mock, dhcp_mock): + def test_deploy(self, power_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: - dhcp_opts = pxe_utils.dhcp_options_for_instance(task) driver_return = self.driver.deploy(task) self.assertEqual(driver_return, states.DEPLOYWAIT) - dhcp_mock.assert_called_once_with(mock.ANY, task, dhcp_opts, None) - bootdev_mock.assert_called_once_with(task, 'pxe', persistent=True) - power_mock.assert_called_once_with(task, - states.REBOOT) + power_mock.assert_called_once_with(task, states.REBOOT) @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) def test_tear_down(self, power_mock): @@ -273,69 +244,160 @@ class TestAgentDeploy(db_base.DbTestCase): power_mock.assert_called_once_with(task, states.POWER_OFF) self.assertEqual(driver_return, states.DELETED) - @mock.patch.object(pxe_utils, 'clean_up_pxe_config', autospec=True) - @mock.patch.object(agent, 'AgentTFTPImageCache', autospec=True) - @mock.patch('ironic.common.utils.unlink_without_raise', autospec=True) - @mock.patch.object(agent, '_get_tftp_image_info', autospec=True) - def test__clean_up_pxe(self, info_mock, unlink_mock, cache_mock, - clean_mock): - info_mock.return_value = {'label': ['fake1', 'fake2']} + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk') + @mock.patch.object(agent, 'build_agent_options') + @mock.patch.object(agent, 'build_instance_info_for_deploy') + def test_prepare(self, build_instance_info_mock, build_options_mock, + pxe_prepare_ramdisk_mock): with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: - agent._clean_up_pxe(task) - info_mock.assert_called_once_with(task.node) - unlink_mock.assert_called_once_with('fake2') - clean_mock.assert_called_once_with(task) + task.node.provision_state = states.DEPLOYING + build_instance_info_mock.return_value = {'foo': 'bar'} + build_options_mock.return_value = {'a': 'b'} - @mock.patch.object(pxe_utils, 'clean_up_pxe_config', autospec=True) - @mock.patch.object(agent.AgentTFTPImageCache, 'clean_up', autospec=True) - @mock.patch('ironic.common.utils.unlink_without_raise', autospec=True) - @mock.patch.object(agent, '_get_tftp_image_info', autospec=True) - def test__clean_up_pxe_manage_tftp_false( - self, info_mock, unlink_mock, cache_mock, clean_mock): - self.config(manage_tftp=False, group='agent') - info_mock.return_value = {'label': ['fake1', 'fake2']} + self.driver.prepare(task) + + build_instance_info_mock.assert_called_once_with(task) + build_options_mock.assert_called_once_with(task.node) + pxe_prepare_ramdisk_mock.assert_called_once_with( + task, {'a': 'b'}) + + self.node.refresh() + self.assertEqual('bar', self.node.instance_info['foo']) + + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk') + @mock.patch.object(agent, 'build_agent_options') + @mock.patch.object(agent, 'build_instance_info_for_deploy') + def test_prepare_manage_agent_boot_false( + self, build_instance_info_mock, build_options_mock, + pxe_prepare_ramdisk_mock): + self.config(group='agent', manage_agent_boot=False) with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: - agent._clean_up_pxe(task) - self.assertFalse(info_mock.called) - self.assertFalse(unlink_mock.called) - self.assertFalse(cache_mock.called) - self.assertFalse(clean_mock.called) + task.node.provision_state = states.DEPLOYING + build_instance_info_mock.return_value = {'foo': 'bar'} + self.driver.prepare(task) + + build_instance_info_mock.assert_called_once_with(task) + self.assertFalse(build_options_mock.called) + self.assertFalse(pxe_prepare_ramdisk_mock.called) + + self.node.refresh() + self.assertEqual('bar', self.node.instance_info['foo']) + + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk') + @mock.patch.object(agent, 'build_agent_options') + @mock.patch.object(agent, 'build_instance_info_for_deploy') + def test_prepare_active( + self, build_instance_info_mock, build_options_mock, + pxe_prepare_ramdisk_mock): + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + task.node.provision_state = states.ACTIVE + + self.driver.prepare(task) + + self.assertFalse(build_instance_info_mock.called) + self.assertFalse(build_options_mock.called) + self.assertFalse(pxe_prepare_ramdisk_mock.called) + + @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk') + def test_clean_up(self, pxe_clean_up_ramdisk_mock): + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + self.driver.clean_up(task) + pxe_clean_up_ramdisk_mock.assert_called_once_with(task) + + @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk') + def test_clean_up_manage_agent_boot_false(self, pxe_clean_up_ramdisk_mock): + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + self.config(group='agent', manage_agent_boot=False) + self.driver.clean_up(task) + self.assertFalse(pxe_clean_up_ramdisk_mock.called) + + @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) + @mock.patch.object(agent, 'build_agent_options', autospec=True) @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports', autospec=True) @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.create_cleaning_ports', autospec=True) - @mock.patch('ironic.drivers.modules.agent._do_pxe_boot', autospec=True) - @mock.patch('ironic.drivers.modules.agent._prepare_pxe_boot', - autospec=True) - def test_prepare_cleaning(self, prepare_mock, boot_mock, create_mock, - delete_mock): - ports = [{'ports': self.ports}] - create_mock.return_value = ports + def _test_prepare_cleaning(self, create_mock, delete_mock, + build_options_mock, power_mock, + return_vif_port_id=True): + if return_vif_port_id: + create_mock.return_value = {self.ports[0].uuid: 'vif-port-id'} + else: + create_mock.return_value = {} + build_options_mock.return_value = {'a': 'b'} with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: self.assertEqual(states.CLEANWAIT, self.driver.prepare_cleaning(task)) - prepare_mock.assert_called_once_with(task) - boot_mock.assert_called_once_with(task, ports) create_mock.assert_called_once_with(mock.ANY, task) delete_mock.assert_called_once_with(mock.ANY, task) + power_mock.assert_called_once_with(task, states.REBOOT) self.assertEqual(task.node.driver_internal_info.get( 'agent_erase_devices_iterations'), 1) + self.ports[0].refresh() + self.assertEqual('vif-port-id', self.ports[0].extra['vif_port_id']) + + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True) + def test_prepare_cleaning(self, prepare_ramdisk_mock): + self._test_prepare_cleaning() + prepare_ramdisk_mock.assert_called_once_with( + mock.ANY, mock.ANY, {'a': 'b'}) + + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True) + def test_prepare_cleaning_no_vif_port_id(self, prepare_ramdisk_mock): + self.assertRaises( + exception.NodeCleaningFailure, self._test_prepare_cleaning, + return_vif_port_id=False) + + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True) + def test_prepare_cleaning_manage_agent_boot_false( + self, prepare_ramdisk_mock): + self.config(group='agent', manage_agent_boot=False) + self._test_prepare_cleaning() + self.assertFalse(prepare_ramdisk_mock.called) + + @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk', autospec=True) @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports', autospec=True) - @mock.patch('ironic.drivers.modules.agent._clean_up_pxe', autospec=True) @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) - def test_tear_down_cleaning(self, power_mock, cleanup_mock, neutron_mock): + def test_tear_down_cleaning(self, power_mock, neutron_mock, + clean_up_ramdisk_mock): + extra_dict = self.ports[0].extra + extra_dict['vif_port_id'] = 'vif-port-id' + self.ports[0].extra = extra_dict + self.ports[0].save() with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: self.assertIsNone(self.driver.tear_down_cleaning(task)) power_mock.assert_called_once_with(task, states.POWER_OFF) - cleanup_mock.assert_called_once_with(task) neutron_mock.assert_called_once_with(mock.ANY, task) + clean_up_ramdisk_mock.assert_called_once_with( + task.driver.boot, task) + + self.ports[0].refresh() + self.assertNotIn('vif_port_id', self.ports[0].extra) + + @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk', autospec=True) + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports', + autospec=True) + @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) + def test_tear_down_cleaning_manage_agent_boot_false( + self, power_mock, neutron_mock, + clean_up_ramdisk_mock): + self.config(group='agent', manage_agent_boot=False) + with task_manager.acquire( + self.context, self.node['uuid'], shared=False) as task: + self.assertIsNone(self.driver.tear_down_cleaning(task)) + power_mock.assert_called_once_with(task, states.POWER_OFF) + neutron_mock.assert_called_once_with(mock.ANY, task) + self.assertFalse(clean_up_ramdisk_mock.called) @mock.patch('ironic.drivers.modules.deploy_utils.agent_get_clean_steps', autospec=True) @@ -433,33 +495,30 @@ class TestAgentVendor(db_base.DbTestCase): task.node.target_provision_state) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) + @mock.patch.object(fake.FakePower, 'get_power_state', + spec=types.FunctionType) @mock.patch.object(agent_client.AgentClient, 'power_off', spec=types.FunctionType) @mock.patch('ironic.conductor.utils.node_set_boot_device', autospec=True) @mock.patch('ironic.drivers.modules.agent.AgentVendorInterface' '.check_deploy_success', autospec=True) - @mock.patch.object(agent, '_clean_up_pxe', autospec=True) - def _test_reboot_to_instance(self, clean_pxe_mock, check_deploy_mock, - bootdev_mock, power_off_mock, - node_power_action_mock, - get_power_state_mock, - uses_pxe=True): + @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk', autospec=True) + def test_reboot_to_instance(self, clean_pxe_mock, check_deploy_mock, + bootdev_mock, power_off_mock, + get_power_state_mock, node_power_action_mock): check_deploy_mock.return_value = None self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE self.node.save() - with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: get_power_state_mock.return_value = states.POWER_OFF task.node.driver_internal_info['is_whole_disk_image'] = True + self.passthru.reboot_to_instance(task) - if uses_pxe: - clean_pxe_mock.assert_called_once_with(task) - else: - self.assertFalse(clean_pxe_mock.called) + clean_pxe_mock.assert_called_once_with(task.driver.boot, task) check_deploy_mock.assert_called_once_with(mock.ANY, task.node) bootdev_mock.assert_called_once_with(task, 'disk', persistent=True) power_off_mock.assert_called_once_with(task.node) @@ -469,41 +528,6 @@ class TestAgentVendor(db_base.DbTestCase): self.assertEqual(states.ACTIVE, task.node.provision_state) self.assertEqual(states.NOSTATE, task.node.target_provision_state) - @mock.patch.object(fake.FakePower, 'get_power_state', - spec=types.FunctionType) - def test_reboot_to_instance_fake_driver(self, get_power_state_mock): - self._test_reboot_to_instance( - get_power_state_mock=get_power_state_mock) - - @mock.patch.object(ipmitool.IPMIPower, 'get_power_state', - spec=types.FunctionType) - def test_reboot_to_instance_agent_ipmitool_driver( - self, get_power_state_mock): - mgr_utils.mock_the_extension_manager(driver='agent_ipmitool') - self.node.driver = 'agent_ipmitool' - self.node.save() - self._test_reboot_to_instance( - get_power_state_mock=get_power_state_mock) - - @mock.patch.object(ilo_power.IloPower, 'get_power_state', - spec=types.FunctionType) - def test_reboot_to_instance_agent_ilo_driver(self, get_power_state_mock): - mgr_utils.mock_the_extension_manager(driver='agent_ilo') - self.node.driver = 'agent_ilo' - self.node.save() - self._test_reboot_to_instance( - get_power_state_mock=get_power_state_mock, uses_pxe=False) - - @mock.patch.object(irmc_power.IRMCPower, 'get_power_state', - spec=types.FunctionType) - def test_reboot_to_instance_agent_irmc_driver(self, get_power_state_mock): - irmc_deploy._check_share_fs_mounted_patcher.start() - mgr_utils.mock_the_extension_manager(driver='agent_irmc') - self.node.driver = 'agent_irmc' - self.node.save() - self._test_reboot_to_instance( - get_power_state_mock=get_power_state_mock, uses_pxe=False) - @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) def test_deploy_has_started(self, mock_get_cmd): @@ -575,35 +599,3 @@ class TestAgentVendor(db_base.DbTestCase): mock_get_cmd.return_value = [{'command_name': 'prepare_image', 'command_status': 'RUNNING'}] self.assertFalse(self.passthru.deploy_is_done(task)) - - def _build_pxe_config_options(self, root_device_hints=False): - self.config(api_url='api-url', group='conductor') - self.config(agent_pxe_append_params='foo bar', group='agent') - - if root_device_hints: - self.node.properties['root_device'] = {'model': 'FakeModel'} - - pxe_info = { - 'deploy_kernel': ('glance://deploy-kernel', - 'fake-node/deploy_kernel'), - 'deploy_ramdisk': ('glance://deploy-ramdisk', - 'fake-node/deploy_ramdisk'), - } - options = agent._build_pxe_config_options(self.node, pxe_info) - expected = {'deployment_aki_path': 'fake-node/deploy_kernel', - 'deployment_ari_path': 'fake-node/deploy_ramdisk', - 'ipa-api-url': 'api-url', - 'ipa-driver-name': u'fake_agent', - 'coreos.configdrive': 0, - 'pxe_append_params': 'foo bar'} - - if root_device_hints: - expected['root_device'] = 'model=FakeModel' - - self.assertEqual(expected, options) - - def test__build_pxe_config_options(self): - self._build_pxe_config_options() - - def test__build_pxe_config_options_root_device_hints(self): - self._build_pxe_config_options(root_device_hints=True) diff --git a/ironic/tests/drivers/test_pxe.py b/ironic/tests/drivers/test_pxe.py index e3fc80cab4..12dfe5b1e1 100644 --- a/ironic/tests/drivers/test_pxe.py +++ b/ironic/tests/drivers/test_pxe.py @@ -128,6 +128,34 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase): mgr_utils.mock_the_extension_manager(driver="fake_pxe") self.node = obj_utils.create_test_node(self.context, **n) + def _test_get_pxe_conf_option(self, driver, expected_value): + mgr_utils.mock_the_extension_manager(driver=driver) + self.node.driver = driver + self.node.save() + + with task_manager.acquire(self.context, self.node.uuid) as task: + returned_value = pxe._get_pxe_conf_option( + task, 'pxe_config_template') + + self.assertEqual(expected_value, returned_value) + + def test_get_pxe_conf_option_iscsi_deploy(self): + self.config(group='pxe', pxe_config_template='my-pxe-config-template') + self._test_get_pxe_conf_option('fake_pxe', + 'my-pxe-config-template') + + def test_get_pxe_conf_option_agent_deploy_default(self): + self.config(group='pxe', pxe_config_template='my-pxe-config-template') + self._test_get_pxe_conf_option('fake_agent', + 'my-pxe-config-template') + + def test_get_pxe_conf_option_agent_deploy_not_default(self): + self.config(group='agent', + agent_pxe_config_template='my-agent-config-template') + self.config(group='pxe', pxe_config_template='my-pxe-config-template') + self._test_get_pxe_conf_option('fake_agent', + 'my-agent-config-template') + def test__parse_driver_info_missing_deploy_kernel(self): del self.node.driver_info['deploy_kernel'] self.assertRaises(exception.MissingParameterValue, @@ -230,7 +258,10 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase): self.config(api_url='http://192.168.122.184:6385', group='conductor') self.config(disk_devices='sda', group='pxe') - self.node.driver_internal_info['is_whole_disk_image'] = whle_dsk_img + driver_internal_info = self.node.driver_internal_info + driver_internal_info['is_whole_disk_image'] = whle_dsk_img + self.node.driver_internal_info = driver_internal_info + self.node.save() tftp_server = CONF.pxe.tftp_server @@ -286,9 +317,10 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase): os.path.join(root_dir, self.node.uuid, 'ramdisk'))} - options = pxe._build_pxe_config_options(self.node, - image_info, - self.context) + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + options = pxe._build_pxe_config_options(task, image_info) self.assertEqual(expected_options, options) def test__build_pxe_config_options(self): @@ -351,10 +383,14 @@ class PXEPrivateMethodsTestCase(db_base.DbTestCase): self.node.uuid, 'deploy_ramdisk')), } - self.node.driver_internal_info['is_whole_disk_image'] = True - options = pxe._build_pxe_config_options(self.node, - image_info, - self.context) + driver_internal_info = self.node.driver_internal_info + driver_internal_info['is_whole_disk_image'] = True + self.node.driver_internal_info = driver_internal_info + self.node.save() + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + options = pxe._build_pxe_config_options(task, image_info) self.assertEqual(expected_options, options) @mock.patch.object(deploy_utils, 'fetch_images', autospec=True)