Bug 23141 - The Debian scripts do not use the myql portnumber
Summary: The Debian scripts do not use the myql portnumber
Status: Needs Signoff
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: 2019-12-03 11:10 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
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

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 M. 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 M. 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 M. 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.