Merge "Refactor agent_ilo driver to use new boot interface"

ironic/drivers/ilo.py

@@ -72,6 +72,7 @@ class IloVirtualMediaAgentDriver(base.BaseDriver):
                 reason=_("Unable to import proliantutils library"))
 
         self.power = power.IloPower()
+        self.boot = boot.IloVirtualMediaBoot()
         self.deploy = deploy.IloVirtualMediaAgentDeploy()
         self.console = console.IloConsoleInterface()
         self.management = management.IloManagement()

ironic/drivers/modules/ilo/deploy.py

@@ -19,14 +19,12 @@ from oslo_config import cfg
 from oslo_log import log as logging
 
 from ironic.common import boot_devices
-from ironic.common import dhcp_factory
 from ironic.common import exception
 from ironic.common.i18n import _
 from ironic.common.i18n import _LW
 from ironic.common import states
 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 deploy_utils
 from ironic.drivers.modules.ilo import boot as ilo_boot
@@ -178,7 +176,7 @@ class IloVirtualMediaIscsiDeploy(iscsi_deploy.ISCSIDeploy):
         super(IloVirtualMediaIscsiDeploy, self).prepare(task)
 
 
-class IloVirtualMediaAgentDeploy(base.DeployInterface):
+class IloVirtualMediaAgentDeploy(agent.AgentDeploy):
     """Interface for deploy-related actions."""
 
     def get_properties(self):
@@ -188,71 +186,28 @@ class IloVirtualMediaAgentDeploy(base.DeployInterface):
         """
         return ilo_boot.COMMON_PROPERTIES
 
-    def validate(self, task):
-        """Validate the driver-specific Node deployment info.
-
-        :param task: a TaskManager instance
-        :raises: MissingParameterValue if some parameters are missing.
-        """
-
-        deploy_utils.validate_capabilities(task.node)
-        ilo_boot.parse_driver_info(task.node)
-
-    @task_manager.require_exclusive_lock
-    def deploy(self, task):
-        """Perform a deployment to a node.
-
-        Prepares the options for the agent ramdisk and sets the node to boot
-        from virtual media cdrom.
-
-        :param task: a TaskManager instance.
-        :returns: states.DEPLOYWAIT
-        :raises: ImageCreationFailed, if it failed while creating the floppy
-            image.
-        :raises: IloOperationError, if some operation on iLO fails.
-        """
-        _prepare_agent_vmedia_boot(task)
-
-        return states.DEPLOYWAIT
-
     @task_manager.require_exclusive_lock
     def tear_down(self, task):
         """Tear down a previous deployment on the task's node.
 
         :param task: a TaskManager instance.
         :returns: states.DELETED
+        :raises: IloOperationError, if some operation on iLO failed.
         """
         manager_utils.node_power_action(task, states.POWER_OFF)
         _disable_secure_boot_if_supported(task)
-        return states.DELETED
+        return super(IloVirtualMediaAgentDeploy, self).tear_down(task)
 
     def prepare(self, task):
         """Prepare the deployment environment for this node.
 
         :param task: a TaskManager instance.
+        :raises: IloOperationError, if some operation on iLO failed.
         """
         if task.node.provision_state != states.ACTIVE:
-            node = task.node
-            node.instance_info = agent.build_instance_info_for_deploy(task)
-            node.save()
             _prepare_node_for_deploy(task)
 
-    def clean_up(self, task):
+        super(IloVirtualMediaAgentDeploy, self).prepare(task)
-        """Clean up the deployment environment for this node.
-
-        Ejects the attached virtual media from the iLO and also removes
-        the floppy image from Swift, if it exists.
-
-        :param task: a TaskManager instance.
-        """
-        ilo_common.cleanup_vmedia_boot(task)
-
-    def take_over(self, task):
-        """Take over management of this node from a dead conductor.
-
-        :param task: a TaskManager instance.
-        """
-        pass
 
     def get_clean_steps(self, task):
         """Get the list of clean steps from the agent.
@@ -260,6 +215,11 @@ class IloVirtualMediaAgentDeploy(base.DeployInterface):
         :param task: a TaskManager object containing the node
         :returns: A list of clean step dictionaries
         """
+
+        # TODO(stendulker): All drivers use CONF.deploy.erase_devices_priority
+        # agent_ilo driver should also use the same. Defect has been filed for
+        # the same.
+        # https://bugs.launchpad.net/ironic/+bug/1515871
         new_priorities = {
             'erase_devices': CONF.ilo.clean_priority_erase_devices,
         }
@@ -267,43 +227,6 @@ class IloVirtualMediaAgentDeploy(base.DeployInterface):
             task, interface='deploy',
             override_priorities=new_priorities)
 
-    def execute_clean_step(self, task, step):
-        """Execute a clean step asynchronously on the agent.
-
-        :param task: a TaskManager object containing the node
-        :param step: a clean step dictionary to execute
-        :returns: states.CLEANWAIT to signify the step will be completed async
-        """
-        return deploy_utils.agent_execute_clean_step(task, step)
-
-    def prepare_cleaning(self, task):
-        """Boot into the agent to prepare for cleaning."""
-        # Create cleaning ports if necessary
-        provider = dhcp_factory.DHCPFactory().provider
-
-        # If we have left over ports from a previous cleaning, remove them
-        if getattr(provider, 'delete_cleaning_ports', None):
-            provider.delete_cleaning_ports(task)
-
-        if getattr(provider, 'create_cleaning_ports', None):
-            provider.create_cleaning_ports(task)
-
-        # Append required config parameters to node's driver_internal_info
-        # to pass to IPA.
-        deploy_utils.agent_add_clean_params(task)
-
-        _prepare_agent_vmedia_boot(task)
-        # 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."""
-        manager_utils.node_power_action(task, states.POWER_OFF)
-        # If we created cleaning ports, delete them
-        provider = dhcp_factory.DHCPFactory().provider
-        if getattr(provider, 'delete_cleaning_ports', None):
-            provider.delete_cleaning_ports(task)
-
 
 class IloPXEDeploy(iscsi_deploy.ISCSIDeploy):
 

ironic/drivers/modules/ilo/vendor.py

@@ -53,12 +53,6 @@ class IloVirtualMediaAgentVendorInterface(agent.AgentVendorInterface):
         super(IloVirtualMediaAgentVendorInterface,
               self).reboot_to_instance(task, **kwargs)
 
-    @task_manager.require_exclusive_lock
-    def continue_deploy(self, task, **kwargs):
-        ilo_common.cleanup_vmedia_boot(task)
-        super(IloVirtualMediaAgentVendorInterface,
-              self).continue_deploy(task, **kwargs)
-
 
 class VendorPassthru(iscsi_deploy.VendorPassthru):
     """Vendor-specific interfaces for iLO deploy drivers."""

ironic/tests/unit/drivers/modules/ilo/test_deploy.py

@@ -13,7 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-"""Test class for common methods used by iLO modules."""
+"""Test class for deploy methods used by iLO modules."""
 
 import mock
 from oslo_config import cfg
@@ -26,7 +26,6 @@ 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 deploy_utils
-from ironic.drivers.modules.ilo import boot as ilo_boot
 from ironic.drivers.modules.ilo import common as ilo_common
 from ironic.drivers.modules.ilo import deploy as ilo_deploy
 from ironic.drivers.modules import iscsi_deploy
@@ -53,32 +52,6 @@ class IloDeployPrivateMethodsTestCase(db_base.DbTestCase):
         self.node = obj_utils.create_test_node(
             self.context, driver='iscsi_ilo', driver_info=INFO_DICT)
 
-    @mock.patch.object(manager_utils, 'node_power_action',
-                       spec_set=True, autospec=True)
-    @mock.patch.object(ilo_common, 'eject_vmedia_devices',
-                       spec_set=True, autospec=True)
-    @mock.patch.object(ilo_common, 'setup_vmedia', spec_set=True,
-                       autospec=True)
-    @mock.patch.object(deploy_utils, 'build_agent_options', spec_set=True,
-                       autospec=True)
-    def test__prepare_agent_vmedia_boot(self, build_options_mock,
-                                        setup_media_mock, eject_mock,
-                                        power_action_mock):
-        deploy_opts = {'a': 'b'}
-        build_options_mock.return_value = deploy_opts
-        with task_manager.acquire(self.context, self.node.uuid,
-                                  shared=False) as task:
-            task.node.driver_info['ilo_deploy_iso'] = 'deploy-iso-uuid'
-
-            ilo_deploy._prepare_agent_vmedia_boot(task)
-
-            eject_mock.assert_called_once_with(task)
-            build_options_mock.assert_called_once_with(task.node)
-            setup_media_mock.assert_called_once_with(task,
-                                                     'deploy-iso-uuid',
-                                                     deploy_opts)
-            power_action_mock.assert_called_once_with(task, states.REBOOT)
-
     @mock.patch.object(ilo_common, 'set_secure_boot_mode', spec_set=True,
                        autospec=True)
     @mock.patch.object(ilo_common, 'get_secure_boot_mode', spec_set=True,
@@ -319,45 +292,28 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase):
         self.node = obj_utils.create_test_node(
             self.context, driver='agent_ilo', driver_info=INFO_DICT)
 
-    @mock.patch.object(deploy_utils, 'validate_capabilities',
+    @mock.patch.object(agent.AgentDeploy, 'tear_down', spec_set=True,
-                       spec_set=True, autospec=True)
-    @mock.patch.object(ilo_boot, 'parse_driver_info', spec_set=True,
                        autospec=True)
-    def test_validate(self,
-                      parse_driver_info_mock,
-                      validate_capability_mock):
-        with task_manager.acquire(self.context, self.node.uuid,
-                                  shared=False) as task:
-            task.driver.deploy.validate(task)
-            validate_capability_mock.assert_called_once_with(task.node)
-            parse_driver_info_mock.assert_called_once_with(task.node)
-
-    @mock.patch.object(ilo_deploy, '_prepare_agent_vmedia_boot', spec_set=True,
-                       autospec=True)
-    def test_deploy(self, vmedia_boot_mock):
-        with task_manager.acquire(self.context, self.node.uuid,
-                                  shared=False) as task:
-            returned_state = task.driver.deploy.deploy(task)
-            vmedia_boot_mock.assert_called_once_with(task)
-            self.assertEqual(states.DEPLOYWAIT, returned_state)
-
     @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True,
                        autospec=True)
     @mock.patch.object(manager_utils, 'node_power_action', spec_set=True,
                        autospec=True)
     def test_tear_down(self,
                        node_power_action_mock,
-                       update_secure_boot_mode_mock):
+                       update_secure_boot_mode_mock,
+                       agent_teardown_mock):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
+            agent_teardown_mock.return_value = states.DELETED
             returned_state = task.driver.deploy.tear_down(task)
             node_power_action_mock.assert_called_once_with(task,
                                                            states.POWER_OFF)
             update_secure_boot_mode_mock.assert_called_once_with(task, False)
             self.assertEqual(states.DELETED, returned_state)
 
-    @mock.patch.object(ilo_deploy.LOG, 'warning',
+    @mock.patch.object(agent.AgentDeploy, 'tear_down', spec_set=True,
-                       spec_set=True, autospec=True)
+                       autospec=True)
+    @mock.patch.object(ilo_deploy.LOG, 'warning', spec_set=True, autospec=True)
     @mock.patch.object(ilo_deploy, 'exception', spec_set=True, autospec=True)
     @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True,
                        autospec=True)
@@ -367,83 +323,47 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase):
                                         node_power_action_mock,
                                         update_secure_boot_mode_mock,
                                         exception_mock,
-                                        mock_log):
+                                        mock_log,
+                                        agent_teardown_mock):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
+            agent_teardown_mock.return_value = states.DELETED
             exception_mock.IloOperationNotSupported = Exception
             update_secure_boot_mode_mock.side_effect = Exception
             returned_state = task.driver.deploy.tear_down(task)
             node_power_action_mock.assert_called_once_with(task,
                                                            states.POWER_OFF)
             update_secure_boot_mode_mock.assert_called_once_with(task, False)
+            agent_teardown_mock.assert_called_once_with(mock.ANY, task)
             self.assertTrue(mock_log.called)
             self.assertEqual(states.DELETED, returned_state)
 
     @mock.patch.object(ilo_deploy, '_prepare_node_for_deploy', spec_set=True,
                        autospec=True)
-    @mock.patch.object(agent, 'build_instance_info_for_deploy', spec_set=True,
+    @mock.patch.object(agent.AgentDeploy, 'prepare', spec_set=True,
                        autospec=True)
     def test_prepare(self,
-                     build_instance_info_mock,
+                     agent_prepare_mock,
                      func_prepare_node_for_deploy):
-        deploy_opts = {'a': 'b'}
-        build_instance_info_mock.return_value = deploy_opts
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.driver.deploy.prepare(task)
-            self.assertEqual(deploy_opts, task.node.instance_info)
             func_prepare_node_for_deploy.assert_called_once_with(task)
+            agent_prepare_mock.assert_called_once_with(mock.ANY, task)
 
+    @mock.patch.object(agent.AgentDeploy, 'prepare', spec_set=True,
+                       autospec=True)
     @mock.patch.object(ilo_deploy, '_prepare_node_for_deploy', spec_set=True,
                        autospec=True)
-    @mock.patch.object(agent, 'build_instance_info_for_deploy', spec_set=True,
-                       autospec=True)
     def test_prepare_active_node(self,
-                                 build_instance_info_mock,
+                                 func_prepare_node_for_deploy,
-                                 func_prepare_node_for_deploy):
+                                 agent_prepare_mock):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.node.provision_state = states.ACTIVE
             task.driver.deploy.prepare(task)
-            self.assertFalse(build_instance_info_mock.called)
             self.assertFalse(func_prepare_node_for_deploy.called)
-
+            agent_prepare_mock.assert_called_once_with(mock.ANY, task)
-    @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports',
-                spec_set=True, autospec=True)
-    @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.create_cleaning_ports',
-                spec_set=True, autospec=True)
-    @mock.patch.object(ilo_deploy, '_prepare_agent_vmedia_boot', spec_set=True,
-                       autospec=True)
-    def test_prepare_cleaning(self, vmedia_boot_mock, create_port_mock,
-                              delete_mock):
-        with task_manager.acquire(self.context, self.node.uuid,
-                                  shared=False) as task:
-            returned_state = task.driver.deploy.prepare_cleaning(task)
-            vmedia_boot_mock.assert_called_once_with(task)
-            self.assertEqual(states.CLEANWAIT, returned_state)
-            create_port_mock.assert_called_once_with(mock.ANY, task)
-            delete_mock.assert_called_once_with(mock.ANY, task)
-            self.assertEqual(task.node.driver_internal_info.get(
-                             'agent_erase_devices_iterations'), 1)
-
-    @mock.patch('ironic.dhcp.neutron.NeutronDHCPApi.delete_cleaning_ports',
-                spec_set=True, autospec=True)
-    @mock.patch.object(manager_utils, 'node_power_action', spec_set=True,
-                       autospec=True)
-    def test_tear_down_cleaning(self, power_mock, delete_mock):
-        with task_manager.acquire(
-                self.context, self.node['uuid'], shared=False) as task:
-            task.driver.deploy.tear_down_cleaning(task)
-            power_mock.assert_called_once_with(task, states.POWER_OFF)
-            delete_mock.assert_called_once_with(mock.ANY, task)
-
-    @mock.patch.object(deploy_utils, 'agent_execute_clean_step', spec_set=True,
-                       autospec=True)
-    def test_execute_clean_step(self, execute_mock):
-        with task_manager.acquire(
-                self.context, self.node['uuid'], shared=False) as task:
-            task.driver.deploy.execute_clean_step(task, 'fake-step')
-            execute_mock.assert_called_once_with(task, 'fake-step')
 
     @mock.patch.object(deploy_utils, 'agent_get_clean_steps', spec_set=True,
                        autospec=True)

ironic/tests/unit/drivers/modules/ilo/test_vendor.py

@@ -206,18 +206,3 @@ class IloVirtualMediaAgentVendorInterfaceTestCase(db_base.DbTestCase):
             self.assertFalse(func_update_secure_boot_mode.called)
             agent_reboot_to_instance_mock.assert_called_once_with(
                 mock.ANY, task, **kwargs)
-
-    @mock.patch.object(ilo_common, 'cleanup_vmedia_boot',
-                       spec_set=True, autospec=True)
-    @mock.patch.object(agent.AgentVendorInterface, 'continue_deploy',
-                       spec_set=True, autospec=True)
-    def test_continue_deploy(self, agent_continue_deploy_mock,
-                             cleanup_mock):
-        CONF.ilo.use_web_server_for_images = True
-        kwargs = {'address': '123456'}
-        with task_manager.acquire(self.context, self.node.uuid,
-                                  shared=False) as task:
-            task.driver.vendor.continue_deploy(task, **kwargs)
-            cleanup_mock.assert_called_once_with(task)
-            agent_continue_deploy_mock.assert_called_once_with(
-                mock.ANY, task, **kwargs)