-
Notifications
You must be signed in to change notification settings - Fork 308
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 container plus volume management #23
Conversation
Hey @gpongelli would you mind fixing the merge conflict? we just put each of the menu items on seperate lines so that merge conflicts in the future are easier to manage. I'm also about to merge #11 in. Would you prefer to use qbittorrent, or you think Transmission also has a place? |
Good idea to put menu items on separate lines, fixed using web client. I think that all three containers can cohabit, so anyone can choose which it's best. |
Awesome, this PR is next on my list for testing then. Just finishing up testing #19 and will start on this one. |
.templates/transmission/volumes.yml
Outdated
driver_opts: | ||
o: bind | ||
type: none | ||
device: <transm_download_volume absolute path on disk> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! I put it as a bookmark to be changed after running menu.sh, so anyone can use the absolute path he/she want (e.g. I have them on an external hd).
@@ -140,6 +143,9 @@ function yml_builder() { | |||
#if an env file exists check for timezone | |||
[ -f "./services/$1/$1.env" ] && timezones ./services/$1/$1.env | |||
|
|||
# if a volumes.yml exists, append to overall volumes.yml file | |||
[ -f "./services/$1/volumes.yml" ] && cat "./services/$1/volumes.yml" >> docker-volumes.yml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will likely need to be updated in the future once we have yaml merging on. Just letting you know in case it breaks your current setup when that upgrade happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok no problem. I did in this way to have the code written in a similar fashion to the original
Hey @gpongelli I just tried running this on a fresh installation and getting the following error:
Could you update the installation steps so that it create the required files and folders? It should be able to run once installing it, with the ability for the user to change the docker image if they choose to. I would help, but I can't push changes to commits from forked repos. |
Yes, unlike usual volume management, those folder must exist before running docker-compose.
I've no idea on how to do that, what do you mean with "installation steps" ? |
Just pushed the changes to your github and approved the PR here. Transmission should now run without errors with the named volumes, and have the directories automatically created for them at build time without any config changes. |
@gpongelli PR #38 may break your current system. There's details in the PR on how to mitigate. |
This PR is to add Transmission container, no more included as plugin into OpenMediaVault 5 release, that uses OMV shared folders.
To make sure the folders are shared between OMV5 and Transmission container, a volume is needed as explained in approach #2 here.
For this reason, it's required the management of new "volumes.yml" file into menu.sh, that can be used by any template, to append all the volume's declaration at the end of docker-compose.yml
Thank you of your awesome job, hope you'll enjoy this PR!