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.

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

Transmission home mountpoint set to /config #1974

Merged
merged 1 commit into from
Oct 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ ENV OPENVPN_USERNAME=**None** \
OPENVPN_PASSWORD=**None** \
OPENVPN_PROVIDER=**None** \
GLOBAL_APPLY_PERMISSIONS=true \
TRANSMISSION_HOME=/data/transmission-home \
TRANSMISSION_HOME=/config/transmission-home \
TRANSMISSION_RPC_PORT=9091 \
TRANSMISSION_DOWNLOAD_DIR=/data/completed \
TRANSMISSION_INCOMPLETE_DIR=/data/incomplete \
Expand Down
4 changes: 2 additions & 2 deletions docs/config-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ In previous versions of this container the settings were not persistent but was
This had the benefit of being very explicit and reproducable but you had to provide Transmission config as environment variables if you
wanted them to stay that way between container restarts. This felt cumbersome to many.

As of version 3.0 this is no longer true. Settings are now persisted in the `/data/transmission-home` folder in the container and as
long as you mount `/data` you should be able to configure Transmission using the UI as you normally would.
As of version 3.0 this is no longer true. Settings are now persisted in the `/config/transmission-home` folder in the container and as
long as you mount `/config` you should be able to configure Transmission using the UI as you normally would.

You may still override Transmission options by setting environment variables if that's your thing.
The variables are named after the transmission config they target but are prefixed with `TRANSMISSION_`, capitalized, and `-` is converted to `_`.
Expand Down
4 changes: 2 additions & 2 deletions docs/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
## How do I enable authentication in the web ui

You can do this either by setting the appropriate fields in `settings.json` which is
found in TRANSMISSION_HOME which defaults to `/data/transmission-home` so it will be available
on your host where you mount the `/data` volume. Remember that Transmission overwrites the config
found in TRANSMISSION_HOME which defaults to `/config/transmission-home` so it will be available
on your host where you mount the `/config` volume. Remember that Transmission overwrites the config
when it shuts down, so do this when the container is not running.

Or you can set it using the convenience environment variables. They will then override the settings
Expand Down
27 changes: 21 additions & 6 deletions transmission/userSetup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,37 @@ if [ -n "$PUID" ] && [ ! "$(id -u root)" -eq "$PUID" ]; then
chown ${RUN_AS}:${RUN_AS} /dev/stdout
fi

if [[ "${TRANSMISSION_HOME%/*}" != "/config" ]]; then
echo "WARNING: TRANSMISSION_HOME mountpoint is not on default /config, this is not recommended."
fi

#If migration is enabled, attempt to move transmission-home
if [[ "$TRANSMISSION_HOME_MIGRATE" = true ]]; then
TRANSMISSION_HOME_SUBNAME=${TRANSMISSION_HOME##*/}
echo "Attempting to migrate old TRANSMISSION_HOME from /data/$TRANSMISSION_HOME_SUBNAME to /config/$TRANSMISSION_HOME_SUBNAME "
if [ -d "/data/$TRANSMISSION_HOME_SUBNAME" ] && [ ! -d "/config/$TRANSMISSION_HOME_SUBNAME" ]; then
mv "/data/$TRANSMISSION_HOME_SUBNAME" "/config/$TRANSMISSION_HOME_SUBNAME"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think many people have not mounted /config. So doing a mv will possibly move the home folder to a non-mounted file system within the container and it would be gone when the container is re-created.

I know that's not 100% true because we specify VOLUME /config in the Dockerfile, but... For a new Docker-user it's probably tricky to re-locate their transmission-home inside a volume.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, if user has not set TRANSMISSION_HOME_MIGRATE then only the warning is shown..
While it makes more sense to keep it under /config… I’m not against a user saying they don’t want that etc..
this would mainly affect new installs and optionally the user can let the system migrate it for them..
Perhaps add another check to see if the user has mounted /config or not?

Copy link
Owner

@haugene haugene Oct 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry. I was a bit quick there, hadn't read it properly it seems 🙈
EDIT: And then I did it again, not reading all the code. I'm signing off for now, but the gist of what I was trying to communicate is...

If we change the default TRANSMISSION_HOME value in our Dockerfile I think most users will come to a blank Transmission state after they re-create the image next time (it will still be in /data though). The NAS gang usually has TRANSMISSION_HOME set because they get all the env vars in their GUI, but I would expect most of the rest to not have the env var set and just rely on the default.

If I'm not mistaken about the fallout here, then that's a bit too much breakage IMO. So I think we have to look for /data/transmission-home and then use that if it exists and log that they are using a non-default setup.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, I ate and I’ll fix a fallback after the little one has been dropped off at dagis ;)

TRANSMISSION_HOME="/config/$TRANSMISSION_HOME_SUBNAME"
TRANSMISSION_HOME_MIGRATE=false
else
echo "Could not migrate, please check for existing folder in /config or missing folder in /data"
fi
fi

# Make sure directories exist before chown and chmod
mkdir -p /config \
"${TRANSMISSION_HOME}" \
"${TRANSMISSION_DOWNLOAD_DIR}" \
"${TRANSMISSION_INCOMPLETE_DIR}" \
"${TRANSMISSION_WATCH_DIR}"

echo "Enforcing ownership on transmission config directories"
echo "Enforcing ownership on transmission config directory"
chown -R ${RUN_AS}:${RUN_AS} \
/config \
"${TRANSMISSION_HOME}"
/config

echo "Applying permissions to transmission config directories"
echo "Applying permissions to transmission config directory"
chmod -R go=rX,u=rwX \
/config \
"${TRANSMISSION_HOME}"
/config

if [ "$GLOBAL_APPLY_PERMISSIONS" = true ] ; then
echo "Setting owner for transmission paths to ${PUID}:${PGID}"
Expand Down