linuxserver / docker-jackett

GNU General Public License v3.0
395 stars 95 forks source link

Enable auto-updates #102

Closed ngosang closed 4 years ago

ngosang commented 4 years ago

Enable auto-updates

This commit enables auto-updates. Jackett project supports more than 500 torrent indexers and we publish releases every day. Users cannot update the Docker image daily. With this change the users can deploy any Docker image and they will get fresh updates every day automatically. It's also possible to disable updates in the Jackett UI (not recommended).

@thelamer @sparklyballs I'm a Jackett developer. This PR is well tested. You have to update the changelog and update instructions in readme.

LinuxServer-CI commented 4 years ago

I am a bot, here are the test results for this PR: https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/jackett/v0.16.527-pkg-b1abb87b-pr-102/index.html https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/jackett/v0.16.527-pkg-b1abb87b-pr-102/shellcheck-result.xml

thelamer commented 4 years ago

We usually don't like to do this because you get into situations where the whole userbase is running a different spread of containers that have updated applications internally but not system packages. We had run into that a lot with Plex in the past which we are forced into doing in contianer updating for Plex pass.

There are auto container updating solutions and jackett closes cleanly on term signals afaik, is there any reason users cannot leverage something like watchtower to auto update? Keep in mind we ingest your pre-releases/releases hourly .

ngosang commented 4 years ago

We usually don't like to do this because you get into situations where the whole userbase is running a different spread of containers that have updated applications internally but not system packages.

I understand, but I think that is not going to happen with Jackett because we don't have external dependencies and we don't use system libraries. If Jackett works in the current Docker image, following updates will work too.

It would be interesting if users update the Docker image only when it contains changes in the base image or the libraries. I don't know what is the best solution for that. Maybe you could separate the Jackett version from the Docker image and rely on automatic updates. You can trigger the automatic update when you start the container or something. From a technical view, updating Jackett is faster and lighter than updating the entire Docker image.

There are auto container updating solutions and jackett closes cleanly on term signals afaik, is there any reason users cannot leverage something like watchtower to auto update?

Apparently not all users use an automatic update mechanism with Docker. Every day we receive issues with problems that are already fixed in the latest release.

I have spent time opening this PR because we don't have enough manpower to handle all those issues caused by outdated releases. We are only 2 developers in a project with thousands of users and new releases every day. Maybe there is another solution but I can't think of anything.

thelamer commented 4 years ago

Still discussing this with the team , but it seems to be leaning towards doing an env flag with some guards . Most users copy and paste the create/compose examples so they would enable it by default. Depending on that env flag being set we would mod the run command and run the chown. Might take a bit to get consensus, will update when I have more.

CHBMB commented 4 years ago

I agree with @thelamer I don't think it should become the default, kind of goes against all the good container principles.

All our containers used to auto-update at start and we deliberately moved away from that for precisely the reasons you've highlighted, we just couldn't support a moving target.

Lets see if we can find a compromise of sorts

thelamer commented 4 years ago

@ngosang I added the optional env var to your feature branch, keep in mind most users copy and paste the run commands so it would likely be set to true especially for novice users. Let me know if this works.

LinuxServer-CI commented 4 years ago

I am a bot, here are the test results for this PR: https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/jackett/v0.16.527-pkg-b1abb87b-pr-102/index.html https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/jackett/v0.16.527-pkg-b1abb87b-pr-102/shellcheck-result.xml

ngosang commented 4 years ago

I do not come to tell you how you have to maintain your software, but understand that it is very difficult for us to support thousands of people who use a Docker image that we do not maintain. The current solution is comfortable for you because issues are resolved by us. You have only 70 closed issues in this repository and we have almost 7000. https://github.com/Jackett/Jackett/issues

We need all users to have the latest version of Jackett always, every day. Every day several of the 500 sites we support break (new domains, layouts, cloudflare, sites down..) and the automation tools stop working (Sonarr, Radarr, Lidarr, Potato..). If you put that together with the more than 1200 different releases we have, it is impossible for us. Disabling updates should be the exception. I can't think of any reason to disable them. If you do, Jackett stops working well in a few weeks. Also, I don't see the new environment variable as necessary. It is very easy to disable updates in the Jackett UI.

The proposed solution does not work for the users that already have Jackett installed/configured. They will keep opening issues in our repository, as they always do, and we have to teach them how to change the configuration. It could take years until all of them have auto-updates.

Please, propose a better solution.

trapexit commented 4 years ago

@ngosang I don't know enough about Jackett and C# but could you add updating to Jackett itself? Downloading the latest version and exec'ing into it or reloading relevant components? Not pretty but can work well.

thelamer commented 4 years ago

@ngosang how about if the env var is unset or set to true (default shown in readme) ? This would default to auto updating but also keep a simple option for corner cases to turn it off.

Let me know if that works.

ngosang commented 4 years ago

I don't know enough about Jackett and C# but could you add updating to Jackett itself? Downloading the latest version and exec'ing into it or reloading relevant components?

Jackett already do that but it's disabled in Docker. This RP tries to enable it.

how about if the env var is unset or set to true (default shown in readme) ? This would default to auto updating but also keep a simple option for corner cases to turn it off.

You can rename the env var to DISABLE_UPDATE=false (optional) and change the conditions to execute the updater by default. Btw, you have 2 typos in Jacket (it's with 2 t). That will work for us, but it's almost the same I proposed in the first commit.

nemchik commented 4 years ago

@ngosang I'm really glad you've brought this up and are talking with us about it. Certainly not every software we make images for can be handled 100% the same, and we don't want to make extra work for you. That being said we do want to use the capabilities of docker the best we can and that includes publishing tagged image versions. Our build system keeps pace with your releases hourly. End users may not update their containers as quickly as we publish new tags/versions. That is really best resolved by encouraging users to utilize tools like watchtower.

Another option I wonder if you'd be ok with is detecting if jackett is running inside a container (we can help a bit on our end by placing a file in the container indicating that the user is running in docker) and jackett could display a message in the UI if the user is out of date reminding them to update or setup a tool like watchtower to automatically update for them?

Like I said, I know not every software handles updates the same, but a valid example supporting how we do things actually comes from my experience with an image we don't maintain: bitwarden_rs. They do versioning the same way we do. They published a version with a bug. I was able to roll back the docker image tag to a previous version and everything worked. The bug was reported and fixed and then I rolled up to latest. This is a huge benefit of docker (quick version switching). We try to keep all our containers fairly consistent, but still do what the software needs.

Anyway what do you think about the UI warning for users when running out of date in docker?

LinuxServer-CI commented 4 years ago

I am a bot, here are the test results for this PR: https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/jackett/v0.16.527-pkg-b1abb87b-pr-102/index.html https://lsio-ci.ams3.digitaloceanspaces.com/lspipepr/jackett/v0.16.527-pkg-b1abb87b-pr-102/shellcheck-result.xml

ngosang commented 4 years ago

I have been refactoring the updater all afternoon to conclude that it's a bad solution. https://github.com/Jackett/Jackett/pull/8766

First of all, I want to say that I have deep knowledge about Docker, CI, CD and version control. I know that all artifacts must have closed versions (Docker images too) to be able to reproduce issues, do rollbacks, track regressions, validate integrity... The theory is right in most cases but Jackett is not a regular software. How many software do you know that publish production releases every day?

  1. I have been inspecting your Docker image. The full image with all layers occupies 257.4 MB. If you download several images they only share the base image lsiobase/ubuntu:bionic 63.2 MB. That means that with each release (everyday) you have to download 200MB aprox. from Internet.

  2. I have been playing with Watchtower and that is not enough because of 1). As far as I know Watchtower doesn't remove old images. You have to automate that process too because each new image takes 200MB aprox. That is 6GB of disk per month. Furthermore you are you're discouraged Watchtower in the readme.

  3. We support a lot of OS (Win, Mac, Linux64, LinuxARM64, LinuxARM32, Mono, FreeBSD and most *nix, NAS ...). As I said earlier, we are a bit overwhelmed and having different update procedures for each OS makes things worse. I don't know if Watchtower will work in FreeBSD or in the NAS with Docker support or in any madness that our users do.

  4. Out updater solves all the previous issues:

    • It will download only 40MB (instead of 200MB) and it deletes the previous version files, the downloaded artifacts, etc... The Docker volume won't increase the size over the time and we don't have to worry about removing old images.
    • In the future we are going to release incremental updates so the download will be even smaller. Will Docker updates you will still need the full size image.
    • Or updater is smart and it can recover from some failures and notify the user if something went wrong.
    • It's made with the same technology/libraries than Jackett. If Jackett works the updater will work too. So, if the user is able to install Docker and Jackett starts he don't have to worry about updates anymore.
    • We already have around 15.000 users with Jackett installed on premise (without Docker) and the updater has been working perfect for years. We are very carefully about changing the updater. imagen

You can keep publishing all the releases (I think is a waste of resources but I can live with that) but the users won't need to have the latest Docker image. If an accident occurs and the updater fails (In that case our problem is much bigger than yours, remember the 15.000 users without Docker). The users can download the latest Docker image manually.

Ideally, you should release only new versions of the Docker image when you make changes to the Dockerfile. The tag will become outdated over time, but we can make Jackett to check for updates at the startup. If there is a problem, you can publish a version that disables updates or whatever. You are not losing all the control. Think about it.

thelamer commented 4 years ago

I appreciate all the data, please look at the current head of the feature branch, a person would have to go out of their way to disable the updates with the current logic .

ngosang commented 4 years ago

Do not merge yet, it is necessary to make a change in our part. I will be back in few days.

ngosang commented 4 years ago

@thelamer We are ready to merge. I noticed that you can remove wget from the Docker files. It's not used by us neither you.