From 6dc09f31488731bb92b13c9d4bd87c2755ff9f09 Mon Sep 17 00:00:00 2001 From: Josh Gachnang <josh@pcsforeducation.com> Date: Thu, 20 Mar 2014 15:18:48 -0700 Subject: [PATCH 1/3] Getting the heartbeat from Ironic instead --- ironic_python_agent/agent.py | 20 +++++++------- ironic_python_agent/ironic_api_client.py | 8 +++--- ironic_python_agent/tests/agent.py | 22 +++++++++++----- .../tests/ironic_api_client.py | 26 ++++++++++++++++--- 4 files changed, 53 insertions(+), 23 deletions(-) diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index 089548bc9..e71651a21 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -81,16 +81,17 @@ class IronicPythonAgentHeartbeater(threading.Thread): interval = 0 while not self.stop_event.wait(interval): - next_heartbeat_by = self.do_heartbeat() + self.do_heartbeat() + # next_heartbeat_by = _time() + self.agent.heartbeat_timeout interval_multiplier = random.uniform(self.min_jitter_multiplier, self.max_jitter_multiplier) - interval = (next_heartbeat_by - _time()) * interval_multiplier + interval = self.agent.heartbeat_timeout * interval_multiplier log_msg = 'sleeping before next heartbeat, interval: {0}' self.log.info(log_msg.format(interval)) def do_heartbeat(self): try: - deadline = self.api.heartbeat( + self.api.heartbeat( uuid=self.agent.get_node_uuid(), advertise_address=self.agent.advertise_address ) @@ -98,12 +99,8 @@ class IronicPythonAgentHeartbeater(threading.Thread): self.log.info('heartbeat successful') except Exception: self.log.exception('error sending heartbeat') - deadline = _time() + self.error_delay self.error_delay = min(self.error_delay * self.backoff_factor, self.max_delay) - pass - - return deadline def stop(self): self.log.info('stopping heartbeater') @@ -123,6 +120,7 @@ class IronicPythonAgent(object): self.api = app.VersionSelectorApplication(self) self.command_results = utils.get_ordereddict() self.heartbeater = IronicPythonAgentHeartbeater(self) + self.heartbeat_timeout = None self.hardware = hardware.get_manager() self.command_lock = threading.Lock() self.log = log.getLogger(__name__) @@ -213,9 +211,13 @@ class IronicPythonAgent(object): """Run the Ironic Python Agent.""" self.started_at = _time() # Get the UUID so we can heartbeat to Ironic - self.node = self.api_client.lookup_node( - hardware_info=self.hardware.list_hardware_info(), + content = self.api_client.lookup_node( + hardware_info=self.hardware.list_hardware_info() ) + if 'node' not in content or 'heartbeat_timeout' not in content: + raise LookupError('Lookup return needs node and heartbeat_timeout') + self.node = content['node'] + self.heartbeat_timeout = content['heartbeat_timeout'] self.heartbeater.start() wsgi = simple_server.make_server( self.listen_address[0], diff --git a/ironic_python_agent/ironic_api_client.py b/ironic_python_agent/ironic_api_client.py index a3f06dee4..e4d820c76 100644 --- a/ironic_python_agent/ironic_api_client.py +++ b/ironic_python_agent/ironic_api_client.py @@ -96,9 +96,11 @@ class APIClient(object): + str(e)) if 'node' not in content or 'uuid' not in content['node']: - raise errors.LookupNodeError('Got invalid data from the API: ' - '{0}'.format(content)) - return content['node'] + raise errors.LookupNodeError('Got invalid node data from the API:' + ' {0}'.format(content)) + if 'heartbeat_timeout' not in content: + raise errors.LookupNodeError('Got invalid') + return content def _get_agent_url(self, advertise_address): return 'http://{0}:{1}'.format(advertise_address[0], diff --git a/ironic_python_agent/tests/agent.py b/ironic_python_agent/tests/agent.py index 024879142..a4f4f30b6 100644 --- a/ironic_python_agent/tests/agent.py +++ b/ironic_python_agent/tests/agent.py @@ -73,7 +73,7 @@ class TestHeartbeater(unittest.TestCase): time_responses.append(50) # SECOND RUN: - # (100 - 50) * .5 = 25 (t becomes ~75) + # 50 * .5 = 25 expected_stop_event_calls.append(mock.call(25.0)) wait_responses.append(False) # next heartbeat due at t=180 @@ -84,8 +84,8 @@ class TestHeartbeater(unittest.TestCase): time_responses.append(80) # THIRD RUN: - # (180 - 80) * .4 = 40 (t becomes ~120) - expected_stop_event_calls.append(mock.call(40.0)) + # 50 * .4 = 20 + expected_stop_event_calls.append(mock.call(20.0)) wait_responses.append(False) # this heartbeat attempt fails heartbeat_responses.append(Exception('uh oh!')) @@ -97,21 +97,23 @@ class TestHeartbeater(unittest.TestCase): time_responses.append(125.5) # FOURTH RUN: - # (125.5 - 125.0) * .5 = 0.25 - expected_stop_event_calls.append(mock.call(0.25)) + # 50 * .5 = 25 + expected_stop_event_calls.append(mock.call(25)) # Stop now wait_responses.append(True) # Hook it up and run it mocked_time.side_effect = time_responses mocked_uniform.side_effect = uniform_responses + self.mock_agent.heartbeat_timeout = 50 self.heartbeater.api.heartbeat.side_effect = heartbeat_responses self.heartbeater.stop_event.wait.side_effect = wait_responses self.heartbeater.run() # Validate expectations - self.assertEqual(self.heartbeater.stop_event.wait.call_args_list, - expected_stop_event_calls) + self.assertEqual(expected_stop_event_calls, + self.heartbeater.stop_event.wait.call_args_list, + ) self.assertEqual(self.heartbeater.error_delay, 2.7) @@ -164,6 +166,12 @@ class TestBaseAgent(unittest.TestCase): self.agent.heartbeater = mock.Mock() self.agent.api_client.lookup_node = mock.Mock() + self.agent.api_client.lookup_node.return_value = { + 'node': { + 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' + }, + 'heartbeat_timeout': 300 + } self.agent.run() listen_addr = ('192.0.2.1', 9999) diff --git a/ironic_python_agent/tests/ironic_api_client.py b/ironic_python_agent/tests/ironic_api_client.py index 8dd4dfb24..3eb0b4348 100644 --- a/ironic_python_agent/tests/ironic_api_client.py +++ b/ironic_python_agent/tests/ironic_api_client.py @@ -104,7 +104,8 @@ class TestBaseIronicPythonAgent(unittest.TestCase): response = httmock.response(status_code=200, content={ 'node': { 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' - } + }, + 'heartbeat_timeout': 300 }) self.api_client.session.request = mock.Mock() @@ -144,10 +145,12 @@ class TestBaseIronicPythonAgent(unittest.TestCase): self.assertRaises(errors.LookupNodeError, self.api_client.lookup_node, hardware_info=self.hardware_info, - ) + ) def test_lookup_node_bad_response_data(self): - response = httmock.response(status_code=200, content='a') + response = httmock.response(status_code=200, content={ + 'heartbeat_timeout': 300 + }) self.api_client.session.request = mock.Mock() self.api_client.session.request.return_value = response @@ -155,7 +158,22 @@ class TestBaseIronicPythonAgent(unittest.TestCase): self.assertRaises(errors.LookupNodeError, self.api_client.lookup_node, hardware_info=self.hardware_info - ) + ) + + def test_lookup_node_no_heartbeat_timeout(self): + response = httmock.response(status_code=200, content={ + 'node': { + 'uuid': 'deadbeef-dabb-ad00-b105-f00d00bab10c' + } + }) + + self.api_client.session.request = mock.Mock() + self.api_client.session.request.return_value = response + + self.assertRaises(errors.LookupNodeError, + self.api_client.lookup_node, + hardware_info=self.hardware_info + ) def test_lookup_node_bad_response_body(self): response = httmock.response(status_code=200, content={ From 234d2097298791e5d6fa64b4ce67ffb888401e33 Mon Sep 17 00:00:00 2001 From: Josh Gachnang <josh@pcsforeducation.com> Date: Thu, 20 Mar 2014 15:51:54 -0700 Subject: [PATCH 2/3] Removing commented out code --- ironic_python_agent/agent.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index e71651a21..bab3449ee 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -82,7 +82,6 @@ class IronicPythonAgentHeartbeater(threading.Thread): while not self.stop_event.wait(interval): self.do_heartbeat() - # next_heartbeat_by = _time() + self.agent.heartbeat_timeout interval_multiplier = random.uniform(self.min_jitter_multiplier, self.max_jitter_multiplier) interval = self.agent.heartbeat_timeout * interval_multiplier From d8c0f1b7949da842e9e2095d878571e34ee01408 Mon Sep 17 00:00:00 2001 From: Josh Gachnang <josh@pcsforeducation.com> Date: Thu, 20 Mar 2014 15:57:20 -0700 Subject: [PATCH 3/3] Actual exception message, stylistic changes --- ironic_python_agent/ironic_api_client.py | 5 +++-- ironic_python_agent/tests/agent.py | 3 +-- ironic_python_agent/tests/ironic_api_client.py | 16 +++++----------- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/ironic_python_agent/ironic_api_client.py b/ironic_python_agent/ironic_api_client.py index e4d820c76..c9d8d44fe 100644 --- a/ironic_python_agent/ironic_api_client.py +++ b/ironic_python_agent/ironic_api_client.py @@ -97,9 +97,10 @@ class APIClient(object): if 'node' not in content or 'uuid' not in content['node']: raise errors.LookupNodeError('Got invalid node data from the API:' - ' {0}'.format(content)) + '%s' % content) if 'heartbeat_timeout' not in content: - raise errors.LookupNodeError('Got invalid') + raise errors.LookupNodeError('Got invalid heartbeat from the API:' + '%s' % content) return content def _get_agent_url(self, advertise_address): diff --git a/ironic_python_agent/tests/agent.py b/ironic_python_agent/tests/agent.py index a4f4f30b6..9d3badf1d 100644 --- a/ironic_python_agent/tests/agent.py +++ b/ironic_python_agent/tests/agent.py @@ -112,8 +112,7 @@ class TestHeartbeater(unittest.TestCase): # Validate expectations self.assertEqual(expected_stop_event_calls, - self.heartbeater.stop_event.wait.call_args_list, - ) + self.heartbeater.stop_event.wait.call_args_list) self.assertEqual(self.heartbeater.error_delay, 2.7) diff --git a/ironic_python_agent/tests/ironic_api_client.py b/ironic_python_agent/tests/ironic_api_client.py index 3eb0b4348..3fb4384b6 100644 --- a/ironic_python_agent/tests/ironic_api_client.py +++ b/ironic_python_agent/tests/ironic_api_client.py @@ -111,9 +111,7 @@ class TestBaseIronicPythonAgent(unittest.TestCase): self.api_client.session.request = mock.Mock() self.api_client.session.request.return_value = response - self.api_client.lookup_node( - hardware_info=self.hardware_info, - ) + self.api_client.lookup_node(hardware_info=self.hardware_info) request_args = self.api_client.session.request.call_args[0] self.assertEqual(request_args[0], 'POST') @@ -144,8 +142,7 @@ class TestBaseIronicPythonAgent(unittest.TestCase): self.assertRaises(errors.LookupNodeError, self.api_client.lookup_node, - hardware_info=self.hardware_info, - ) + hardware_info=self.hardware_info) def test_lookup_node_bad_response_data(self): response = httmock.response(status_code=200, content={ @@ -157,8 +154,7 @@ class TestBaseIronicPythonAgent(unittest.TestCase): self.assertRaises(errors.LookupNodeError, self.api_client.lookup_node, - hardware_info=self.hardware_info - ) + hardware_info=self.hardware_info) def test_lookup_node_no_heartbeat_timeout(self): response = httmock.response(status_code=200, content={ @@ -172,8 +168,7 @@ class TestBaseIronicPythonAgent(unittest.TestCase): self.assertRaises(errors.LookupNodeError, self.api_client.lookup_node, - hardware_info=self.hardware_info - ) + hardware_info=self.hardware_info) def test_lookup_node_bad_response_body(self): response = httmock.response(status_code=200, content={ @@ -185,5 +180,4 @@ class TestBaseIronicPythonAgent(unittest.TestCase): self.assertRaises(errors.LookupNodeError, self.api_client.lookup_node, - hardware_info=self.hardware_info, - ) + hardware_info=self.hardware_info)