diff --git a/doc/source/deploy/cleaning.rst b/doc/source/deploy/cleaning.rst index 16d3ffc439..556c379a09 100644 --- a/doc/source/deploy/cleaning.rst +++ b/doc/source/deploy/cleaning.rst @@ -91,8 +91,8 @@ currently requires use of a custom HardwareManager. The only exception is erase_devices, which can have its priority set in ironic.conf. For instance, to disable erase_devices, you'd use the following config:: - [agent] - agent_erase_devices_priority=0 + [deploy] + erase_devices_priority=0 To enable/disable the in-band disk erase using ``agent_ilo`` driver, use the following config:: diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 56b6d27c8c..dbbf766cc5 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -1,33 +1,5 @@ [DEFAULT] -# -# Options defined in oslo.service.service -# - -# Enable eventlet backdoor. Acceptable values are 0, , -# and :, where 0 results in listening on a random -# tcp port number; results in listening on the -# specified port number (and not enabling backdoor if that -# port is in use); and : results in listening on -# the smallest unused port number within the specified range -# of port numbers. The chosen port is displayed in the -# service's log file. (string value) -#backdoor_port= - -# Enables or disables logging values of all registered options -# when starting a service (at DEBUG level). (boolean value) -#log_options=true - - -# -# Options defined in oslo.service.periodic_task -# - -# Some periodic tasks can be run in a separate process. Should -# we run them here? (boolean value) -#run_external_periodic_tasks=true - - # # Options defined in oslo.log # @@ -121,6 +93,34 @@ #fatal_deprecations=false +# +# Options defined in oslo.service.service +# + +# Enable eventlet backdoor. Acceptable values are 0, , +# and :, where 0 results in listening on a random +# tcp port number; results in listening on the +# specified port number (and not enabling backdoor if that +# port is in use); and : results in listening on +# the smallest unused port number within the specified range +# of port numbers. The chosen port is displayed in the +# service's log file. (string value) +#backdoor_port= + +# Enables or disables logging values of all registered options +# when starting a service (at DEBUG level). (boolean value) +#log_options=true + + +# +# Options defined in oslo.service.periodic_task +# + +# Some periodic tasks can be run in a separate process. Should +# we run them here? (boolean value) +#run_external_periodic_tasks=true + + # # Options defined in oslo.messaging # @@ -363,16 +363,6 @@ # 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 -# Agent ramdisk. If unset, will use the priority set in the -# ramdisk (defaults to 10 for the GenericHardwareManager). If -# set to 0, will not run during cleaning. (integer value) -#agent_erase_devices_priority= - -# Number of iterations to be run for erasing devices. (integer -# value) -#agent_erase_devices_iterations=1 - # 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) @@ -818,6 +808,18 @@ # ironic-conductor node's HTTP root path. (string value) #http_root=/httpboot +# Priority to run in-band erase devices via the Ironic Python +# Agent ramdisk. If unset, will use the priority set in the +# ramdisk (defaults to 10 for the GenericHardwareManager). If +# set to 0, will not run during cleaning. (integer value) +# Deprecated group/name - [agent]/agent_erase_devices_priority +#erase_devices_priority= + +# Number of iterations to be run for erasing devices. (integer +# value) +# Deprecated group/name - [agent]/agent_erase_devices_iterations +#erase_devices_iterations=1 + [dhcp] diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index a3bb1bf3ae..17eaaa7e10 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -19,7 +19,6 @@ from oslo_log import log from oslo_utils import excutils from oslo_utils import units -from ironic.common import dhcp_factory from ironic.common import exception from ironic.common.glance_service import service_utils from ironic.common.i18n import _ @@ -28,7 +27,6 @@ from ironic.common.i18n import _LI from ironic.common.i18n import _LW from ironic.common import image_service from ironic.common import images -from ironic.common import keystone from ironic.common import paths from ironic.common import raid from ironic.common import states @@ -54,15 +52,6 @@ agent_opts = [ '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 ' - 'set in the ramdisk (defaults to 10 for the ' - 'GenericHardwareManager). If set to 0, will not run ' - 'during cleaning.')), - cfg.IntOpt('agent_erase_devices_iterations', - default=1, - help=_('Number of iterations to be run for erasing devices.')), cfg.BoolOpt('manage_agent_boot', default=True, deprecated_name='manage_tftp', @@ -83,6 +72,8 @@ agent_opts = [ CONF = cfg.CONF CONF.import_opt('my_ip', 'ironic.netconf') +CONF.import_opt('erase_devices_priority', + 'ironic.drivers.modules.deploy_utils', group='deploy') CONF.register_opts(agent_opts, group='agent') LOG = log.getLogger(__name__) @@ -107,28 +98,6 @@ def _get_client(): return client -def build_agent_options(node): - """Build the options to be passed to the agent ramdisk. - - :param node: an ironic node object - :returns: a dictionary containing the parameters to be passed to - agent ramdisk. - """ - ironic_api = (CONF.conductor.api_url or - keystone.get_service_url()).rstrip('/') - agent_config_opts = { - 'ipa-api-url': ironic_api, - 'ipa-driver-name': node.driver, - # NOTE: The below entry is a temporary workaround for bug/1433812 - 'coreos.configdrive': 0, - } - root_device = deploy_utils.parse_root_device_hints(node) - if root_device: - agent_config_opts['root_device'] = root_device - - return agent_config_opts - - def build_instance_info_for_deploy(task): """Build instance_info necessary for deploying to a node. @@ -289,7 +258,7 @@ class AgentDeploy(base.DeployInterface): node.instance_info = build_instance_info_for_deploy(task) node.save() if CONF.agent.manage_agent_boot: - deploy_opts = build_agent_options(node) + deploy_opts = deploy_utils.build_agent_options(node) task.driver.boot.prepare_ramdisk(task, deploy_opts) def clean_up(self, task): @@ -330,12 +299,12 @@ class AgentDeploy(base.DeployInterface): :returns: A list of clean step dictionaries """ steps = deploy_utils.agent_get_clean_steps(task) - if CONF.agent.agent_erase_devices_priority is not None: + if CONF.deploy.erase_devices_priority is not None: for step in steps: if (step.get('step') == 'erase_devices' and step.get('interface') == 'deploy'): # Override with operator set priority - step['priority'] = CONF.agent.agent_erase_devices_priority + step['priority'] = CONF.deploy.erase_devices_priority return steps def execute_clean_step(self, task, step): @@ -357,47 +326,8 @@ class AgentDeploy(base.DeployInterface): be removed or if new cleaning ports cannot be created :returns: states.CLEANWAIT to signify an asynchronous prepare """ - provider = dhcp_factory.DHCPFactory() - # If we have left over ports from a previous cleaning, remove them - if getattr(provider.provider, 'delete_cleaning_ports', None): - # Allow to raise if it fails, is caught and handled in conductor - provider.provider.delete_cleaning_ports(task) - - # Create cleaning ports if necessary - 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) - - 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 + return deploy_utils.prepare_inband_cleaning( + task, manage_boot=CONF.agent.manage_agent_boot) def tear_down_cleaning(self, task): """Clean up the PXE and DHCP files after cleaning. @@ -406,22 +336,8 @@ class AgentDeploy(base.DeployInterface): :raises NodeCleaningFailure: if the cleaning ports cannot be removed """ - manager_utils.node_power_action(task, states.POWER_OFF) - if CONF.agent.manage_agent_boot: - task.driver.boot.clean_up_ramdisk(task) - - # If we created cleaning ports, delete them - provider = dhcp_factory.DHCPFactory() - if getattr(provider.provider, 'delete_cleaning_ports', None): - # 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() + deploy_utils.tear_down_inband_cleaning( + task, manage_boot=CONF.agent.manage_agent_boot) class AgentVendorInterface(agent_base_vendor.BaseAgentVendor): diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index fb802b8254..788e0b59c5 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -36,6 +36,7 @@ import requests import six from six.moves.urllib import parse +from ironic.common import dhcp_factory from ironic.common import disk_partitioner from ironic.common import exception from ironic.common.i18n import _ @@ -44,6 +45,7 @@ from ironic.common.i18n import _LI from ironic.common.i18n import _LW from ironic.common import image_service from ironic.common import images +from ironic.common import keystone from ironic.common import states from ironic.common import utils from ironic.conductor import utils as manager_utils @@ -73,6 +75,21 @@ deploy_opts = [ default='/httpboot', help='ironic-conductor node\'s HTTP root path.', deprecated_group='pxe'), + # TODO(rameshg87): Remove the deprecated names for the below two options in + # Mitaka release. + cfg.IntOpt('erase_devices_priority', + deprecated_name='agent_erase_devices_priority', + deprecated_group='agent', + help=_('Priority to run in-band erase devices via the Ironic ' + 'Python Agent ramdisk. If unset, will use the priority ' + 'set in the ramdisk (defaults to 10 for the ' + 'GenericHardwareManager). If set to 0, will not run ' + 'during cleaning.')), + cfg.IntOpt('erase_devices_iterations', + deprecated_name='agent_erase_devices_iterations', + deprecated_group='agent', + default=1, + help=_('Number of iterations to be run for erasing devices.')), ] CONF = cfg.CONF CONF.register_opts(deploy_opts, group='deploy') @@ -1006,9 +1023,8 @@ def agent_add_clean_params(task): :param task: a TaskManager instance. """ - agent_params = CONF.agent info = task.node.driver_internal_info - passes = agent_params.agent_erase_devices_iterations + passes = CONF.deploy.erase_devices_iterations info['agent_erase_devices_iterations'] = passes task.node.driver_internal_info = info task.node.save() @@ -1262,3 +1278,153 @@ def get_boot_option(node): """ capabilities = parse_instance_info_capabilities(node) return capabilities.get('boot_option', 'netboot').lower() + + +def prepare_cleaning_ports(task): + """Prepare the Ironic ports of the node for cleaning. + + This method deletes the cleaning ports currently existing + for all the ports of the node and then creates a new one + for each one of them. It also adds 'vif_port_id' to port.extra + of each Ironic port, after creating the cleaning ports. + + :param task: a TaskManager object containing the node + :raises NodeCleaningFailure: if the previous cleaning ports cannot + be removed or if new cleaning ports cannot be created + """ + provider = dhcp_factory.DHCPFactory() + # If we have left over ports from a previous cleaning, remove them + if getattr(provider.provider, 'delete_cleaning_ports', None): + # Allow to raise if it fails, is caught and handled in conductor + provider.provider.delete_cleaning_ports(task) + + # Create cleaning ports if necessary + 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() + + +def tear_down_cleaning_ports(task): + """Deletes the cleaning ports created for each of the Ironic ports. + + This method deletes the cleaning port created before cleaning + was started. + + :param task: a TaskManager object containing the node + :raises NodeCleaningFailure: if the cleaning ports cannot be + removed. + """ + # If we created cleaning ports, delete them + provider = dhcp_factory.DHCPFactory() + if getattr(provider.provider, 'delete_cleaning_ports', None): + # 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() + + +def build_agent_options(node): + """Build the options to be passed to the agent ramdisk. + + :param node: an ironic node object + :returns: a dictionary containing the parameters to be passed to + agent ramdisk. + """ + ironic_api = (CONF.conductor.api_url or + keystone.get_service_url()).rstrip('/') + agent_config_opts = { + 'ipa-api-url': ironic_api, + 'ipa-driver-name': node.driver, + # NOTE: The below entry is a temporary workaround for bug/1433812 + 'coreos.configdrive': 0, + } + root_device = parse_root_device_hints(node) + if root_device: + agent_config_opts['root_device'] = root_device + + return agent_config_opts + + +def prepare_inband_cleaning(task, manage_boot=True): + """Prepares the node to boot into agent for in-band cleaning. + + This method does the following: + 1. Prepares the cleaning ports for the bare metal + node and updates the clean parameters in node's driver_internal_info. + 2. If 'manage_boot' parameter is set to true, it also calls the + 'prepare_ramdisk' method of boot interface to boot the agent ramdisk. + 3. Reboots the bare metal node. + + :param task: a TaskManager object containing the node + :param manage_boot: If this is set to True, this method calls the + 'prepare_ramdisk' method of boot interface to boot the agent + ramdisk. If False, it skips preparing the boot agent ramdisk using + boot interface, and assumes that the environment is setup to + automatically boot agent ramdisk every time bare metal node is + rebooted. + :returns: states.CLEANWAIT to signify an asynchronous prepare. + :raises NodeCleaningFailure: if the previous cleaning ports cannot + be removed or if new cleaning ports cannot be created + """ + prepare_cleaning_ports(task) + + # Append required config parameters to node's driver_internal_info + # to pass to IPA. + agent_add_clean_params(task) + + if manage_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 + + +def tear_down_inband_cleaning(task, manage_boot=True): + """Tears down the environment setup for in-band cleaning. + + This method does the following: + 1. Powers off the bare metal node. + 2. If 'manage_boot' parameter is set to true, it also + calls the 'clean_up_ramdisk' method of boot interface to clean up + the environment that was set for booting agent ramdisk. + 3. Deletes the cleaning ports which were setup as part + of cleaning. + + :param task: a TaskManager object containing the node + :param manage_boot: If this is set to True, this method calls the + 'clean_up_ramdisk' method of boot interface to boot the agent + ramdisk. If False, it skips this step. + :raises NodeCleaningFailure: if the cleaning ports cannot be + removed. + """ + manager_utils.node_power_action(task, states.POWER_OFF) + if manage_boot: + task.driver.boot.clean_up_ramdisk(task) + + tear_down_cleaning_ports(task) diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py index f68e68db8e..388ca6e07a 100644 --- a/ironic/drivers/modules/ilo/deploy.py +++ b/ironic/drivers/modules/ilo/deploy.py @@ -343,7 +343,7 @@ def _prepare_agent_vmedia_boot(task): # during deploy. ilo_common.eject_vmedia_devices(task) - deploy_ramdisk_opts = agent.build_agent_options(task.node) + deploy_ramdisk_opts = deploy_utils.build_agent_options(task.node) deploy_iso = task.node.driver_info['ilo_deploy_iso'] _reboot_into(task, deploy_iso, deploy_ramdisk_opts) @@ -515,7 +515,7 @@ class IloVirtualMediaIscsiDeploy(base.DeployInterface): iscsi_deploy.check_image_size(task) deploy_ramdisk_opts = iscsi_deploy.build_deploy_ramdisk_options(node) - agent_opts = agent.build_agent_options(node) + agent_opts = deploy_utils.build_agent_options(node) deploy_ramdisk_opts.update(agent_opts) deploy_nic_mac = deploy_utils.get_single_nic_with_vif_port_id(task) deploy_ramdisk_opts['BOOTIF'] = deploy_nic_mac diff --git a/ironic/drivers/modules/irmc/deploy.py b/ironic/drivers/modules/irmc/deploy.py index 3f94766958..8177bfe372 100644 --- a/ironic/drivers/modules/irmc/deploy.py +++ b/ironic/drivers/modules/irmc/deploy.py @@ -602,7 +602,7 @@ class IRMCVirtualMediaIscsiDeploy(base.DeployInterface): iscsi_deploy.check_image_size(task) deploy_ramdisk_opts = iscsi_deploy.build_deploy_ramdisk_options(node) - agent_opts = agent.build_agent_options(node) + agent_opts = deploy_utils.build_agent_options(node) deploy_ramdisk_opts.update(agent_opts) deploy_nic_mac = deploy_utils.get_single_nic_with_vif_port_id(task) deploy_ramdisk_opts['BOOTIF'] = deploy_nic_mac @@ -706,7 +706,7 @@ class IRMCVirtualMediaAgentDeploy(base.DeployInterface): image. :raises: IRMCOperationError, if some operation on iRMC fails. """ - deploy_ramdisk_opts = agent.build_agent_options(task.node) + deploy_ramdisk_opts = deploy_utils.build_agent_options(task.node) _reboot_into_deploy_iso(task, deploy_ramdisk_opts) return states.DEPLOYWAIT diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 69b6ac0256..02915ba0bf 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -33,7 +33,6 @@ 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 from ironic.drivers.modules import agent_base_vendor from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import image_cache @@ -721,7 +720,7 @@ class ISCSIDeploy(base.DeployInterface): # NOTE(lucasagomes): We are going to extend the normal PXE config # to also contain the agent options so it could be used for # both the DIB ramdisk and the IPA ramdisk - agent_opts = agent.build_agent_options(node) + agent_opts = deploy_utils.build_agent_options(node) deploy_opts.update(agent_opts) task.driver.boot.prepare_ramdisk(task, deploy_opts) diff --git a/ironic/tests/drivers/ilo/test_deploy.py b/ironic/tests/drivers/ilo/test_deploy.py index 6b13902c5f..df886cd090 100644 --- a/ironic/tests/drivers/ilo/test_deploy.py +++ b/ironic/tests/drivers/ilo/test_deploy.py @@ -362,7 +362,7 @@ class IloDeployPrivateMethodsTestCase(db_base.DbTestCase): spec_set=True, autospec=True) @mock.patch.object(ilo_deploy, '_reboot_into', spec_set=True, autospec=True) - @mock.patch.object(agent, 'build_agent_options', spec_set=True, + @mock.patch.object(deploy_utils, 'build_agent_options', spec_set=True, autospec=True) def test__prepare_agent_vmedia_boot(self, build_options_mock, reboot_into_mock, eject_mock): @@ -734,7 +734,7 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase): autospec=True) @mock.patch.object(deploy_utils, 'get_single_nic_with_vif_port_id', spec_set=True, autospec=True) - @mock.patch.object(agent, 'build_agent_options', spec_set=True, + @mock.patch.object(deploy_utils, 'build_agent_options', spec_set=True, autospec=True) @mock.patch.object(iscsi_deploy, 'build_deploy_ramdisk_options', spec_set=True, autospec=True) diff --git a/ironic/tests/drivers/irmc/test_deploy.py b/ironic/tests/drivers/irmc/test_deploy.py index 519a1b4596..36fd2b0143 100644 --- a/ironic/tests/drivers/irmc/test_deploy.py +++ b/ironic/tests/drivers/irmc/test_deploy.py @@ -904,7 +904,7 @@ class IRMCVirtualMediaIscsiDeployTestCase(db_base.DbTestCase): spec_set=True, autospec=True) @mock.patch.object(deploy_utils, 'get_single_nic_with_vif_port_id', spec_set=True, autospec=True) - @mock.patch.object(agent, 'build_agent_options', spec_set=True, + @mock.patch.object(deploy_utils, 'build_agent_options', spec_set=True, autospec=True) @mock.patch.object(iscsi_deploy, 'build_deploy_ramdisk_options', spec_set=True, autospec=True) @@ -1004,7 +1004,7 @@ class IRMCVirtualMediaAgentDeployTestCase(db_base.DbTestCase): @mock.patch.object(irmc_deploy, '_reboot_into_deploy_iso', spec_set=True, autospec=True) - @mock.patch.object(agent, 'build_agent_options', spec_set=True, + @mock.patch.object(deploy_utils, 'build_agent_options', spec_set=True, autospec=True) def test_deploy(self, build_agent_options_mock, _reboot_into_deploy_iso_mock): diff --git a/ironic/tests/drivers/test_agent.py b/ironic/tests/drivers/test_agent.py index 5648da8d25..aadf1dcb99 100644 --- a/ironic/tests/drivers/test_agent.py +++ b/ironic/tests/drivers/test_agent.py @@ -20,7 +20,6 @@ from oslo_config import cfg from ironic.common import exception from ironic.common import image_service from ironic.common import images -from ironic.common import keystone from ironic.common import raid from ironic.common import states from ironic.conductor import task_manager @@ -50,31 +49,6 @@ class TestAgentMethods(db_base.DbTestCase): self.node = object_utils.create_test_node(self.context, driver='fake_agent') - def test_build_agent_options_conf(self): - self.config(api_url='api-url', group='conductor') - options = agent.build_agent_options(self.node) - self.assertEqual('api-url', options['ipa-api-url']) - self.assertEqual('fake_agent', options['ipa-driver-name']) - self.assertEqual(0, options['coreos.configdrive']) - - @mock.patch.object(keystone, 'get_service_url', autospec=True) - def test_build_agent_options_keystone(self, get_url_mock): - - self.config(api_url=None, group='conductor') - get_url_mock.return_value = 'api-url' - options = agent.build_agent_options(self.node) - self.assertEqual('api-url', options['ipa-api-url']) - self.assertEqual('fake_agent', options['ipa-driver-name']) - self.assertEqual(0, options['coreos.configdrive']) - - def test_build_agent_options_root_device_hints(self): - self.config(api_url='api-url', group='conductor') - self.node.properties['root_device'] = {'model': 'fake_model'} - options = agent.build_agent_options(self.node) - self.assertEqual('api-url', options['ipa-api-url']) - self.assertEqual('fake_agent', options['ipa-driver-name']) - self.assertEqual('model=fake_model', options['root_device']) - @mock.patch.object(image_service, 'GlanceImageService', autospec=True) def test_build_instance_info_for_deploy_glance_image(self, glance_mock): i_info = self.node.instance_info @@ -302,7 +276,7 @@ class TestAgentDeploy(db_base.DbTestCase): self.assertEqual(driver_return, states.DELETED) @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk') - @mock.patch.object(agent, 'build_agent_options') + @mock.patch.object(deploy_utils, '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): @@ -323,7 +297,7 @@ class TestAgentDeploy(db_base.DbTestCase): 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(deploy_utils, '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, @@ -344,7 +318,7 @@ class TestAgentDeploy(db_base.DbTestCase): 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(deploy_utils, 'build_agent_options') @mock.patch.object(agent, 'build_instance_info_for_deploy') def test_prepare_active( self, build_instance_info_mock, build_options_mock, @@ -374,88 +348,6 @@ class TestAgentDeploy(db_base.DbTestCase): 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) - 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)) - 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.conductor.utils.node_power_action', autospec=True) - 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) - 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) def test_get_clean_steps(self, mock_get_clean_steps): @@ -473,7 +365,7 @@ class TestAgentDeploy(db_base.DbTestCase): def test_get_clean_steps_config_priority(self, mock_get_clean_steps): # Test that we can override the priority of get clean steps # Use 0 because it is an edge case (false-y) and used in devstack - self.config(agent_erase_devices_priority=0, group='agent') + self.config(erase_devices_priority=0, group='deploy') mock_steps = [{'priority': 10, 'interface': 'deploy', 'step': 'erase_devices'}] expected_steps = [{'priority': 0, 'interface': 'deploy', @@ -484,6 +376,44 @@ class TestAgentDeploy(db_base.DbTestCase): mock_get_clean_steps.assert_called_once_with(task) self.assertEqual(expected_steps, steps) + @mock.patch.object(deploy_utils, 'prepare_inband_cleaning', autospec=True) + def test_prepare_cleaning(self, prepare_inband_cleaning_mock): + prepare_inband_cleaning_mock.return_value = states.CLEANWAIT + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertEqual( + states.CLEANWAIT, self.driver.prepare_cleaning(task)) + prepare_inband_cleaning_mock.assert_called_once_with( + task, manage_boot=True) + + @mock.patch.object(deploy_utils, 'prepare_inband_cleaning', autospec=True) + def test_prepare_cleaning_manage_agent_boot_false( + self, prepare_inband_cleaning_mock): + prepare_inband_cleaning_mock.return_value = states.CLEANWAIT + self.config(group='agent', manage_agent_boot=False) + with task_manager.acquire(self.context, self.node.uuid) as task: + self.assertEqual( + states.CLEANWAIT, self.driver.prepare_cleaning(task)) + prepare_inband_cleaning_mock.assert_called_once_with( + task, manage_boot=False) + + @mock.patch.object(deploy_utils, 'tear_down_inband_cleaning', + autospec=True) + def test_tear_down_cleaning(self, tear_down_cleaning_mock): + with task_manager.acquire(self.context, self.node.uuid) as task: + self.driver.tear_down_cleaning(task) + tear_down_cleaning_mock.assert_called_once_with( + task, manage_boot=True) + + @mock.patch.object(deploy_utils, 'tear_down_inband_cleaning', + autospec=True) + def test_tear_down_cleaning_manage_agent_boot_false( + self, tear_down_cleaning_mock): + self.config(group='agent', manage_agent_boot=False) + with task_manager.acquire(self.context, self.node.uuid) as task: + self.driver.tear_down_cleaning(task) + tear_down_cleaning_mock.assert_called_once_with( + task, manage_boot=False) + class TestAgentVendor(db_base.DbTestCase): diff --git a/ironic/tests/drivers/test_deploy_utils.py b/ironic/tests/drivers/test_deploy_utils.py index 7b7f22f97f..bbb163f16d 100644 --- a/ironic/tests/drivers/test_deploy_utils.py +++ b/ironic/tests/drivers/test_deploy_utils.py @@ -35,6 +35,7 @@ from ironic.common import disk_partitioner from ironic.common import exception from ironic.common import image_service from ironic.common import images +from ironic.common import keystone from ironic.common import states from ironic.common import utils as common_utils from ironic.conductor import task_manager @@ -1877,9 +1878,10 @@ class TrySetBootDeviceTestCase(db_base.DbTestCase): task, boot_devices.DISK, persistent=True) -class AgentCleaningTestCase(db_base.DbTestCase): +class AgentMethodsTestCase(db_base.DbTestCase): + def setUp(self): - super(AgentCleaningTestCase, self).setUp() + super(AgentMethodsTestCase, self).setUp() mgr_utils.mock_the_extension_manager(driver='fake_agent') n = {'driver': 'fake_agent', 'driver_internal_info': {'agent_url': 'http://127.0.0.1:9999'}} @@ -2001,13 +2003,136 @@ class AgentCleaningTestCase(db_base.DbTestCase): self.assertEqual(states.CLEANWAIT, response) def test_agent_add_clean_params(self): - cfg.CONF.agent.agent_erase_devices_iterations = 2 + cfg.CONF.deploy.erase_devices_iterations = 2 with task_manager.acquire( self.context, self.node['uuid'], shared=False) as task: utils.agent_add_clean_params(task) self.assertEqual(task.node.driver_internal_info.get( 'agent_erase_devices_iterations'), 2) + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports', + autospec=True) + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.create_cleaning_ports', + autospec=True) + def _test_prepare_inband_cleaning_ports( + self, create_mock, delete_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 = {} + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + utils.prepare_cleaning_ports(task) + create_mock.assert_called_once_with(mock.ANY, task) + delete_mock.assert_called_once_with(mock.ANY, task) + + self.ports[0].refresh() + self.assertEqual('vif-port-id', self.ports[0].extra['vif_port_id']) + + def test_prepare_inband_cleaning_ports(self): + self._test_prepare_inband_cleaning_ports() + + def test_prepare_inband_cleaning_ports_no_vif_port_id(self): + self.assertRaises( + exception.NodeCleaningFailure, + self._test_prepare_inband_cleaning_ports, + return_vif_port_id=False) + + @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports', + autospec=True) + def test_tear_down_inband_cleaning_ports(self, neutron_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: + utils.tear_down_cleaning_ports(task) + neutron_mock.assert_called_once_with(mock.ANY, task) + + self.ports[0].refresh() + self.assertNotIn('vif_port_id', self.ports[0].extra) + + @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True) + @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) + @mock.patch.object(utils, 'build_agent_options', autospec=True) + @mock.patch.object(utils, 'prepare_cleaning_ports', autospec=True) + def _test_prepare_inband_cleaning( + self, prepare_cleaning_ports_mock, + build_options_mock, power_mock, prepare_ramdisk_mock, + manage_boot=True): + build_options_mock.return_value = {'a': 'b'} + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.assertEqual( + states.CLEANWAIT, + utils.prepare_inband_cleaning(task, manage_boot=manage_boot)) + prepare_cleaning_ports_mock.assert_called_once_with(task) + power_mock.assert_called_once_with(task, states.REBOOT) + self.assertEqual(task.node.driver_internal_info.get( + 'agent_erase_devices_iterations'), 1) + if manage_boot: + prepare_ramdisk_mock.assert_called_once_with( + mock.ANY, mock.ANY, {'a': 'b'}) + build_options_mock.assert_called_once_with(task.node) + else: + self.assertFalse(prepare_ramdisk_mock.called) + self.assertFalse(build_options_mock.called) + + def test_prepare_inband_cleaning(self): + self._test_prepare_inband_cleaning() + + def test_prepare_inband_cleaning_manage_boot_false(self): + self._test_prepare_inband_cleaning(manage_boot=False) + + @mock.patch.object(pxe.PXEBoot, 'clean_up_ramdisk', autospec=True) + @mock.patch.object(utils, 'tear_down_cleaning_ports', autospec=True) + @mock.patch('ironic.conductor.utils.node_power_action', autospec=True) + def _test_tear_down_inband_cleaning( + self, power_mock, tear_down_ports_mock, + clean_up_ramdisk_mock, manage_boot=True): + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + utils.tear_down_inband_cleaning(task, manage_boot=manage_boot) + power_mock.assert_called_once_with(task, states.POWER_OFF) + tear_down_ports_mock.assert_called_once_with(task) + if manage_boot: + clean_up_ramdisk_mock.assert_called_once_with( + task.driver.boot, task) + else: + self.assertFalse(clean_up_ramdisk_mock.called) + + def test_tear_down_inband_cleaning(self): + self._test_tear_down_inband_cleaning(manage_boot=True) + + def test_tear_down_inband_cleaning_manage_boot_false(self): + self._test_tear_down_inband_cleaning(manage_boot=False) + + def test_build_agent_options_conf(self): + self.config(api_url='api-url', group='conductor') + options = utils.build_agent_options(self.node) + self.assertEqual('api-url', options['ipa-api-url']) + self.assertEqual('fake_agent', options['ipa-driver-name']) + self.assertEqual(0, options['coreos.configdrive']) + + @mock.patch.object(keystone, 'get_service_url', autospec=True) + def test_build_agent_options_keystone(self, get_url_mock): + + self.config(api_url=None, group='conductor') + get_url_mock.return_value = 'api-url' + options = utils.build_agent_options(self.node) + self.assertEqual('api-url', options['ipa-api-url']) + self.assertEqual('fake_agent', options['ipa-driver-name']) + self.assertEqual(0, options['coreos.configdrive']) + + def test_build_agent_options_root_device_hints(self): + self.config(api_url='api-url', group='conductor') + self.node.properties['root_device'] = {'model': 'fake_model'} + options = utils.build_agent_options(self.node) + self.assertEqual('api-url', options['ipa-api-url']) + self.assertEqual('fake_agent', options['ipa-driver-name']) + self.assertEqual('model=fake_model', options['root_device']) + @mock.patch.object(utils, 'is_block_device', autospec=True) @mock.patch.object(utils, 'login_iscsi', lambda *_: None) diff --git a/ironic/tests/drivers/test_iscsi_deploy.py b/ironic/tests/drivers/test_iscsi_deploy.py index f1c913393d..5e381da9c5 100644 --- a/ironic/tests/drivers/test_iscsi_deploy.py +++ b/ironic/tests/drivers/test_iscsi_deploy.py @@ -31,7 +31,6 @@ 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.modules import agent from ironic.drivers.modules import agent_base_vendor from ironic.drivers.modules import agent_client from ironic.drivers.modules import deploy_utils @@ -993,7 +992,7 @@ class ISCSIDeployTestCase(db_base.DbTestCase): prepare_instance_mock.assert_called_once_with( task.driver.boot, task) - @mock.patch.object(agent, 'build_agent_options', autospec=True) + @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) @mock.patch.object(iscsi_deploy, 'build_deploy_ramdisk_options', autospec=True) @mock.patch.object(pxe.PXEBoot, 'prepare_ramdisk', autospec=True)