Catch errors when scraping datadir from help output

[yosifkit]

Before this change, a user would just get the container exiting and zero output. Now a user will get some friendly output about the command that was run and then the error messages that mysqld emitted.

[yosifkit]

Related to #216 (comment)

[tianon]

LGTM!

cc @ltangvald

[ltangvald]

Maybe separate it from the datadir fetch? We have a commit for this in the upstream image, actually:
mysql@05833af

[yosifkit]

I'm not sure I understand the need to separate the error detection from the datadir scraping. This code ensures we only have to run mysqld --verbose --help once to get the error and datadir. Plus, now that we have the _datadir function this ensures that the command is checked for errors any time it is called (which is currently two times, once as root, line 27/37, and once as mysql user, line 35/45)

[ltangvald]

It's somewhat a personal preference since I think it's easier to read and understand the code, but there is some benefit to keeping different logical operations (fetch datadir and check for errors) separate, since it makes future debugging and maintenance simpler.

[yosifkit]

That makes sense, though the only problem I have with separating them is that they could become out of sync. The reason for checking for errors is to make sure the datadir scraping will succeed.

As an alternative solution, we could just ignore the failure and add an || true. Aside from the non-zero exit, are there any mysqld --verbose -help errors that would cause the datadir scrape to fail? Perhaps we just allow it to fail later when it does mysql_install_db or --initialize-insecure.

[ltangvald]

Yeah, we added it as a more general error checking to fail fast and notify users if e.g. a custom config contains invalid entries, not specifically for the datadir scrape. I think it's a useful thing to have, since it lets us print a decent error message in such cases.

Can't really think of anything else that would cause the scrape to fail.

[yosifkit]

Updated to separate config error check and datadir scraping.

[yosifkit]

I need to update this text to something more appropriate.

[ltangvald]

LGTM

[tianon]

👍