Skip to content

Commit

Permalink
Clarify connection error on heartbeats
Browse files Browse the repository at this point in the history
Heartbeat connection errors are often a sign of a transitory
network failures which may resolve themselves. But an operator
looking at the screen doesn't necessarilly know that.

They don't understand that there could have been a network
failure, or a misconfiguration that caused the connectivity
failure and soft of kind of default to "well it failed"
without further clarification.

As such, this patch adds explicit catching of the requests
ConnectionError exception and rasies a new internal error
with a more verbose error message in that event to provide
operators with additional clarity.

Change-Id: I4cb2c0d1f577df1c4451308bd86efa8f94390b0c
Story: 2008046
Task: 40709
  • Loading branch information
juliakreger committed Aug 20, 2020
1 parent ba6ca24 commit f670f70
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 0 deletions.
10 changes: 10 additions & 0 deletions ironic_python_agent/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,3 +348,13 @@ class ClockSyncError(RESTError):
"""Error raised when attempting to sync the system clock."""

message = 'Error syncing system clock'


class HeartbeatConnectionError(IronicAPIError):
"""Transitory connection failure occured attempting to contact the API."""

message = ("Error attempting to heartbeat - Possible transitory network "
"failure or blocking port may be present.")

def __init__(self, details):
super(HeartbeatConnectionError, self).__init__(details)
2 changes: 2 additions & 0 deletions ironic_python_agent/ironic_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ def heartbeat(self, uuid, advertise_address, advertise_protocol='http'):

try:
response = self._request('POST', path, data=data, headers=headers)
except requests.exceptions.ConnectionError as e:
raise errors.HeartbeatConnectionError(str(e))
except Exception as e:
raise errors.HeartbeatError(str(e))

Expand Down
10 changes: 10 additions & 0 deletions ironic_python_agent/tests/unit/test_ironic_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,16 @@ def test_heartbeat_409_status_code(self):
uuid='deadbeef-dabb-ad00-b105-f00d00bab10c',
advertise_address=('192.0.2.1', '9999'))

def test_heartbeat_requests_connection_error(self):
self.api_client.session.request = mock.Mock()
self.api_client.session.request.side_effect = \
requests.exceptions.ConnectionError
self.assertRaisesRegex(errors.HeartbeatConnectionError,
'transitory network failure or blocking port',
self.api_client.heartbeat,
uuid='meow',
advertise_address=('192.0.2.1', '9999'))

@mock.patch('eventlet.greenthread.sleep', autospec=True)
@mock.patch('ironic_python_agent.ironic_api_client.APIClient._do_lookup',
autospec=True)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
other:
- |
Adds an explicit capture of connectivity failures in the heartbeat
process to provide a more verbose error message in line with what is
occuring as opposed to just indicating that an error occured. This
new exception is called ``HeartbeatConnectionError`` and is likely only
going to be visible if there is a local connectivity failure such as a
router failure, switchport in a blocking state, or connection centered
transient failure.

0 comments on commit f670f70

Please sign in to comment.