add support for exposing of database port

[skodak]

Hi @stronk7 ,

I am using PHPUnit with docker, I needed to expose DB port so that PHPStorm can access the database to do autocompletion and syntax checking.

Ciao, Petr

[skodak]

I have included this in https://github.com/moodlehq/moodle-docker/pull/229.

[stronk7]

Hi super @skodak ,

I like the changes, and they are implemented in a similar way to the web port, so... great!

The only point that, maybe, we should consider, is to make it have a completely parallel behaviour that other ports have. The key here is that, by default, when using the port, it's bound to all the computer interfaces, hence accesible from outside.

To remediate that, in #66 we made both VNC and WEB ports to be bound to localhost (127.0.0.1) by default, still allowing to be configured to other interfaces. I think we can easily add that very same feature to the DB port, so all them work the same.

Would you mind updating the PR to support that extra stuff?

[stronk7]

BTW, also... why don't we have the very same for Oracle and SQL*Server? I think that in my Windows laptop, where I always use those databases via docker... I've added the port and it's working perfectly.... am I missing anything?

[skodak]

done, I was just lazy to add sqlsrv and oracle support because it does not run with Roseta emulation on M2

[skodak]

oh, I forgot to update the cmd file too, just a moment

[stronk7]

Thanks @skodak !

Doing some local tests along the day and surely will proceed with this.

Patch-wise the only detail that I find a little bit strange is to document the DBs l/p in the MOODLE_DOCKER_DB_PORT explanation. But honestly, I've not been able to suggest a better place, so all ok, I think.

Ciao :-)

[stronk7]

Ok, these are the results under Unix (Mac x64), should be the same for linux and other flavours.

1. No MOODLE_DOCKER_DB_PORT set
   • PostgreSQL: 5432/tcp
   • MySQL: 3306/tcp, 33060/tcp
   • MariaDB: 3306/tcp
   • SQL*Server: 1433/tcp
   • Oracle: --nothing!--
2. MOODLE_DOCKER_DB_PORT=0
   • PostgreSQL: 5432/tcp
   • Skipped the other databases.
3. MOODLE_DOCKER_DB_PORT=1234 (verified connectivity with all them)
   • PostgreSQL: 127.0.0.1:1234->5432/tcp
   • MySQL: 33060/tcp, 127.0.0.1:1234->3306/tcp
   • MariaDB: 127.0.0.1:1234->3306/tcp
   • SQL*Server: 127.0.0.1:1234->1433/tcp
   • Oracle: 127.0.0.1:1234->1521/tcp
4. MOODLE_DOCKER_DB_PORT=127.0.0.1:1234 (verified connectivity)
   • PostgreSQL: 127.0.0.1:1234->5432/tcp
   • Skipped the other databases.
5. MOODLE_DOCKER_DB_PORT=10.10.10.10:1234 (verified connectivity)
   • PostgreSQL: 10.10.10.10:1234->5432/tcp
   • Skipped the other databases.
6. MOODLE_DOCKER_DB_PORT=0.0.0.0:1234 (verified connectivity)
   • PostgreSQL: 0.0.0.0:1234->5432/tcp
   • Skipped the other databases.

So all ok, so far, nice! Going to try a few under Windows now...

[stronk7]

Uhm, I'm having some problems with Windows, I just set, for example:

set MOODLE_DOCKER_DB_PORT=8998

And, after running bin\moodle-docker-compose.cmd up -d I can see that MOODLE_DOCKER_DB_PORT has been correctly set to 127.0.0.1:8998, but I'm not getting the port exposed.

Same applies to any other MOODLE_DOCKER_DB_PORT value. Never exposed.

Will look later, maybe it's a local glitch, coz I used to have my Windows moodle-docker locally hacked to publish the ports always...

Ciao :-)

[stronk7]

Wow, debugging filename (before the if exist %filename% ( it comes with this value, that I cannot understand:

filename=C:\Users\stronk7\git_moodle\moodle-docker\db.pgsql.7.4.yml

Its' sort of the PHP version mixed there... I must be missing something obvious somewhere, lol. Looking...

[stronk7]

Ah, it seems to be caused because the variables are being set within a parenthesised code block... and then... it's using the global scope one that was declared some lines above when calculating the web port... crazy CMD!

Looking for alternatives...

[stronk7]

Ok, just using a different variable name seems to do he trick. I've read that there are other way to force the same variable to be re-evaluated (using DelayedExpansion and other techniques), but this way it seems to be working ok:

diff --git a/bin/moodle-docker-compose.cmd b/bin/moodle-docker-compose.cmd
index 07ec875..3966ac4 100644
--- a/bin/moodle-docker-compose.cmd
+++ b/bin/moodle-docker-compose.cmd
@@ -45,9 +45,9 @@ IF "%MOODLE_DOCKER_DB_PORT%"=="" (
         IF "%MOODLE_DOCKER_DB_PORT%"=="%MOODLE_DOCKER_DB_PORT::=%" (
             SET MOODLE_DOCKER_DB_PORT=127.0.0.1:%MOODLE_DOCKER_DB_PORT%
         )
-        SET filename=%BASEDIR%\db.%MOODLE_DOCKER_DB%.port.yml
-        if exist %filename% (
-            SET DOCKERCOMPOSE=%DOCKERCOMPOSE% -f "%filename%"
+        SET filedbport=%BASEDIR%\db.%MOODLE_DOCKER_DB%.port.yml
+        IF EXIST %filedbport% (
+            SET DOCKERCOMPOSE=%DOCKERCOMPOSE% -f "%filedbport%"
         )
     )
 )

Would you mind amend the .cmd, please?

[stronk7]

And these are the results with the patch above applied and Windows:

1. No MOODLE_DOCKER_DB_PORT set
   • PostgreSQL: 5432/tcp
   • Skipped the other databases.
2. MOODLE_DOCKER_DB_PORT=0
   • PostgreSQL: 5432/tcp
   • Skipped the other databases.
3. MOODLE_DOCKER_DB_PORT=8998 (verified connectivity)
   • PostgreSQL:127.0.0.1:8998->5432/tcp
   • Skipped the other databases.
4. MOODLE_DOCKER_DB_PORT=127.0.0.1:8998 (verified connectivity)
   • PostgreSQL: 127.0.0.1:8998->5432/tcp
   • Skipped the other databases.
5. MOODLE_DOCKER_DB_PORT=10.10.10.16:8998 (verified connectivity)
   • PostgreSQL: 10.10.10.16:8998->5432/tcp
   • Skipped the other databases.
6. MOODLE_DOCKER_DB_PORT=0.0.0.0:8998 (verified connectivity)
   • PostgreSQL: 0.0.0.0:8998->5432/tcp
   • Skipped the other databases.

So, I think that once the .cmd has been fixed, this is 100% ready to land. Thanks!

[skodak]

I have amended and force-pushed the change with filedbport

[stronk7]

And merged! Thanks!