-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
An error is raised when using pull-through caching in a disconnected environment #1499
Comments
The following change fixes the error on the pulp-container's side. --- a/pulp_container/app/registry.py
+++ b/pulp_container/app/registry.py
@@ -8,7 +8,7 @@ from contextlib import suppress
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.content import Handler, PathNotResolved
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
@@ -154,11 +155,12 @@ class Registry(Handler):
)
tag_url = urljoin(remote.url, relative_url)
downloader = remote.get_downloader(url=tag_url)
+ downloader.max_retries = 0
try:
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 fe0e5662..68018114 100644
--- a/pulp_container/app/registry_api.py
+++ b/pulp_container/app/registry_api.py
@@ -11,7 +11,7 @@ import logging
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
@@ -28,6 +28,7 @@ from pulpcore.plugin.models import Artifact, ContentArtifact, UploadChunk
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,
@@ -99,8 +100,11 @@ IGNORED_PULL_THROUGH_REMOTE_ATTRIBUTES = [
"pulp_id",
"url",
"name",
+ "connect_timeout",
]
+REMOTE_CONNECTION_TIMEOUT = 2
+
class ContentRenderer(BaseRenderer):
"""
@@ -316,6 +320,7 @@ class ContainerRegistryApiMixin:
name=path,
upstream_name=upstream_name.strip("/"),
url=pull_through_cache_distribution.remote.url,
+ connect_timeout=REMOTE_CONNECTION_TIMEOUT,
**remote_data,
)
@@ -1098,6 +1103,7 @@ class Manifests(RedirectsMixin, ContainerRegistryApiMixin, ViewSet):
)
tag_url = urljoin(remote.url, relative_url)
downloader = remote.get_downloader(url=tag_url)
+ downloader.max_retries = 0
try:
response = downloader.fetch(
extra_data={"headers": V2_ACCEPT_HEADERS, "http_method": "head"}
@@ -1110,6 +1116,9 @@ class Manifests(RedirectsMixin, ContainerRegistryApiMixin, ViewSet):
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 ManifestNotFound(reference=pk)
else:
digest = response.headers.get("docker-content-digest")
return models.Manifest.objects.filter(digest=digest).first() |
It might be worth considering adding a migration that set timeouts and backoffs to correct value for already existing remotes created during the pull-through caching.
|
I am waiting to get this merged: pulp/pulpcore#5025. |
I would expect similar traceback would be shown also during regular sync. None of these features are designed to work in air gapped env. |
It does make sense for this feature to be able to deal with connectivity issues. Once you pull-through the content, it should be saved in the Pulp instance and be able to deliver the image whenever requested. |
It makes sense to deliver content which is already locally cached, however for the cases when pulp instance needs to reach out to the internet whether during the first pull or during sync we should not raise 404 but propagate the error. |
Yes, with the change, we should also refactor the following code paths: pulp_container/pulp_container/app/registry_api.py Line 1111 in 59e06e5
pulp_container/pulp_container/app/registry.py Line 429 in 59e06e5
|
|
This happens when using the pull-through caching workflow with no internet connection.
Traceback:
The text was updated successfully, but these errors were encountered: