Extract host and port from DB_HOST also support DB_PORT to test connection instead of only use default port 3306

[thohng]

---

• [x] I have read the contributing guideline and understand that I have made the correct modifications

---

Description:

Improve Bookstack app startup from 30 seconds delayed and trying if the connection to MySQL DB_HOST use the non default port 3306.

Benefits of this PR and context:

To fix issue #135 It will extract the actual host and port from DB_HOST and provide to the nc process. This will support DB_HOST using the endpoint format or host only (with default port 3306):

• DB_HOST=hostOrFQDN
• DB_HOST=hostOrFQDN:port

~Limitations: not support host IPv6~

How Has This Been Tested?

Has been tested by mount the new file to the file in docker container

environment:
  - APP_URL=https://domain
  - DB_HOST=10.109.0.2:25060
  - DB_USER=user
  - DB_PASS=password
  - DB_DATABASE=database_name
volumes:
  - /opt/bookstack/config:/config
  - /opt/bookstack/develop/50-config:/etc/cont-init.d/50-config

Logs after fixed

.....
Running config - db_user set
/var/run/s6/etc/cont-init.d/50-config: line 98: warning: command substitution: ignored null byte in input
/var/run/s6/etc/cont-init.d/50-config: line 98: warning: command substitution: ignored null byte in input
Nothing to migrate.
[cont-init.d] 50-config: exited 0.
....

Source / References:

N/A

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-c3f8e947-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-c3f8e947-pr-136/shellcheck-result.xml

[thohng]

Properly support specific non default port 3306 in DB_HOST and also improve experience for these issues #31, #86 and #88 without working around.

[drizuid]

I'm a bit unsure why we didn't have DB_PORT as an envvar like other similar containers, and while I like this change a lot, it's inconsistent with our other containers which would have the user specify db_port in compose. we'll discuss this a bit internally.

[thohng]

If we prefer to use DB_PORT, I can submit the PR to support DB_PORT and also solve the 30s delay. I just checked on BookStack, it support both DB_PORT or DB_HOST with port, and port in DB_HOST will take precedence.

https://github.com/BookStackApp/BookStack/blob/development/app/Config/database.php#L45

// MYSQL
// Split out port from host if set
$mysql_host = env('DB_HOST', 'localhost');
$mysql_host_exploded = explode(':', $mysql_host);
$mysql_port = env('DB_PORT', 3306);
if (count($mysql_host_exploded) > 1) {
    $mysql_host = $mysql_host_exploded[0];
    $mysql_port = intval($mysql_host_exploded[1]);
}

[drizuid]

that would be fine i think. If you take a look at netbox (we have others but netbox is the only one popping into my mind right now) you can see how we are handling DB_PORT, obviously consistency is our goal. I'll make sure this repo is marked for hacktoberfest too, so you can get some credit :)

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/shellcheck-result.xml

[thohng]

Update PR to support DB_PORT

• Insert default line DB_PORT=3306 to .env if missing. The current .env.example does not have line DB_PORT=3306
• Use port from DB_HOST if any
• If no port on DB_HOST, will use DB_PORT if provided, or DB_PORT will be default 3306

The issue #135 is fixed both case port on DB_HOST or DB_PORT provided

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/shellcheck-result.xml

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/shellcheck-result.xml

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/shellcheck-result.xml

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/shellcheck-result.xml

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/shellcheck-result.xml

[drizuid]

I've asked the team to review the changes, if this was JUST adding DB_PORT i would've merged it but since it also includes the logic for <address>:<port> I'll need team consensus.

[thohng]

Please review NetLah-external#1

Hi Eric Nemchik, I would fix the Alpine 3.15 not support grep -qP and improve the support to domain:port or [::1]:port or [1:2::7:8]:port ~Please help review #141~

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/shellcheck-result.xml

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.09.1-pkg-dc94345f-pr-136/shellcheck-result.xml

[jaytomten]

Awesome. Waiting on this update.

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.10.2-pkg-9a84e426-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.10.2-pkg-9a84e426-pr-136/shellcheck-result.xml

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[thohng]

Hi, can we help preview and merge this PR? This PR is not just improvement to support the DB_PORT but also fix the issue that connection can update from docker environment variable to file /config/www/.env. Thanks!

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.11-pkg-9a84e426-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.11-pkg-9a84e426-pr-136/shellcheck-result.xml

[nemchik]

@thohng Please check my review comment above requesting small changes. I believe this is nearly ready. Once those changes are made we will re-review and merge this.

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.11.1-pkg-9a84e426-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.11.1-pkg-9a84e426-pr-136/shellcheck-result.xml

[thohng]

@thohng Please check my review comment above requesting small changes. I believe this is nearly ready. Once those changes are made we will re-review and merge this.

Hi Eric Nemchik, I actually have merged your improvement https://github.com/NetLah-external/docker-bookstack/pull/1 2 months ago, and the PR is ready, but that time I have not triggered the re-review. Please help preview again.

The recent merge PR #151 actually already in my PR 2 months ago, to fix the issue that .env not update again when db info changed (like changing on host, port, username, or password after first time initialized)

I also resolved the conflicts by the PR 151 recently.

[nemchik]

Wow, I had a pending review that I did not submit. That's entirely my fault. Sorry!

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.11.1-pkg-9a84e426-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.11.1-pkg-9a84e426-pr-136/shellcheck-result.xml

[LinuxServer-CI]

I am a bot, here are the test results for this PR: https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.11.1-pkg-9a84e426-pr-136/index.html https://ci-tests.linuxserver.io/lspipepr/bookstack/v22.11.1-pkg-9a84e426-pr-136/shellcheck-result.xml

[thohng]

New version working well on my side: https://hub.docker.com/layers/linuxserver/bookstack/22.11.1/images/sha256-356ae10976e45c62c57fb15e1ad257b002c3cd133f8e34fb3703ee3484530e01?context=explore