Bug 23141 - The Debian scripts do not use the MySQL port number
Summary: The Debian scripts do not use the MySQL port number
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: unspecified
Hardware: All Linux
: P5 - low normal (vote)
Assignee: Andreas Jonsson
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-06-17 18:12 UTC by Andreas Jonsson
Modified: 2020-11-16 15:40 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 23141: Add port number to koha-dump script (939 bytes, patch)
2019-06-17 18:59 UTC, Andreas Jonsson
Details | Diff | Splinter Review
Bug 23141: Add support for configuring database port in koha-dump, koha-mysql and koha-mysqlcheck (2.69 KB, patch)
2019-06-17 19:00 UTC, Andreas Jonsson
Details | Diff | Splinter Review
Bug 23141: Add mysql portnumber support when populating db (2.17 KB, patch)
2019-06-17 19:00 UTC, Andreas Jonsson
Details | Diff | Splinter Review
Bug 23141: Add --dbport parameter to koha-create (3.22 KB, patch)
2019-07-04 17:27 UTC, Andreas Jonsson
Details | Diff | Splinter Review
Bug 23141: Parse mysql portnumber from /etc/mysql/koha-common.cnf (1.67 KB, patch)
2019-07-04 17:28 UTC, Andreas Jonsson
Details | Diff | Splinter Review
Bug 23141: Add option to select db credentials source when populating db. (4.37 KB, patch)
2019-07-04 20:17 UTC, Andreas Jonsson
Details | Diff | Splinter Review
Bug 23141: Add option to select db credentials source when populating db. (4.42 KB, patch)
2019-07-08 15:00 UTC, Andreas Jonsson
Details | Diff | Splinter Review
Bug 23141: Add port number to koha-dump script (900 bytes, patch)
2020-06-16 10:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23141: Add support for configuring database port in koha-dump, koha-mysql and koha-mysqlcheck (2.69 KB, patch)
2020-06-16 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23141: Add --dbport parameter to koha-create (3.28 KB, patch)
2020-06-16 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23141: Parse mysql portnumber from /etc/mysql/koha-common.cnf (1.67 KB, patch)
2020-06-16 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23141: Add option to select db credentials source when populating db. (4.46 KB, patch)
2020-06-16 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23141: Add port number to koha-dump script (900 bytes, patch)
2020-11-13 14:23 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 23141: Add support for configuring database port in koha-dump, koha-mysql and koha-mysqlcheck (2.69 KB, patch)
2020-11-13 14:23 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 23141: Add --dbport parameter to koha-create (3.45 KB, patch)
2020-11-13 14:23 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 23141: Parse mysql portnumber from /etc/mysql/koha-common.cnf (1.67 KB, patch)
2020-11-13 14:23 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 23141: Add option to select db credentials source when populating db. (4.65 KB, patch)
2020-11-13 14:23 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Jonsson 2019-06-17 18:12:23 UTC
The scripts koha-mysql, koha-mysqldump, and koha-create and possibly others do not use the configured mysql portnumber configured in koha-conf.xml.
Comment 1 Andreas Jonsson 2019-06-17 18:59:37 UTC
Created attachment 90687 [details] [review]
Bug 23141: Add port number to koha-dump script
Comment 2 Andreas Jonsson 2019-06-17 19:00:04 UTC
Testplan for devbox:

* Change port of mariadb by adding the file /etc/mysql/conf.d/port.cnf with contents:

  [mysqld]
  port = 12345

* systemctl restart mariadb
* edit /etc/koha/sites/kohadev/koha-conf.xml and change the elments hostname and port:

 <hostname>127.0.0.1</hostname>
 <port>12345</port>

* Test the scripts:

  sudo ~/kohaclone/debian/scripts/koha-mysql kohadev
  sudo ~/kohaclone/debian/scripts/koha-mysqlcheck kohadev
  sudo ~/kohaclone/debian/scripts/koha-dump kohadev

* Test populate db:

sudo /usr/sbin/koha-create --request-db test
sudo mysqldump --defaults-file=/etc/mysql/koha-common.cnf koha_kohadev >/tmp/kohadev.sql

mysqluser="$( sudo xmlstarlet sel -t -v 'yazgfs/config/user'     /etc/koha/sites/test/koha-conf.xml )"
mysqlpass="$( sudo xmlstarlet sel -t -v 'yazgfs/config/pass'     /etc/koha/sites/test/koha-conf.xml )"

sudo mysql --defaults-file=/etc/mysql/koha-common.cnf -e "CREATE DATABASE koha_test; GRANT ALL ON koha_test.* TO '$mysqluser'@'localhost' IDENTIFIED BY '$mysqlpass'"

