The scripts koha-mysql, koha-mysqldump, and koha-create and possibly others do not use the configured mysql portnumber configured in koha-conf.xml.
Created attachment 90687 [details] [review] Bug 23141: Add port number to koha-dump script
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
Created attachment 90688 [details] [review] Bug 23141: Add support for configuring database port in koha-dump, koha-mysql and koha-mysqlcheck
Created attachment 90689 [details] [review] Bug 23141: Add mysql portnumber support when populating db
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 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.
Marking Failed QA, because of comment #6 (without a doubt) and possibly comment #5.
(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
Created attachment 91291 [details] [review] Bug 23141: Add --dbport parameter to koha-create
Created attachment 91292 [details] [review] Bug 23141: Parse mysql portnumber from /etc/mysql/koha-common.cnf
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.
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
Created attachment 91300 [details] [review] Bug 23141: Add option to select db credentials source when populating db.
Created attachment 91408 [details] [review] Bug 23141: Add option to select db credentials source when populating db.
Adding a few more people to the cc list to get some fresh eyes.
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 ;-)
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...)
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"
Created attachment 105895 [details] [review] Bug 23141: Add port number to koha-dump script
Created attachment 105896 [details] [review] Bug 23141: Add support for configuring database port in koha-dump, koha-mysql and koha-mysqlcheck
Created attachment 105897 [details] [review] Bug 23141: Add --dbport parameter to koha-create
Created attachment 105898 [details] [review] Bug 23141: Parse mysql portnumber from /etc/mysql/koha-common.cnf
Created attachment 105899 [details] [review] Bug 23141: Add option to select db credentials source when populating db.
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.
Created attachment 113605 [details] [review] Bug 23141: Add port number to koha-dump script
Created attachment 113606 [details] [review] Bug 23141: Add support for configuring database port in koha-dump, koha-mysql and koha-mysqlcheck
Created attachment 113607 [details] [review] Bug 23141: Add --dbport parameter to koha-create
Created attachment 113608 [details] [review] Bug 23141: Parse mysql portnumber from /etc/mysql/koha-common.cnf
Created attachment 113609 [details] [review] Bug 23141: Add option to select db credentials source when populating db.
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.
> * 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.