Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fs.copyFile not working with docker volumes #21329

Closed
crisward opened this issue Jun 14, 2018 · 11 comments
Closed

fs.copyFile not working with docker volumes #21329

crisward opened this issue Jun 14, 2018 · 11 comments
Labels
fs Issues and PRs related to the fs subsystem / file system. libuv Issues and PRs related to the libuv dependency or the uv binding.

Comments

@crisward
Copy link

crisward commented Jun 14, 2018 •
edited
Loading

  • Version:v10.4.1 (but seems to apply to any version of node using copyFile, tried 8.11 too)
  • Platform: Linux 9c6e47477060 3.13.0-86-generic # 131-Ubuntu SMP Thu May 12 23:33:13 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux (inside docker container)
  • Subsystem: filesystem / fs
require("fs").copyFile("/somevolume/text.txt","/somevolume/text2.txt",(err)=>{console.log(err)})

Expected behaviour - it copies the file
Actual behaviour - it fails to copy but no error is reported. This is not a file permissions issue as the command line cp command works as the same user. Also if the destination exists, it gets deleted, but the overwriting file doesn't get written.

The above code works if the files are not on a docker volume. The old method using createReadStream and createWriteStream also works.

The docker image I am using is the heroku buildpack. https://github.com/heroku/heroku-buildpack-nodejs. (with dokku)
@addaleax addaleax added fs Issues and PRs related to the fs subsystem / file system. libuv Issues and PRs related to the libuv dependency or the uv binding. labels Jun 14, 2018
@addaleax
Copy link
Member

@nodejs/fs @nodejs/libuv

@santigimeno
Copy link
Member

santigimeno commented Jun 14, 2018 •
edited
Loading

@crisward it's working for me with the official node docker image (v10.4.1).

@crisward
Copy link
Author

@santigimeno Thanks for checking, I'll dig further and see what underlying docker image actually gets pulled.

@crisward
Copy link
Author

crisward commented Jun 14, 2018 •
edited
Loading

Are there any specific dependencies that copyFile would use, that I could check for inside the container (dynamic libs perhaps)?

I'm not sure where to go with this, if it's a heroku buildpack issues I could open something there, or it may be something I need add to herokuish. Perhaps knowing about missing dependencies would help.

@santigimeno
Copy link
Member

@crisward I don't know if it's possible for you to run that code with strace (strace -f node script.js) and post the output somewhere. (You'll need to run the container with --cap-add SYS_PTRACE)

@crisward
Copy link
Author

Hope this helps.

https://gist.github.com/crisward/254896e0df574217dc70b9479dfbea70

@cjihrig
Copy link
Contributor

cjihrig commented Jun 14, 2018

It looks like fchmod() is failing: fchmod(11, 0100666) = -1 EPERM (Operation not permitted)

@santigimeno
Copy link
Member

Yeah, but from the original comment it seems the error is not reported

@crisward
Copy link
Author

That's correct, the error comes back null.

@cjihrig
Copy link
Contributor

cjihrig commented Jun 14, 2018

Figured it out. PR for libuv coming...

@cjihrig cjihrig mentioned this issue Jun 14, 2018
Closed
cjihrig added a commit to cjihrig/libuv that referenced this issue Jun 14, 2018
@cjihrig
unix: set errno in uv_fs_copyfile()
96d38d9 
When the specific value of -1 is returned, uv__fs_work() uses
the value of errno. This commit sets errno in uv__fs_copyfile().

Fixes: nodejs/node#21329
PR-URL: libuv#1881
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@targos
Copy link
Member

targos commented Jun 14, 2018

Reopening until we get a libuv update

@targos targos reopened this Jun 14, 2018
targos pushed a commit that referenced this issue Jun 25, 2018
@cjihrig @targos
deps: upgrade to libuv 1.21.0
f162939 
Notable changes:

- Building via cmake is now supported.
  PR-URL: libuv/libuv#1850
- Stricter checks have been added to prevent watching the same
  file descriptor multiple times.
  PR-URL: libuv/libuv#1851
  Refs: #3604
- An IPC deadlock on Windows has been fixed.
  PR-URL: libuv/libuv#1843
  Fixes: #9706
  Fixes: #7657
- uv_fs_lchown() has been added.
  PR-URL: libuv/libuv#1826
  Refs: #19868
- uv_fs_copyfile() sets errno on error.
  PR-URL: libuv/libuv#1881
  Fixes: #21329
- uv_fs_fchmod() supports -A files on Windows.
  PR-URL: libuv/libuv#1819
  Refs: #12803

PR-URL: #21466
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this issue Nov 5, 2018
@cjihrig @MylesBorins
deps: upgrade to libuv 1.21.0
4de5458 
Notable changes:

- Building via cmake is now supported.
  PR-URL: libuv/libuv#1850
- Stricter checks have been added to prevent watching the same
  file descriptor multiple times.
  PR-URL: libuv/libuv#1851
  Refs: nodejs#3604
- An IPC deadlock on Windows has been fixed.
  PR-URL: libuv/libuv#1843
  Fixes: nodejs#9706
  Fixes: nodejs#7657
- uv_fs_lchown() has been added.
  PR-URL: libuv/libuv#1826
  Refs: nodejs#19868
- uv_fs_copyfile() sets errno on error.
  PR-URL: libuv/libuv#1881
  Fixes: nodejs#21329
- uv_fs_fchmod() supports -A files on Windows.
  PR-URL: libuv/libuv#1819
  Refs: nodejs#12803

PR-URL: nodejs#21466
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Nov 11, 2018
@cjihrig @MylesBorins
deps: upgrade to libuv 1.21.0
11cb09b 
Notable changes:

- Building via cmake is now supported.
  PR-URL: libuv/libuv#1850
- Stricter checks have been added to prevent watching the same
  file descriptor multiple times.
  PR-URL: libuv/libuv#1851
  Refs: #3604
- An IPC deadlock on Windows has been fixed.
  PR-URL: libuv/libuv#1843
  Fixes: #9706
  Fixes: #7657
- uv_fs_lchown() has been added.
  PR-URL: libuv/libuv#1826
  Refs: #19868
- uv_fs_copyfile() sets errno on error.
  PR-URL: libuv/libuv#1881
  Fixes: #21329
- uv_fs_fchmod() supports -A files on Windows.
  PR-URL: libuv/libuv#1819
  Refs: #12803

Backport-PR-URL: #24103
PR-URL: #21466
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
fs Issues and PRs related to the fs subsystem / file system. libuv Issues and PRs related to the libuv dependency or the uv binding.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@addaleax @santigimeno @targos @cjihrig @crisward