Skip to content

Conversation

AEnterprise
Copy link
Contributor

Right now you can't deploy koel in docker without manual intervation, this pr resolves that by automatically running koel:init (as non interactive), as well as koel:doctor to report the status. If no config is present it will just "start" but not work as before until the user provides/mounts a valid .env file

However the big difference is now that once this config file exists, koel can self-update and no longer needs manual intervention. the init and doctor output during startup also makes it easier to diagnose issues for users unfamiliar with the koel inner workings. This behavior is on by default but can be disabled with the SKIP_INIT flag

side note: smtp not being configured is currently an "error", this should probably be a warning since it work just fine without

This script can also optimize the config loading, since it has some pitfals for unaware users (requiring a config reboot to apply) i've left this off by default and needs the OPTIMIZE_CONFIG to apply. While needing a reboot to apply config changes is fairly standard in docker containers, i didn't want to break current behavior. Maybe could be enabled by default in a later major release?

@AEnterprise AEnterprise force-pushed the koel-init branch 6 times, most recently from 2b3354e to 24076e4 Compare August 31, 2025 11:49
@@ -1,10 +1,13 @@
#!/bin/bash
#!/bin/sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just more compatible, it's not using anything bash related

@phanan phanan closed this Aug 31, 2025
@phanan
Copy link
Member

phanan commented Aug 31, 2025

koel:init needs to be run manually at least for the installation, because there's no DB and other configs available yet. After that, it can be automated indeed, but not with the current state of the PR, sorry.
I think another, simpler, approach would be to run php artisan koel:init --no-interaction if we detect the existence of .env (which indicates an upgrade and not a first time installation).

@AEnterprise
Copy link
Contributor Author

with this setup, if there is no env file it will just create one with an key, fail the installer but still proceed to boot as the old setup did. nothing materially changed here. it will always fail and never really break anything since there will not be a database yet and it won't proceed past that step

image

@phanan
Copy link
Member

phanan commented Aug 31, 2025

with this setup, if there is no env file it will just create one with an key, fail the installer but still proceed to boot as the old setup did. nothing materially changed here. it will always fail and never really break anything since there will not be a database yet and it won't proceed past that step

Yes, but consider two approaches:

  1. To instruct the user to run the script to set things up manually
  2. To just silently run the script and fail on its face :D

I think (2) will look bad, hence the suggestion.

@AEnterprise
Copy link
Contributor Author

well it's imo still a massive improvement over the old container just silently starting as if everything is fine and throwing error 500 on basically everything

but i can give 1 a go

@AEnterprise
Copy link
Contributor Author

just thought of another case though while working on these: all these options can technically get this all working without any .env file at all.

might it be worth to otherwise

  1. require the .env file to be present
  2. have a ENV_ONLY mode where all params need to be in the env. this would force the config optimalization option (since it can't change at runtime anymore) and not let the container start if koel:init fails (and give a link to the env example with a message to add the needed env vars). as this indicate missing properties they need to set.

@phanan
Copy link
Member

phanan commented Aug 31, 2025

I'd actually prefer the .env file to be left for the app itself and have nothing to do with the Docker installation.

@AEnterprise
Copy link
Contributor Author

added the warning and early script exit for when the file is missing

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants