From f670f704f3e4bf9b05a97d1f39ab99febfe29cf7 Mon Sep 17 00:00:00 2001
From: Julia Kreger <juliaashleykreger@gmail.com>
Date: Thu, 20 Aug 2020 08:21:50 -0700
Subject: [PATCH] Clarify connection error on heartbeats

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
---
 ironic_python_agent/errors.py                          | 10 ++++++++++
 ironic_python_agent/ironic_api_client.py               |  2 ++
 .../tests/unit/test_ironic_api_client.py               | 10 ++++++++++
 ...y-heartbeat-connection-errors-2af152bf2d7928e2.yaml | 10 ++++++++++
 4 files changed, 32 insertions(+)
 create mode 100644 releasenotes/notes/clarify-heartbeat-connection-errors-2af152bf2d7928e2.yaml

diff --git a/ironic_python_agent/errors.py b/ironic_python_agent/errors.py
index 99bbe2b81..bf0253dd8 100644
--- a/ironic_python_agent/errors.py
+++ b/ironic_python_agent/errors.py
@@ -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)
diff --git a/ironic_python_agent/ironic_api_client.py b/ironic_python_agent/ironic_api_client.py
index 747b133d2..4738f22d1 100644
--- a/ironic_python_agent/ironic_api_client.py
+++ b/ironic_python_agent/ironic_api_client.py
@@ -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))
 
diff --git a/ironic_python_agent/tests/unit/test_ironic_api_client.py b/ironic_python_agent/tests/unit/test_ironic_api_client.py
index 562f06be2..bd710e77f 100644
--- a/ironic_python_agent/tests/unit/test_ironic_api_client.py
+++ b/ironic_python_agent/tests/unit/test_ironic_api_client.py
@@ -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)
diff --git a/releasenotes/notes/clarify-heartbeat-connection-errors-2af152bf2d7928e2.yaml b/releasenotes/notes/clarify-heartbeat-connection-errors-2af152bf2d7928e2.yaml
new file mode 100644
index 000000000..dace1cd50
--- /dev/null
+++ b/releasenotes/notes/clarify-heartbeat-connection-errors-2af152bf2d7928e2.yaml
@@ -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.