docker cp not working properly

[andergnet]

Hello,
Fist of all, thanks for the driver and the work you put into it, it's great, it saves me a lot of time when creating containers.

I discovered a bug, the issue is the following:
When I start the container the volume correctly mounts.
Then, while the container is running I run a "docker cp".
When the "docker cp" ends the volume unmounts (i.e. the "mountpoint" folder is no longer there)
The running container does not like that.

Doing a "docker cp" with an stopped container or starting and stoping the container work just fine.

If you could fix it or give me some indications on where to look would be really nice (I cannot do it without help due to my total lack of knowledge of Go and Docker drivers)

Thans!

[kolayne]

Good day! I am glad you found docker-on-top useful. I might be able to have a deeper look in your problem in a few days.

Meanwhile, it would be useful if you provided a minimal reproducible example: which containers I need to start and which commands to run (starting with the volume creation) to reproduce the issue.

Logs can be useful too. If you're running the docker-on-top executable manually, you can just provide the output it generates. If you're using a systemd service, you can access the logs via sudo journalctl -u docker-on-top.service (to see logs generated previously) or sudo journalctl -u docker-on-top.service --follow to access logs in the real time.

[andergnet]

WoW, that was quick!
I have attached a zip file with the reproductible example: test.zip

It contains these files:

• An script with the steps to do to reproduce the issue. Is better to follow it step by step rather than executing it.
• A docker-compose.yaml file with the container and volume information
• The logs of an erroneous run. From what I see, it mounts the volume first when the container is run, then again with the cp, then an unmount also with the cp, and an unmount with an error when the container is stopped.

спасибо (which is most of the Russian I remember from the time when I had time to learn languages 😉)

[kolayne]

Hello again!

I was able to reproduce the issue. The docker-on-top code erroneously assumes that a volume could only be mounted several times if there are several containers using it, and, vice versa, if it's only mounted in one container, it should be mounted just once. The assumption breaks when using docker cp (apparently, docker temporarily mounts the same volume to another place to perform the copy).

I will look into it soon and, hopefully, fix it by the end of the week

[kolayne]

Hm. After a deeper investigation, it appears to me that this is a bug of dockerd rather than docker-on-top. At least, according to the documentation, the assumption I described above that docker-on-top makes is valid.

Upstream issue: moby/moby#47964

It's taking longer than I was hoping it would... But I hope this week I will have enough time to try and submit a patch for dockerd

[andergnet]

Thanks for all the effort you are putting into it even if the bug it's not in docker-on-top. I will keep an eye also in the upstream issue to see if it gets fixed.

[kolayne]

Hello there! It's been a while.

A few days ago I submitted a PR: moby/moby#48096. It fixes the issue on Linux, however, it's not yet ready for merge:

• There are a few TODOs fixing which will improve the code quality,
• I only implemented the linux/unix version, the current patched version won't build on windows at all,
• Integration tests are not implemented,
• etc. See the first PR message for details.

I opened the PR because I wanted to synchronize with the maintainers and agree in which direction I should be going (and also ask them some questions about the code) but opening the PR alone did not attract them. Maybe I should ping a particular person but after (briefly) glancing through the contribution guidelines I didn't find which person I should ping.

Moreover, in the next couple of weeks I may not have much time to work on this, even if someone does come there to answer my questions.

Anyway! Back to Docker-on-Top! I see three things that can be done about this issue (#18):

0. Before it's resolved, there's a workaround. If you need to docker cp from/to a container that uses a docker-on-top volume, stop the container first, then docker cp, then start the container again. Kinda sucks but... ¯\_(ツ)_/¯

1. The right way to fix this issue would be to patch the upstream (https://github.com/moby/moby, aka dockerd) because this bug occurs due to dockerd violating what's stated in their plugin API docs, namely, the fact that mount ID is unique for every mount.
   If you, @andergnet, or anyone else decide to take this path, you are free to base your solution on top of what I've already implemented in Refactor MountPoint IDs moby/moby#48096, if you want to.

2. But I have a feeling that the upstream bug is not going to be resolved anytime soon. So, I'm ready to accept a patch for docker-on-top itself, working around the plugin API issue.

   Details about what to patch in docker-on-top

   The patch would need to alter the way docker-on-top tracks mounts: currently, for every mount ID there's a corresponding (empty) file created when it's mounted and deleted when it's unmounted, and it's only the first mount and the last unmount that we care about: that's when there's real work to do.

   But it doesn't work because with docker cp on a running container, dockerd may issue a Mount/Unmount request with the same ID as it did before. So, it should be reworked to track multiple mounts with the same id.

   Before making the patch, check if docker allows two docker cps running on the same container at the same time. If not (i.e., they are scheduled one after another), there should be no more than 2 mounts with the same ID possible (it will make your work much easier).

[andergnet]

Thanks for the update, I have been following the [lack] of progress of the PR, I hope someone gets in touch and fixes it.

The workaround I have taken for the time being is to check whether the container is running or not, if it's not do a docker cp, if it is do a cp from the mountpoint folder. It's a script so it does not really hurt.

I may try to send you a PR, I have looked into the code and it does not seem too hard:

1. Add a counter to the VolumeInfo structure
2. On mount check whether the file exists or not, if it does exist, increase the counter
3. On unmount read the file, decrease the counter and only delete it when the counter reaches zero.
   Theory looks solid, we'll see how practice goes

[andergnet]

Just a quick update, I'm working on a solution I hope to finish it by the end of the week.

[andergnet]

I have just sent you a pull request that fixes this issue, I hope it is to your liking ☺.