openstack/ironic
Code Issues Proposed changes

Merge "Make method public to support out-of-band cleaning"

Browse Source
This commit is contained in:
Zuul 2018-06-11 05:14:27 +00:00 committed by Gerrit Code Review
commit 9059a48508
5 changed files with 36 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
ironic/conductor/utils.py
View File

@ -784,3 +784,15 @@ def validate_instance_info_traits(node):
"node. Node %(node)s is missing traits %(traits)s") % "node. Node %(node)s is missing traits %(traits)s") %
{"node": node.uuid, "traits": ", ".join(missing)}) {"node": node.uuid, "traits": ", ".join(missing)})
raise exception.InvalidParameterValue(err) 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)

20
ironic/drivers/modules/agent_base_vendor.py
View File

@ -28,7 +28,6 @@ from ironic.common import boot_devices
from ironic.common import exception from ironic.common import exception
from ironic.common.i18n import _ from ironic.common.i18n import _
from ironic.common import states from ironic.common import states
from ironic.conductor import rpcapi
from ironic.conductor import utils as manager_utils from ironic.conductor import utils as manager_utils
from ironic.conf import CONF from ironic.conf import CONF
from ironic.drivers.modules import agent_client from ironic.drivers.modules import agent_client
@ -152,17 +151,6 @@ def _cleaning_reboot(task):
task.node.save() 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): def _get_completed_cleaning_command(task, commands):
"""Returns None or a completed cleaning command from the agent. """Returns None or a completed cleaning command from the agent.
@ -339,7 +327,7 @@ class HeartbeatMixin(object):
manager_utils.set_node_cleaning_steps(task) manager_utils.set_node_cleaning_steps(task)
# The exceptions from RPC are not possible as we using cast # The exceptions from RPC are not possible as we using cast
# here # here
_notify_conductor_resume_clean(task) manager_utils.notify_conductor_resume_clean(task)
else: else:
msg = _('Node failed to check cleaning progress.') msg = _('Node failed to check cleaning progress.')
self.continue_cleaning(task) self.continue_cleaning(task)
@ -478,7 +466,7 @@ class AgentDeployMixin(HeartbeatMixin):
info = task.node.driver_internal_info info = task.node.driver_internal_info
info.pop('cleaning_reboot', None) info.pop('cleaning_reboot', None)
task.node.driver_internal_info = info task.node.driver_internal_info = info
_notify_conductor_resume_clean(task) manager_utils.notify_conductor_resume_clean(task)
return return
else: else:
# Agent has no commands whatsoever # Agent has no commands whatsoever
@ -543,7 +531,7 @@ class AgentDeployMixin(HeartbeatMixin):
LOG.exception(msg) LOG.exception(msg)
return manager_utils.cleaning_error_handler(task, 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': elif command.get('command_status') == 'SUCCEEDED':
clean_step_hook = _get_post_clean_step_hook(node) 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, ' LOG.info('Agent on node %s returned cleaning command success, '
'moving to next clean step', node.uuid) 'moving to next clean step', node.uuid)
_notify_conductor_resume_clean(task) manager_utils.notify_conductor_resume_clean(task)
else: else:
msg = (_('Agent returned unknown status for clean step %(step)s ' msg = (_('Agent returned unknown status for clean step %(step)s '
'on node %(node)s : %(err)s.') % 'on node %(node)s : %(err)s.') %

4
ironic/drivers/modules/drac/raid.py
View File

@ -28,9 +28,9 @@ from ironic.common.i18n import _
from ironic.common import raid as raid_common from ironic.common import raid as raid_common
from ironic.common import states from ironic.common import states
from ironic.conductor import task_manager from ironic.conductor import task_manager
from ironic.conductor import utils as manager_utils
from ironic.conf import CONF from ironic.conf import CONF
from ironic.drivers import base 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 common as drac_common
from ironic.drivers.modules.drac import job as drac_job from ironic.drivers.modules.drac import job as drac_job
@ -889,4 +889,4 @@ class DracRAID(base.RAIDInterface):
def _resume_cleaning(self, task): def _resume_cleaning(self, task):
raid_common.update_raid_info( raid_common.update_raid_info(
task.node, self.get_logical_disks(task)) task.node, self.get_logical_disks(task))
agent_base_vendor._notify_conductor_resume_clean(task) manager_utils.notify_conductor_resume_clean(task)

10
ironic/tests/unit/drivers/modules/drac/test_periodic_task.py
View File

@ -19,7 +19,7 @@ import mock
from ironic.common import driver_factory from ironic.common import driver_factory
from ironic.conductor import task_manager 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 common as drac_common
from ironic.drivers.modules.drac import raid as drac_raid from ironic.drivers.modules.drac import raid as drac_raid
from ironic.tests.unit.db import base as db_base from ironic.tests.unit.db import base as db_base
@ -144,7 +144,7 @@ class DracPeriodicTaskTestCase(db_base.DbTestCase):
autospec=True) autospec=True)
@mock.patch.object(drac_raid.DracRAID, 'get_logical_disks', @mock.patch.object(drac_raid.DracRAID, 'get_logical_disks',
spec_set=True, autospec=True) 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( def test__check_node_raid_jobs_with_completed_job(
self, mock_notify_conductor_resume_clean, self, mock_notify_conductor_resume_clean,
mock_get_logical_disks, mock_get_drac_client): mock_get_logical_disks, mock_get_drac_client):
@ -212,7 +212,7 @@ class DracPeriodicTaskTestCase(db_base.DbTestCase):
autospec=True) autospec=True)
@mock.patch.object(drac_raid.DracRAID, 'get_logical_disks', @mock.patch.object(drac_raid.DracRAID, 'get_logical_disks',
spec_set=True, autospec=True) 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( def test__check_node_raid_jobs_with_completed_job_already_failed(
self, mock_notify_conductor_resume_clean, self, mock_notify_conductor_resume_clean,
mock_get_logical_disks, mock_get_drac_client): mock_get_logical_disks, mock_get_drac_client):
@ -252,7 +252,7 @@ class DracPeriodicTaskTestCase(db_base.DbTestCase):
autospec=True) autospec=True)
@mock.patch.object(drac_raid.DracRAID, 'get_logical_disks', @mock.patch.object(drac_raid.DracRAID, 'get_logical_disks',
spec_set=True, autospec=True) 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( def test__check_node_raid_jobs_with_multiple_jobs_completed(
self, mock_notify_conductor_resume_clean, self, mock_notify_conductor_resume_clean,
mock_get_logical_disks, mock_get_drac_client): mock_get_logical_disks, mock_get_drac_client):
@ -293,7 +293,7 @@ class DracPeriodicTaskTestCase(db_base.DbTestCase):
autospec=True) autospec=True)
@mock.patch.object(drac_raid.DracRAID, 'get_logical_disks', @mock.patch.object(drac_raid.DracRAID, 'get_logical_disks',
spec_set=True, autospec=True) 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( def test__check_node_raid_jobs_with_multiple_jobs_failed(
self, mock_notify_conductor_resume_clean, self, mock_notify_conductor_resume_clean,
mock_get_logical_disks, mock_get_drac_client): mock_get_logical_disks, mock_get_drac_client):

26
ironic/tests/unit/drivers/modules/test_agent_base_vendor.py
View File

@ -82,7 +82,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
autospec=True) autospec=True)
@mock.patch.object(agent_base_vendor.HeartbeatMixin, @mock.patch.object(agent_base_vendor.HeartbeatMixin,
'reboot_to_instance', autospec=True) '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) autospec=True)
def test_heartbeat_in_maintenance(self, ncrc_mock, rti_mock, cd_mock): def test_heartbeat_in_maintenance(self, ncrc_mock, rti_mock, cd_mock):
# NOTE(pas-ha) checking only for states that are not noop # NOTE(pas-ha) checking only for states that are not noop
@ -112,7 +112,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
autospec=True) autospec=True)
@mock.patch.object(agent_base_vendor.HeartbeatMixin, @mock.patch.object(agent_base_vendor.HeartbeatMixin,
'reboot_to_instance', autospec=True) '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) autospec=True)
def test_heartbeat_with_reservation(self, ncrc_mock, rti_mock, cd_mock): def test_heartbeat_with_reservation(self, ncrc_mock, rti_mock, cd_mock):
# NOTE(pas-ha) checking only for states that are not noop # NOTE(pas-ha) checking only for states that are not noop
@ -138,7 +138,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
autospec=True) autospec=True)
@mock.patch.object(agent_base_vendor.HeartbeatMixin, @mock.patch.object(agent_base_vendor.HeartbeatMixin,
'reboot_to_instance', autospec=True) '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) autospec=True)
def test_heartbeat_noops_in_wrong_state(self, ncrc_mock, rti_mock, def test_heartbeat_noops_in_wrong_state(self, ncrc_mock, rti_mock,
cd_mock): cd_mock):
@ -213,7 +213,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
@mock.patch.object(agent_base_vendor.HeartbeatMixin, @mock.patch.object(agent_base_vendor.HeartbeatMixin,
'refresh_clean_steps', autospec=True) 'refresh_clean_steps', autospec=True)
@mock.patch.object(manager_utils, 'set_node_cleaning_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) autospec=True)
def test_heartbeat_resume_clean(self, mock_notify, mock_set_steps, def test_heartbeat_resume_clean(self, mock_notify, mock_set_steps,
mock_refresh, mock_touch): mock_refresh, mock_touch):
@ -234,7 +234,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest):
@mock.patch.object(agent_base_vendor.HeartbeatMixin, @mock.patch.object(agent_base_vendor.HeartbeatMixin,
'refresh_clean_steps', autospec=True) 'refresh_clean_steps', autospec=True)
@mock.patch.object(manager_utils, 'set_node_cleaning_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) autospec=True)
def test_heartbeat_resume_clean_fails(self, mock_notify, mock_set_steps, def test_heartbeat_resume_clean_fails(self, mock_notify, mock_set_steps,
mock_refresh, mock_touch, mock_refresh, mock_touch,
@ -913,7 +913,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest):
self.assertFalse(prepare_mock.called) self.assertFalse(prepare_mock.called)
self.assertFalse(failed_state_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) autospec=True)
@mock.patch.object(agent_client.AgentClient, 'get_commands_status', @mock.patch.object(agent_client.AgentClient, 'get_commands_status',
autospec=True) autospec=True)
@ -996,7 +996,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest):
self.deploy.continue_cleaning(task) self.deploy.continue_cleaning(task)
reboot_mock.assert_called_once_with(task, states.REBOOT) 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) autospec=True)
@mock.patch.object(agent_client.AgentClient, 'get_commands_status', @mock.patch.object(agent_client.AgentClient, 'get_commands_status',
autospec=True) autospec=True)
@ -1023,7 +1023,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest):
@mock.patch.object(agent_base_vendor, @mock.patch.object(agent_base_vendor,
'_get_post_clean_step_hook', autospec=True) '_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) autospec=True)
@mock.patch.object(agent_client.AgentClient, 'get_commands_status', @mock.patch.object(agent_client.AgentClient, 'get_commands_status',
autospec=True) autospec=True)
@ -1050,7 +1050,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest):
hook_mock.assert_called_once_with(task, command_status) hook_mock.assert_called_once_with(task, command_status)
notify_mock.assert_called_once_with(task) 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) autospec=True)
@mock.patch.object(agent_base_vendor, @mock.patch.object(agent_base_vendor,
'_get_post_clean_step_hook', autospec=True) '_get_post_clean_step_hook', autospec=True)
@ -1083,7 +1083,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest):
error_handler_mock.assert_called_once_with(task, mock.ANY) error_handler_mock.assert_called_once_with(task, mock.ANY)
self.assertFalse(notify_mock.called) 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) autospec=True)
@mock.patch.object(agent_client.AgentClient, 'get_commands_status', @mock.patch.object(agent_client.AgentClient, 'get_commands_status',
autospec=True) autospec=True)
@ -1112,7 +1112,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest):
self.deploy.continue_cleaning(task) self.deploy.continue_cleaning(task)
self.assertFalse(notify_mock.called) 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) autospec=True)
@mock.patch.object(agent_client.AgentClient, 'get_commands_status', @mock.patch.object(agent_client.AgentClient, 'get_commands_status',
autospec=True) autospec=True)
@ -1144,7 +1144,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest):
error_mock.assert_called_once_with(task, mock.ANY) error_mock.assert_called_once_with(task, mock.ANY)
@mock.patch.object(manager_utils, 'set_node_cleaning_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) autospec=True)
@mock.patch.object(agent_base_vendor.AgentDeployMixin, @mock.patch.object(agent_base_vendor.AgentDeployMixin,
'refresh_clean_steps', autospec=True) '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, 'cleaning_error_handler', autospec=True)
@mock.patch.object(manager_utils, 'set_node_cleaning_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) autospec=True)
@mock.patch.object(agent_base_vendor.AgentDeployMixin, @mock.patch.object(agent_base_vendor.AgentDeployMixin,
'refresh_clean_steps', autospec=True) 'refresh_clean_steps', autospec=True)