pms-docker icon indicating copy to clipboard operation
pms-docker copied to clipboard

feat: Move /.firstRunComplete to /tmp to prepare readonly rootfs

Open mkilchhofer opened this issue 2 years ago • 12 comments

I currently migrate all my Kubernetes workloads to containers with read-only rootfs.

The plex container is a bit tricky with the /.firstRunComplete file. If you'd move that file to /tmp, it would be possible to mount a temporary filesystem which is writable (Kubernetes emptyDir) below /tmp.

The s6 suite also have issues with read-only rootfs but it (officially) supports it by setting S6_READ_ONLY_ROOT=1. Ref: https://github.com/just-containers/s6-overlay#read-only-root-filesystem

mkilchhofer avatar Feb 25 '23 18:02 mkilchhofer

The /.firstRunComplete is used to indicate whether this is the first time the container is run or whether it is a subsequent run. Moving this to /tmp will likely completely break this (because /tmp is by definition transitory and content there can be deleted without warning).

gbooker avatar Feb 27 '23 14:02 gbooker

I would be fine to move it completely elsewhere but having it inside the / (the root) is a really bad practice for the reason I mentioned above. Are you fine if we move it elsewhere?

Additional I'd like to ask why does something break if the "first run" is executed twice (in case /tmp is cleared to an arbitrary mechanism)? What I understand from reading all init scripts is that if we skip it, we gain speed due to skip of a recursive file chown.

mkilchhofer avatar Feb 27 '23 20:02 mkilchhofer

@gbooker any update?

mkilchhofer avatar Mar 22 '23 17:03 mkilchhofer

As stated, moving this indicator to /tmp will break functionality. This PR hasn't changed from that. Any change to this behavior would have to also not break the existing functionality.

Perhaps there's an id or something which doesn't change across updates which could be used to indicate that it is in the same effective container and this be stored in a more permanent location in /config?

gbooker avatar Mar 22 '23 17:03 gbooker

What ID do you mean? If you recreate the container from a container image (eg. on an update) its not the same container.

In which cases should the firstRun be triggered?

mkilchhofer avatar Mar 23 '23 20:03 mkilchhofer

I was afraid the container id changes across updates.

Ideally, the firstRun should be triggered when a fresh container is created but not when a container is upgraded. Additionally not when a configuration is changed (such as a volume map, etc) It's a tricky thing to accomplish since docker is really missing a mechanism to do this.

The rationale is the first-run can perform some expensive operations but these should only be done once (such as chown a directory tree).

gbooker avatar Mar 23 '23 21:03 gbooker

What container upgrade are you talking about here? The root filesystem does not persist when a container is recreated (or upgraded as you state).

saltydk avatar Mar 24 '23 04:03 saltydk

As I am still affected of this issue, can we talk about it again?

@gbooker is your idea that this script is only executed when plex is initialized for the first time or every time Plex is upgraded (and thus the container is replaced from a new container image)?

/cc: @MarshallAsch as you implemented a helm chart for plex (PR #82) which could also benefit from this change.

mkilchhofer avatar Apr 27 '24 10:04 mkilchhofer

is your idea that this script is only executed when plex is initialized for the first time or every time Plex is upgraded (and thus the container is replaced from a new container image)?

I previously stated:

Ideally, the firstRun should be triggered when a fresh container is created but not when a container is upgraded. Additionally not when a configuration is changed (such as a volume map, etc) It's a tricky thing to accomplish since docker is really missing a mechanism to do this.

The first run does some things (such as a recursive chown or signing in the PMS with the result stored in prefs) which should only be run once. However, if a server is migrated from one system to another (or potentially migrate to a new container), it likely should be run again. I don't like the idea of modifying the container root image that much (as it is antithetical to how docker is supposed to work) but I couldn't find any good mechanism to accomplish this.

gbooker avatar Apr 29 '24 13:04 gbooker

I don't like the idea of modifying the container root image that much (as it is antithetical to how docker is supposed to work) but I couldn't find any good mechanism to accomplish this.

Its not a big deal. The real issue is, that this file should not live in a directory which contains other files. We can move it to wherever you want. Would you be okay with another location?

mkilchhofer avatar Apr 30 '24 15:04 mkilchhofer

Might be missing some context here, but what about a newer directory that we declare with the VOLUME directive in the dockerfile? This allows users to override it with persistent volumes and signals there's some sort of statefulness expected to operators

cilindrox avatar Apr 30 '24 17:04 cilindrox

Might be missing some context here, but what about a newer directory that we declare with the VOLUME directive in the dockerfile? This allows users to override it with persistent volumes and signals there's some sort of statefulness expected to operators

That would be great!

mkilchhofer avatar Apr 30 '24 18:04 mkilchhofer