Refactor agent {prepare,tear_down}_cleaning into deploy_utils

This commit refactors code from AgentDeploy.prepare_cleaning and
AgentDeploy.tear_down_cleaning into a common place so that it
can be reused for ISCSIDeploy. It also moves the method
agent.build_agent_options to deploy_utils module.

The following CONF options have been deprecated and renamed:
[agent]agent_erase_devices_priority => [deploy]erase_devices_priority
[agent]agent_erase_devices_iterations => [deploy]erase_devices_iterations

The old CONF options will be removed in the Mitaka release.

Change-Id: I818c9f81f5469bf5029495dc9b65259718c41cd5
Implements: blueprint iscsi-deploy-in-band-cleaning

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]
+  [deploy]
-  agent_erase_devices_priority=0
+  erase_devices_priority=0
 
 To enable/disable the in-band disk erase using ``agent_ilo`` driver, use the
 following config::

etc/ironic/ironic.conf.sample

@@ -1,33 +1,5 @@
 [DEFAULT]
 
-#
-# Options defined in oslo.service.service
-#
-
-# Enable eventlet backdoor.  Acceptable values are 0, <port>,
-# and <start>:<end>, where 0 results in listening on a random
-# tcp port number; <port> results in listening on the
-# specified port number (and not enabling backdoor if that
-# port is in use); and <start>:<end> 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=<None>
-
-# 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, <port>,
+# and <start>:<end>, where 0 results in listening on a random
+# tcp port number; <port> results in listening on the
+# specified port number (and not enabling backdoor if that
+# port is in use); and <start>:<end> 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=<None>
+
+# 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=<None>
-
-# 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=<None>
+
+# Number of iterations to be run for erasing devices. (integer
+# value)
+# Deprecated group/name - [agent]/agent_erase_devices_iterations
+#erase_devices_iterations=1
+
 
 [dhcp]
 

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()
+        return deploy_utils.prepare_inband_cleaning(
-        # If we have left over ports from a previous cleaning, remove them
+            task, manage_boot=CONF.agent.manage_agent_boot)
-        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
 
     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)
+        deploy_utils.tear_down_inband_cleaning(
-        if CONF.agent.manage_agent_boot:
+            task, manage_boot=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()
 
 
 class AgentVendorInterface(agent_base_vendor.BaseAgentVendor):

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)

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

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

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)

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)

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):

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):
 

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)

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)