ensure if service restart to wait till mysql is up

[vicinus]

The old mysql init scripts waited till mysql was up and running till they finished. With ubuntu 14.04 and upstart (re)starting mysql will no longer wait for mysql to finish starting up. So in some cases, puppet will try to run mysql queries against the mysql server which is still starting up.

This patch prevents that by waiting till the configured mysql socket is available if the mysql service gets updated.

[igalic]

rather than refreshonly, you may wanna use:

exec { 'wait_for_mysql_socket_to_open':
  command   => '/bin/true',
  tries     => '3'.
  try_sleep => '10',
  subscribe => Service['mysqld'],
  unless    => "/bin/test -S ${mysqlsocket}",
}

[vicinus]

ok, your solution is more elegant. I tested it and the only things I changed are: 'test' is located in /usr/bin on ubuntu and centos. As I don't know if there is a distribution which places 'test' in /bin, I added path => '/bin:/usr/bin'. Also I changed subscribe to require, because without refreshonly, I see no reason to use subscribe.

Also how should I proceed? Should I close this pull request and make a completely new pull request with one squashed commit?

[igalic]

you could use git push --force
however, you've been working on your own master branch, which might make things very confusing, especially in case you have to rebase, etc…

i highly recommend using the github flow work flow.

[vicinus]

I have integrated your changes. Is there anything else I should do?

[igalic]

@vicinus nope, i think that's all good so far…
@DavidS / @tphoney any comments on this?

[DavidS]

@vicinus basically ok. The question I have is whether the socket is always there or whether there would be a way of configuring the mysql server so that this would always run (and do the wrong thing), (and whether we should care about such a setup)?

[vicinus]

I see no technically reason why it shouldn't be possible to run a mysql server without a file system socket. But I'm not sure if anyone is running such a setup with puppet. To clear the socket value the mysql::server::options hash has to be cleared by setting it explicitly.

I would suggest to surround the exec with an if statement:

if $mysqlsocket != undef {
  exec { 'wait_for_mysql_socket_to_open': 
    ... 
  }
}

That way if someone disabled the socket configuration, he isn't placed in a worse situation as before. And if he happens to get in the same situation we can look if we want to add a fall back to check for network sockets instead.

Please let me know if my proposal is acceptable and I will update the pull request with the aforementioned if statement around the exec.

[igalic]

my main issue: when mysql crashes and doesn't cleanup the socket, does that mean we won't re/start it?

[vicinus]

The exec only looks if a file exists and is a socket. If that's the case nothing happens. So if mysql crashes and doesn't cleanup the socket, then the exec will do nothing. If the puppet service configuration prior detects that mysql has crashed and is not running is another story and not part of this change. Or am I mistaken?

Also the command to run must be /bin/false not /bin/true, because otherwise if the socket isn't there /bin/true will tell the exec that everything is ok, and no retry will occur.

[DavidS]

Using /bin/false as command it will create an error in the report - even if the socket was created successfully. What about using the same test -S check as in the unless?

It reminds me of the health checks in app orchestration (https://docs.puppetlabs.com/pe/latest/app_orchestration_availability_tests.html) and the validators in https://github.com/puppet-community/puppet-healthcheck

Especially the latter might be great to add proper socket validation there, available for all to use.

[igalic]

@vicinus ping? any comments?

[vicinus]

sorry, because of the "Christmas madness" I didn't have time to think it through.
The connection validator solution seems like the more general solution. But at the moment I think this will add a lot of overhead, because the only solution to use it, that i can think of is to add a dependency to every possible mysql usage like that:

Class['mysql_connection_validatior'] -> Mysql_user <| |>
Class['mysql_connection_validatior'] -> Mysql_grant <| |>
Class['mysql_connection_validatior'] -> Mysql_database <| |>
Class['mysql_connection_validatior'] -> Mysql_plugin <| |>

And I'm not sure, but my first consideration was that this will increase the catalog generation significantly and that is something I want to avoid if possible. But as I said before, I didn't have time to test it.

To the point regarding the usage of /bin/false: test -S can be used also for the command. But with puppet 4.3 /bin/false would work also. Is there an issue with older puppet versions, that test -S is preferable?

[vicinus]

I have updated the patch to only use 'test -S'.

I also looked into mysql connection validation and for me there are a lot of open questions like should username and password also be validated like the postgresql validate_db_connection.pp. Should the validation be done via a exec in puppet code or in ruby with plain ruby code or should the ruby mysql library used and so on. And for the foreseeable future I won't have the time needed to work on this.