Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

Simplify Traefik v2 setup #405

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

masterkoppa
Copy link

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.

Since we can't use the built in management from traefik
in host mode, move most of the configuration
to the toml file for simplicity and ease of management.
@michaelkrieger
Copy link
Contributor

michaelkrieger commented Aug 16, 2020

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.

@masterkoppa
Copy link
Author

@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.

@joshuaboniface
Copy link
Member

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

@Artiume
Copy link
Contributor

Artiume commented Apr 29, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@michaelkrieger
Copy link
Contributor

michaelkrieger commented Apr 29, 2021

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.

@Artiume
Copy link
Contributor

Artiume commented Apr 29, 2021

then let's fix it or close it

@dkanada
Copy link
Member

dkanada commented Aug 6, 2021

@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?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants