openstack/ironic
Code Issues Proposed changes

Do not log an error on heartbeat in deploying/cleaning/rescuing

Browse Source 
It is completely normal for IPA to heartbeat in these states while
ironic is busy doing something with the node. Just ignore it.

Also fixes the incorrect unit test on invalid states that was only
testing available in reality.

Partially reverts the effect of 2b8581af8c.

Change-Id: I2d7218285de97f4769a0e42797c272af95413fa2
This commit is contained in:
Dmitry Tantsur 2019-09-17 12:48:44 +02:00
parent aba282a7fc
commit 93bd867bef
2 changed files with 36 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
ironic/drivers/modules/agent_base_vendor.py
View File

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

34
ironic/tests/unit/drivers/modules/test_agent_base_vendor.py
View File

@ -231,6 +231,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,
@ -238,19 +239,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,
'in_core_deploy_step', autospec=True)
@ -469,6 +476,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,