Merge pull request #1646 from rackerlabs/retry_nautobot_intermittent_disconnects

feat: PUC-1466: Added retry logic for transient Nautobot failures

Author: cardoe

.pre-commit-config.yaml

@@ -106,6 +106,7 @@ repos:
         - "sushy"
         - "types-requests"
         - "netapp-ontap==9.17.1.0"
+        - "tenacity"
 
   - repo: local
     hooks:

python/understack-workflows/pyproject.toml

@@ -26,6 +26,7 @@ dependencies = [
     "sushy>=5.3.0,<6",
     "kubernetes==33.1.0",
     "netapp-ontap==9.17.1.0",
+    "tenacity>=8.0.0",
 ]
 
 [project.scripts]

python/understack-workflows/tests/test_nautobot_device_sync.py

@@ -5,6 +5,7 @@
 from unittest.mock import patch
 
 import pytest
+import requests
 
 from understack_workflows.oslo_event.nautobot_device_sync import EXIT_STATUS_FAILURE
 from understack_workflows.oslo_event.nautobot_device_sync import EXIT_STATUS_SUCCESS
@@ -15,6 +16,7 @@
 )
 from understack_workflows.oslo_event.nautobot_device_sync import _generate_device_name
 from understack_workflows.oslo_event.nautobot_device_sync import _get_record_value
