"Autoupdate" feature. Update S6. Shellcheck. #81

midzelis · 2023-06-21T01:19:39Z

I've always been annoyed that my containerized Plex server says that theres a new update. Its pretty annoying to have to pull a new docker image or to set up things like watchtower.

Instead, I modified the official pms-docker image to support autoupdates (optionally.) It's fully backwards compatible, and it should be buildable exactly the same way as before (however you did that.)

There's a new ARG 'autoupdate' default=false. If false, its 100% backwards compatible.

If its true, it will skip downloading the ARG TAG completely. Instead, it will download (and cache) the binary on startup. It will also install and run a cron job (randomized between 4am-4:30am) to check for updates. If it finds an update it will stop the server, install the update, and re-start the server.

I also updated S6 to latest version.

I also ran shellcheck and fixed all the warnings.

Oh, and also added sha256sum checking of all downloads.

gbooker

Your changes need to be separated into individual commits before this PR can even begin to be considered. Essentially each commit should contain small but atomic changes.
Additionally many of your files are missing the trailing newline.

midzelis · 2023-06-22T18:36:36Z

Thanks. I split up the PR into much more bite-sized changes. Additionally, the image is available up on: https://hub.docker.com/r/midzelis/pms-docker/tags

Looking forward to your review/comments.

gbooker

A few minor changes and some questions.

Thank you for splitting up the commits as it makes individual changes much easier to review.

dev/build.amd64.sh

dev/debug.386.sh

dev/debug.amd64.sh

Dockerfile

root/etc/plex/service.cron.run

gbooker · 2023-06-22T18:59:23Z

root/etc/plex/plex-update

-# Short-circuit test of version before remote check to see if it's already installed.
-if [ "${versionToInstall}" = "${installedVersion}" ]; then
-  exit 0
-fi


We have docker images which have a pre-installed PMS version that do not update. The above code effectively maintains that because the version is the full version of the PMS installed in the image (such as 1.32.4.7195-7c8f9d3b6). This behavior must be maintained for those images.

Ok, this is a good call out. I was tripped up by using the version as a TAG and having TAG also be a build-arg.

I switched things up a bit: now, you can build the image with TAG=autoupdate (this obsoletes beta/public - but you can still build the images with public/beta tags - it will behave as if it was 'autoupdate' but it will ignore the AUTO_UPDATE_CHANNEL and will be hardcoded to 'beta' or 'public').

root/plex-common.sh

root/plex-envvars

gbooker · 2023-06-22T19:04:43Z

root/plex_service.sh

-  s6-svc "$1" /var/run/s6/services/plex
-else
-  echo "No argument supplied; must be -u, -d, or -r."
-fi


Why is this removed? This file is sometimes used by people who wish to stop the service inside a shell in the container without stopping the container.

sztomi

I only have a small nit, otherwise LGTM

sztomi · 2023-07-06T14:36:01Z

Dockerfile

+ARG S6_OVERLAY_ARCH=arm
+ARG PLEX_BUILD=linux-armv7hf_neon
+
+FROM i386/ubuntu:18.04 as base-386


I'd call this i386 to not change how this arch is spelled in this context.

Unfortunately, I can't really change this. The way this multi-arch Dockerfile works is that it pulls from various arch-specific ubuntu images and then names them using the pattern base-TARGETARCH where arch comes from the env var TARGETARCH that is automatically set bydocker build[x]. The actual selected image is selected on line 24 below.

Looks like everyone has opinions, and they are not standardized, on what platforms mean what.

In the case of i386/ubuntu, this is meant to be used when TARGETARCH=386

S6-overlays' has its own opinions, and it maps TARGETARCH=386 to i686. See https://github.com/just-containers/s6-overlay#which-architecture-to-use-depending-on-your-targetarch

More details can be found here: https://github.com/BretFisher/multi-platform-docker-build/blob/main/README.md#the-problem-with-downloading-binaries-in-dockerfiles

Technically, the 32bit Ubuntu image is stuck at v18.04, which is way past EOL. Maybe this build should just be removed?

18.04 only left support a month ago + a few days but that it still out of support and likely should still be removed.

I think it should stay until the team formally decides to stop supporting it.

midzelis · 2023-08-15T14:23:58Z

@gbooker @sztomi - is there anything more to do here?

gbooker

I have a few additional comments/questions.

Typically we prefer fixups to be squashed (we review PRs by commit). While you commits are not actually fixup commits there are instances of one commit correcting something in a previous commit (such as newlines). In these cases we prefer the correction to exist in the original commit. I did some obvious squashes in a branch on the original repo that you can use for a starting point: https://github.com/plexinc/pms-docker/tree/graham/updateSquash (which has the same tree in the final commit as your branch).
There are many less obvious such fixups that would be better squashed together. For example the creation of several scripts which are then removed and consolidated into one in a latter commit. This is likely better as just the one commit with the end result of the unified script.

gbooker · 2023-08-18T14:39:54Z

Dockerfile

-ARG AUTOUPDATE=false
-ARG TAG=beta
+# TAG can be "autoupdate" or an explicit version, like 1.32.4.7195-7c8f9d3b6
+ARG TAG="autoupdate"


I'm not certain about making the auto update the default behavior. It is antithetical to docker but then so is the current default of beta so 🤷 .

When I made this comment, I didn't realize that your work is effectively supplanting the public and beta tags. So, assuming that I have that correct, then this change isn't really a change.

gbooker · 2023-08-18T14:42:09Z

Dockerfile

+    /installBinary.sh \
+    && \
+# Clean up installer
+    rm /installBinary.sh


Was this intended to cleanup a deb file instead of removing the script to install debs?

gbooker · 2023-08-18T14:45:08Z

dev/dev.sh

+
+# This bakes the specific binary into the image, you can override these vars on command line
+# DOCKERHUB_IMAGE=myorg/mycontainer ./dev/sh bake
+VERSION_TO_BUILD=${VERSION_TO_BUILD:-"1.32.4.7195-7c8f9d3b6"}


Not sure it should default to a particular version as this script is likely not to be updated often as the server advances in version.

gbooker · 2023-08-18T14:51:04Z

root/plex-logs.sh

+. /plex-envvars
+
+logs="$PLEX_MEDIA_SERVER_APPLICATION_SUPPORT_DIR/Plex Media Server/Logs"
+multitail --mergeall "$logs"/*


You may want a regex here to exclude \.\d+\.log files otherwise the log rotation may interfere with this.

gbooker · 2023-08-18T14:51:54Z

dev/dev.sh

@@ -6,9 +6,12 @@ IFS=$'\n\t'
 # Assumes this is running on Linux (any distro) using (intel/amd).

 # This bakes the specific binary into the image, you can override these vars on command line
-# DOCKERHUB_IMAGE=myorg/mycontainer ./dev/sh bake
+# prompt> DOCKERHUB_IMAGE=myorg/mycontainer ./dev/sh bake


Sierra1011 · 2024-04-22T09:28:33Z

This is very anti-pattern for containerised applications.

In any normal situation the root filesystem isn't (and shouldn't be) writable.

The exception I can think of is when Docker is used purely as an abstraction layer e.g. where Plex couldn't be installed natively, and instead one could mount a volume to the entire container filesystem, rather than just config files and a small bit of state.

In my setup (k3s) the autoupdate behaviour described would result in a startup loop that looks like restart > update > restart > update because the only persisted state is under /config.

There are as many ways to solve this particular issue as there are installations of it; I might suggest watchtower (as previously mentioned) is the most efficient and most appropriate one for a container environment, but as I said it's all subjective.

cilindrox · 2024-07-04T14:37:33Z

Should we close this @gbooker ?

gbooker requested changes Jun 21, 2023

View reviewed changes

midzelis added 8 commits June 22, 2023 13:48

Add multi-arch build/debug helper scripts

156169f

Upgrade to latest Ubuntu LTS (and fix arch base image)

a09c55b

Upgrade to s6-overlay v3

63c3c8c

Run shellcheck, handle all warnings

b0ae80d

Add 'strict' mode to bash scripts

d504440

Move scripts and dynamically link up services.d/cont-init.d

c5f4f70

Autoupdate functionality

c5da1c4

Use single multi-arch Dockerfile

83062cd

midzelis force-pushed the master branch from 27d0064 to 83062cd Compare June 22, 2023 18:31

gbooker reviewed Jun 22, 2023

View reviewed changes

midzelis added 6 commits June 24, 2023 16:01

Allow version-specific builds. Improve build scripts. Readmes.

502203f

Log viewer /plex-logs.sh

42fdfc9

Backwards compat. FORCE_UPDATE var

709a6de

newlines

e2734a5

Do not install on every container start.

c2fad2c

fix cron schedule

2cae64a

midzelis requested a review from gbooker June 30, 2023 15:24

sztomi approved these changes Jul 6, 2023

View reviewed changes

gbooker reviewed Aug 18, 2023

View reviewed changes

bash strict mode prevent hw-transcode script from working properly

fcd1c43

Rip9776 approved these changes Jul 4, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

"Autoupdate" feature. Update S6. Shellcheck. #81

"Autoupdate" feature. Update S6. Shellcheck. #81

midzelis commented Jun 21, 2023

gbooker left a comment

midzelis commented Jun 22, 2023

gbooker left a comment

gbooker Jun 22, 2023

midzelis Jun 24, 2023

gbooker Jun 22, 2023

midzelis Jun 24, 2023

sztomi left a comment

sztomi Jul 6, 2023

midzelis Jul 6, 2023

midzelis Jul 6, 2023

gbooker Jul 6, 2023

sztomi Jul 6, 2023

midzelis commented Aug 15, 2023

gbooker left a comment

gbooker Aug 18, 2023

gbooker Aug 18, 2023

gbooker Aug 18, 2023

gbooker Aug 18, 2023

gbooker Aug 18, 2023

gbooker Aug 18, 2023

Sierra1011 commented Apr 22, 2024

cilindrox commented Jul 4, 2024

"Autoupdate" feature. Update S6. Shellcheck. #81

Are you sure you want to change the base?

"Autoupdate" feature. Update S6. Shellcheck. #81

Conversation

midzelis commented Jun 21, 2023

gbooker left a comment

Choose a reason for hiding this comment

midzelis commented Jun 22, 2023

gbooker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sztomi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

midzelis commented Aug 15, 2023

gbooker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Sierra1011 commented Apr 22, 2024

cilindrox commented Jul 4, 2024