openstack/ironic
Code Issues Proposed changes

Stop console at tearing down without unsetting console_enabled

Browse Source 
A node's console_enabled is reset to False at tearing down not to
expose the console to an instance user. This requires an operator to
set The console_enabled to True when another instance is created on the
node.

This patch stops a console at tearing down a node with keeping
console_enabled True. Then, the console is started at deployment.

Change-Id: I42c209af80465b12720b41fd96ae9477091eaf24
Story: #2003972
Task: #26914
This commit is contained in:
Hironori Shiina 2018-10-05 16:35:32 +09:00
parent d2c9bba6f9
commit 37c92111a0
3 changed files with 106 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

52
ironic/conductor/manager.py
View File

@ -943,7 +943,23 @@ class ConductorManager(base_manager.BaseConductorManager):
# do it in this thread since we're already out of the main
# conductor thread.
if node.console_enabled:
self._set_console_mode(task, False)
notify_utils.emit_console_notification(
task, 'console_stop', fields.NotificationStatus.START)
try:
# Keep console_enabled=True for next deployment
task.driver.console.stop_console(task)
except Exception as err:
with excutils.save_and_reraise_exception():
LOG.error('Failed to stop console while tearing down '
'the node %(node)s: %(err)s.',
{'node': node.uuid, 'err': err})
notify_utils.emit_console_notification(
task, 'console_stop',
fields.NotificationStatus.ERROR)
else:
notify_utils.emit_console_notification(
task, 'console_stop', fields.NotificationStatus.END)
task.driver.deploy.clean_up(task)
task.driver.deploy.tear_down(task)
except Exception as e:
@ -3574,6 +3590,7 @@ def _old_rest_of_do_node_deploy(task, conductor_id, no_deploy_steps):
# NOTE(deva): Some drivers may return states.DEPLOYWAIT
# eg. if they are waiting for a callback
if new_state == states.DEPLOYDONE:
_start_console_in_deploy(task)
task.process_event('done')
LOG.info('Successfully deployed node %(node)s with '
'instance %(instance)s.',
@ -3679,12 +3696,45 @@ def _do_next_deploy_step(task, step_index, conductor_id):
node.driver_internal_info = driver_internal_info
node.save()
_start_console_in_deploy(task)
task.process_event('done')
LOG.info('Successfully deployed node %(node)s with '
'instance %(instance)s.',
{'node': node.uuid, 'instance': node.instance_uuid})
def _start_console_in_deploy(task):
"""Start console at the end of deployment.
Console is stopped at tearing down not to be exposed to an instance user.
Then, restart at deployment.
:param task: a TaskManager instance with an exclusive lock
"""
if not task.node.console_enabled:
return
notify_utils.emit_console_notification(
task, 'console_restore', fields.NotificationStatus.START)
try:
task.driver.console.start_console(task)
except Exception as err:
msg = (_('Failed to start console while deploying the '
'node %(node)s: %(err)s.') % {'node': task.node.uuid,
'err': err})
LOG.error(msg)
task.node.last_error = msg
task.node.console_enabled = False
task.node.save()
notify_utils.emit_console_notification(
task, 'console_restore', fields.NotificationStatus.ERROR)
else:
notify_utils.emit_console_notification(
task, 'console_restore', fields.NotificationStatus.END)
@task_manager.require_exclusive_lock
def handle_sync_power_state_max_retries_exceeded(task, actual_power_state,
exception=None):

54
ironic/tests/unit/conductor/test_manager.py
View File

@ -2164,6 +2164,24 @@ class DoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.assertEqual(self.service.conductor.id,
task.node.conductor_affinity)
@mock.patch('ironic.conductor.manager._start_console_in_deploy',
autospec=True)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.deploy', autospec=True)
def test__old_rest_of_do_node_deploy_console(self, mock_deploy,
mock_console):
self._start_service()
node = obj_utils.create_test_node(self.context, driver='fake-hardware')
task = task_manager.TaskManager(self.context, node.uuid)
task.process_event('deploy')
mock_deploy.return_value = states.DEPLOYDONE
manager._old_rest_of_do_node_deploy(task, self.service.conductor.id,
True)
mock_deploy.assert_called_once_with(mock.ANY, task)
mock_console.assert_called_once_with(task)
self.assertEqual(self.service.conductor.id,
task.node.conductor_affinity)
@mgr_utils.mock_record_keepalive
class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin,
@ -2243,9 +2261,14 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin,
mock_execute.assert_called_once_with(mock.ANY, task,
self.deploy_steps[1])
@mock.patch('ironic.drivers.modules.fake.FakeConsole.start_console',
autospec=True)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.execute_deploy_step',
autospec=True)
def test__do_next_deploy_step_last_step_done(self, mock_execute):
def _test__do_next_deploy_step_last_step_done(self, mock_execute,
mock_console,
console_enabled=False,
console_error=False):
# Resume where last_step is the last deploy step that was executed
driver_internal_info = {'deploy_step_index': 1,
'deploy_steps': self.deploy_steps}
@ -2255,8 +2278,11 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin,
provision_state=states.DEPLOYWAIT,
target_provision_state=states.ACTIVE,
driver_internal_info=driver_internal_info,
deploy_step=self.deploy_steps[1])
deploy_step=self.deploy_steps[1],
console_enabled=console_enabled)
mock_execute.return_value = None
if console_error:
mock_console.side_effect = exception.ConsoleError()
task = task_manager.TaskManager(self.context, node.uuid)
task.process_event('resume')
@ -2270,6 +2296,20 @@ class DoNextDeployStepTestCase(mgr_utils.ServiceSetUpMixin,
self.assertNotIn('deploy_step_index', node.driver_internal_info)
self.assertIsNone(node.driver_internal_info['deploy_steps'])
self.assertFalse(mock_execute.called)
if console_enabled:
mock_console.assert_called_once_with(mock.ANY, task)
else:
self.assertFalse(mock_console.called)
def test__do_next_deploy_step_last_step_done(self):
self._test__do_next_deploy_step_last_step_done()
def test__do_next_deploy_step_last_step_done_with_console(self):
self._test__do_next_deploy_step_last_step_done(console_enabled=True)
def test__do_next_deploy_step_last_step_done_with_console_error(self):
self._test__do_next_deploy_step_last_step_done(console_enabled=True,
console_error=True)
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.execute_deploy_step',
autospec=True)
@ -2565,7 +2605,7 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.assertEqual({}, node.instance_info)
mock_tear_down.assert_called_once_with(task)
@mock.patch('ironic.conductor.manager.ConductorManager._set_console_mode')
@mock.patch('ironic.drivers.modules.fake.FakeConsole.stop_console')
def test_do_node_tear_down_console_raises_error(self, mock_console):
# test when _set_console_mode raises exception
node = obj_utils.create_test_node(
@ -2588,11 +2628,11 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.assertIsNotNone(node.last_error)
# Assert instance_info was erased
self.assertEqual({}, node.instance_info)
mock_console.assert_called_once_with(task, False)
mock_console.assert_called_once_with(task)
# TODO(TheJulia): Since we're functionally bound to neutron support
# by default, the fake drivers still invoke neutron.
@mock.patch('ironic.conductor.manager.ConductorManager._set_console_mode')
@mock.patch('ironic.drivers.modules.fake.FakeConsole.stop_console')
@mock.patch('ironic.common.neutron.unbind_neutron_port')
@mock.patch('ironic.conductor.manager.ConductorManager._do_node_clean')
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.tear_down')
@ -2635,9 +2675,9 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
self.assertEqual({}, port.internal_info)
mock_unbind.assert_called_once_with('foo', context=mock.ANY)
if enabled_console:
mock_console.assert_called_once_with(task, False)
mock_console.assert_called_once_with(task)
else:
mock_console.assert_not_called()
self.assertFalse(mock_console.called)
def test__do_node_tear_down_ok_without_console(self):
self._test__do_node_tear_down_ok(enabled_console=False)

8
releasenotes/notes/bug-2003972-dae9b7d0f6180339.yaml Normal file
View File

@ -0,0 +1,8 @@
---
fixes:
- |
Fixes a bug that a node's ``console_enabled`` is reset to ``False`` at
undeploying the node, which requires an operator to set it to ``True``
before deploying again. By this fix, while the console is stopped at
tearing down, ``console_enabled`` remains ``True``. When the node is
deployed again, the console is started automatically.