Create mount source if it does not exist by aiordache · Pull Request #1563 · docker-archive/compose-cli

aiordache · 2021-04-16T08:51:12Z

Create a directory if a bind-mount source does not exist to align with docker-compose.

services:
  frontend:
    image: nginx
    volumes:
      - /tmp/doesnotexist:/test

[example]$ docker-compose up -d
Creating network "example_default" with the default driver
Creating example_frontend_1 ... done

[example]$ ls -al /tmp | grep doesnotexist
drwxr-xr-x  2 root root      40 Apr 16 10:45 doesnotexist

[example]$ docker compose up -d
[+] Running 4/4
 ⠿ Network "example_default"     Created                                                                                                           0.0s
 ⠿ Container example_frontend_1  Started                                                                                                           0.7s

[example]$ ls -al /tmp | grep doesnotexist
drwxr-xr-x  2 anca users     40 Apr 16 10:45 doesnotexist

Closes #1543

ndeloof

incorrect implementation creating source on CLI hosts, not Dockerd hosts

ndeloof · 2021-04-16T10:40:00Z

+		if m.Type == mount.TypeBind {
+			// mkdir if bind source path does not exist
+			if _, err := os.Stat(m.Source); err != nil && os.IsNotExist(err) {
+				err = os.Mkdir(m.Source, 0755)


👎 doing so you create source on the CLI host, which might not be the dockerHost. Obviously this is what most people use for development, but this is wrong here. Need to understand how our bind configuration differs from docker/cli doing the same

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

thaJeztah · 2021-04-26T13:53:07Z

This is likely caused if it's using the wrong API for the short-hand notation. The old "binds" API is used for the shorthand notation (-v <host-path>:<container-path>[:<options>]), which automatically creates the host-path. Automatically creating the host-path has been problematic, and we tried to deprecate/remove it, but doing so caused to many issues / broke too many people, so we kept it for the old "binds" API. When using the "advanced" notation (--mounts or "extended notation in compose"), the new 'mounts' API should be used, which does not have this behavior.

Here's a comparison:

Shorthand:

docker run --rm -v /shorthand:/foo hello-world

And the associated request that's made:

DEBU[2021-04-26T13:40:31.135461497Z] form data: {"AttachStderr":true,"AttachStdin":false,"AttachStdout":true,"Cmd":null,"Domainname":"","Entrypoint":null,"Env":[],"HostConfig":{"AutoRemove":true,"Binds":["/shorthand:/foo"],"BlkioDeviceReadBps":null,"BlkioDeviceReadIOps":null,"BlkioDeviceWriteBps":null,"BlkioDeviceWriteIOps":null,"BlkioWeight":0,"BlkioWeightDevice":null,"CapAdd":null,"CapDrop":null,"Cgroup":"","CgroupParent":"","ConsoleSize":[0,0],"ContainerIDFile":"","CpuCount":0,"CpuPercent":0,"CpuPeriod":0,"CpuQuota":0,"CpuRealtimePeriod":0,"CpuRealtimeRuntime":0,"CpuShares":0,"CpusetCpus":"","CpusetMems":"","DeviceCgroupRules":null,"Devices":[],"DiskQuota":0,"Dns":[],"DnsOptions":[],"DnsSearch":[],"ExtraHosts":null,"GroupAdd":null,"IOMaximumBandwidth":0,"IOMaximumIOps":0,"IpcMode":"","Isolation":"","KernelMemory":0,"Links":null,"LogConfig":{"Config":{},"Type":""},"Memory":0,"MemoryReservation":0,"MemorySwap":0,"MemorySwappiness":-1,"NanoCpus":0,"NetworkMode":"default","OomKillDisable":false,"OomScoreAdj":0,"PidMode":"","PidsLimit":0,"PortBindings":{},"Privileged":false,"PublishAllPorts":false,"ReadonlyRootfs":false,"RestartPolicy":{"MaximumRetryCount":0,"Name":"no"},"SecurityOpt":null,"ShmSize":0,"UTSMode":"","Ulimits":null,"UsernsMode":"","VolumeDriver":"","VolumesFrom":null},"Hostname":"","Image":"hello-world","Labels":{},"NetworkingConfig":{"EndpointsConfig":{}},"OnBuild":null,"OpenStdin":false,"StdinOnce":false,"Tty":false,"User":"","Volumes":{},"WorkingDir":""}

Request;

Details

{
  "AttachStderr": true,
  "AttachStdin": false,
  "AttachStdout": true,
  "Cmd": null,
  "Domainname": "",
  "Entrypoint": null,
  "Env": [],
  "HostConfig": {
    "AutoRemove": true,
    "Binds": [
      "/shorthand:/foo"
    ],
    "BlkioDeviceReadBps": null,
    "BlkioDeviceReadIOps": null,
    "BlkioDeviceWriteBps": null,
    "BlkioDeviceWriteIOps": null,
    "BlkioWeight": 0,
    "BlkioWeightDevice": null,
    "CapAdd": null,
    "CapDrop": null,
    "Cgroup": "",
    "CgroupParent": "",
    "ConsoleSize": [
      0,
      0
    ],
    "ContainerIDFile": "",
    "CpuCount": 0,
    "CpuPercent": 0,
    "CpuPeriod": 0,
    "CpuQuota": 0,
    "CpuRealtimePeriod": 0,
    "CpuRealtimeRuntime": 0,
    "CpuShares": 0,
    "CpusetCpus": "",
    "CpusetMems": "",
    "DeviceCgroupRules": null,
    "Devices": [],
    "DiskQuota": 0,
    "Dns": [],
    "DnsOptions": [],
    "DnsSearch": [],
    "ExtraHosts": null,
    "GroupAdd": null,
    "IOMaximumBandwidth": 0,
    "IOMaximumIOps": 0,
    "IpcMode": "",
    "Isolation": "",
    "KernelMemory": 0,
    "Links": null,
    "LogConfig": {
      "Config": {},
      "Type": ""
    },
    "Memory": 0,
    "MemoryReservation": 0,
    "MemorySwap": 0,
    "MemorySwappiness": -1,
    "NanoCpus": 0,
    "NetworkMode": "default",
    "OomKillDisable": false,
    "OomScoreAdj": 0,
    "PidMode": "",
    "PidsLimit": 0,
    "PortBindings": {},
    "Privileged": false,
    "PublishAllPorts": false,
    "ReadonlyRootfs": false,
    "RestartPolicy": {
      "MaximumRetryCount": 0,
      "Name": "no"
    },
    "SecurityOpt": null,
    "ShmSize": 0,
    "UTSMode": "",
    "Ulimits": null,
    "UsernsMode": "",
    "VolumeDriver": "",
    "VolumesFrom": null
  },
  "Hostname": "",
  "Image": "hello-world",
  "Labels": {},
  "NetworkingConfig": {
    "EndpointsConfig": {}
  },
  "OnBuild": null,
  "OpenStdin": false,
  "StdinOnce": false,
  "Tty": false,
  "User": "",
  "Volumes": {},
  "WorkingDir": ""
}

And the "long form" (--mount);

docker run --rm --mount type=bind,src=/long,target=/foo hello-world

And associated request:

DEBU[2021-04-26T13:43:12.728500981Z] form data: {"AttachStderr":true,"AttachStdin":false,"AttachStdout":true,"Cmd":null,"Domainname":"","Entrypoint":null,"Env":[],"HostConfig":{"AutoRemove":true,"Binds":null,"BlkioDeviceReadBps":null,"BlkioDeviceReadIOps":null,"BlkioDeviceWriteBps":null,"BlkioDeviceWriteIOps":null,"BlkioWeight":0,"BlkioWeightDevice":null,"CapAdd":null,"CapDrop":null,"Cgroup":"","CgroupParent":"","ConsoleSize":[0,0],"ContainerIDFile":"","CpuCount":0,"CpuPercent":0,"CpuPeriod":0,"CpuQuota":0,"CpuRealtimePeriod":0,"CpuRealtimeRuntime":0,"CpuShares":0,"CpusetCpus":"","CpusetMems":"","DeviceCgroupRules":null,"Devices":[],"DiskQuota":0,"Dns":[],"DnsOptions":[],"DnsSearch":[],"ExtraHosts":null,"GroupAdd":null,"IOMaximumBandwidth":0,"IOMaximumIOps":0,"IpcMode":"","Isolation":"","KernelMemory":0,"Links":null,"LogConfig":{"Config":{},"Type":""},"Memory":0,"MemoryReservation":0,"MemorySwap":0,"MemorySwappiness":-1,"Mounts":[{"Source":"/long","Target":"/foo","Type":"bind"}],"NanoCpus":0,"NetworkMode":"default","OomKillDisable":false,"OomScoreAdj":0,"PidMode":"","PidsLimit":0,"PortBindings":{},"Privileged":false,"PublishAllPorts":false,"ReadonlyRootfs":false,"RestartPolicy":{"MaximumRetryCount":0,"Name":"no"},"SecurityOpt":null,"ShmSize":0,"UTSMode":"","Ulimits":null,"UsernsMode":"","VolumeDriver":"","VolumesFrom":null},"Hostname":"","Image":"hello-world","Labels":{},"NetworkingConfig":{"EndpointsConfig":{}},"OnBuild":null,"OpenStdin":false,"StdinOnce":false,"Tty":false,"User":"","Volumes":{},"WorkingDir":""}

Request:

Details

{
  "AttachStderr": true,
  "AttachStdin": false,
  "AttachStdout": true,
  "Cmd": null,
  "Domainname": "",
  "Entrypoint": null,
  "Env": [],
  "HostConfig": {
    "AutoRemove": true,
    "Binds": null,
    "BlkioDeviceReadBps": null,
    "BlkioDeviceReadIOps": null,
    "BlkioDeviceWriteBps": null,
    "BlkioDeviceWriteIOps": null,
    "BlkioWeight": 0,
    "BlkioWeightDevice": null,
    "CapAdd": null,
    "CapDrop": null,
    "Cgroup": "",
    "CgroupParent": "",
    "ConsoleSize": [
      0,
      0
    ],
    "ContainerIDFile": "",
    "CpuCount": 0,
    "CpuPercent": 0,
    "CpuPeriod": 0,
    "CpuQuota": 0,
    "CpuRealtimePeriod": 0,
    "CpuRealtimeRuntime": 0,
    "CpuShares": 0,
    "CpusetCpus": "",
    "CpusetMems": "",
    "DeviceCgroupRules": null,
    "Devices": [],
    "DiskQuota": 0,
    "Dns": [],
    "DnsOptions": [],
    "DnsSearch": [],
    "ExtraHosts": null,
    "GroupAdd": null,
    "IOMaximumBandwidth": 0,
    "IOMaximumIOps": 0,
    "IpcMode": "",
    "Isolation": "",
    "KernelMemory": 0,
    "Links": null,
    "LogConfig": {
      "Config": {},
      "Type": ""
    },
    "Memory": 0,
    "MemoryReservation": 0,
    "MemorySwap": 0,
    "MemorySwappiness": -1,
    "Mounts": [
      {
        "Source": "/long",
        "Target": "/foo",
        "Type": "bind"
      }
    ],
    "NanoCpus": 0,
    "NetworkMode": "default",
    "OomKillDisable": false,
    "OomScoreAdj": 0,
    "PidMode": "",
    "PidsLimit": 0,
    "PortBindings": {},
    "Privileged": false,
    "PublishAllPorts": false,
    "ReadonlyRootfs": false,
    "RestartPolicy": {
      "MaximumRetryCount": 0,
      "Name": "no"
    },
    "SecurityOpt": null,
    "ShmSize": 0,
    "UTSMode": "",
    "Ulimits": null,
    "UsernsMode": "",
    "VolumeDriver": "",
    "VolumesFrom": null
  },
  "Hostname": "",
  "Image": "hello-world",
  "Labels": {},
  "NetworkingConfig": {
    "EndpointsConfig": {}
  },
  "OnBuild": null,
  "OpenStdin": false,
  "StdinOnce": false,
  "Tty": false,
  "User": "",
  "Volumes": {},
  "WorkingDir": ""
}

Diff between both;

git diff --no-index ./bind-shorthand.json ./bind-mounts.json
diff --git a/./bind-shorthand.json b/./bind-mounts.json
index d1d3311..003f482 100644
--- a/./bind-shorthand.json
+++ b/./bind-mounts.json
@@ -8,9 +8,7 @@
   "Env": [],
   "HostConfig": {
     "AutoRemove": true,
-    "Binds": [
-      "/shorthand:/foo"
-    ],
+    "Binds": null,
     "BlkioDeviceReadBps": null,
     "BlkioDeviceReadIOps": null,
     "BlkioDeviceWriteBps": null,
@@ -57,6 +55,13 @@
     "MemoryReservation": 0,
     "MemorySwap": 0,
     "MemorySwappiness": -1,
+    "Mounts": [
+      {
+        "Source": "/long",
+        "Target": "/foo",
+        "Type": "bind"
+      }
+    ],
     "NanoCpus": 0,
     "NetworkMode": "default",
     "OomKillDisable": false,

So compose should switch between using one or the other API, depending on which form is used (long form / shorthand)

ndeloof · 2021-04-26T14:22:45Z

@thaJeztah unfortunately we don't know which notation has been used by user after the compose file is parsed into compose-go types :'(

thaJeztah · 2021-04-26T14:52:40Z

Would it be an option to have an internal "CreateMissing", or Options: ["create-missing"] (or something like that) property that can be set, and can be used to switch between one and the other?

ndeloof · 2021-04-26T15:21:40Z

That could be an option indeed, but then the code don't just have to prepare MountOptions but will return distinct types. Need to dig into code to check impact

thaJeztah · 2021-04-26T15:37:57Z

For the "engine API" side, possibly it could take that option as well, but that would have to be discussed / accepted, and won't be in a release just yet, so wouldn't help in the short/medium term.

ndeloof · 2021-04-27T10:36:53Z

@thaJeztah That's indeed sad the mount API does not expose this option that would make it a potential full replacement for the legacy bind API. But that's what we have ¯\(ツ)/¯
Created moby/moby#42332 so that we might get a simpler way in the future

ndeloof · 2021-04-28T07:54:16Z

closed for #1604

aiordache requested review from ndeloof and ulyssessouza April 16, 2021 08:51

github-actions Bot added the local Local context (moby) label Apr 16, 2021

aiordache force-pushed the mount_source branch from bd4c46f to 7c51d36 Compare April 16, 2021 09:25

ndeloof suggested changes Apr 16, 2021

View reviewed changes

aiordache force-pushed the mount_source branch from 7c51d36 to 9f59408 Compare April 21, 2021 08:03

Create mount source if it does not exist

39117ee

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

aiordache force-pushed the mount_source branch from 9f59408 to 39117ee Compare April 21, 2021 08:04

Update volume tests

0db72e1

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

ndeloof mentioned this pull request Apr 25, 2021

BUG: Compose not exposing docker.sock in Docker Desktop for Windows/WSL #1591

Closed

thaJeztah mentioned this pull request Apr 26, 2021

invalid mount config for type "bind": bind source path does not exist: #1543

Closed

This was referenced Apr 27, 2021

introduce create_host_path compose-spec/compose-spec#163

Merged

Mount API can't fully replace the legacy Bind API moby/moby#42332

Open

ndeloof mentioned this pull request Apr 28, 2021

when bind mount has CreateHostPath set, use bind API #1604

Merged

ndeloof closed this Apr 28, 2021

aiordache deleted the mount_source branch June 25, 2021 09:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create mount source if it does not exist#1563

Create mount source if it does not exist#1563
aiordache wants to merge 2 commits intodocker-archive:mainfrom
aiordache:mount_source

aiordache commented Apr 16, 2021

Uh oh!

ndeloof left a comment

Uh oh!

ndeloof Apr 16, 2021

Uh oh!

thaJeztah commented Apr 26, 2021

Uh oh!

ndeloof commented Apr 26, 2021

Uh oh!

thaJeztah commented Apr 26, 2021

Uh oh!

ndeloof commented Apr 26, 2021

Uh oh!

thaJeztah commented Apr 26, 2021

Uh oh!

ndeloof commented Apr 27, 2021

Uh oh!

ndeloof commented Apr 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

aiordache commented Apr 16, 2021

Uh oh!

ndeloof left a comment

Choose a reason for hiding this comment

Uh oh!

ndeloof Apr 16, 2021

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Apr 26, 2021

Uh oh!

ndeloof commented Apr 26, 2021

Uh oh!

thaJeztah commented Apr 26, 2021

Uh oh!

ndeloof commented Apr 26, 2021

Uh oh!

thaJeztah commented Apr 26, 2021

Uh oh!

ndeloof commented Apr 27, 2021

Uh oh!

ndeloof commented Apr 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants