diff --git a/ironic/drivers/modules/agent_base_vendor.py b/ironic/drivers/modules/agent_base_vendor.py index 3a1c507827..a158daddd0 100644 --- a/ironic/drivers/modules/agent_base_vendor.py +++ b/ironic/drivers/modules/agent_base_vendor.py @@ -67,11 +67,14 @@ VENDOR_PROPERTIES = { 'older deploy ramdisks. Defaults to False. Optional.') } -__HEARTBEAT_RECORD_ONLY = (states.ENROLL, states.MANAGEABLE, - states.AVAILABLE) +__HEARTBEAT_RECORD_ONLY = (states.ENROLL, states.MANAGEABLE, states.AVAILABLE, + states.CLEANING, states.DEPLOYING, states.RESCUING) _HEARTBEAT_RECORD_ONLY = frozenset(__HEARTBEAT_RECORD_ONLY) -_HEARTBEAT_ALLOWED = (states.DEPLOYWAIT, states.CLEANWAIT, states.RESCUEWAIT) +_HEARTBEAT_ALLOWED = (states.DEPLOYWAIT, states.CLEANWAIT, states.RESCUEWAIT, + # These are allowed but don't cause any actions since + # they're also in HEARTBEAT_RECORD_ONLY. + states.DEPLOYING, states.CLEANING, states.RESCUING) HEARTBEAT_ALLOWED = frozenset(_HEARTBEAT_ALLOWED) _FASTTRACK_HEARTBEAT_ALLOWED = (states.DEPLOYWAIT, states.CLEANWAIT, diff --git a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py index 4ac6497d45..2fcf4d2e05 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base_vendor.py @@ -269,6 +269,7 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): self.assertEqual(0, rti_mock.call_count) self.assertEqual(0, cd_mock.call_count) + @mock.patch.object(agent_base_vendor.LOG, 'error', autospec=True) @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'continue_deploy', autospec=True) @mock.patch.object(agent_base_vendor.HeartbeatMixin, @@ -276,19 +277,25 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): @mock.patch.object(manager_utils, 'notify_conductor_resume_clean', autospec=True) def test_heartbeat_noops_in_wrong_state(self, ncrc_mock, rti_mock, - cd_mock): - allowed = {states.DEPLOYWAIT, states.CLEANWAIT} + cd_mock, log_mock): + allowed = {states.DEPLOYWAIT, states.CLEANWAIT, states.RESCUEWAIT, + states.DEPLOYING, states.CLEANING, states.RESCUING} for state in set(states.machine.states) - allowed: - for m in (ncrc_mock, rti_mock, cd_mock): + for m in (ncrc_mock, rti_mock, cd_mock, log_mock): m.reset_mock() with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: - self.node.provision_state = state + task.node.provision_state = state self.deploy.heartbeat(task, 'url', '1.0.0') self.assertTrue(task.shared) + self.assertNotIn('agent_last_heartbeat', + task.node.driver_internal_info) self.assertEqual(0, ncrc_mock.call_count) self.assertEqual(0, rti_mock.call_count) self.assertEqual(0, cd_mock.call_count) + log_mock.assert_called_once_with(mock.ANY, + {'node': self.node.uuid, + 'state': state}) @mock.patch.object(agent_base_vendor.HeartbeatMixin, 'continue_deploy', autospec=True) @@ -530,6 +537,25 @@ class HeartbeatMixinTest(AgentDeployMixinBaseTest): task.node.driver_internal_info['agent_last_heartbeat']) mock_touch.assert_called_once_with(mock.ANY) + @mock.patch.object(agent_base_vendor.LOG, 'error', autospec=True) + def test_heartbeat_records_cleaning_deploying(self, log_mock): + for provision_state in (states.CLEANING, states.DEPLOYING): + self.node.driver_internal_info = {} + self.node.provision_state = provision_state + self.node.save() + with task_manager.acquire( + self.context, self.node.uuid, shared=False) as task: + self.deploy.heartbeat(task, 'http://127.0.0.1:8080', '3.2.0') + self.assertEqual('http://127.0.0.1:8080', + task.node.driver_internal_info['agent_url']) + self.assertEqual('3.2.0', + task.node.driver_internal_info[ + 'agent_version']) + self.assertIsNotNone( + task.node.driver_internal_info['agent_last_heartbeat']) + self.assertEqual(provision_state, task.node.provision_state) + self.assertFalse(log_mock.called) + def test_heartbeat_records_fast_track(self): self.config(fast_track=True, group='deploy') for provision_state in [states.ENROLL, states.MANAGEABLE,