Remove agent vendor passthru from iLO drvers

Change-Id: Ic686eaa744bf2c9eac44fe3c437ebb67e6153489
Partial-Bug: #1640533

ironic/drivers/ilo.py

@@ -77,6 +77,5 @@ class IloVirtualMediaAgentDriver(base.BaseDriver):
         self.deploy = deploy.IloVirtualMediaAgentDeploy()
         self.console = console.IloConsoleInterface()
         self.management = management.IloManagement()
-        self.vendor = vendor.IloVirtualMediaAgentVendorInterface()
         self.inspect = inspect.IloInspect()
         self.raid = agent.AgentRAID()

ironic/drivers/modules/ilo/deploy.py

@@ -160,10 +160,56 @@ def _validate(task):
                 "the given URL is not reachable.") % deploy_iso)
 
 
-class IloVirtualMediaIscsiDeploy(iscsi_deploy.ISCSIDeploy):
+class IloIscsiDeployMixin(object):
 
-    def get_properties(self):
+    @METRICS.timer('IloIscsiDeployMixin.tear_down')
-        return {}
+    @task_manager.require_exclusive_lock
+    def tear_down(self, task):
+        """Tear down a previous deployment on the task's node.
+
+        Power off the node. All actual clean-up is done in the clean_up()
+        method which should be called separately.
+
+        :param task: a TaskManager instance containing the node to act on.
+        :returns: deploy state 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 super(IloIscsiDeployMixin, self).tear_down(task)
+
+    @METRICS.timer('IloIscsiDeployMixin.prepare_cleaning')
+    def prepare_cleaning(self, task):
+        """Boot into the agent to prepare for cleaning.
+
+        :param task: a TaskManager object containing the node
+        :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
+        :raises: IloOperationError, if some operation on iLO failed.
+        """
+        # Powering off the Node before initiating boot for node cleaning.
+        # If node is in system POST, setting boot device fails.
+        manager_utils.node_power_action(task, states.POWER_OFF)
+        return super(IloIscsiDeployMixin, self).prepare_cleaning(task)
+
+    @METRICS.timer('IloIscsiDeployMixin.continue_deploy')
+    @task_manager.require_exclusive_lock
+    def continue_deploy(self, task, **kwargs):
+        """Method invoked when deployed with the IPA ramdisk.
+
+        This method is invoked during a heartbeat from an agent when
+        the node is in wait-call-back state.
+        This updates boot mode and secure boot settings, if required.
+        """
+        ilo_common.update_boot_mode(task)
+        ilo_common.update_secure_boot_mode(task, True)
+        super(IloIscsiDeployMixin, self).continue_deploy(task, **kwargs)
+
+
+class IloVirtualMediaIscsiDeploy(IloIscsiDeployMixin,
+                                 iscsi_deploy.ISCSIDeploy):
 
     @METRICS.timer('IloVirtualMediaIscsiDeploy.validate')
     def validate(self, task):
@@ -180,23 +226,6 @@ class IloVirtualMediaIscsiDeploy(iscsi_deploy.ISCSIDeploy):
         _validate(task)
         super(IloVirtualMediaIscsiDeploy, self).validate(task)
 
-    @METRICS.timer('IloVirtualMediaIscsiDeploy.tear_down')
-    @task_manager.require_exclusive_lock
-    def tear_down(self, task):
-        """Tear down a previous deployment on the task's node.
-
-        Power off the node. All actual clean-up is done in the clean_up()
-        method which should be called separately.
-
-        :param task: a TaskManager instance containing the node to act on.
-        :returns: deploy state 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 super(IloVirtualMediaIscsiDeploy, self).tear_down(task)
-
     @METRICS.timer('IloVirtualMediaIscsiDeploy.prepare')
     def prepare(self, task):
         """Prepare the deployment environment for this task's node.
@@ -209,21 +238,6 @@ class IloVirtualMediaIscsiDeploy(iscsi_deploy.ISCSIDeploy):
 
         super(IloVirtualMediaIscsiDeploy, self).prepare(task)
 
-    @METRICS.timer('IloVirtualMediaIscsiDeploy.prepare_cleaning')
-    def prepare_cleaning(self, task):
-        """Boot into the agent to prepare for cleaning.
-
-        :param task: a TaskManager object containing the node
-        :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
-        :raises: IloOperationError, if some operation on iLO failed.
-        """
-        # Powering off the Node before initiating boot for node cleaning.
-        # If node is in system POST, setting boot device fails.
-        manager_utils.node_power_action(task, states.POWER_OFF)
-        return super(IloVirtualMediaIscsiDeploy, self).prepare_cleaning(task)
-
 
 class IloVirtualMediaAgentDeploy(agent.AgentDeploy):
     """Interface for deploy-related actions."""
@@ -314,8 +328,25 @@ class IloVirtualMediaAgentDeploy(agent.AgentDeploy):
             task, interface='deploy',
             override_priorities=new_priorities)
 
+    @METRICS.timer('IloVirtualMediaAgentDeploy.reboot_to_instance')
+    def reboot_to_instance(self, task):
+        node = task.node
+        LOG.debug('Preparing to reboot to instance for node %s',
+                  node.uuid)
 
-class IloPXEDeploy(iscsi_deploy.ISCSIDeploy):
+        error = self.check_deploy_success(node)
+        if error is None:
+            # Set boot mode
+            ilo_common.update_boot_mode(task)
+
+            # Need to enable secure boot, if being requested
+            ilo_common.update_secure_boot_mode(task, True)
+
+        super(IloVirtualMediaAgentDeploy,
+              self).reboot_to_instance(task)
+
+
+class IloPXEDeploy(IloIscsiDeployMixin, iscsi_deploy.ISCSIDeploy):
 
     @METRICS.timer('IloPXEDeploy.prepare')
     def prepare(self, task):
@@ -359,32 +390,3 @@ class IloPXEDeploy(iscsi_deploy.ISCSIDeploy):
         """
         manager_utils.node_set_boot_device(task, boot_devices.PXE)
         return super(IloPXEDeploy, self).deploy(task)
-
-    @METRICS.timer('IloPXEDeploy.tear_down')
-    @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
-        """
-        # Powering off the Node before disabling secure boot. If the node is
-        # in POST, disable secure boot will fail.
-        manager_utils.node_power_action(task, states.POWER_OFF)
-        _disable_secure_boot_if_supported(task)
-        return super(IloPXEDeploy, self).tear_down(task)
-
-    @METRICS.timer('IloPXEDeploy.prepare_cleaning')
-    def prepare_cleaning(self, task):
-        """Boot into the agent to prepare for cleaning.
-
-        :param task: a TaskManager object containing the node
-        :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
-        :raises: IloOperationError, if some operation on iLO failed.
-        """
-        # Powering off the Node before initiating boot for node cleaning.
-        # If node is in system POST, setting boot device fails.
-        manager_utils.node_power_action(task, states.POWER_OFF)
-        return super(IloPXEDeploy, self).prepare_cleaning(task)

ironic/drivers/modules/ilo/vendor.py

@@ -24,40 +24,20 @@ 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 common as ilo_common
-from ironic.drivers.modules import iscsi_deploy
 
 LOG = logging.getLogger(__name__)
 
 METRICS = metrics_utils.get_metrics_logger(__name__)
 
 
-class IloVirtualMediaAgentVendorInterface(agent.AgentVendorInterface):
+class VendorPassthru(base.VendorInterface):
-    """Interface for vendor passthru related actions."""
-
-    @METRICS.timer('IloVirtualMediaAgentVendorInterface.reboot_to_instance')
-    def reboot_to_instance(self, task):
-        node = task.node
-        LOG.debug('Preparing to reboot to instance for node %s',
-                  node.uuid)
-
-        error = self.check_deploy_success(node)
-        if error is None:
-            # Set boot mode
-            ilo_common.update_boot_mode(task)
-
-            # Need to enable secure boot, if being requested
-            ilo_common.update_secure_boot_mode(task, True)
-
-        super(IloVirtualMediaAgentVendorInterface,
-              self).reboot_to_instance(task)
-
-
-class VendorPassthru(iscsi_deploy.VendorPassthru):
     """Vendor-specific interfaces for iLO deploy drivers."""
 
+    def get_properties(self):
+        return {}
+
     @METRICS.timer('IloVendorPassthru.validate')
     def validate(self, task, method, **kwargs):
         """Validate vendor-specific actions.
@@ -79,19 +59,6 @@ class VendorPassthru(iscsi_deploy.VendorPassthru):
             return
         super(VendorPassthru, self).validate(task, method, **kwargs)
 
-    @METRICS.timer('IloVendorPassthru.continue_deploy')
-    @task_manager.require_exclusive_lock
-    def continue_deploy(self, task, **kwargs):
-        """Method invoked when deployed with the IPA ramdisk.
-
-        This method is invoked during a heartbeat from an agent when
-        the node is in wait-call-back state.
-        This updates boot mode and secure boot settings, if required.
-        """
-        ilo_common.update_boot_mode(task)
-        ilo_common.update_secure_boot_mode(task, True)
-        super(VendorPassthru, self).continue_deploy(task, **kwargs)
-
     def _validate_boot_into_iso(self, task, kwargs):
         """Validates if attach_iso can be called and if inputs are proper."""
         if not (task.node.provision_state == states.MANAGEABLE or

ironic/tests/unit/conductor/test_manager.py

@@ -4667,7 +4667,7 @@ class ManagerTestProperties(tests_db_base.DbTestCase):
         expected = ['ilo_address', 'ilo_username', 'ilo_password',
                     'client_port', 'client_timeout', 'ilo_deploy_iso',
                     'console_port', 'ilo_change_password',
-                    'deploy_forces_oob_reboot', 'ca_file']
+                    'ca_file']
         self._check_driver_properties("agent_ilo", expected)
 
     def test_driver_properties_fail(self):

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

@@ -385,6 +385,24 @@ class IloVirtualMediaIscsiDeployTestCase(db_base.DbTestCase):
                                                            states.POWER_OFF)
             iscsi_prep_clean_mock.assert_called_once_with(mock.ANY, task)
 
+    @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'continue_deploy',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(ilo_common, 'update_secure_boot_mode', autospec=True)
+    @mock.patch.object(ilo_common, 'update_boot_mode', autospec=True)
+    def test_continue_deploy(self,
+                             func_update_boot_mode,
+                             func_update_secure_boot_mode,
+                             pxe_vendorpassthru_mock):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.provision_state = states.DEPLOYWAIT
+            task.node.target_provision_state = states.ACTIVE
+            task.driver.deploy.continue_deploy(task)
+            func_update_boot_mode.assert_called_once_with(task)
+            func_update_secure_boot_mode.assert_called_once_with(task, True)
+            pxe_vendorpassthru_mock.assert_called_once_with(
+                mock.ANY, task)
+
 
 class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase):
 
@@ -561,6 +579,52 @@ class IloVirtualMediaAgentDeployTestCase(db_base.DbTestCase):
                                                            states.POWER_OFF)
             agent_prep_clean_mock.assert_called_once_with(mock.ANY, task)
 
