summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--ironic_python_agent/agent.py21
-rw-r--r--ironic_python_agent/tests/unit/test_agent.py44
-rw-r--r--releasenotes/notes/coalesce_heartbeats-fb8899a5f9fe4709.yaml10
3 files changed, 66 insertions, 9 deletions
diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py
index 8e36af2c..a9f8077f 100644
--- a/ironic_python_agent/agent.py
+++ b/ironic_python_agent/agent.py
@@ -88,6 +88,8 @@ class IronicPythonAgentHeartbeater(threading.Thread):
self.stop_event = threading.Event()
self.api = agent.api_client
self.interval = 0
+ self.heartbeat_forced = False
+ self.previous_heartbeat = 0
def run(self):
"""Start the heartbeat thread."""
@@ -95,10 +97,21 @@ class IronicPythonAgentHeartbeater(threading.Thread):
LOG.info('Starting heartbeater')
self.agent.set_agent_advertise_addr()
- while not self.stop_event.wait(self.interval):
- self.do_heartbeat()
+ while not self.stop_event.wait(min(self.interval, 5)):
+ if self._heartbeat_expected():
+ self.do_heartbeat()
eventlet.sleep(0)
+ def _heartbeat_expected(self):
+ # Normal heartbeating
+ if _time() > self.previous_heartbeat + self.interval:
+ return True
+
+ # Forced heartbeating, but once in 5 seconds
+ if (self.heartbeat_forced
+ and _time() > self.previous_heartbeat + 5):
+ return True
+
def do_heartbeat(self):
"""Send a heartbeat to Ironic."""
try:
@@ -109,6 +122,8 @@ class IronicPythonAgentHeartbeater(threading.Thread):
generated_cert=self.agent.generated_cert,
)
LOG.info('heartbeat successful')
+ self.heartbeat_forced = False
+ self.previous_heartbeat = _time()
except errors.HeartbeatConflictError:
LOG.warning('conflict error sending heartbeat to {}'.format(
self.agent.api_url))
@@ -123,7 +138,7 @@ class IronicPythonAgentHeartbeater(threading.Thread):
LOG.info(log_msg.format(self.interval))
def force_heartbeat(self):
- self.do_heartbeat()
+ self.heartbeat_forced = True
def stop(self):
"""Stop the heartbeat thread."""
diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py
index 646a852e..b020f692 100644
--- a/ironic_python_agent/tests/unit/test_agent.py
+++ b/ironic_python_agent/tests/unit/test_agent.py
@@ -83,8 +83,7 @@ class TestHeartbeater(ironic_agent_base.IronicAgentTest):
time_responses.append(50)
# SECOND RUN:
- # 50 * .5 = 25
- expected_stop_calls.append(mock.call(25.0))
+ expected_stop_calls.append(mock.call(5))
wait_responses.append(False)
# next heartbeat due at t=180
heartbeat_responses.append(180)
@@ -92,10 +91,11 @@ class TestHeartbeater(ironic_agent_base.IronicAgentTest):
uniform_responses.append(0.4)
# time is now 80
time_responses.append(80)
+ # add one response for _time in _heartbeat_expected
+ time_responses.append(80)
# THIRD RUN:
- # 50 * .4 = 20
- expected_stop_calls.append(mock.call(20.0))
+ expected_stop_calls.append(mock.call(5))
wait_responses.append(False)
# this heartbeat attempt fails
heartbeat_responses.append(Exception('uh oh!'))
@@ -107,8 +107,7 @@ class TestHeartbeater(ironic_agent_base.IronicAgentTest):
time_responses.append(125.5)
# FOURTH RUN:
- # 50 * .5 = 20
- expected_stop_calls.append(mock.call(25.0))
+ expected_stop_calls.append(mock.call(5))
# Stop now
wait_responses.append(True)
@@ -123,6 +122,39 @@ class TestHeartbeater(ironic_agent_base.IronicAgentTest):
# Validate expectations
self.assertEqual(expected_stop_calls,
self.heartbeater.stop_event.wait.call_args_list)
+ self.assertEqual(self.heartbeater.api.heartbeat.call_count, 2)
+ self.assertEqual(mock_time.call_count, 5)
+
+ @mock.patch('ironic_python_agent.agent._time', autospec=True)
+ def test__heartbeat_expected(self, mock_time):
+
+ # initial setting
+ self.heartbeater.previous_heartbeat = 0
+ self.heartbeater.interval = 0
+ self.heartbeater.heartbeat_forced = False
+ mock_time.return_value = 0
+ self.assertFalse(self.heartbeater._heartbeat_expected())
+
+ # 1st cadence
+ self.heartbeater.previous_heartbeat = 0
+ self.heartbeater.interval = 100
+ self.heartbeater.heartbeat_forced = False
+ mock_time.return_value = 5
+ self.assertFalse(self.heartbeater._heartbeat_expected())
+
+ # 2nd cadence with a forced heartbeat
+ self.heartbeater.previous_heartbeat = 0
+ self.heartbeater.interval = 100
+ self.heartbeater.heartbeat_forced = True
+ mock_time.return_value = 10
+ self.assertTrue(self.heartbeater._heartbeat_expected())
+
+ # 11th cadence with a scheduled heartbeat
+ self.heartbeater.previous_heartbeat = 0
+ self.heartbeater.interval = 100
+ self.heartbeater.heartbeat_forced = False
+ mock_time.return_value = 110
+ self.assertTrue(self.heartbeater._heartbeat_expected())
@mock.patch.object(hardware, '_md_scan_and_assemble', lambda: None)
diff --git a/releasenotes/notes/coalesce_heartbeats-fb8899a5f9fe4709.yaml b/releasenotes/notes/coalesce_heartbeats-fb8899a5f9fe4709.yaml
new file mode 100644
index 00000000..db785bd5
--- /dev/null
+++ b/releasenotes/notes/coalesce_heartbeats-fb8899a5f9fe4709.yaml
@@ -0,0 +1,10 @@
+---
+features:
+ - |
+ Heartbeats to the conductor are grouped when they are scheduled or
+ requested within a time interval of five seconds to avoid sending
+ them in quick succession.
+fixes:
+ - |
+ Fixes an issue where a quick succession of heartbeats exposes a race
+ condition in the conductor's RPC handling.