Comment 4 for bug 1922412

Revision history for this message
Bryce Harrington (bryce) wrote :

Actually, it's the os.listdir() call that throws the exception, so this patch won't solve the crash. The try would need to be at the next indentation level up.

However, I think we should redo the code a bit here. Look at attach_dmi() in /usr/lib/python3/dist-packages/apport/hookutils.py as an example to emulate. Instead of a try/except, use a call to os.path.isdir() like attach_dmi() does. Probably wouldn't hurt to also check for /etc/mysql/ before making the other two _add_my_conf_files() calls, since these could also fail if that dir was missing.

Also, printing out the error message is probably not going to result in a useful behavior. The user won't see this error message during apport execution; it'll either end up in the body of the bug report or maybe silently dropped (I think Apport expects "Key: Value" output from hooks).

There's three things we could do:

a. Ignore it.

File the bug anyway, just without this particular information. Instead of iterating the config directory, add an entry like report['MySQLConf.etc.mysql.conf.d'] = "No such directory" or whatever, that'll clue us in if these show up.

b. Bail, refusing to report the bug.

In this case, we set the key report['UnreportableReason'] to the error message and then return False from add_info(). Also down in __main__ check return value on add_info() and exit if its False. (See /usr/share/apport/package-hooks/source_xorg.py for an example of this pattern.) The user will be informed that bug won't be filed. I don't remember if they'll be told why or if it will just silently exit.

c. Prompt the user.

Apport is capable of displaying GUI dialogs to the user, to give them advice or prompt them about things (like whether to include certain files). Routines like ui.yesno(), ui.choice(), and ui.information() exist to do this. See for example line 234 in /usr/share/apport/package-hooks/source_xorg.py for a ui.information() call as it bails.

Give some thought to which of these three options you think is going to be best. Offhand, I think if mysql can be expected to operate properly without the /etc/mysql/conf.d or /etc/mysql/mysql.conf.d directories, then we should probably just ignore (option a.); if it's a legitimate issue we should work on figuring out, then option b. may be better; if we're 100% sure it's just user error doing something dumb, then option c. (with a ui.information() call) would probably be the best all-around experience for them and us.

Lastly, in case you didn't already know, note that for testing/debugging purposes you can run the apport hook directly. I.e.:

root@triage-hirsute:/usr/share/apport/package-hooks# python3 ./source_mysql-8.0.py
Logs.var.log.daemon.log:
Logs.var.log.mysql.error.log: 2021-04-05T17:27:07.759239Z 0 [System] [MY-013169] [Server] /usr/sbin/mysqld (mysqld 8.0.23-3ubuntu1) initializing of server in progress as process 145151
KernLog: audit(
ProcVersionSignature: Ubuntu 5.4.0-70.78-generic 5.4.94
ProcCmdline: BOOT_IMAGE=/boot/vmlinuz-5.4.0-70-generic root=UUID=3637f658-4489-4f1f-acd9-2b0ce513d2db ro cgroup_enable=memory swapaccount=1
.etc.apparmor.d.usr.sbin.mysqld: # vim:syntax=apparmor
MySQLConf.etc.mysql.my.cnf: #
MySQLConf.etc.mysql.mysql.cnf: #
MySQLConf.etc.mysql.conf.d.mysqldump.cnf: [mysqldump]
MySQLConf.etc.mysql.conf.d.mysql.cnf: [mysql]
MySQLConf.etc.mysql.mysql.conf.d.mysql.cnf: #
MySQLConf.etc.mysql.mysql.conf.d.mysqld.cnf: #
MySQLVarLibDirListing: ['client-key.pem', 'server-cert.pem', 'undo_002', '#ib_16384_1.dblwr', 'performance_schema', 'mysql.ibd', '#ib_16384_0.dblwr', 'client-cert.pem', 'debian-5.7.flag', 'ib_logfile1', 'undo_001', 'binlog.000003', 'ibdata1', 'private_key.pem', 'ca-key.pem', 'ca.pem', 'ib_logfile0', 'sys', 'ib_buffer_pool', 'mysql', 'binlog.index', 'binlog.000001', 'server-key.pem', '#innodb_temp', 'triage-hirsute.pid', 'ibtmp1', 'public_key.pem', 'binlog.000002', 'auto.cnf']

If we add apport ui calls, this'll need to be run from something that has X11 running, but if not then it can be run and tested in a container.