lloesche / valheim-server-docker

Valheim dedicated gameserver with automatic update, World backup, BepInEx and ValheimPlus mod support
https://hub.docker.com/r/lloesche/valheim-server
Apache License 2.0
1.94k stars 272 forks source link

common script file perm check #300

Closed tacitDev closed 3 years ago

tacitDev commented 3 years ago

I ran into a situation when using Kubernetes NFS volumes and volume mounts in the /config dir due to the ensure_permissions() chmod. This workaround allows empty or existing NFS directories that already have the correct permissions to be mounted

lloesche commented 3 years ago

Hi @tacitDev thank you for your PR. What is the error this PR is solving? Also can you check if the latest version of the container already solves your issue? For #299 I changed the way permission change errors are handled and made them non-critical.

tacitDev commented 3 years ago

Hi @lloesche! Thanks for the awesome Docker image. After re-reading the PR description, I was a bit vague :laughing:.

Now that I've stepped back, the problem might be fixable on my end via changing something with my NFS setup (currently using root_squash,subtree_check,rw). I'll update if it works, and will post example logs for posterity and searchability in case anyone runs into this issue in the future. For what it's worth, it's similar to this issue here: https://github.com/kubernetes/kubernetes/issues/54601#issuecomment-339928466

For the record, I was using the latest image in Docker Hub as it was pulled more than a few times today while troubleshooting.

The issue is related to using the image via a Kubernetes deployment. I have an NFS service on a different machine that stores config files. When mounting the NFS directory in the pod, with pre-populated files that have correct permissions, valheim-docker throws errors when trying to change their permissions (even if they're set to be the same). This caused the valheimplus merge step to fail. The container is able to read/write to the file fine, but not change the permissions.

So, for example:

With this setup, I was getting logs similar to (might not be word for word, have to recreate it later):

chown: changing ownership of /config/worlds/world1.fwl`
Failed to extract and install ValheimPlus

The changes in common > ensure_permissions() of this PR check if the directory and files are already set to the correct permissions before setting them with chmod.

Anyway, for now, I'll close this PR.

lloesche commented 3 years ago

Thanks for the description.

throws errors when trying to change their permissions

The latest image of the container (from like 5 min before I originally answered to this PR) should make all permission related errors in ensure_permissions() non-critical in the BepInEx/ValheimPlus install process. So you will still see the error in the log file, which I think is valuable info when debugging issues down the line, but it will no longer abort the ValheimPlus/BepInEx install process.