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

Khushiyant · 2024-06-13T13:29:55Z

This PR aims to implement subpath support for volumes in Docker-Py, allowing users to specify a subpath when creating a volume or a container. This feature is essential for scenarios where a container needs to access a specific directory within a volume.

Added a subpath parameter to the create_volume method in docker/api/volume.py.
Updated the Volume model in docker/models/volumes.py to include a subpath attribute.
Modified the create_container method in docker/api/container.py to include the subpath parameter when creating a mount.
Updated the Mount class in docker/types/mounts.py to include a subpath attribute.

This implementation is based on the Docker API documentation and might require further testing and refinements.
The subpath parameter is optional, and users can still create volumes and containers without specifying a subpath.

Adds support for specifying a subpath when creating a container This allows users to specify a subdirectory within the volume to use as the container's root. Changes: * Added `subpath` parameter to `create_container` method in `docker/api/container.py` * Updated `Mount` class in `docker/types/mounts.py` to include `subpath` attribute * Modified `Container` model in `docker/models/containers.py` to include `subpath` attribute BREAKING CHANGE: None TESTED: Yes, added unit tests to verify subpath functionality Signed-off-by: Khushiyant <[email protected]>

rayrapetyan · 2024-09-16T00:47:19Z

Hello, can this PR be merged ASAP?

Khushiyant · 2024-09-16T04:50:41Z

@rayrapetyan Even, I'm also waiting for the review

rayrapetyan · 2024-09-16T05:24:29Z

@thaJeztah @krissetto, could you provide a review please, this seems like a very simple and straightforward change. Thanks!

rayrapetyan · 2024-09-16T17:19:08Z

@Khushiyant, could you re-trigger the build as seems one of the integration tests has failed before.

Signed-off-by: Khushiyant <[email protected]>

Khushiyant · 2024-09-17T10:01:30Z

@Khushiyant, could you re-trigger the build as seems one of the integration tests has failed before.

Yeah, I triggered the build and now, all tests have passed

krissetto

Thanks for the contribution!
I gave this a quick look for now, we'll try to get it in soon

krissetto · 2024-09-17T10:08:13Z

docker/api/volume.py

@@ -45,6 +45,9 @@ def create_volume(self, name=None, driver=None, driver_opts=None,
            driver (str): Name of the driver used to create the volume
            driver_opts (dict): Driver options as a key-value dictionary
            labels (dict): Labels to set on the volume
+            subpath (str): Subpath within the volume to use as the volume's


I'd change this to

subpath (str): Path inside a volume to mount instead of the volume root.

To better match documentation we have elsewhere (eg https://github.com/docker/docs/blob/23f3fbd0639a24d0e2280fd0edb7f72b3f0cb1ca/content/reference/compose-file/services.md?plain=1#L1865)

@dvdksn WDYT?

krissetto · 2024-09-17T10:09:23Z

docker/types/services.py

@@ -242,14 +242,16 @@ class Mount(dict):
          for the ``volume`` type.
        driver_config (DriverConfig): Volume driver configuration. Only valid
          for the ``volume`` type.
+        subpath (string): The path within the volume where the container's


I'd keep this the same as the suggestion above ☝️

krissetto · 2024-09-17T10:20:37Z

tests/integration/api_container_test.py

+        mount = docker.types.Mount(
+            type="volume", source=helpers.random_name(),
+            target=self.mount_dest, read_only=True,
+            subpath='subdir'


From a quick glance, it doesn't seem that the subpath is being checked in this test

Made changes accordingly

krissetto · 2024-09-17T10:21:44Z

docker/types/services.py

@@ -280,6 +282,8 @@ def __init__(self, target, source, type='volume', read_only=False,
                volume_opts['Labels'] = labels
            if driver_config:
                volume_opts['DriverConfig'] = driver_config
+            if subpath:
+                volume_opts['SubPath'] = subpath


I think this should be written as Subpath here, right @vvoland?

Yeah, it is Subpath instead of SubPath: moby#45687. I'll change this quick

Khushiyant · 2024-09-17T10:58:08Z

docker/types/services.py

@@ -280,6 +282,8 @@ def __init__(self, target, source, type='volume', read_only=False,
                volume_opts['Labels'] = labels
            if driver_config:
                volume_opts['DriverConfig'] = driver_config
+            if subpath:
+                volume_opts['Subpath'] = subpath


Change the SubPath to Subpath

krissetto · 2024-09-17T12:13:09Z

tests/integration/api_container_test.py

+        mount_data = filtered[0]
+        assert mount['Source'] == mount_data['Name']
+        assert mount_data['RW'] is False
+        assert mount["VolumeOptions"]["Subpath"] == "subdir"


This is testing that the input mount options, as defined above on line 628, are the same.

The test needs to check that the path mounted inside the container corresponds to the subdir as defined in the mount options. The volume mounted must therefore contain the subdir, or else there should be an error.

Maybe you could change to check mount_data instead, if it contains the necessary info? I don't have much time now but I'd have to check how these volume tests are defined in a bit more detail later

Oh, sure. Will do it later today

Khushiyant changed the title ~~Implement Subpath Support for Volumes in Docker-Py~~ Implement Subpath Support for Volumes in Docker-Py (#3243) Jun 13, 2024

Khushiyant force-pushed the volume-subpath branch from 32bb4c5 to 5edcbe4 Compare June 13, 2024 13:32

Khushiyant marked this pull request as ready for review September 16, 2024 05:13

rayrapetyan approved these changes Sep 16, 2024

View reviewed changes

Build Re-trigger

b9fc1ea

Signed-off-by: Khushiyant <[email protected]>

Khushiyant force-pushed the volume-subpath branch from 6b8bbb5 to b9fc1ea Compare September 17, 2024 09:53

krissetto reviewed Sep 17, 2024

View reviewed changes

chore: change--SubPath keyvalue, add assert Subpath in test

39ed407

Khushiyant commented Sep 17, 2024

View reviewed changes

krissetto reviewed Sep 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Khushiyant commented Jun 13, 2024 •

edited

Loading

rayrapetyan commented Sep 16, 2024

Khushiyant commented Sep 16, 2024

rayrapetyan commented Sep 16, 2024

rayrapetyan commented Sep 16, 2024

Khushiyant commented Sep 17, 2024

krissetto left a comment

krissetto Sep 17, 2024

dvdksn Sep 17, 2024

krissetto Sep 17, 2024

krissetto Sep 17, 2024

Khushiyant Sep 17, 2024

krissetto Sep 17, 2024

Khushiyant Sep 17, 2024 •

edited

Loading

Khushiyant Sep 17, 2024

krissetto Sep 17, 2024

Khushiyant Sep 17, 2024 •

edited

Loading

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

Are you sure you want to change the base?

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

Conversation

Khushiyant commented Jun 13, 2024 • edited Loading

rayrapetyan commented Sep 16, 2024

Khushiyant commented Sep 16, 2024

rayrapetyan commented Sep 16, 2024

rayrapetyan commented Sep 16, 2024

Khushiyant commented Sep 17, 2024

krissetto left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Khushiyant Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Khushiyant Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Khushiyant commented Jun 13, 2024 •

edited

Loading

Khushiyant Sep 17, 2024 •

edited

Loading

Khushiyant Sep 17, 2024 •

edited

Loading