jellyfin-docs icon indicating copy to clipboard operation
jellyfin-docs copied to clipboard

Simplify Traefik v2 setup

Open masterkoppa opened this issue 4 years ago • 8 comments

While troubleshooting an issue with my personal Traefik v2 setup, I referenced the docs to see if I was missing anything new (Thank you @michaelkrieger for putting these together!). However, I found the label setup here unnecessary and overly complex. Especially if you already have a Traefik v2 setup for other things.

Since we can't use the built in management from Traefik in host mode, I moved most of the configuration of the Jellyfin configuration to the toml file, instead of using labels. As a bonus, this should in theory allow folks to use a non-Docker Jellyfin installation with Traefik v2.

NOTE: Since this has a lot of placeholders and I used yml instead of toml on my own setup, I might have missed some quotes or messed something up so a second set of eyes would be appreciated.

masterkoppa avatar Aug 16 '20 01:08 masterkoppa

Thanks @masterkoppa I’d suggest, if you like this pull, adding it to the documentation as an alternate configuration block instead of replacing the setup. Anyone which has or wants to set up multiple containers (torrents, pihole, VPN, NAS, etc) often wishes to keep the configuration together with the Docker container as much as possible. To go and put the configuration only in the yaml file removes the (probably) most common use case. Another note is that it doesn’t actually -need- host networking except for certain discovery features, so to say that it can’t use host networking is incorrect. A final note is that this shortcoming should be fixed in future Docker versions.

I’ll suggest adding this good contribution instead of replacing an otherwise working/common/good configuration. A paragraph that says “if you wish to not use labels in your configuration or if you want to use Traefik v2 without the Docker provider (ie: a non-Docker Jellyfin installation), you can replace the labels of the jellyfin service with the following code in the TOML file” and then having the code block... that I think makes more sense.

It’s a great idea to add this to the documentation. I don’t think it’s better to make this the default configuration nor better to remove the Docker provider configuration though.

michaelkrieger avatar Aug 16 '20 02:08 michaelkrieger

@michaelkrieger

I had debated that approach at first, but I didn't like the potential for duplication when I started it. But I didn't think of making it a potential addendum like you suggested, I'll update the PR with something like that.

masterkoppa avatar Aug 16 '20 02:08 masterkoppa

I agree with the add-rather-than-replace idea, otherwise good. CC @masterkoppa

joshuaboniface avatar Sep 15 '20 02:09 joshuaboniface

/azp run

Artiume avatar Apr 29 '21 11:04 Artiume

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Apr 29 '21 11:04 azure-pipelines[bot]

Are we doing the replace based on the approval? I don't think the new configuration is any more ideal, though adding it to the documentation as an alternate implementation is fine. Replacing a working implementation using Docker Labels for a TOML configuration for the sake of it just doesn't make sense.

Especially since that one Traefik bug will be fixed.

michaelkrieger avatar Apr 29 '21 11:04 michaelkrieger

then let's fix it or close it

Artiume avatar Apr 29 '21 11:04 Artiume

@masterkoppa we have several comments in here requesting this method be added rather than replace the main configuration so could you make that change before this gets merged?

dkanada avatar Aug 06 '21 14:08 dkanada