diff --git a/ironic/drivers/ilo.py b/ironic/drivers/ilo.py index 0051da3d4a..8f64e67fdd 100644 --- a/ironic/drivers/ilo.py +++ b/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() diff --git a/ironic/drivers/modules/ilo/deploy.py b/ironic/drivers/modules/ilo/deploy.py index 601a2cb82e..d5dc2c9c1b 100644 --- a/ironic/drivers/modules/ilo/deploy.py +++ b/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): - return {} + @METRICS.timer('IloIscsiDeployMixin.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(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) diff --git a/ironic/drivers/modules/ilo/vendor.py b/ironic/drivers/modules/ilo/vendor.py index f5aac370bb..bbf482527f 100644 --- a/ironic/drivers/modules/ilo/vendor.py +++ b/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): - """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): +class VendorPassthru(base.VendorInterface): """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 diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 39d9c3d312..5b6394801e 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/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): diff --git a/ironic/tests/unit/drivers/modules/ilo/test_deploy.py b/ironic/tests/unit/drivers/modules/ilo/test_deploy.py index b83ce61304..6e5457a1f8 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_deploy.py +++ b/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) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_vendor.py b/ironic/tests/unit/drivers/modules/ilo/test_vendor.py index 87e72cfb55..8b43e2542a 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_vendor.py +++ b/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) diff --git a/releasenotes/notes/remove-agent-passthru-ilo-2d1870bc2d8e8d60.yaml b/releasenotes/notes/remove-agent-passthru-ilo-2d1870bc2d8e8d60.yaml new file mode 100644 index 0000000000..c556f697a6 --- /dev/null +++ b/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.