+from understack_workflows.oslo_event.nautobot_device_sync import _is_retryable_error
 from understack_workflows.oslo_event.nautobot_device_sync import _normalise_manufacturer
 from understack_workflows.oslo_event.nautobot_device_sync import (
     _populate_from_inventory,
@@ -807,3 +809,85 @@ def test_handle_delete_event_success(self, mock_delete, mock_conn, mock_nautobot
 
         assert result == EXIT_STATUS_SUCCESS
         mock_delete.assert_called_once_with(node_uuid, mock_nautobot)
+
+
+class TestIsRetryableError:
+    """Test cases for _is_retryable_error function."""
+
+    def test_connection_error_is_retryable(self):
+        exc = requests.exceptions.ConnectionError("Connection refused")
+        assert _is_retryable_error(exc) is True
+
+    def test_503_service_unavailable_is_retryable(self):
+        from pynautobot import RequestError
+
+        mock_response = MagicMock()
+        mock_response.status_code = 503
+        mock_response.reason = "Service Unavailable"
+        mock_response.json.return_value = {"detail": "Service temporarily unavailable"}
+        mock_response.request.body = None
+        mock_response.url = "http://nautobot/api/dcim/devices/"
+        mock_response.text = "Service Unavailable"
+
+        exc = RequestError(mock_response)
+        assert _is_retryable_error(exc) is True
+
+    def test_502_bad_gateway_is_retryable(self):
+        from pynautobot import RequestError
+
+        mock_response = MagicMock()
+        mock_response.status_code = 502
+        mock_response.reason = "Bad Gateway"
+        mock_response.json.return_value = {}
+        mock_response.request.body = None
+        mock_response.url = "http://nautobot/api/dcim/devices/"
+        mock_response.text = "Bad Gateway"
+
+        exc = RequestError(mock_response)
+        assert _is_retryable_error(exc) is True
+
+    def test_504_gateway_timeout_is_retryable(self):
+        from pynautobot import RequestError
+
+        mock_response = MagicMock()
+        mock_response.status_code = 504
+        mock_response.reason = "Gateway Timeout"
+        mock_response.json.return_value = {}
+        mock_response.request.body = None
+        mock_response.url = "http://nautobot/api/dcim/devices/"
+        mock_response.text = "Gateway Timeout"
+
+        exc = RequestError(mock_response)
+        assert _is_retryable_error(exc) is True
+
+    def test_400_bad_request_is_not_retryable(self):
+        from pynautobot import RequestError
+
+        mock_response = MagicMock()
+        mock_response.status_code = 400
+        mock_response.reason = "Bad Request"
+        mock_response.json.return_value = {"device_type": ["Not found"]}
+        mock_response.request.body = None
+        mock_response.url = "http://nautobot/api/dcim/devices/"
+        mock_response.text = "Bad Request"
+
+        exc = RequestError(mock_response)
+        assert _is_retryable_error(exc) is False
+
+    def test_404_not_found_is_not_retryable(self):
+        from pynautobot import RequestError
+
+        mock_response = MagicMock()
+        mock_response.status_code = 404
+        mock_response.reason = "Not Found"
+        mock_response.json.return_value = {}
+        mock_response.request.body = None
+        mock_response.url = "http://nautobot/api/dcim/devices/123/"
+        mock_response.text = "Not Found"
+
+        exc = RequestError(mock_response)
+        assert _is_retryable_error(exc) is False
+
+    def test_value_error_is_not_retryable(self):
+        exc = ValueError("Invalid value")
+        assert _is_retryable_error(exc) is False

python/understack-workflows/understack_workflows/oslo_event/nautobot_device_sync.py

@@ -8,14 +8,18 @@
 Can be called from any event handler - provision, inspect, CRUD, etc.
 """
 
+import logging
 import re
 from dataclasses import dataclass
 from dataclasses import field
 from typing import Any
 from uuid import UUID
 
+import requests
+import tenacity
 from ironicclient.common.apiclient import exceptions as ironic_exceptions
 from openstack.connection import Connection
+from pynautobot import RequestError
 from pynautobot.core.api import Api as Nautobot
 
 from understack_workflows.helpers import setup_logger
@@ -30,6 +34,28 @@
 EXIT_STATUS_SUCCESS = 0
 EXIT_STATUS_FAILURE = 1
 
+# Retry configuration for transient failures
+RETRY_ATTEMPTS = 5
+RETRY_WAIT_MIN = 2
+RETRY_WAIT_MAX = 30
+
+
+def _is_retryable_error(exc: BaseException) -> bool:
+    """Determine if an exception is retryable.
+
+    Retries on:
+    - Connection errors (network issues, DNS failures)
+    - 503 Service Unavailable (Nautobot temporarily down)
+    - 502 Bad Gateway (proxy/load balancer issues)
+    - 504 Gateway Timeout
+    """
+    if isinstance(exc, requests.exceptions.ConnectionError):
+        return True
+    if isinstance(exc, RequestError):
+        status_code = getattr(exc.req, "status_code", None)
+        return status_code in (502, 503, 504)
+    return False
+
 
 @dataclass
 class DeviceInfo:
@@ -205,6 +231,9 @@ def _set_location_from_switches(
         device_info.rack_id = rack_id
 
     except Exception as e:
+        # Re-raise retryable errors (503, connection issues) to trigger retry
+        if _is_retryable_error(e):
+            raise
         logger.error(
             "Failed to determine location for node %s: %s", device_info.uuid, e
         )
@@ -484,6 +513,15 @@ def _find_or_create_nautobot_device(
     return _create_nautobot_device(ironic_node_info, nautobot_client)
 
 
+@tenacity.retry(
+    retry=tenacity.retry_if_exception(_is_retryable_error),
+    wait=tenacity.wait_random_exponential(
+        multiplier=1, min=RETRY_WAIT_MIN, max=RETRY_WAIT_MAX
+    ),
+    stop=tenacity.stop_after_attempt(RETRY_ATTEMPTS),
+    before_sleep=tenacity.before_sleep_log(logger, logging.WARNING),
+    reraise=True,
+)
 def sync_device_to_nautobot(
     node_uuid: str,
     nautobot_client: Nautobot,
@@ -496,7 +534,8 @@ def sync_device_to_nautobot(
     2. Creates or updates the device in Nautobot
     3. Optionally syncs interfaces (ports) to Nautobot
 
-    Can be called from any event handler.
+    Can be called from any event handler. Automatically retries on transient
+    failures (503, connection errors) with exponential backoff.
 
     Args:
         node_uuid: Ironic node UUID
@@ -539,7 +578,10 @@ def sync_device_to_nautobot(
         logger.info(str(e))
         return EXIT_STATUS_FAILURE
 
-    except Exception:
+    except Exception as e:
+        # Re-raise retryable errors to trigger tenacity retry
+        if _is_retryable_error(e):
+            raise
         logger.exception("Failed to sync device %s to Nautobot", node_uuid)
         return EXIT_STATUS_FAILURE