Stop long prunes failing with a bare 500 (0.60.1)
A prune that deletes a lot of data (image prune -a in particular) easily runs past docker-py's 60s read timeout. requests' ReadTimeout is not a DockerException, so it slipped past safe_call and the UI only showed "Request failed with status code 500". Prunes now use a client with a 540s timeout (below nginx's 600s proxy_read_timeout), and safe_call turns transport timeouts and connection errors into DockerError, so they surface as a 502 with a message. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
8edea8f971
commit
f766ded528
7 files changed
+120
-11
No files matched your search
@@ -9,6 +9,7 @@ import functools
|
|||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
import docker
|
import docker
|
||||||
|
import requests
|
||||||
from docker.errors import APIError, DockerException, NotFound
|
from docker.errors import APIError, DockerException, NotFound
|
||||||
|
|
||||||
from config import settings
|
from config import settings
|
||||||
@@ -23,19 +24,37 @@ class DockerError(Exception):
|
|||||||
super().__init__(f"{error}: {detail}" if detail else error)
|
super().__init__(f"{error}: {detail}" if detail else error)
|
||||||
|
|
||||||
|
|
||||||
@functools.lru_cache(maxsize=1)
|
# Read timeout for get_long_client(). docker-py's default of 60s is fine for
|
||||||
def get_client() -> docker.DockerClient:
|
# interactive calls, but a prune that deletes gigabytes of volume data or image
|
||||||
|
# layers routinely takes longer. Kept below nginx's proxy_read_timeout (600s)
|
||||||
|
# so the caller still gets our error instead of a bare gateway timeout.
|
||||||
|
LONG_TIMEOUT_SECONDS = 540
|
||||||
|
|
||||||
|
|
||||||
|
def _connect(**kwargs) -> docker.DockerClient:
|
||||||
try:
|
try:
|
||||||
base_url = f"unix://{settings.DOCKER_SOCKET}"
|
base_url = f"unix://{settings.DOCKER_SOCKET}"
|
||||||
client = docker.DockerClient(base_url=base_url)
|
client = docker.DockerClient(base_url=base_url, **kwargs)
|
||||||
client.ping()
|
client.ping()
|
||||||
return client
|
return client
|
||||||
except DockerException as exc: # pragma: no cover - environment dependent
|
except DockerException as exc: # pragma: no cover - environment dependent
|
||||||
raise DockerError("docker_unavailable", str(exc)) from exc
|
raise DockerError("docker_unavailable", str(exc)) from exc
|
||||||
|
|
||||||
|
|
||||||
|
@functools.lru_cache(maxsize=1)
|
||||||
|
def get_client() -> docker.DockerClient:
|
||||||
|
return _connect()
|
||||||
|
|
||||||
|
|
||||||
|
@functools.lru_cache(maxsize=1)
|
||||||
|
def get_long_client() -> docker.DockerClient:
|
||||||
|
"""A client for calls that can legitimately run for minutes (prunes)."""
|
||||||
|
return _connect(timeout=LONG_TIMEOUT_SECONDS)
|
||||||
|
|
||||||
|
|
||||||
def reset_client() -> None:
|
def reset_client() -> None:
|
||||||
get_client.cache_clear()
|
get_client.cache_clear()
|
||||||
|
get_long_client.cache_clear()
|
||||||
|
|
||||||
|
|
||||||
def safe_call(fn, *args, **kwargs) -> Any:
|
def safe_call(fn, *args, **kwargs) -> Any:
|
||||||
@@ -48,3 +67,12 @@ def safe_call(fn, *args, **kwargs) -> Any:
|
|||||||
raise DockerError("docker_api_error", str(exc)) from exc
|
raise DockerError("docker_api_error", str(exc)) from exc
|
||||||
except DockerException as exc:
|
except DockerException as exc:
|
||||||
raise DockerError("docker_error", str(exc)) from exc
|
raise DockerError("docker_error", str(exc)) from exc
|
||||||
|
# docker-py lets transport errors from requests through unwrapped; without
|
||||||
|
# these a slow daemon surfaced as a bare 500 with no message.
|
||||||
|
except requests.exceptions.Timeout as exc:
|
||||||
|
raise DockerError(
|
||||||
|
"docker_timeout",
|
||||||
|
"Docker did not answer in time; the operation may still be running on the host",
|
||||||
|
) from exc
|
||||||
|
except requests.exceptions.RequestException as exc:
|
||||||
|
raise DockerError("docker_unavailable", str(exc)) from exc
|
||||||
@@ -1,7 +1,7 @@
|
|||||||
"""Image listing for the images router."""
|
"""Image listing for the images router."""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
from docker_client import DockerError, get_client, safe_call
|
from docker_client import DockerError, get_client, get_long_client, safe_call
|
||||||
from services import update_service
|
from services import update_service
|
||||||
|
|
||||||
COMPOSE_PROJECT_LABEL = "com.docker.compose.project"
|
COMPOSE_PROJECT_LABEL = "com.docker.compose.project"
|
||||||
@@ -51,7 +51,7 @@ def prune_images(all_unused: bool = False) -> dict:
|
|||||||
"""Remove unused images. By default only dangling (untagged) images are
|
"""Remove unused images. By default only dangling (untagged) images are
|
||||||
removed; ``all_unused=True`` removes every image not referenced by a
|
removed; ``all_unused=True`` removes every image not referenced by a
|
||||||
container (``docker image prune -a``)."""
|
container (``docker image prune -a``)."""
|
||||||
client = get_client()
|
client = get_long_client()
|
||||||
# dangling=false tells the engine to also consider tagged-but-unused images.
|
# dangling=false tells the engine to also consider tagged-but-unused images.
|
||||||
filters = {"dangling": False} if all_unused else {"dangling": True}
|
filters = {"dangling": False} if all_unused else {"dangling": True}
|
||||||
result = safe_call(client.images.prune, filters=filters)
|
result = safe_call(client.images.prune, filters=filters)
|
||||||
|
|||||||
@@ -6,7 +6,7 @@ from typing import Optional
|
|||||||
from docker.errors import APIError
|
from docker.errors import APIError
|
||||||
from docker.types import IPAMConfig, IPAMPool
|
from docker.types import IPAMConfig, IPAMPool
|
||||||
|
|
||||||
from docker_client import DockerError, get_client, safe_call
|
from docker_client import DockerError, get_client, get_long_client, safe_call
|
||||||
|
|
||||||
COMPOSE_PROJECT_LABEL = "com.docker.compose.project"
|
COMPOSE_PROJECT_LABEL = "com.docker.compose.project"
|
||||||
DEFAULT_NETWORKS = {"bridge", "host", "none"}
|
DEFAULT_NETWORKS = {"bridge", "host", "none"}
|
||||||
@@ -133,5 +133,5 @@ def delete_network(network_id: str) -> None:
|
|||||||
|
|
||||||
|
|
||||||
def prune_networks() -> dict:
|
def prune_networks() -> dict:
|
||||||
client = get_client()
|
client = get_long_client()
|
||||||
return safe_call(client.networks.prune)
|
return safe_call(client.networks.prune)
|
||||||
@@ -7,7 +7,7 @@ from typing import Literal, Optional
|
|||||||
|
|
||||||
import yaml
|
import yaml
|
||||||
|
|
||||||
from docker_client import get_client, safe_call
|
from docker_client import get_client, get_long_client, safe_call
|
||||||
|
|
||||||
COMPOSE_PROJECT_LABEL = "com.docker.compose.project"
|
COMPOSE_PROJECT_LABEL = "com.docker.compose.project"
|
||||||
|
|
||||||
@@ -86,7 +86,7 @@ def remove_volume(name: str, force: bool = False) -> None:
|
|||||||
|
|
||||||
|
|
||||||
def prune_volumes() -> dict:
|
def prune_volumes() -> dict:
|
||||||
client = get_client()
|
client = get_long_client()
|
||||||
return safe_call(client.volumes.prune)
|
return safe_call(client.volumes.prune)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,81 @@
|
|||||||
|
"""Slow Docker calls must fail with a message, not a bare 500.
|
||||||
|
|
||||||
|
A volume or image prune that deletes a lot of data easily runs past docker-py's
|
||||||
|
60s read timeout. requests' ``ReadTimeout`` is not a ``DockerException``, so it
|
||||||
|
slipped past ``safe_call`` and the UI only showed "Request failed with status
|
||||||
|
code 500". Prunes now use a client with a long timeout, and any transport
|
||||||
|
timeout that still happens is turned into a ``DockerError`` (-> 502 + detail).
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
import requests
|
||||||
|
|
||||||
|
|
||||||
|
def test_safe_call_wraps_read_timeout():
|
||||||
|
from docker_client import DockerError, safe_call
|
||||||
|
|
||||||
|
def slow():
|
||||||
|
raise requests.exceptions.ReadTimeout("Read timed out. (read timeout=60)")
|
||||||
|
|
||||||
|
with pytest.raises(DockerError) as info:
|
||||||
|
safe_call(slow)
|
||||||
|
assert info.value.error == "docker_timeout"
|
||||||
|
|
||||||
|
|
||||||
|
def test_safe_call_wraps_connection_error():
|
||||||
|
from docker_client import DockerError, safe_call
|
||||||
|
|
||||||
|
def gone():
|
||||||
|
raise requests.exceptions.ConnectionError("socket went away")
|
||||||
|
|
||||||
|
with pytest.raises(DockerError) as info:
|
||||||
|
safe_call(gone)
|
||||||
|
assert info.value.error == "docker_unavailable"
|
||||||
|
|
||||||
|
|
||||||
|
def test_volume_prune_timeout_is_a_502_with_detail(as_admin, monkeypatch):
|
||||||
|
from services import volume_service
|
||||||
|
|
||||||
|
class Volumes:
|
||||||
|
def prune(self, filters=None):
|
||||||
|
raise requests.exceptions.ReadTimeout("Read timed out.")
|
||||||
|
|
||||||
|
class Client:
|
||||||
|
volumes = Volumes()
|
||||||
|
|
||||||
|
monkeypatch.setattr(volume_service, "get_long_client", lambda: Client())
|
||||||
|
r = as_admin.post("/api/volumes/prune")
|
||||||
|
assert r.status_code == 502
|
||||||
|
assert r.json()["error"] == "docker_timeout"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"module, fn, args",
|
||||||
|
[
|
||||||
|
("volume_service", "prune_volumes", ()),
|
||||||
|
("image_service", "prune_images", (True,)),
|
||||||
|
("network_service", "prune_networks", ()),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_prunes_use_the_long_timeout_client(monkeypatch, module, fn, args):
|
||||||
|
import importlib
|
||||||
|
|
||||||
|
svc = importlib.import_module(f"services.{module}")
|
||||||
|
used = []
|
||||||
|
|
||||||
|
class Any_:
|
||||||
|
def prune(self, filters=None):
|
||||||
|
used.append("long")
|
||||||
|
return {}
|
||||||
|
|
||||||
|
class Client:
|
||||||
|
volumes = images = networks = Any_()
|
||||||
|
|
||||||
|
def short():
|
||||||
|
raise AssertionError("prune must not use the 60s client")
|
||||||
|
|
||||||
|
monkeypatch.setattr(svc, "get_client", short)
|
||||||
|
monkeypatch.setattr(svc, "get_long_client", lambda: Client())
|
||||||
|
getattr(svc, fn)(*args)
|
||||||
|
assert used == ["long"]
|
||||||
+1
-1
@@ -1,3 +1,3 @@
|
|||||||
"""Single source of truth for the StackPilot release version."""
|
"""Single source of truth for the StackPilot release version."""
|
||||||
|
|
||||||
APP_VERSION = "0.60.0"
|
APP_VERSION = "0.60.1"
|
||||||
@@ -1,7 +1,7 @@
|
|||||||
{
|
{
|
||||||
"name": "stackpilot-frontend",
|
"name": "stackpilot-frontend",
|
||||||
"private": true,
|
"private": true,
|
||||||
"version": "0.60.0",
|
"version": "0.60.1",
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
"dev": "vite",
|
"dev": "vite",
|
||||||
|
|||||||
Reference in new issue
Block a user