Merge "Stop console at tearing down without unsetting console_enabled"

ironic/conductor/manager.py

@@ -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:
@@ -3564,6 +3580,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.',
@@ -3669,12 +3686,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):

ironic/tests/unit/conductor/test_manager.py

@@ -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)

releasenotes/notes/bug-2003972-dae9b7d0f6180339.yaml

@@ -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.