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

Fix for issue #420, entrypoint has a race condition where installation fails if DB is not ready in time. #1435

Closed
wants to merge 3 commits into from

Conversation

dghodgson
Copy link

@dghodgson dghodgson commented Mar 5, 2021

The NextCloud installer seems to set a default DB user and password in config.php if the first installation attempt fails (see issue nextcloud/server#41074 ) . This PR gets around that issue by waiting for the DB service to become available before proceeding with installation.

This adds wait-for-it to /usr/bin in the dockerfile templates. The script is called in entrypoint.sh right before installation is attempted.

A helper function get_host_string() is also added to set default ports for MySQL/Postgres if no port is provided in the DB host string.

I attempted to implement the wait_for() function from wait-for-it into the entrypoint script directly, but ran into various issues (including the use of /bin/sh by entrypoint.sh instead of /bin/bash, which complicated things a bit). Calling the script just works without any issues, but that does mean adding another file to the docker image.

Not sure if running the update script to update the files in 19/20/21 is required as part of the pull request, so I opted not to.

…n fails if DB is not ready in time.

Signed-off-by: Daniel Hodgson <toasty27@gmail.com>
Signed-off-by: Daniel Hodgson <toasty27@gmail.com>
Signed-off-by: Daniel Hodgson <toasty27@gmail.com>
@dghodgson
Copy link
Author

Seeing the alpine images failing due to a lack of bash.

Bash provides /dev/tcp which can be used to test network connections without an added dependency like netcat. Will have to look into another solution that works on alpine.

@J0WI J0WI added wontfix and removed wontfix labels Jan 9, 2024
@J0WI
Copy link
Contributor

J0WI commented Jan 9, 2024

Closing this because ADD is discouraged, especially from url, and the script doesn't work anyway on Alpine. IMHO it's not worth to add this kind of dependency management to this image. There are many tools for container deployment that can already take care of it.

@J0WI J0WI closed this Jan 9, 2024
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