Tear down console during unprovisioning

Before this patch, when unprovisioning a node, the console was left
running. This allowed a user to view the console even after the instance
was gone. Stop the console during unprovisioning to block this.

ConductorManager._set_console_mode will now bubble up any exceptions
raised, so that we can explode as needed during unprovisioning. This
does not have any side effects, as elsewhere it is run in it's own
thread and execution was complete after handling the exception.

Also change a few mock.ANY in the relevant unit tests to the actual task
object, as cleanup.

Conflicts:
	ironic/tests/unit/conductor/test_manager.py

Change-Id: Iec128444d692e0b0fbc1737eb21b0e6f69b7ec1e
Partial-Bug: #1769817
Story: #2002000
Task: #19634
(cherry picked from commit 7a8b26db6b)
This commit is contained in:
Jim Rollenhagen 2018-05-10 08:06:19 -04:00
parent a88bd7cbcd
commit 00914c802d
4 changed files with 74 additions and 25 deletions

View File

@ -293,13 +293,12 @@ a node console are:
* ``baremetal.node.console_restore.end``
* ``baremetal.node.console_restore.error``
``console_set`` action is used when start or stop console is initiated via API
request. The ``console_restore`` action is used when the console was already
enabled, but a driver must restart the console because an ironic-conductor was
restarted. This may also be sent when an ironic-conductor takes over a node
that was being managed by another ironic-conductor. "start" and "end"
notifications have INFO level, "error" has ERROR. Example of node console
notification::
``console_set`` action is used when start or stop console is initiated. The
``console_restore`` action is used when the console was already enabled, but a
driver must restart the console because an ironic-conductor was restarted. This
may also be sent when an ironic-conductor takes over a node that was being
managed by another ironic-conductor. "start" and "end" notifications have INFO
level, "error" has ERROR. Example of node console notification::
{
"priority": "info",

View File

@ -651,6 +651,11 @@ class ConductorManager(base_manager.BaseConductorManager):
"""Internal RPC method to tear down an existing node deployment."""
node = task.node
try:
# stop the console
# 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)
task.driver.deploy.clean_up(task)
task.driver.deploy.tear_down(task)
except Exception as e:
@ -1822,23 +1827,25 @@ class ConductorManager(base_manager.BaseConductorManager):
# take_over() right now.
else:
task.driver.console.stop_console(task)
except Exception as e:
op = _('enabling') if enabled else _('disabling')
msg = (_('Error %(op)s the console on node %(node)s. '
'Reason: %(error)s') % {'op': op,
'node': node.uuid,
'error': e})
node.last_error = msg
LOG.error(msg)
node.save()
notify_utils.emit_console_notification(
task, 'console_set', fields.NotificationStatus.ERROR)
else:
node.console_enabled = enabled
node.last_error = None
node.save()
notify_utils.emit_console_notification(
task, 'console_set', fields.NotificationStatus.END)
with excutils.save_and_reraise_exception():
op = _('enabling') if enabled else _('disabling')
msg = (_('Error %(op)s the console on node %(node)s. '
'Reason: %(error)s') % {'op': op,
'node': node.uuid,
'error': e})
node.last_error = msg
LOG.error(msg)
node.save()
notify_utils.emit_console_notification(
task, 'console_set', fields.NotificationStatus.ERROR)
node.console_enabled = enabled
node.last_error = None
node.save()
notify_utils.emit_console_notification(
task, 'console_set', fields.NotificationStatus.END)
@METRICS.timer('ConductorManager.update_port')
@messaging.expected_exceptions(exception.NodeLocked,

View File

@ -1568,17 +1568,43 @@ class DoNodeDeployTearDownTestCase(mgr_utils.ServiceSetUpMixin,
self.assertIsNotNone(node.last_error)
# Assert instance_info was erased
self.assertEqual({}, node.instance_info)
mock_tear_down.assert_called_once_with(mock.ANY)
mock_tear_down.assert_called_once_with(task)
@mock.patch('ironic.conductor.manager.ConductorManager._set_console_mode')
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(
self.context, driver='fake', provision_state=states.DELETING,
target_provision_state=states.AVAILABLE,
instance_info={'foo': 'bar'},
console_enabled=True,
driver_internal_info={'is_whole_disk_image': False})
task = task_manager.TaskManager(self.context, node.uuid)
self._start_service()
mock_console.side_effect = exception.ConsoleError('test')
self.assertRaises(exception.ConsoleError,
self.service._do_node_tear_down, task)
node.refresh()
self.assertEqual(states.ERROR, node.provision_state)
self.assertEqual(states.NOSTATE, node.target_provision_state)
self.assertIsNotNone(node.last_error)
# Assert instance_info was erased
self.assertEqual({}, node.instance_info)
mock_console.assert_called_once_with(task, False)
@mock.patch('ironic.conductor.manager.ConductorManager._set_console_mode')
@mock.patch('ironic.conductor.manager.ConductorManager._do_node_clean')
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.tear_down')
def test__do_node_tear_down_ok(self, mock_tear_down, mock_clean):
def _test__do_node_tear_down_ok(self, mock_tear_down, mock_clean,
mock_console, enabled_console=False):
# test when driver.deploy.tear_down succeeds
node = obj_utils.create_test_node(
self.context, driver='fake', provision_state=states.DELETING,
target_provision_state=states.AVAILABLE,
instance_uuid=uuidutils.generate_uuid(),
instance_info={'foo': 'bar'},
console_enabled=enabled_console,
driver_internal_info={'is_whole_disk_image': False,
'instance': {'ephemeral_gb': 10}})
@ -1595,6 +1621,16 @@ class DoNodeDeployTearDownTestCase(mgr_utils.ServiceSetUpMixin,
self.assertNotIn('instance', node.driver_internal_info)
mock_tear_down.assert_called_once_with(mock.ANY)
mock_clean.assert_called_once_with(mock.ANY)
if enabled_console:
mock_console.assert_called_once_with(task, False)
else:
mock_console.assert_not_called()
def test__do_node_tear_down_ok_without_console(self):
self._test__do_node_tear_down_ok(enabled_console=False)
def test__do_node_tear_down_ok_with_console(self):
self._test__do_node_tear_down_ok(enabled_console=True)
@mock.patch('ironic.conductor.manager.ConductorManager._do_node_clean')
@mock.patch('ironic.drivers.modules.fake.FakeDeploy.tear_down')

View File

@ -0,0 +1,7 @@
---
security:
- |
Fixes an issue where an enabled console could be left running after a node
was unprovisioned. This allowed a user to view the console even after the
instance was gone. Ironic now stops the console during unprovisioning to
block this.