sudo ~/kohaclone/debian/scripts/koha-create --populate-db --defaultsql /tmp/kohadev.sql test
Comment 3 Andreas Jonsson 2019-06-17 19:00:20 UTC
Created attachment 90688 [details] [review]
Bug 23141: Add support for configuring database port in koha-dump, koha-mysql and koha-mysqlcheck
Comment 4 Andreas Jonsson 2019-06-17 19:00:51 UTC
Created attachment 90689 [details] [review]
Bug 23141: Add mysql portnumber support when populating db
Comment 5 Mark Tompsett 2019-06-18 00:13:44 UTC
Comment on attachment 90689 [details] [review]
Bug 23141: Add mysql portnumber support when populating db

Review of attachment 90689 [details] [review]:
-----------------------------------------------------------------

::: debian/scripts/koha-create
@@ +118,4 @@
>  }
>  
>  getmysqlhost() {
> +    if [ ! -f /etc/mysql/koha-common.cnf ]

No. Because koha-common.cnf was (at least last time I checked) a soft link to debian.cnf -- which triggers noise when accessing a broken link.
Comment 6 Mark Tompsett 2019-06-18 00:26:02 UTC
Comment on attachment 90689 [details] [review]
Bug 23141: Add mysql portnumber support when populating db

Review of attachment 90689 [details] [review]:
-----------------------------------------------------------------

::: debian/scripts/koha-create
@@ +694,4 @@
>      # something.
>      mysqluser=$(getinstancemysqluser $name)
>      mysqldb=$(getinstancemysqldatabase $name)
> +    mysqlhost=$(getinstancemysqlhost $name)

No. Because the logic for determining the mysqlhost is at the top of the script (542-576 approx). Command line over rides password file over rides the "host" listed in the mysql [client] section. Now, perhaps in the case of populate, we should insert an override based on the koha-conf.xml file, but it doesn't belong here.
Comment 7 Mark Tompsett 2019-06-18 00:27:07 UTC
Marking Failed QA, because of comment #6 (without a doubt) and possibly comment #5.
Comment 8 Andreas Jonsson 2019-06-18 06:16:04 UTC
(In reply to M. Tompsett from comment #5)
> Comment on attachment 90689 [details] [review] [review]
> Bug 23141: Add mysql portnumber support when populating db
> 
> Review of attachment 90689 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: debian/scripts/koha-create
> @@ +118,4 @@
> >  }
> >  
> >  getmysqlhost() {
> > +    if [ ! -f /etc/mysql/koha-common.cnf ]
> 
> No. Because koha-common.cnf was (at least last time I checked) a soft link
> to debian.cnf -- which triggers noise when accessing a broken link.


Sorry! This change was not supposed to be included.

But I have to point out that it does not make sense to check for the existance of /etc/mysql/debian.cnf and then proceed to use /etc/mysql/koha-common.cnf
Comment 9 Andreas Jonsson 2019-07-04 17:27:52 UTC
Created attachment 91291 [details] [review]
Bug 23141: Add --dbport parameter to koha-create
Comment 10 Andreas Jonsson 2019-07-04 17:28:45 UTC
Created attachment 91292 [details] [review]
Bug 23141: Parse mysql portnumber from /etc/mysql/koha-common.cnf
Comment 11 Andreas Jonsson 2019-07-04 17:32:34 UTC
The database connection details have three different purposes: 1. to create the database and database user 2. to set the connection details in koha-conf.xml and 3. to load data.

There are five different sources for acquiring the connection details: 1. command line, 2. password file, 3. /etc/mysql/koha-common.cnf (if /etc/myslq/debian.cnf exists!?) 4. koha-conf.xml and 5. default values.

So, to add support for port number we could:

1. Add command line parameter for portnumber
2. Add an optional field to the password file
3. Parse port number from /etc/mysql/koha-common.cnf
4. Add command line flag to select which source should have priority when loading data.

But then there is also the question on how to prioritize which source should be used in what situation.  For my particular use case I need to use the connection details of koha-conf.xml when populating the database.

But when using the credentials from the instance configuration it would be better to call koha-mysql.  I suggest adding the parameter --defaultsql-creds for this purpose.
Comment 12 Andreas Jonsson 2019-07-04 17:34:13 UTC
Updated testplan for devbox:

* Change port of mariadb by adding the file /etc/mysql/conf.d/port.cnf with contents:

  [mysqld]
  port = 12345

* systemctl restart mariadb
* edit /etc/koha/sites/kohadev/koha-conf.xml and change the elments hostname and port:

 <hostname>127.0.0.1</hostname>
 <port>12345</port>

* Test the scripts:

  sudo ~/kohaclone/debian/scripts/koha-mysql kohadev
  sudo ~/kohaclone/debian/scripts/koha-mysqlcheck kohadev
  sudo ~/kohaclone/debian/scripts/koha-dump kohadev

* Make sure the template is installed in /etc/koha/koha-conf-site.xml.in (sudo cp $HOME/kohaclone/debian/templates/koha-conf-site.xml.in /etc/koha)

* Test koha-create using command line parameter:

sudo $HOME/kohaclone/debian/scripts/koha-create --dbport 12345 --create-db test

* Verify portnumber: sudo xmlstarlet sel -t -v 'yazgfs/config/port' "/etc/koha/sites/test/koha-conf.xml"

* Test koha-create using configuration file:
  * Edit /etc/mysql/koha-common.cnf and add port = 12345 in section client.

sudo $HOME/kohaclone/debian/scripts/koha-create --create-db test2

* Verify portnumber: sudo xmlstarlet sel -t -v 'yazgfs/config/port' "/etc/koha/sites/test2/koha-conf.xml"

* Test koha-create using password file:
  * Edit /etc/koha/passwd and add a column with portnumber ':12345'

sudo $HOME/kohaclone/debian/scripts/koha-create --create-db test3

* Verify portnumber: sudo xmlstarlet sel -t -v 'yazgfs/config/port' "/etc/koha/sites/test2/koha-conf.xml"

* Test separate db population

sudo $HOME/kohaclone/debian/scripts/koha-create --request-db test4

sudo mysqldump --defaults-file=/etc/mysql/koha-common.cnf koha_kohadev >/tmp/kohadev.sql

mysqluser="$( sudo xmlstarlet sel -t -v 'yazgfs/config/user'     /etc/koha/sites/test4/koha-conf.xml )"
mysqlpass="$( sudo xmlstarlet sel -t -v 'yazgfs/config/pass'     /etc/koha/sites/test4/koha-conf.xml )"

sudo mysql --defaults-file=/etc/mysql/koha-common.cnf -e "CREATE DATABASE koha_test4; GRANT ALL ON koha_test4.* TO '$mysqluser'@'localhost' IDENTIFIED BY '$mysqlpass'"

sudo ~/kohaclone/debian/scripts/koha-create --populate-db --defaultsql /tmp/kohadev.sql test4


* Test separate db population using instance credentials

sudo $HOME/kohaclone/debian/scripts/koha-create --request-db test5

mysqluser="$( sudo xmlstarlet sel -t -v 'yazgfs/config/user'     /etc/koha/sites/test5/koha-conf.xml )"
mysqlpass="$( sudo xmlstarlet sel -t -v 'yazgfs/config/pass'     /etc/koha/sites/test5/koha-conf.xml )"

sudo mysql --defaults-file=/etc/mysql/koha-common.cnf -e "CREATE DATABASE koha_test5; GRANT ALL ON koha_test5.* TO '$mysqluser'@'localhost' IDENTIFIED BY '$mysqlpass'"

sudo ~/kohaclone/debian/scripts/koha-create --populate-db --defaultsql --defaultsql-creds instance --dbport 1 /tmp/kohadev.sql test5
Comment 13 Andreas Jonsson 2019-07-04 20:17:49 UTC
Created attachment 91300 [details] [review]
Bug 23141: Add option to select db credentials source when populating db.
Comment 14 Andreas Jonsson 2019-07-08 15:00:59 UTC
Created attachment 91408 [details] [review]
Bug 23141: Add option to select db credentials source when populating db.
Comment 15 Martin Renvoize 2020-03-09 10:22:19 UTC
Adding a few more people to the cc list to get some fresh eyes.
Comment 16 Andreas Roussos 2020-03-11 18:43:34 UTC
Fixed a small typo (myql -> MySQL) in the Bug Summary and split "portnumber" in
two words -- all in the name of making it easier to locate this bug in future
searches ;-)
Comment 17 David Cook 2020-04-02 00:56:20 UTC
I was going to say that it assumes that Koha is connecting to MySQL using TCP rather than a Unix socket, but after checking Koha::Database, it looks like we mandate use of TCP anyway, so never mind me.

(That being said, should we support Unix socket connections for databases? I suppose it would just add complexity to all the Koha management scripts at this point. I imagine most production implementations have a separate database server anyway. That said, I've pretty sure I've found with PostgreSQL at least that a Unix socket is considerably faster than TCP over localhost. But that's enough of a tangent for now...)
Comment 18 Victor Grousset/tuxayo 2020-05-12 17:11:54 UTC
Conflict in debian/scripts/koha-dump

    # Dump database.
    mysqlhost="$( xmlstarlet sel -t -v 'yazgfs/config/hostname' $kohaconfig )"
    mysqldb="$( xmlstarlet sel -t -v 'yazgfs/config/database' $kohaconfig )"
    mysqluser="$( xmlstarlet sel -t -v 'yazgfs/config/user' $kohaconfig )"
    mysqlpass="$( xmlstarlet sel -t -v 'yazgfs/config/pass' $kohaconfig )"
    backupdir="$( xmlstarlet sel -t -v 'yazgfs/config/backupdir' $kohaconfig || true )"
    [ -z "$backupdir" ] && backupdir="/var/spool/koha/$name"
    dbdump="$backupdir/$name-$date.sql.gz"
    [ "$quiet" = "no" ] && echo "* DB to $dbdump"
<<<<<<< HEAD
    dbflag="--databases"
    [ "$without_db_name" = "yes" ] && dbflag=""
    mysqldump $dbflag --host="$mysqlhost" \
||||||| constructed merge base
    mysqldump --databases --host="$mysqlhost" \
=======
    mysqldump --databases --skip-triggers --host="$mysqlhost" --port="$mysqlport" \
>>>>>>> Bug 23141: Add port number to koha-dump script
        --user="$mysqluser" --password="$mysqlpass" "$mysqldb" |
        gzip > "$dbdump"
    chown "root:$name-koha" "$dbdump"
    chmod g+r "$dbdump"
Comment 19 Jonathan Druart 2020-06-16 10:07:56 UTC
Created attachment 105895 [details] [review]
Bug 23141: Add port number to koha-dump script
Comment 20 Jonathan Druart 2020-06-16 10:08:00 UTC
Created attachment 105896 [details] [review]
Bug 23141: Add support for configuring database port in koha-dump, koha-mysql and koha-mysqlcheck
Comment 21 Jonathan Druart 2020-06-16 10:08:04 UTC
Created attachment 105897 [details] [review]
Bug 23141: Add --dbport parameter to koha-create
Comment 22 Jonathan Druart 2020-06-16 10:08:07 UTC
Created attachment 105898 [details] [review]
Bug 23141: Parse mysql portnumber from /etc/mysql/koha-common.cnf
Comment 23 Jonathan Druart 2020-06-16 10:08:11 UTC
Created attachment 105899 [details] [review]
Bug 23141: Add option to select db credentials source when populating db.
Comment 24 Jonathan Druart 2020-06-16 10:09:17 UTC
Patches rebased against master.

I noticed a change that should not be part of this patchset in my opinion: --skip-triggers is added to the mysqldump call in koha-dump.
Comment 25 Victor Grousset/tuxayo 2020-11-13 14:23:39 UTC
Created attachment 113605 [details] [review]
Bug 23141: Add port number to koha-dump script
Comment 26 Victor Grousset/tuxayo 2020-11-13 14:23:42 UTC
Created attachment 113606 [details] [review]
Bug 23141: Add support for configuring database port in koha-dump, koha-mysql and koha-mysqlcheck
Comment 27 Victor Grousset/tuxayo 2020-11-13 14:23:46 UTC
Created attachment 113607 [details] [review]
Bug 23141: Add --dbport parameter to koha-create
Comment 28 Victor Grousset/tuxayo 2020-11-13 14:23:49 UTC
Created attachment 113608 [details] [review]
Bug 23141: Parse mysql portnumber from /etc/mysql/koha-common.cnf
Comment 29 Victor Grousset/tuxayo 2020-11-13 14:23:53 UTC
Created attachment 113609 [details] [review]
Bug 23141: Add option to select db credentials source when populating db.
Comment 30 Victor Grousset/tuxayo 2020-11-13 14:50:55 UTC
Sorry for the delay, I missed in the notification that it was me who asked for the rebase.

First, here is a another needed rebase. I'll try to get my head around this again.
Comment 31 Victor Grousset/tuxayo 2020-11-16 15:40:54 UTC
> * Test koha-create using command line parameter:

> vagrant@kohadevbox:~$ sudo $HOME/kohaclone/debian/scripts/koha-create --dbport 12345 --create-db test
> Internal error processing command line arguments

Looking at code, --dbport is there. I don't know what is wrong. koha-create Still works without --dbport so that confirm is linked to it.