analytics icon indicating copy to clipboard operation
analytics copied to clipboard

Have entrypoint script run required steps directly

Open RealOrangeOne opened this issue 2 years ago • 5 comments

Changes

This removes the need for a custom command in docker-compose.yml. https://github.com/plausible/hosting/blob/f7682057104671b42e3fff3a106f93a32ea14775/docker-compose.yml#L30

It intentionally removes init-admin from the setup, as this should really only be done once.

This should make the installation much simpler, as it works more expected, and means people can unset the environment variables required for the admin setup script. It should also make it start up marginally faster.

Interested to hear what people think.

Tests

  • [ ] Automated tests have been added
  • [x] This PR does not require tests

Changelog

  • [ ] Entry has been added to changelog
  • [ ] This PR does not make a user-facing change

Documentation

  • [ ] Docs have been updated (I'll do this once the approach is approved, but before merge)
  • [ ] This change does not need a documentation update

Dark mode

  • [ ] The UI has been tested both in dark and light mode
  • [x] This PR does not change the UI

RealOrangeOne avatar May 06 '22 17:05 RealOrangeOne

BundleMon

Unchanged files (7)
Status Path Size Limits
:white_check_mark: static/css/app.css
515.1KB -
:white_check_mark: static/js/dashboard.js
287.08KB -
:white_check_mark: static/js/app.js
12.13KB -
:white_check_mark: static/js/embed.host.js
5.58KB -
:white_check_mark: static/js/embed.content.js
5.06KB -
:white_check_mark: tracker/js/plausible.js
748B -
:white_check_mark: static/js/applyTheme.js
314B -

No change in files bundle size

Final result: :white_check_mark:

View report in BundleMon website ➡️


Current branch size history | Target branch size history

bundlemon[bot] avatar May 06 '22 17:05 bundlemon[bot]

I'm generally in favour of this.

How would people run init-admin with this setup? Eventually I would like to build that behaviour into the UI so people can just create the admin account through the UI. But for now we would need to be able to run init-admin manually during setup.

ukutaht avatar Jun 02 '22 08:06 ukutaht

How would people run init-admin with this setup

Once they start the plausible container (and its dependencies), it'd be something like docker-compose exec plausible ADMIN_USER_EMAIL=email ADMIN_USER_NAME=name ADMIN_USER_PASS=password /app/init-admin.sh. I'd like to change the arguments to be passed through command line, but I don't know enough Elixir to do that.

I'll get a docs PR together which updates the install docs, which should help the picture a little too

RealOrangeOne avatar Jun 04 '22 19:06 RealOrangeOne

It should also make it start up marginally faster.

Why? Because init-admin isn't executed every time on startup? It's one user lookup by email address per startup. The performance gain is negligible IMO. I'd be curious about how long that query takes on the main plausible.io instance 🤔

create-db is also something that should be run only once. Also, running migrations implicitly is not something everybody would want. We should at least add a DISABLE_MIGRATIONS env var switch.

Having said all that, I don't really see the point of these changes just to simplify the docker-compose code a bit.


As example, I'm currently implementing a Helm chart for the TrueCharts community repo. The resulting Kubernetes deployment does this:

  1. Run an init container awaiting both Plausible and ClickHouse to accept connections (instead of naively running sleep 10)
  2. Run one of the following init containers:
    • when installing the Helm chart, run: createdb, migrate, init-admin
    • when upgrading the Helm chart, run: migrate
  3. Run the main container with default run

It's a matter of preference, really, but I like the explicit approach more.

schnerring avatar Jul 11 '22 23:07 schnerring

When running in a k8s environment, yes using init containers is a better solution. But outside of that (eg docker-compose), that doesn't exist.

I agree though it's definitely best to allow users to opt for the functionality they want. So in the k8s case, being able to disable migrations etc on container start is probably ideal (even if most of the time they'd noop)

RealOrangeOne avatar Jul 21 '22 08:07 RealOrangeOne

Having thought about it, this change will not work for us since we run this docker image in a non-self-hosted environment. We want to be in manual control over migrations etc. and don't want the image itself to run these scripts. I think the plausible/hosting repo is an appropriate place for that.

The long term view is to build this admin stuff into the UI so people don't need to run the init-admin script and stuff like that themselves.

ukutaht avatar Sep 02 '22 13:09 ukutaht