search

linuxserver / docker-nzbget

GNU General Public License v3.0
149 stars 83 forks source link

Recursively chown /downloads #121

Closed rfrowe closed 3 years ago

rfrowe commented 3 years ago

linuxserver.io

Description:

So I was trying to mount an SSD-backed volume for the intermediate directory. I read the note about this in the README but happened upon two facts:

  1. I have to manually configure NZBGet to use /intermediate
  2. This directory is not chowned by cont-init.d, resulting in permissions issues, again requiring manual intervention

I could have submitted a PR to add conditional chown of /intermediate to cont-init.d, but I decided to fix both problems. By recursively chowning /downloads, I can mount my volume to /downloads/intermediate since docker supports nested volume mounts. This then fixes both issues.

Benefits of this PR and context:

Plug-and-play functionality for intermediates volume.

How Has This Been Tested?

Did docker-compose down -v and altered my docker-compose.yaml to replace image: with:

build: https://github.com/rfrowe/docker-nzbget#patch-1

Then did docker-compose up. No longer receive permissions error about /downloads/intermediate while downloading.

Source / References:

N/A

LinuxServer-CI commented 3 years ago

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/nzbget/v21.0-pkg-a7f1c907-pr-121/index.html https://ci-tests.linuxserver.io/lspipepr/nzbget/v21.0-pkg-a7f1c907-pr-121/shellcheck-result.xml

nemchik commented 3 years ago

Thanks for the PR, but this could have a significant impact slowing down container init times for many users. I would advise if you require this functionality to do the permissions manually as needed or check out https://blog.linuxserver.io/2019/09/14/customizing-our-containers/

rfrowe commented 3 years ago

@nemchik May I ask how you would feel about a PR which does [[ -d /intermediate ]] && chown abc:abc /intermediate? A similar chown is done for /config and /downloads which may also be mounted volumes. It seems only fair that /intermediate be chowned as well since it's officially supported in the README. This would solve issue (2) of my original post, which is a good enough solution for me.