Fix ignored umask env var by removing overwrite (testing)

JorisMolnar commented 4 years ago

We welcome all PR’s though this doesn’t guarantee it will be accepted.

Description:

I bumped into an issue that the new UMASK environment variable did not seem to work for the NZBGet service. The UMask was always 022.

4 years ago (so long before the UMASK env var) commit 64dc5f4cf9edb516dd3fb95b6f143c338291e9d6 added a line in the nzbget service run file to set the UMask to 022. I could not find a reason for this change. #26 doesn't give more information.

This static UMask got applied after the env var UMask got applied.

This PR removes the static UMask. This results in:

UMASK env var is filled: The configured UMask is applied.
UMASK env var is empty: The default UMask of the default abc user in the container is used. This is 022.

Benefits of this PR and context:

This PR fixes a bug for users that have configured a UMask using the environment variable. Users that have not configured a special UMask get the default 022, just like before. That should make the change safe for everyone.

How Has This Been Tested?

I started a container using the following docker-compose.yml service snippet:

  nzbget:
    container_name: nzbget
    build: ./nzbget-code/docker-nzbget
    restart: unless-stopped
    networks:
      - frontend
    volumes:
      - /mnt/Data1/media/NZBGet:/downloads
      - ./nzbget/config:/config
      - ./shared:/shared
    environment:
      - UMASK=002
      - PUID=${PUID_NZBGET}
      - PGID
      - TZ

Next I started a download and enter a shell in the container. There I ran the commands described in the Discord conversation below to check the UMask of the currently running nzbget and unrar processes.

root@f19036f8a0a2:/# ps aux
USER         PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root           1  0.0  0.0    196     4 ?        Ss   23:05   0:00 s6-svscan -t0 /var/run/s6/services
root          35  0.0  0.0    196     4 ?        S    23:05   0:00 s6-supervise s6-fdholderd
root         254  0.0  0.0    196     4 ?        S    23:05   0:00 s6-supervise nzbget
abc          258 39.9  1.0  49152 41716 ?        Ssl  23:05   3:58 /app/nzbget/nzbget -s -c /config/nzbget.conf -o OutputMode=log
root         282  0.0  0.0   2424  1148 pts/0    Ss   23:05   0:00 /bin/bash
abc        16577 23.2  0.0   4396  2100 ?        Ds   23:11   0:57 /app/nzbget/unrar x -y -p- -o+ *.rar /downloads/Downloads/Some.Random.Name/_unpack/
root       17132  0.0  0.0   1616   524 pts/0    R+   23:15   0:00 ps aux
root@f19036f8a0a2:/# grep '^Umask:' "/proc/16577/status"
Umask:  0002
root@f19036f8a0a2:/# grep '^Umask:' "/proc/258/status"
Umask:  0002

Source / References:

The following is the short Discord discussion I had about this subject.

JMolnar Hi, I'm trying to configure the umask for my nzbget container as described here: https://github.com/linuxserver/docker-nzbget#umask-for-running-applications But it seems like this setting doesn't work.

I test this by connecting to the container using sudo docker exec -it nzbget /bin/bash and then running grep '^Umask:' "/proc/12345/status" where 12345 is the PID of the /app/nzbget/nzbget process. (I get the PID by running ps aux) The response I get in the nzbget container is Umask: 0022. I also tried running it in the rutorrent container, where I get Umask: 0002 as expected.

My docker-compose service snippet is:

  nzbget:
    container_name: nzbget
    image: linuxserver/nzbget
    restart: unless-stopped
    networks:
      - frontend
    volumes:
      - /mnt/Data1/media/NZBGet:/downloads
      - ./nzbget/config:/config
      - ./shared:/shared
    environment:
      - UMASK=002
      - PUID=${PUID_NZBGET}
      - PGID
      - TZ

j0nnymoe @JorisMolnar you can set the UMASK via the nzbget settings I believe aswell

JMolnar I noticed the following file always sets the umask to 022, but I don't know if there is some other system that should change it again: https://github.com/linuxserver/docker-nzbget/blob/master/root/etc/services.d/nzbget/run Thanks @j0nnymoe, I'll take a look at that. I found the setting in nzbget and can confirm that works :smile: Thanks The original setting was 1000, which nzbget says disables changing the umask, so it seems there still is an issue with the nzbget image. But at least there is a workaround :slight_smile:

JMolnar Regarding that umask issue I mentioned 2 hours ago, it seems like we aren't completely there yet. NZBGet just extracted something and it had the wrong permissions. I'd have to test further, but it's probably because the unrar process doesn't inherit the umask configured in nzbget settings. So unrar got 022 again :sweat_smile: I'll take a look if I can spot the difference between the containers where the umask env variable works and doesn't work.

JMolnar I think I fixed the issue. If my test succeeds I'll submit a pull request.

LinuxServer-CI commented 4 years ago

I am a bot, here are the test results for this PR: https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/nzbget/v21.1-r2311-pkg-4d7b0218-pr-112/index.html https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/nzbget/v21.1-r2311-pkg-4d7b0218-pr-112/shellcheck-result.xml

LinuxServer-CI commented 4 years ago

I am a bot, here are the test results for this PR: https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/nzbget/v21.1-r2311-pkg-4d7b0218-pr-112/index.html https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/nzbget/v21.1-r2311-pkg-4d7b0218-pr-112/shellcheck-result.xml

linuxserver / docker-nzbget