+    @mock.patch.object(agent.AgentDeploy, 'reboot_to_instance',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(agent.AgentDeploy, 'check_deploy_success',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(ilo_common, 'update_boot_mode', spec_set=True,
+                       autospec=True)
+    @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True,
+                       autospec=True)
+    def test_reboot_to_instance(self, func_update_secure_boot_mode,
+                                func_update_boot_mode,
+                                check_deploy_success_mock,
+                                agent_reboot_to_instance_mock):
+        check_deploy_success_mock.return_value = None
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.driver.deploy.reboot_to_instance(task)
+            check_deploy_success_mock.assert_called_once_with(
+                mock.ANY, task.node)
+            func_update_boot_mode.assert_called_once_with(task)
+            func_update_secure_boot_mode.assert_called_once_with(task, True)
+            agent_reboot_to_instance_mock.assert_called_once_with(
+                mock.ANY, task)
+
+    @mock.patch.object(agent.AgentDeploy, 'reboot_to_instance',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(agent.AgentDeploy, 'check_deploy_success',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(ilo_common, 'update_boot_mode', spec_set=True,
+                       autospec=True)
+    @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True,
+                       autospec=True)
+    def test_reboot_to_instance_deploy_fail(self, func_update_secure_boot_mode,
+                                            func_update_boot_mode,
+                                            check_deploy_success_mock,
+                                            agent_reboot_to_instance_mock):
+        check_deploy_success_mock.return_value = "Error"
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.driver.deploy.reboot_to_instance(task)
+            check_deploy_success_mock.assert_called_once_with(
+                mock.ANY, task.node)
+            self.assertFalse(func_update_boot_mode.called)
+            self.assertFalse(func_update_secure_boot_mode.called)
+            agent_reboot_to_instance_mock.assert_called_once_with(
+                mock.ANY, task)
+
 
 class IloPXEDeployTestCase(db_base.DbTestCase):
 
@@ -705,3 +769,21 @@ class IloPXEDeployTestCase(db_base.DbTestCase):
             node_power_action_mock.assert_called_once_with(task,
                                                            states.POWER_OFF)
             iscsi_prep_clean_mock.assert_called_once_with(mock.ANY, task)
+
+    @mock.patch.object(iscsi_deploy.ISCSIDeploy, 'continue_deploy',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(ilo_common, 'update_secure_boot_mode', autospec=True)
+    @mock.patch.object(ilo_common, 'update_boot_mode', autospec=True)
+    def test_continue_deploy(self,
+                             func_update_boot_mode,
+                             func_update_secure_boot_mode,
+                             pxe_vendorpassthru_mock):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            task.node.provision_state = states.DEPLOYWAIT
+            task.node.target_provision_state = states.ACTIVE
+            task.driver.deploy.continue_deploy(task)
+            func_update_boot_mode.assert_called_once_with(task)
+            func_update_secure_boot_mode.assert_called_once_with(task, True)
+            pxe_vendorpassthru_mock.assert_called_once_with(
+                mock.ANY, task)

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

@@ -21,11 +21,9 @@ from ironic.common import exception
 from ironic.common import states
 from ironic.conductor import task_manager
 from ironic.conductor import utils as manager_utils
-from ironic.drivers.modules import agent
 from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules.ilo import common as ilo_common
 from ironic.drivers.modules.ilo import vendor as ilo_vendor
-from ironic.drivers.modules import iscsi_deploy
 from ironic.tests.unit.conductor import mgr_utils
 from ironic.tests.unit.db import base as db_base
 from ironic.tests.unit.db import utils as db_utils
@@ -108,76 +106,3 @@ class VendorPassthruTestCase(db_base.DbTestCase):
                 task, info)
             validate_image_prop_mock.assert_called_once_with(
                 task.context, {'image_source': 'foo'}, [])
-
-    @mock.patch.object(iscsi_deploy.VendorPassthru, 'continue_deploy',
-                       spec_set=True, autospec=True)
-    @mock.patch.object(ilo_common, 'update_secure_boot_mode', autospec=True)
-    @mock.patch.object(ilo_common, 'update_boot_mode', autospec=True)
-    def test_continue_deploy(self,
-                             func_update_boot_mode,
-                             func_update_secure_boot_mode,
-                             pxe_vendorpassthru_mock):
-        with task_manager.acquire(self.context, self.node.uuid,
-                                  shared=False) as task:
-            task.node.provision_state = states.DEPLOYWAIT
-            task.node.target_provision_state = states.ACTIVE
-            task.driver.vendor.continue_deploy(task)
-            func_update_boot_mode.assert_called_once_with(task)
-            func_update_secure_boot_mode.assert_called_once_with(task, True)
-            pxe_vendorpassthru_mock.assert_called_once_with(
-                mock.ANY, task)
-
-
-class IloVirtualMediaAgentVendorInterfaceTestCase(db_base.DbTestCase):
-
-    def setUp(self):
-        super(IloVirtualMediaAgentVendorInterfaceTestCase, self).setUp()
-        mgr_utils.mock_the_extension_manager(driver="agent_ilo")
-        self.node = obj_utils.create_test_node(
-            self.context, driver='agent_ilo', driver_info=INFO_DICT)
-
-    @mock.patch.object(agent.AgentVendorInterface, 'reboot_to_instance',
-                       spec_set=True, autospec=True)
-    @mock.patch.object(agent.AgentVendorInterface, 'check_deploy_success',
-                       spec_set=True, autospec=True)
-    @mock.patch.object(ilo_common, 'update_boot_mode', spec_set=True,
-                       autospec=True)
-    @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True,
-                       autospec=True)
-    def test_reboot_to_instance(self, func_update_secure_boot_mode,
-                                func_update_boot_mode,
-                                check_deploy_success_mock,
-                                agent_reboot_to_instance_mock):
-        check_deploy_success_mock.return_value = None
-        with task_manager.acquire(self.context, self.node.uuid,
-                                  shared=False) as task:
-            task.driver.vendor.reboot_to_instance(task)
-            check_deploy_success_mock.assert_called_once_with(
-                mock.ANY, task.node)
-            func_update_boot_mode.assert_called_once_with(task)
-            func_update_secure_boot_mode.assert_called_once_with(task, True)
-            agent_reboot_to_instance_mock.assert_called_once_with(
-                mock.ANY, task)
-
-    @mock.patch.object(agent.AgentVendorInterface, 'reboot_to_instance',
-                       spec_set=True, autospec=True)
-    @mock.patch.object(agent.AgentVendorInterface, 'check_deploy_success',
-                       spec_set=True, autospec=True)
-    @mock.patch.object(ilo_common, 'update_boot_mode', spec_set=True,
-                       autospec=True)
-    @mock.patch.object(ilo_common, 'update_secure_boot_mode', spec_set=True,
-                       autospec=True)
-    def test_reboot_to_instance_deploy_fail(self, func_update_secure_boot_mode,
-                                            func_update_boot_mode,
-                                            check_deploy_success_mock,
-                                            agent_reboot_to_instance_mock):
-        check_deploy_success_mock.return_value = "Error"
-        with task_manager.acquire(self.context, self.node.uuid,
-                                  shared=False) as task:
-            task.driver.vendor.reboot_to_instance(task)
-            check_deploy_success_mock.assert_called_once_with(
-                mock.ANY, task.node)
-            self.assertFalse(func_update_boot_mode.called)
-            self.assertFalse(func_update_secure_boot_mode.called)
-            agent_reboot_to_instance_mock.assert_called_once_with(
-                mock.ANY, task)

releasenotes/notes/remove-agent-passthru-ilo-2d1870bc2d8e8d60.yaml

@@ -0,0 +1,17 @@
+---
+features:
+  - |
+    Agent lookup/heartbeat as vendor passthru is removed from iLO drivers.
+    Affected drivers are
+
+    * agent_ilo
+    * iscsi_ilo
+    * pxe_ilo
+
+    Other existing vendor passthru methods are left inplace.
+
+upgrade:
+  - Agent lookup/heartbeat as vendor passthru is removed from iLO drivers.
+    That means that iLO drivers become incompatible with IPA < 1.5.0.
+    Operators are required to update their IPA-based deploy ramdisks to
+    contain IPA >= 1.5.0.