Merge pull request #3270 from Khushiyant/volume-subpath

Implement Subpath Support for Volumes in Docker-Py (#3243)

Dockerfile

@@ -6,7 +6,7 @@ FROM python:${PYTHON_VERSION}
 WORKDIR /src
 COPY . .
 
-ARG VERSION
+ARG VERSION=0.0.0.dev0
 RUN --mount=type=cache,target=/cache/pip \
     PIP_CACHE_DIR=/cache/pip \
     SETUPTOOLS_SCM_PRETEND_VERSION=${VERSION} \

Dockerfile-docs

@@ -13,7 +13,7 @@ RUN addgroup --gid $gid sphinx \
 WORKDIR /src
 COPY . .
 
-ARG VERSION
+ARG VERSION=0.0.0.dev0
 RUN --mount=type=cache,target=/cache/pip \
     PIP_CACHE_DIR=/cache/pip \
     SETUPTOOLS_SCM_PRETEND_VERSION=${VERSION} \

Makefile

@@ -1,5 +1,5 @@
-TEST_API_VERSION ?= 1.44
+TEST_API_VERSION ?= 1.45
-TEST_ENGINE_VERSION ?= 25.0
+TEST_ENGINE_VERSION ?= 26.1
 
 ifeq ($(OS),Windows_NT)
     PLATFORM := Windows
@@ -13,7 +13,7 @@ endif
 
 SETUPTOOLS_SCM_PRETEND_VERSION_DOCKER ?= $(shell git describe --match '[0-9]*' --dirty='.m' --always --tags 2>/dev/null | sed -r 's/-([0-9]+)/.dev\1/' | sed 's/-/+/')
 ifeq ($(SETUPTOOLS_SCM_PRETEND_VERSION_DOCKER),)
-	SETUPTOOLS_SCM_PRETEND_VERSION_DOCKER = "dev"
+	SETUPTOOLS_SCM_PRETEND_VERSION_DOCKER = "0.0.0.dev0"
 endif
 
 .PHONY: all
@@ -33,7 +33,7 @@ build-dind-ssh:
 		--build-arg VERSION=${SETUPTOOLS_SCM_PRETEND_VERSION_DOCKER} \
 		--build-arg ENGINE_VERSION=${TEST_ENGINE_VERSION} \
 		--build-arg API_VERSION=${TEST_API_VERSION} \
-		--build-arg APT_MIRROR .
+		.
 
 .PHONY: build
 build:
@@ -42,7 +42,7 @@ build:
 		-t docker-sdk-python3 \
 		-f tests/Dockerfile \
 		--build-arg VERSION=${SETUPTOOLS_SCM_PRETEND_VERSION_DOCKER} \
-		--build-arg APT_MIRROR .
+		.
 
 .PHONY: build-docs
 build-docs:
@@ -76,9 +76,6 @@ integration-test: build
 setup-network:
 	docker network inspect dpy-tests || docker network create dpy-tests
 
-.PHONY: integration-dind
-integration-dind: integration-dind
-
 .PHONY: integration-dind
 integration-dind: build setup-network
 	docker rm -vf dpy-dind || :

docker/constants.py

@@ -2,7 +2,7 @@ import sys
 
 from .version import __version__
 
-DEFAULT_DOCKER_API_VERSION = '1.44'
+DEFAULT_DOCKER_API_VERSION = '1.45'
 MINIMUM_DOCKER_API_VERSION = '1.24'
 DEFAULT_TIMEOUT_SECONDS = 60
 STREAM_HEADER_SIZE_BYTES = 8

docker/models/containers.py

@@ -181,7 +181,8 @@ class Container(Model):
             user (str): User to execute command as. Default: root
             detach (bool): If true, detach from the exec command.
                 Default: False
-            stream (bool): Stream response data. Default: False
+            stream (bool): Stream response data. Ignored if ``detach`` is true.
+                Default: False
             socket (bool): Return the connection socket to allow custom
                 read/write operations. Default: False
             environment (dict or list): A dictionary or a list of strings in

docker/models/images.py

@@ -407,8 +407,8 @@ class ImageCollection(Collection):
                 if match:
                     image_id = match.group(2)
                     images.append(image_id)
-            if 'error' in chunk:
+            if 'errorDetail' in chunk:
-                raise ImageLoadError(chunk['error'])
+                raise ImageLoadError(chunk['errorDetail']['message'])
 
         return [self.get(i) for i in images]
 

docker/types/services.py

@@ -242,6 +242,7 @@ class Mount(dict):
           for the ``volume`` type.
         driver_config (DriverConfig): Volume driver configuration. Only valid
           for the ``volume`` type.
+        subpath (str): Path inside a volume to mount instead of the volume root.
         tmpfs_size (int or string): The size for the tmpfs mount in bytes.
         tmpfs_mode (int): The permission mode for the tmpfs mount.
     """
@@ -249,7 +250,7 @@ class Mount(dict):
     def __init__(self, target, source, type='volume', read_only=False,
                  consistency=None, propagation=None, no_copy=False,
                  labels=None, driver_config=None, tmpfs_size=None,
-                 tmpfs_mode=None):
+                 tmpfs_mode=None, subpath=None):
         self['Target'] = target
         self['Source'] = source
         if type not in ('bind', 'volume', 'tmpfs', 'npipe'):
@@ -267,7 +268,7 @@ class Mount(dict):
                 self['BindOptions'] = {
                     'Propagation': propagation
                 }
-            if any([labels, driver_config, no_copy, tmpfs_size, tmpfs_mode]):
+            if any([labels, driver_config, no_copy, tmpfs_size, tmpfs_mode, subpath]):
                 raise errors.InvalidArgument(
                     'Incompatible options have been provided for the bind '
                     'type mount.'
@@ -280,6 +281,8 @@ class Mount(dict):
                 volume_opts['Labels'] = labels
             if driver_config:
                 volume_opts['DriverConfig'] = driver_config
+            if subpath:
+                volume_opts['Subpath'] = subpath
             if volume_opts:
                 self['VolumeOptions'] = volume_opts
             if any([propagation, tmpfs_size, tmpfs_mode]):

tests/Dockerfile

@@ -28,7 +28,7 @@ RUN curl -sSL -o /opt/docker-credential-pass.tar.gz \
 WORKDIR /src
 COPY . .
 
-ARG VERSION
+ARG VERSION=0.0.0.dev0
 RUN --mount=type=cache,target=/cache/pip \
     PIP_CACHE_DIR=/cache/pip \
     SETUPTOOLS_SCM_PRETEND_VERSION=${VERSION} \

tests/Dockerfile-ssh-dind

@@ -1,7 +1,7 @@
 # syntax=docker/dockerfile:1
 
-ARG API_VERSION=1.44
+ARG API_VERSION=1.45
-ARG ENGINE_VERSION=25.0
+ARG ENGINE_VERSION=26.1
 
 FROM docker:${ENGINE_VERSION}-dind
 

tests/integration/api_build_test.py

@@ -275,7 +275,7 @@ class BuildTest(BaseAPIIntegrationTest):
             pass
 
         info = self.client.inspect_image('build1')
-        assert not info['Config']['OnBuild']
+        assert 'OnBuild' not in info['Config'] or not info['Config']['OnBuild']
 
     @requires_api_version('1.25')
     def test_build_with_network_mode(self):

tests/integration/api_container_test.py

@@ -620,6 +620,56 @@ class VolumeBindTest(BaseAPIIntegrationTest):
         assert mount['Source'] == mount_data['Name']
         assert mount_data['RW'] is True
 
+    @requires_api_version('1.45')
+    def test_create_with_subpath_volume_mount(self):
+        source_volume = helpers.random_name()
+        self.client.create_volume(name=source_volume)
+
+        setup_container = None
+        test_container = None
+
+
+        # Create a file structure in the volume to test with
+        setup_container = self.client.create_container(
+            TEST_IMG,
+            [
+                "sh",
+                "-c",
+                'mkdir -p /vol/subdir && echo "test content" > /vol/subdir/testfile.txt',
+            ],
+            host_config=self.client.create_host_config(
+                binds=[f"{source_volume}:/vol"]
+            ),
+        )
+        self.client.start(setup_container)
+        self.client.wait(setup_container)
+
+        # Now test with subpath
+        mount = docker.types.Mount(
+            type="volume",
+            source=source_volume,
+            target=self.mount_dest,
+            read_only=True,
+            subpath="subdir",
+        )
+
+
+        host_config = self.client.create_host_config(mounts=[mount])
+        test_container = self.client.create_container(
+            TEST_IMG,
+            ["cat", os.path.join(self.mount_dest, "testfile.txt")],
+            host_config=host_config,
+        )
+
+        self.client.start(test_container)
+        self.client.wait(test_container)  # Wait for container to finish
+        output = self.client.logs(test_container).decode("utf-8").strip()
+
+        # If the subpath feature is working, we should be able to see the content
+        # of the file in the subdir
+        assert output == "test content"
+
+
     def check_container_data(self, inspect_data, rw, propagation='rprivate'):
         assert 'Mounts' in inspect_data
         filtered = list(filter(

tests/integration/api_service_test.py

@@ -5,7 +5,7 @@ import pytest
 
 import docker
 
-from ..helpers import force_leave_swarm, requires_api_version, requires_experimental
+from ..helpers import force_leave_swarm, requires_api_version
 from .base import TEST_IMG, BaseAPIIntegrationTest
 
 
@@ -140,8 +140,7 @@ class ServiceTest(BaseAPIIntegrationTest):
         assert len(services) == 1
         assert services[0]['ID'] == svc_id['ID']
 
-    @requires_api_version('1.25')
+    @requires_api_version('1.29')
-    @requires_experimental(until='1.29')
     def test_service_logs(self):
         name, svc_id = self.create_simple_service()
         assert self.get_service_container(name, include_stopped=True)

tests/integration/api_volume_test.py

@@ -17,10 +17,16 @@ class TestVolumes(BaseAPIIntegrationTest):
         assert result['Driver'] == 'local'
 
     def test_create_volume_invalid_driver(self):
-        driver_name = 'invalid.driver'
+        # special name to avoid exponential timeout loop
+        # https://github.com/moby/moby/blob/9e00a63d65434cdedc444e79a2b33a7c202b10d8/pkg/plugins/client.go#L253-L254
+        driver_name = 'this-plugin-does-not-exist'
 
-        with pytest.raises(docker.errors.NotFound):
+        with pytest.raises(docker.errors.APIError) as cm:
             self.client.create_volume('perfectcherryblossom', driver_name)
+            assert (
+                cm.value.response.status_code == 404 or
+                cm.value.response.status_code == 400
+            )
 
     def test_list_volumes(self):
         name = 'imperishablenight'

tests/integration/models_containers_test.py

@@ -131,10 +131,9 @@ class ContainerCollectionTest(BaseIntegrationTest):
         assert 'NetworkSettings' in attrs
         assert 'Networks' in attrs['NetworkSettings']
         assert list(attrs['NetworkSettings']['Networks'].keys()) == [net_name]
-        # Expect Aliases to list 'test_alias' and the container's short-id.
+        # Aliases no longer include the container's short-id in API v1.45.
-        # In API version 1.45, the short-id will be removed.
         assert attrs['NetworkSettings']['Networks'][net_name]['Aliases'] \
-               == [test_alias, attrs['Id'][:12]]
+               == [test_alias]
         assert attrs['NetworkSettings']['Networks'][net_name]['DriverOpts'] \
                == test_driver_opt
 
@@ -191,9 +190,9 @@ class ContainerCollectionTest(BaseIntegrationTest):
         assert 'NetworkSettings' in attrs
         assert 'Networks' in attrs['NetworkSettings']
         assert list(attrs['NetworkSettings']['Networks'].keys()) == [net_name]
-        # Aliases should include the container's short-id (but it will be removed
+        # Aliases no longer include the container's short-id in API v1.45.
-        # in API v1.45).
+        assert (attrs['NetworkSettings']['Networks'][net_name]['Aliases']
-        assert attrs['NetworkSettings']['Networks'][net_name]['Aliases'] == [attrs["Id"][:12]]
+                is None)
         assert (attrs['NetworkSettings']['Networks'][net_name]['DriverOpts']
                 is None)
 
@@ -353,14 +352,26 @@ class ContainerTest(BaseIntegrationTest):
         assert exec_output[0] == 0
         assert exec_output[1] == b"hello\n"
 
+    def test_exec_run_error_code_from_exec(self):
+        client = docker.from_env(version=TEST_API_VERSION)
+        container = client.containers.run(
+            "alpine", "sh -c 'sleep 20'", detach=True
+        )
+        self.tmp_containers.append(container.id)
+        exec_output = container.exec_run("sh -c 'exit 42'")
+        assert exec_output[0] == 42
+
     def test_exec_run_failed(self):
         client = docker.from_env(version=TEST_API_VERSION)
         container = client.containers.run(
             "alpine", "sh -c 'sleep 60'", detach=True
         )
         self.tmp_containers.append(container.id)
-        exec_output = container.exec_run("docker ps")
+        exec_output = container.exec_run("non-existent")
-        assert exec_output[0] == 126
+        # older versions of docker return `126` in the case that an exec cannot
+        # be started due to a missing executable. We're fixing this for the
+        # future, so accept both for now.
+        assert exec_output[0] == 127 or exec_output[0] == 126
 
     def test_kill(self):
         client = docker.from_env(version=TEST_API_VERSION)

tests/ssh/api_build_test.py

@@ -266,7 +266,7 @@ class BuildTest(BaseAPIIntegrationTest):
             pass
 
         info = self.client.inspect_image('build1')
-        assert not info['Config']['OnBuild']
+        assert 'OnBuild' not in info['Config'] or not info['Config']['OnBuild']
 
     @requires_api_version('1.25')
     def test_build_with_network_mode(self):