summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJim Rollenhagen <jim@jimrollenhagen.com>2018-05-10 08:06:19 -0400
committerJim Rollenhagen <jim@jimrollenhagen.com>2018-05-18 11:53:38 +0000
commit00102e3e46c7bda873c9975364ea07f22c34d12d (patch)
treec9217601bb9d4693df704c975b0e499961e59c39
parent4ca48b4f60d51e1b0ba8546fd1499270a7ceab09 (diff)
Tear down console during unprovisioningstable/pike
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 7a8b26db6b48f5593786bb92cf09fa1d6afcc1ea)
Notes
Notes (review): Code-Review+2: Ruby Loo <opensrloo@gmail.com> Code-Review+2: Julia Kreger <juliaashleykreger@gmail.com> Workflow+1: Julia Kreger <juliaashleykreger@gmail.com> Verified+2: Zuul Submitted-by: Zuul Submitted-at: Tue, 29 May 2018 23:20:28 +0000 Reviewed-on: https://review.openstack.org/568569 Project: openstack/ironic Branch: refs/heads/stable/pike
-rw-r--r--doc/source/admin/notifications.rst13
-rw-r--r--ironic/conductor/manager.py39
-rw-r--r--ironic/tests/unit/conductor/test_manager.py40
-rw-r--r--releasenotes/notes/stop-console-during-unprovision-a29d8facb3f03be5.yaml7
4 files changed, 74 insertions, 25 deletions
diff --git a/doc/source/admin/notifications.rst b/doc/source/admin/notifications.rst
index c547b26..c627d09 100644
--- a/doc/source/admin/notifications.rst
+++ b/doc/source/admin/notifications.rst
@@ -416,13 +416,12 @@ a node console are:
416* ``baremetal.node.console_restore.end`` 416* ``baremetal.node.console_restore.end``
417* ``baremetal.node.console_restore.error`` 417* ``baremetal.node.console_restore.error``
418 418
419``console_set`` action is used when start or stop console is initiated via API 419``console_set`` action is used when start or stop console is initiated. The
420request. The ``console_restore`` action is used when the console was already 420``console_restore`` action is used when the console was already enabled, but a
421enabled, but a driver must restart the console because an ironic-conductor was 421driver must restart the console because an ironic-conductor was restarted. This
422restarted. This may also be sent when an ironic-conductor takes over a node 422may also be sent when an ironic-conductor takes over a node that was being
423that was being managed by another ironic-conductor. "start" and "end" 423managed by another ironic-conductor. "start" and "end" notifications have INFO
424notifications have INFO level, "error" has ERROR. Example of node console 424level, "error" has ERROR. Example of node console notification::
425notification::
426 425
427 { 426 {
428 "priority": "info", 427 "priority": "info",
diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index c6221a1..6ee1da3 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -665,6 +665,11 @@ class ConductorManager(base_manager.BaseConductorManager):
665 """Internal RPC method to tear down an existing node deployment.""" 665 """Internal RPC method to tear down an existing node deployment."""
666 node = task.node 666 node = task.node
667 try: 667 try:
668 # stop the console
669 # do it in this thread since we're already out of the main
670 # conductor thread.
671 if node.console_enabled:
672 self._set_console_mode(task, False)
668 task.driver.deploy.clean_up(task) 673 task.driver.deploy.clean_up(task)
669 task.driver.deploy.tear_down(task) 674 task.driver.deploy.tear_down(task)
670 except Exception as e: 675 except Exception as e:
@@ -1848,23 +1853,25 @@ class ConductorManager(base_manager.BaseConductorManager):
1848 # take_over() right now. 1853 # take_over() right now.
1849 else: 1854 else:
1850 task.driver.console.stop_console(task) 1855 task.driver.console.stop_console(task)
1856
1851 except Exception as e: 1857 except Exception as e:
1852 op = _('enabling') if enabled else _('disabling') 1858 with excutils.save_and_reraise_exception():
1853 msg = (_('Error %(op)s the console on node %(node)s. ' 1859 op = _('enabling') if enabled else _('disabling')
1854 'Reason: %(error)s') % {'op': op, 1860 msg = (_('Error %(op)s the console on node %(node)s. '
1855 'node': node.uuid, 1861 'Reason: %(error)s') % {'op': op,
1856 'error': e}) 1862 'node': node.uuid,
1857 node.last_error = msg 1863 'error': e})
1858 LOG.error(msg) 1864 node.last_error = msg
1859 node.save() 1865 LOG.error(msg)
1860 notify_utils.emit_console_notification( 1866 node.save()
1861 task, 'console_set', fields.NotificationStatus.ERROR) 1867 notify_utils.emit_console_notification(
1862 else: 1868 task, 'console_set', fields.NotificationStatus.ERROR)
1863 node.console_enabled = enabled 1869
1864 node.last_error = None 1870 node.console_enabled = enabled
1865 node.save() 1871 node.last_error = None
1866 notify_utils.emit_console_notification( 1872 node.save()
1867 task, 'console_set', fields.NotificationStatus.END) 1873 notify_utils.emit_console_notification(
1874 task, 'console_set', fields.NotificationStatus.END)
1868 1875
1869 @METRICS.timer('ConductorManager.create_port') 1876 @METRICS.timer('ConductorManager.create_port')
1870 @messaging.expected_exceptions(exception.NodeLocked, 1877 @messaging.expected_exceptions(exception.NodeLocked,
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index 619451e..eea77ad 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -1707,17 +1707,43 @@ class DoNodeDeployTearDownTestCase(mgr_utils.ServiceSetUpMixin,
1707 self.assertIsNotNone(node.last_error) 1707 self.assertIsNotNone(node.last_error)
1708 # Assert instance_info was erased 1708 # Assert instance_info was erased
1709 self.assertEqual({}, node.instance_info) 1709 self.assertEqual({}, node.instance_info)
1710 mock_tear_down.assert_called_once_with(mock.ANY) 1710 mock_tear_down.assert_called_once_with(task)
1711
1712 @mock.patch('ironic.conductor.manager.ConductorManager._set_console_mode')
1713 def test_do_node_tear_down_console_raises_error(self, mock_console):
1714 # test when _set_console_mode raises exception
1715 node = obj_utils.create_test_node(
1716 self.context, driver='fake', provision_state=states.DELETING,
1717 target_provision_state=states.AVAILABLE,
1718 instance_info={'foo': 'bar'},
1719 console_enabled=True,
1720 driver_internal_info={'is_whole_disk_image': False})
1721
1722 task = task_manager.TaskManager(self.context, node.uuid)
1723 self._start_service()
1724 mock_console.side_effect = exception.ConsoleError('test')
1725 self.assertRaises(exception.ConsoleError,
1726 self.service._do_node_tear_down, task)
1727 node.refresh()
1728 self.assertEqual(states.ERROR, node.provision_state)
1729 self.assertEqual(states.NOSTATE, node.target_provision_state)
1730 self.assertIsNotNone(node.last_error)
1731 # Assert instance_info was erased
1732 self.assertEqual({}, node.instance_info)
1733 mock_console.assert_called_once_with(task, False)
1711 1734
1735 @mock.patch('ironic.conductor.manager.ConductorManager._set_console_mode')
1712 @mock.patch('ironic.conductor.manager.ConductorManager._do_node_clean') 1736 @mock.patch('ironic.conductor.manager.ConductorManager._do_node_clean')
1713 @mock.patch('ironic.drivers.modules.fake.FakeDeploy.tear_down') 1737 @mock.patch('ironic.drivers.modules.fake.FakeDeploy.tear_down')
1714 def test__do_node_tear_down_ok(self, mock_tear_down, mock_clean): 1738 def _test__do_node_tear_down_ok(self, mock_tear_down, mock_clean,
1739 mock_console, enabled_console=False):
1715 # test when driver.deploy.tear_down succeeds 1740 # test when driver.deploy.tear_down succeeds
1716 node = obj_utils.create_test_node( 1741 node = obj_utils.create_test_node(
1717 self.context, driver='fake', provision_state=states.DELETING, 1742 self.context, driver='fake', provision_state=states.DELETING,
1718 target_provision_state=states.AVAILABLE, 1743 target_provision_state=states.AVAILABLE,
1719 instance_uuid=uuidutils.generate_uuid(), 1744 instance_uuid=uuidutils.generate_uuid(),
1720 instance_info={'foo': 'bar'}, 1745 instance_info={'foo': 'bar'},
1746 console_enabled=enabled_console,
1721 driver_internal_info={'is_whole_disk_image': False, 1747 driver_internal_info={'is_whole_disk_image': False,
1722 'instance': {'ephemeral_gb': 10}}) 1748 'instance': {'ephemeral_gb': 10}})
1723 1749
@@ -1734,6 +1760,16 @@ class DoNodeDeployTearDownTestCase(mgr_utils.ServiceSetUpMixin,
1734 self.assertNotIn('instance', node.driver_internal_info) 1760 self.assertNotIn('instance', node.driver_internal_info)
1735 mock_tear_down.assert_called_once_with(mock.ANY) 1761 mock_tear_down.assert_called_once_with(mock.ANY)
1736 mock_clean.assert_called_once_with(mock.ANY) 1762 mock_clean.assert_called_once_with(mock.ANY)
1763 if enabled_console:
1764 mock_console.assert_called_once_with(task, False)
1765 else:
1766 mock_console.assert_not_called()
1767
1768 def test__do_node_tear_down_ok_without_console(self):
1769 self._test__do_node_tear_down_ok(enabled_console=False)
1770
1771 def test__do_node_tear_down_ok_with_console(self):
1772 self._test__do_node_tear_down_ok(enabled_console=True)
1737 1773
1738 @mock.patch('ironic.conductor.manager.ConductorManager._do_node_clean') 1774 @mock.patch('ironic.conductor.manager.ConductorManager._do_node_clean')
1739 @mock.patch('ironic.drivers.modules.fake.FakeDeploy.tear_down') 1775 @mock.patch('ironic.drivers.modules.fake.FakeDeploy.tear_down')
diff --git a/releasenotes/notes/stop-console-during-unprovision-a29d8facb3f03be5.yaml b/releasenotes/notes/stop-console-during-unprovision-a29d8facb3f03be5.yaml
new file mode 100644
index 0000000..71b035a
--- /dev/null
+++ b/releasenotes/notes/stop-console-during-unprovision-a29d8facb3f03be5.yaml
@@ -0,0 +1,7 @@
1---
2security:
3 - |
4 Fixes an issue where an enabled console could be left running after a node
5 was unprovisioned. This allowed a user to view the console even after the
6 instance was gone. Ironic now stops the console during unprovisioning to
7 block this.