diff --git a/docker/errors.py b/docker/errors.py index d7fb78b2bb..b1751cdb20 100644 --- a/docker/errors.py +++ b/docker/errors.py @@ -167,6 +167,10 @@ class ImageLoadError(DockerException): pass +class ImagePullError(DockerException): + pass + + def create_unexpected_kwargs_error(name, kwargs): quoted_kwargs = [f"'{k}'" for k in sorted(kwargs)] text = [f"{name}() "] diff --git a/docker/models/images.py b/docker/models/images.py index 0e8cce3f82..d905d59b1c 100644 --- a/docker/models/images.py +++ b/docker/models/images.py @@ -4,7 +4,12 @@ from ..api import APIClient from ..constants import DEFAULT_DATA_CHUNK_SIZE -from ..errors import BuildError, ImageLoadError, InvalidArgument +from ..errors import ( + BuildError, + ImageLoadError, + ImagePullError, + InvalidArgument, +) from ..utils import parse_repository_tag from ..utils.json_stream import json_stream from .resource import Collection, Model @@ -441,6 +446,9 @@ def pull(self, repository, tag=None, all_tags=False, **kwargs): Raises: :py:class:`docker.errors.APIError` If the server returns an error. + :py:class:`docker.errors.ImagePullError` + If the pull fails after the server accepted the request, + for example because the registry refused it. Example: @@ -462,13 +470,19 @@ def pull(self, repository, tag=None, all_tags=False, **kwargs): del kwargs['stream'] pull_log = self.client.api.pull( - repository, tag=tag, stream=True, all_tags=all_tags, **kwargs + repository, tag=tag, stream=True, decode=True, + all_tags=all_tags, **kwargs ) - for _ in pull_log: - # We don't do anything with the logs, but we need - # to keep the connection alive and wait for the image - # to be pulled. - pass + for chunk in pull_log: + # The daemon answers a failed pull with HTTP 200 and reports the + # failure in the stream, so this is the only place it shows up. + # Without this check the image is simply missing afterwards and + # the caller gets an ImageNotFound that hides the real cause. + if 'errorDetail' in chunk or 'error' in chunk: + detail = chunk.get('errorDetail') or {} + raise ImagePullError( + detail.get('message') or chunk.get('error') + ) if not all_tags: sep = '@' if tag.startswith('sha256:') else ':' return self.get(f'{repository}{sep}{tag}') diff --git a/tests/unit/models_containers_test.py b/tests/unit/models_containers_test.py index 0e2ae341a9..4208c0fcf3 100644 --- a/tests/unit/models_containers_test.py +++ b/tests/unit/models_containers_test.py @@ -257,7 +257,8 @@ def test_run_pull(self): assert container.id == FAKE_CONTAINER_ID client.api.pull.assert_called_with( - 'alpine', platform=None, tag='latest', all_tags=False, stream=True + 'alpine', platform=None, tag='latest', all_tags=False, + stream=True, decode=True, ) def test_run_with_error(self): @@ -354,6 +355,7 @@ def test_run_platform(self): tag='latest', all_tags=False, stream=True, + decode=True, platform='linux/arm64', ) diff --git a/tests/unit/models_images_test.py b/tests/unit/models_images_test.py index 3478c3fedb..19343b0530 100644 --- a/tests/unit/models_images_test.py +++ b/tests/unit/models_images_test.py @@ -1,7 +1,10 @@ import unittest import warnings +import pytest + from docker.constants import DEFAULT_DATA_CHUNK_SIZE +from docker.errors import ImagePullError from docker.models.images import Image from .fake_api import FAKE_IMAGE_ID @@ -46,7 +49,7 @@ def test_pull(self): client = make_fake_client() image = client.images.pull('test_image:test') client.api.pull.assert_called_with( - 'test_image', tag='test', all_tags=False, stream=True + 'test_image', tag='test', all_tags=False, stream=True, decode=True ) client.api.inspect_image.assert_called_with('test_image:test') assert isinstance(image, Image) @@ -56,13 +59,13 @@ def test_pull_tag_precedence(self): client = make_fake_client() image = client.images.pull('test_image:latest', tag='test') client.api.pull.assert_called_with( - 'test_image', tag='test', all_tags=False, stream=True + 'test_image', tag='test', all_tags=False, stream=True, decode=True ) client.api.inspect_image.assert_called_with('test_image:test') image = client.images.pull('test_image') client.api.pull.assert_called_with( - 'test_image', tag='latest', all_tags=False, stream=True + 'test_image', tag='latest', all_tags=False, stream=True, decode=True ) client.api.inspect_image.assert_called_with('test_image:latest') assert isinstance(image, Image) @@ -72,7 +75,7 @@ def test_pull_multiple(self): client = make_fake_client() images = client.images.pull('test_image', all_tags=True) client.api.pull.assert_called_with( - 'test_image', tag='latest', all_tags=True, stream=True + 'test_image', tag='latest', all_tags=True, stream=True, decode=True ) client.api.images.assert_called_with( all=False, name='test_image', filters=None @@ -83,6 +86,27 @@ def test_pull_multiple(self): assert isinstance(image, Image) assert image.id == FAKE_IMAGE_ID + def test_pull_raises_the_error_reported_in_the_stream(self): + client = make_fake_client({ + 'pull.return_value': [ + {'status': 'Pulling from library/test_image'}, + { + 'errorDetail': {'message': 'denied: access forbidden'}, + 'error': 'denied: access forbidden', + }, + ], + }) + with pytest.raises(ImagePullError, match='denied: access forbidden'): + client.images.pull('test_image:test') + client.api.inspect_image.assert_not_called() + + def test_pull_raises_a_bare_error_message(self): + client = make_fake_client({ + 'pull.return_value': [{'error': 'lease does not exist'}], + }) + with pytest.raises(ImagePullError, match='lease does not exist'): + client.images.pull('test_image:test') + def test_pull_with_stream_param(self): client = make_fake_client() with warnings.catch_warnings(record=True) as w: