Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Docker container overhaul #4300

Open
feederbox826 opened this issue Nov 21, 2023 · 24 comments
Open

[Feature] Docker container overhaul #4300

feederbox826 opened this issue Nov 21, 2023 · 24 comments
Assignees

Comments

@feederbox826
Copy link
Contributor

feederbox826 commented Nov 21, 2023

Is your feature request related to a problem? Please describe.

Describe the solution you'd like

Describe alternatives you've considered
N/A

Additional context
I want to work on this. Please assign me to this. I've done this for other projects and I am eager to finally do it for stash

@feederbox826
Copy link
Contributor Author

feederbox826 commented Nov 21, 2023

High level overview and TODO tracker

developing in https://github.com/feederbox826/stash-s6

container

Building

Documentation

  • updates
    • update dockerhub README
    • add links to hwaccel flavors, make images links
    • update docker-compose ENV, mounts, GPU bindings
  • additions
    • docker image
      • add documentation for entrypoint variables
    • hw acceleration
      • breakdown of flavors/ hardware acceleration
      • link test videos for reencoding
      • example arguments
      • device mounts

@Leopere
Copy link
Collaborator

Leopere commented Nov 22, 2023

The theory of why the changes need made is on par with what I’d want implemented. Privilege reduction proper installation of software requirements my biggest issue is that it needs to help users from before keep working ideally

@feederbox826
Copy link
Contributor Author

feederbox826 commented Nov 22, 2023

it needs to help users from before keep working ideally

Yep, my plan right now is to test it in my own repository to iron out any bugs, and write mgirations for both hotio/stash and stashapp/stash so that users can migrate over seamlessly or as seamless as mv && chown -R can get

@stg-annon
Copy link
Collaborator

another problem people have with docker installs is python packages, might be worth taking a look at setting up a venv so that plugins and scrapers that require other packages don't need to reinstall them on every reboot

alternately we may be able to get away with just setting a couple environment variables to a mount from the host
PIP_TARGET
PYTHONHOME

@MaverickD650
Copy link

Could we keep a rootless container option without s6? It causes particular problems on kubernetes as it aggressively CHOWNs files and can cause a container to fail to start by exceeding the probes time limits. It must also be run as root on kubernetes to allow the inital s6 operations to take place which isn't ideal since we're aiming for rootless.

@feederbox826
Copy link
Contributor Author

alternately we may be able to get away with just setting a couple environment variables to a mount from the host
PIP_TARGET
PYTHONHOME

https://github.com/feederbox826/stash-s6/blob/b71eb246ae412192e1d9426cc01b4829c546738f/alpine.Dockerfile#L16-L19

cache and PIP_INSTALL_TARGET are both set to a volume

@feederbox826
Copy link
Contributor Author

Could we keep a rootless container option without s6? It causes particular problems on kubernetes as it aggressively CHOWNs files and can cause a container to fail to start by exceeding the probes time limits. It must also be run as root on kubernetes to allow the inital s6 operations to take place which isn't ideal since we're aiming for rootless.

the chown operations are done by the init scripts, not by s6 itself. Unfortunately my goal with this is to be as beginner-friendly as possible, following lscr.io conventions and s6 allows us some leniency without creating a excessively long docker-entrypoint script and adding in modular dropins for different flavors. I'll look into creating a minimal (alpine + python + binary) image, but I'm not a fan of fragmeting further from the base

@PrivatePuffin
Copy link

A proper LSCR.io

following lscr.io conventions

From a professional perspective, LSIO containers are not good containers.
They are borderline problematic on anything but a docker(-compose installation), for example on kubernetes. Due to, essentually, requiring root. They essentially cut corners from good container practices, to build a container that "just works" for the average joi.

Calling LSIO a "convention", makes it seem like it's somehow a high grade container standard, which it's quite obviously not.
It's better than the average container, but that comes at a cost.

also is a minor security concern #684

To be clear: The main container still runs as root in S6 and requires multiple(!) layers of security precautions in kubernets to be disabled. It also prevents things like supplemental groups to be used correctly.

while USER is an option, it breaks since stash looks under $HOME/.stash for config files

The solution for which is adapting this behavior, not migrating to S6 overlay based containers

@feederbox826
Copy link
Contributor Author

feederbox826 commented Nov 22, 2023

Either way I'm discussing with @Leopere to drop s6 in favor of a (b)ash script but,

From a professional perspective, LSIO containers are not good containers.

They're not good but they work out of the box for 99% of users which imo is more important than accomodating someone who is already building their own helmchart and should know how to build their own container

k8s, as much as we would like to try and dance around, is not exactly docker, but that's a tangent for another time.

To be clear: The main container still runs as root in S6 and requires multiple(!) layers of security precautions in kubernets to be disabled. It also prevents things like supplemental groups to be used correctly.

iirc #684 is not to run a root-less container, is to run as a non-priviledged user within a rooted container.

@feederbox826
Copy link
Contributor Author

I'll try to get the bash scripts out the door, but I have a some personal negative experiences with k8s but PRs are always welcome.

@PrivatePuffin
Copy link

imo is more important than accomodating someone who is already building their own helmchart and should know how to build their own container

Writhing a helm chart is a lot different than writhing containers as they are a LOT easier to standardise and maintain. Easier in terms of dev-time per application. Just because I can technically do something, doesn't mean the project I maintain has the manpower to do so on a scope of 900 help charts.

k8s, as much as we would like to try and dance around, is not exactly docker, but that's a tangent for another time.

Well, in terms of security: Yes it offers a lot more options to lock-down. But in general it has quite clear best-practices that aline with docker itself as well in certain areas:

But it's not rocker science:

  • Accept USER being set
  • Dont change user
  • Don't chown (without copy)
  • Dont chmod
  • Don't write to directories already containing files (only write to folders that can be mounted)

@scruffynerf
Copy link

As someone who has written for and used both k8s and docker, in the context of Stash, k8s makes no sense for most people. Docker is a much more easily supported standard, and more so, the level of support it generates is already high. K8s would not improve matters.
As much as I feel like some of the docker users could be better served by a native app, at least there are reasonable arguments in favor of Dockerizing.

@feederbox826
Copy link
Contributor Author

feederbox826 commented Nov 23, 2023

Dropped s6 in favor of an ash entrypoint script

Limitations of root-less

  • volume mounts are by default not accessible
  • nvenc stream needs root to run ldconfig
  • cannot migrate from root to non-root

but that's inevitable. If you have further suggestions, you seem much more versed in rootless and k8s compatibility so I look forward to your PRs

@feederbox826 feederbox826 changed the title [Feature] Docker container overhaul with LSCR.io-like features [Feature] Docker container overhaul Nov 23, 2023
@jangrewe
Copy link

Please also add the stashapp-tools python module to fix some of the plugins. :-)

@feederbox826
Copy link
Contributor Author

Please also add the stashapp-tools python module to fix some of the plugins. :-)

already bundled :) https://github.com/feederbox826/stash-s6/blob/main/stash/root/defaults/requirements.txt

@feederbox826
Copy link
Contributor Author

Just a quick status update while I'm here - Right now it's down to figuring out mutli-arch workflow and makefile support for local and CI integration. Progress has been slow since I'm not used to this many layers of build dependencies and integrating into the existing CI for multiarch releases

@d3f113
Copy link

d3f113 commented Dec 29, 2023

Would be great to add an environment variable to install packages like it is done in:
https://github.com/jlesage/docker-jdownloader-2
Environment variable is called: INSTALL_PACKAGES

@feederbox826
Copy link
Contributor Author

Would be great to add an environment variable to install packages like it is done in: https://github.com/jlesage/docker-jdownloader-2 Environment variable is called: INSTALL_PACKAGES

not sure if this is a general comment or a critique of the current method - the current method is to search the plugin and scraper dirs reursively for matching requirements.txt as well as it being a mappable override https://github.com/feederbox826/stash-s6/blob/main/stash/root/opt/entrypoint.sh#L231-L290

@toby666123
Copy link

Any news on this, do you have a rough assumption maybe for when the new docker images will become available?

Thanks a lot for your efforts!!

@feederbox826
Copy link
Contributor Author

Sorry, I've been taking an extended break from stash et al. after getting burnt out. No promises but I'm aiming to get back on track by July 😓

@Maista6969
Copy link
Contributor

Take care of your self @feederbox826, your expertise is appreciated but nothing worth burning out over! I'm sure someone else can pick up the great work you've already put in here

@feederbox826
Copy link
Contributor Author

I'm close to being able to wrap up the project independently (build-from-source excluded)

I would love to have some testers that have

  • Intel ARC
  • Pi 4+ with V4L2

and some info if AMF (AMD) is officially supported

@toby666123
Copy link

I'm close to being able to wrap up the project independently (build-from-source excluded)

I would love to have some testers that have

  • Intel ARC
  • Pi 4+ with V4L2

and some info if AMF (AMD) is officially supported

Seems nobody here has the respective Hardware available.
Could you also just finish the project without support for the 2 platforms?

@feederbox826
Copy link
Contributor Author

I got an ARC myself and someone else verified V4L2, I haven't figured out the build process yet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants