diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index 3c2bddc7d..686a5788b 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -177,7 +177,10 @@ class IronicPythonAgentHeartbeater(threading.Thread): """Stop the heartbeat thread.""" LOG.info('stopping heartbeater') self.stop_event.set() - return self.join() + # Only join if the thread was actually started + if self.is_alive(): + return self.join() + return None class IronicPythonAgent(base.ExecuteCommandMixin): @@ -621,13 +624,15 @@ class IronicPythonAgent(base.ExecuteCommandMixin): # rescue operations. LOG.info('Found rescue state marker file, locking down IPA ' 'in disabled mode') - self.heartbeater.stop() + if hasattr(self, 'heartbeater') and self.heartbeater.is_alive(): + self.heartbeater.stop() self.serve_api = False while True: time.sleep(100) if not self.standalone and self.api_urls: - self.heartbeater.stop() + if hasattr(self, 'heartbeater') and self.heartbeater.is_alive(): + self.heartbeater.stop() if self.lockdown: self._lockdown_system() diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index fd0ad7a16..5d643b827 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -184,6 +184,25 @@ class TestHeartbeater(ironic_agent_base.IronicAgentTest): mock_time.return_value = 110 self.assertTrue(self.heartbeater._heartbeat_expected()) + def test_stop_not_started(self): + """Test stop() when thread was never started.""" + # Thread is not alive, should not call join() + self.assertFalse(self.heartbeater.is_alive()) + result = self.heartbeater.stop() + self.assertIsNone(result) + self.assertTrue(self.heartbeater.stop_event.set.called) + + @mock.patch.object(agent.IronicPythonAgentHeartbeater, 'join', + autospec=True) + def test_stop_when_alive(self, mock_join): + """Test stop() when thread is alive.""" + # Mock the thread as alive + with mock.patch.object(self.heartbeater, 'is_alive', + autospec=True, return_value=True): + self.heartbeater.stop() + mock_join.assert_called_once_with(self.heartbeater) + self.assertTrue(self.heartbeater.stop_event.set.called) + @mock.patch.object(hardware, '_md_scan_and_assemble', lambda: None) @mock.patch.object(hardware, '_check_for_iscsi', lambda: None) @@ -800,6 +819,49 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): mock.call('wait_for_disks')], mock_dispatch.call_args_list) + @mock.patch( + 'ironic_python_agent.hardware_managers.cna._detect_cna_card', + mock.Mock()) + @mock.patch('os.path.exists', autospec=True) + @mock.patch.object(hardware, 'get_managers', autospec=True) + @mock.patch.object(time, 'sleep', autospec=True) + @mock.patch.object(agent.IronicPythonAgent, '_wait_for_interface', + autospec=True) + @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True) + def test_run_rescue_mode_heartbeater_not_started( + self, mock_dispatch, mock_wait, mock_sleep, mock_get_managers, + mock_exists): + """Test rescue mode doesn't fail when heartbeater not started.""" + CONF.set_override('inspection_callback_url', '') + + # Mock rescue mode marker file exists + mock_exists.return_value = True + + self.agent.heartbeater = mock.Mock() + self.agent.heartbeater.is_alive.return_value = False + self.agent.api_client.lookup_node = mock.Mock() + self.agent.api_client.lookup_node.return_value = { + 'node': { + 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' + }, + 'config': { + 'heartbeat_timeout': 300 + } + } + + # Setup to exit the infinite loop after first iteration + mock_sleep.side_effect = [None, KeyboardInterrupt()] + + try: + self.agent.run() + except KeyboardInterrupt: + pass + + # Heartbeater should not be started or stopped in rescue mode + self.agent.heartbeater.start.assert_not_called() + self.agent.heartbeater.stop.assert_not_called() + self.assertFalse(self.agent.serve_api) + def test_async_command_success(self): result = base.AsyncCommandResult('foo_command', {'fail': False}, foo_execute) diff --git a/releasenotes/notes/fix-rescue-mode-heartbeater-stop-8f3a2b1c4d5e6f7g.yaml b/releasenotes/notes/fix-rescue-mode-heartbeater-stop-8f3a2b1c4d5e6f7g.yaml new file mode 100644 index 000000000..6534ab0fe --- /dev/null +++ b/releasenotes/notes/fix-rescue-mode-heartbeater-stop-8f3a2b1c4d5e6f7g.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes RuntimeError when entering rescue mode by checking if the + heartbeater thread is alive before attempting to stop it. +