From f40f145f8205d30e2c5a898d9519ef7ed6e9ea07 Mon Sep 17 00:00:00 2001 From: Nguyen Van Trung Date: Wed, 23 May 2018 10:26:29 +0700 Subject: [PATCH] Make method public to support out-of-band cleaning This makes the _notify_conductor_resume_clean method public by removing the leading underscore from its name. And move it into conductor/utils.py. The idrac hardware type's RAID configuration out-of-band cleaning has been using it [1]. The method is going to be used by the RAID configuration support that is being added to the iRMC hardware type [2]. [1] https://github.com/openstack/ironic/blob/580d4338e2bce8e557a5007fcba85e157f0b8d60/ironic/drivers/modules/drac/raid.py#L892 [2] https://review.openstack.org/#/c/512979 Change-Id: Ifd10dd88d65306049119588e6088359a5d38c158 --- ironic/conductor/utils.py | 12 +++++++++ ironic/drivers/modules/agent_base_vendor.py | 20 +++----------- ironic/drivers/modules/drac/raid.py | 4 +-- .../modules/drac/test_periodic_task.py | 10 +++---- .../drivers/modules/test_agent_base_vendor.py | 26 +++++++++---------- 5 files changed, 36 insertions(+), 36 deletions(-) diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 36c9b8300f..68f9cbc2a7 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -784,3 +784,15 @@ def validate_instance_info_traits(node): "node. Node %(node)s is missing traits %(traits)s") % {"node": node.uuid, "traits": ", ".join(missing)}) raise exception.InvalidParameterValue(err) + + +def notify_conductor_resume_clean(task): + LOG.debug('Sending RPC to conductor to resume cleaning for node %s', + task.node.uuid) + from ironic.conductor import rpcapi + uuid = task.node.uuid + rpc = rpcapi.ConductorAPI() + topic = rpc.get_topic_for(task.node) + # Need to release the lock to let the conductor take it + task.release_resources() + rpc.continue_node_clean(task.context, uuid, topic=topic) diff --git a/ironic/drivers/modules/agent_base_vendor.py b/ironic/drivers/modules/agent_base_vendor.py index 16a89b8ee5..4c777c09e6 100644 --- a/ironic/drivers/modules/agent_base_vendor.py +++ b/ironic/drivers/modules/agent_base_vendor.py @@ -28,7 +28,6 @@ from ironic.common import boot_devices from ironic.common import exception from ironic.common.i18n import _ from ironic.common import states -from ironic.conductor import rpcapi from ironic.conductor import utils as manager_utils from ironic.conf import CONF from ironic.drivers.modules import agent_client @@ -152,17 +151,6 @@ def _cleaning_reboot(task): task.node.save() -def _notify_conductor_resume_clean(task): - LOG.debug('Sending RPC to conductor to resume cleaning for node %s', - task.node.uuid) - uuid = task.node.uuid - rpc = rpcapi.ConductorAPI() - topic = rpc.get_topic_for(task.node) - # Need to release the lock to let the conductor take it - task.release_resources() - rpc.continue_node_clean(task.context, uuid, topic=topic) - - def _get_completed_cleaning_command(task, commands): """Returns None or a completed cleaning command from the agent. @@ -339,7 +327,7 @@ class HeartbeatMixin(object): manager_utils.set_node_cleaning_steps(task) # The exceptions from RPC are not possible as we using cast # here - _notify_conductor_resume_clean(task) + manager_utils.notify_conductor_resume_clean(task) else: msg = _('Node failed to check cleaning progress.') self.continue_cleaning(task) @@ -478,7 +466,7 @@ class AgentDeployMixin(HeartbeatMixin): info = task.node.driver_internal_info info.pop('cleaning_reboot', None) task.node.driver_internal_info = info - _notify_conductor_resume_clean(task) + manager_utils.notify_conductor_resume_clean(task) return else: # Agent has no commands whatsoever @@ -543,7 +531,7 @@ class AgentDeployMixin(HeartbeatMixin): LOG.exception(msg) return manager_utils.cleaning_error_handler(task, msg) - _notify_conductor_resume_clean(task) + manager_utils.notify_conductor_resume_clean(task) elif command.get('command_status') == 'SUCCEEDED': clean_step_hook = _get_post_clean_step_hook(node) @@ -573,7 +561,7 @@ class AgentDeployMixin(HeartbeatMixin): LOG.info('Agent on node %s returned cleaning command success, ' 'moving to next clean step', node.uuid) - _notify_conductor_resume_clean(task) + manager_utils.notify_conductor_resume_clean(task) else: msg = (_('Agent returned unknown status for clean step %(step)s ' 'on node %(node)s : %(err)s.') % diff --git a/ironic/drivers/modules/drac/raid.py b/ironic/drivers/modules/drac/raid.py index 4760cc374a..4b54480691 100644 --- a/ironic/drivers/modules/drac/raid.py +++ b/ironic/drivers/modules/drac/raid.py @@ -28,9 +28,9 @@ from ironic.common.i18n import _ from ironic.common import raid as raid_common from ironic.common import states from ironic.conductor import task_manager +from ironic.conductor import utils as manager_utils from ironic.conf import CONF from ironic.drivers import base -from ironic.drivers.modules import agent_base_vendor from ironic.drivers.modules.drac import common as drac_common from ironic.drivers.modules.drac import job as drac_job @@ -889,4 +889,4 @@ class DracRAID(base.RAIDInterface): def _resume_cleaning(self, task): raid_common.update_raid_info( task.node, self.get_logical_disks(task)) - agent_base_vendor._notify_conductor_resume_clean(task) + manager_utils.notify_conductor_resume_clean(task) diff --git a/ironic/tests/unit/drivers/modules/drac/test_periodic_task.py b/ironic/tests/unit/drivers/modules/drac/test_periodic_task.py index 62a62a1c11..5ac80586d2 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_periodic_task.py +++ b/ironic/tests/unit/drivers/modules/drac/test_periodic_task.py @@ -19,7 +19,7 @@ import mock from ironic.common import driver_factory from ironic.conductor import task_manager -from ironic.drivers.modules import agent_base_vendor +from ironic.conductor import utils as manager_utils from ironic.drivers.modules.drac import common as drac_common from ironic.drivers.modules.drac import raid as drac_raid from ironic.tests.unit.db import base as db_base @@ -144,7 +144,7 @@ class DracPeriodicTaskTestCase(db_base.DbTestCase): autospec=True) @mock.patch.object(drac_raid.DracRAID, 'get_logical_disks', spec_set=True, autospec=True) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean') + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean') def test__check_node_raid_jobs_with_completed_job( self, mock_notify_conductor_resume_clean, mock_get_logical_disks, mock_get_drac_client): @@ -212,7 +212,7 @@ class DracPeriodicTaskTestCase(db_base.DbTestCase): autospec=True) @mock.patch.object(drac_raid.DracRAID, 'get_logical_disks', spec_set=True, autospec=True) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean') + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean') def test__check_node_raid_jobs_with_completed_job_already_failed( self, mock_notify_conductor_resume_clean, mock_get_logical_disks, mock_get_drac_client): @@ -252,7 +252,7 @@ class DracPeriodicTaskTestCase(db_base.DbTestCase): autospec=True) @mock.patch.object(drac_raid.DracRAID, 'get_logical_disks', spec_set=True, autospec=True) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean') + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean') def test__check_node_raid_jobs_with_multiple_jobs_completed( self, mock_notify_conductor_resume_clean, mock_get_logical_disks, mock_get_drac_client): @@ -293,7 +293,7 @@ class DracPeriodicTaskTestCase(db_base.DbTestCase): autospec=True) @mock.patch.object(drac_raid.DracRAID, 'get_logical_disks', spec_set=True, autospec=True) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean') + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean') def test__check_node_raid_jobs_with_multiple_jobs_failed( self, mock_notify_conductor_resume_clean, mock_get_logical_disks, mock_get_drac_client): diff --git a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py index b3c5cb3e1b..90c9fe5546 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py @@ -82,7 +82,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): autospec=True) @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'reboot_to_instance', autospec=True) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) def test_heartbeat_in_maintenance(self, ncrc_mock, rti_mock, cd_mock): # NOTE(pas-ha) checking only for states that are not noop @@ -112,7 +112,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): autospec=True) @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'reboot_to_instance', autospec=True) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) def test_heartbeat_with_reservation(self, ncrc_mock, rti_mock, cd_mock): # NOTE(pas-ha) checking only for states that are not noop @@ -138,7 +138,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): autospec=True) @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'reboot_to_instance', autospec=True) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) def test_heartbeat_noops_in_wrong_state(self, ncrc_mock, rti_mock, cd_mock): @@ -213,7 +213,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'refresh_clean_steps', autospec=True) @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) def test_heartbeat_resume_clean(self, mock_notify, mock_set_steps, mock_refresh, mock_touch): @@ -234,7 +234,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'refresh_clean_steps', autospec=True) @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) def test_heartbeat_resume_clean_fails(self, mock_notify, mock_set_steps, mock_refresh, mock_touch, @@ -913,7 +913,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.assertFalse(prepare_mock.called) self.assertFalse(failed_state_mock.called) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) @@ -996,7 +996,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.deploy.continue_cleaning(task) reboot_mock.assert_called_once_with(task, states.REBOOT) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) @@ -1023,7 +1023,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): @mock.patch.object(agent_base_vendor, '_get_post_clean_step_hook', autospec=True) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) @@ -1050,7 +1050,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): hook_mock.assert_called_once_with(task, command_status) notify_mock.assert_called_once_with(task) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) @mock.patch.object(agent_base_vendor, '_get_post_clean_step_hook', autospec=True) @@ -1083,7 +1083,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): error_handler_mock.assert_called_once_with(task, mock.ANY) self.assertFalse(notify_mock.called) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) @@ -1112,7 +1112,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): self.deploy.continue_cleaning(task) self.assertFalse(notify_mock.called) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', autospec=True) @@ -1144,7 +1144,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): error_mock.assert_called_once_with(task, mock.ANY) @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) @mock.patch.object(agent_base_vendor.AgentDeployMixin, 'refresh_clean_steps', autospec=True) @@ -1183,7 +1183,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): @mock.patch.object(manager_utils, 'cleaning_error_handler', autospec=True) @mock.patch.object(manager_utils, 'set_node_cleaning_steps', autospec=True) - @mock.patch.object(agent_base_vendor, '_notify_conductor_resume_clean', + @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) @mock.patch.object(agent_base_vendor.AgentDeployMixin, 'refresh_clean_steps', autospec=True)