From 495430b6aba957875b09c9f62f836b688060ec1d Mon Sep 17 00:00:00 2001
From: Lubos Mjachky <lmjachky@redhat.com>
Date: Tue, 21 May 2024 18:08:29 +0200
Subject: [PATCH] Trap connection errors when fetching data during pull-through
 caching

closes #1499
---
 CHANGES/1499.bugfix                |  1 +
 pulp_container/app/exceptions.py   |  8 +++++++-
 pulp_container/app/registry.py     |  5 +++--
 pulp_container/app/registry_api.py |  9 +++++++--
 pulp_container/app/serializers.py  | 18 ++++++++++++++++++
 5 files changed, 36 insertions(+), 5 deletions(-)
 create mode 100644 CHANGES/1499.bugfix

diff --git a/CHANGES/1499.bugfix b/CHANGES/1499.bugfix
new file mode 100644
index 000000000..acec620eb
--- /dev/null
+++ b/CHANGES/1499.bugfix
@@ -0,0 +1 @@
+Made the pull-through caching machinery resilient to connection errors.
diff --git a/pulp_container/app/exceptions.py b/pulp_container/app/exceptions.py
index 8b2c998ed..c3280f043 100644
--- a/pulp_container/app/exceptions.py
+++ b/pulp_container/app/exceptions.py
@@ -1,4 +1,10 @@
-from rest_framework.exceptions import NotFound, ParseError
+from rest_framework.exceptions import APIException, NotFound, ParseError
+
+
+class ServiceUnavailable(APIException):
+    status_code = 503
+    default_detail = 'Service temporarily unavailable.'
+    default_code = 'service_unavailable'
 
 
 class RepositoryNotFound(NotFound):
diff --git a/pulp_container/app/registry.py b/pulp_container/app/registry.py
index b35c3ba57..e585cf41b 100644
--- a/pulp_container/app/registry.py
+++ b/pulp_container/app/registry.py
@@ -8,7 +8,7 @@
 from urllib.parse import urljoin
 
 from aiohttp import web
-from aiohttp.client_exceptions import ClientResponseError
+from aiohttp.client_exceptions import ClientResponseError, ClientConnectionError
 from aiohttp.web_exceptions import HTTPTooManyRequests
 from django_guid import set_guid
 from django_guid.utils import generate_guid
@@ -21,6 +21,7 @@
 from pulpcore.plugin.models import RemoteArtifact, Content, ContentArtifact
 from pulpcore.plugin.content import ArtifactResponse
 from pulpcore.plugin.tasking import dispatch
+from pulpcore.plugin.exceptions import TimeoutException
 
 from pulp_container.app.cache import RegistryContentCache
 from pulp_container.app.models import ContainerDistribution, Tag, Blob, Manifest, BlobManifest
@@ -157,7 +158,7 @@ async def get_tag(self, request):
                 response = await downloader.run(
                     extra_data={"headers": V2_ACCEPT_HEADERS, "http_method": "head"}
                 )
-            except ClientResponseError:
+            except (ClientResponseError, ClientConnectionError, TimeoutException):
                 # the manifest is not available on the remote anymore
                 # but the old one is still stored in the database
                 pass
diff --git a/pulp_container/app/registry_api.py b/pulp_container/app/registry_api.py
index 73d157377..9c9ca44a9 100644
--- a/pulp_container/app/registry_api.py
+++ b/pulp_container/app/registry_api.py
@@ -12,7 +12,7 @@
 import hashlib
 import re
 
-from aiohttp.client_exceptions import ClientResponseError
+from aiohttp.client_exceptions import ClientResponseError, ClientConnectionError
 from itertools import chain
 from urllib.parse import urljoin, urlparse, urlunparse, parse_qs, urlencode
 from tempfile import NamedTemporaryFile
@@ -29,6 +29,8 @@
 from pulpcore.plugin.files import PulpTemporaryUploadedFile
 from pulpcore.plugin.tasking import add_and_remove, dispatch
 from pulpcore.plugin.util import get_objects_for_user, get_url
+from pulpcore.plugin.exceptions import TimeoutException
+
 from rest_framework.exceptions import (
     AuthenticationFailed,
     NotAuthenticated,
@@ -63,6 +65,7 @@
     ManifestNotFound,
     ManifestInvalid,
     ManifestSignatureInvalid,
+    ServiceUnavailable,
 )
 from pulp_container.app.redirects import (
     FileStorageRedirects,
@@ -1118,8 +1121,10 @@ def fetch_manifest(self, remote, pk):
                 # it is necessary to pass this information back to the client
                 raise Throttled()
             else:
-                # TODO: do not mask out relevant errors, like HTTP 502
                 raise ManifestNotFound(reference=pk)
+        except (ClientConnectionError, TimeoutException):
+            # The remote server is not available
+            raise ServiceUnavailable()
         else:
             digest = response.headers.get("docker-content-digest")
             return models.Manifest.objects.filter(digest=digest).first()
diff --git a/pulp_container/app/serializers.py b/pulp_container/app/serializers.py
index 3cac522bd..4afe03be6 100644
--- a/pulp_container/app/serializers.py
+++ b/pulp_container/app/serializers.py
@@ -308,6 +308,24 @@ class ContainerPullThroughRemoteSerializer(RemoteSerializer):
 
     policy = serializers.ChoiceField(choices=[Remote.ON_DEMAND], default=Remote.ON_DEMAND)
 
+    connect_timeout = serializers.FloatField(
+        default=2.0,
+        required=False,
+        help_text=(
+            "aiohttp.ClientTimeout.connect (q.v.) for download-connections. The default is null, "
+            "which will cause the default from the aiohttp library to be used."
+        ),
+        min_value=0.0,
+    )
+    max_retries = serializers.IntegerField(
+        default=0,
+        help_text=(
+            "Maximum number of retry attempts after a download failure. If not set then the "
+            "default value (3) will be used."
+        ),
+        required=False,
+    )
+
     class Meta:
         fields = RemoteSerializer.Meta.fields
         model = models.ContainerPullThroughRemote