Bug 526311 - Review Request: mysql-mmm - Multi-Master Replication Manager for MySQL
Summary: Review Request: mysql-mmm - Multi-Master Replication Manager for MySQL
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Ruben Kerkhof
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 531912
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-09-29 19:22 UTC by BJ Dierkes
Modified: 2010-01-31 14:32 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-01-31 14:32:04 UTC
Type: ---
Embargoed:
ruben: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description BJ Dierkes 2009-09-29 19:22:51 UTC
Spec URL: http://5dollarwhitebox.org/tmp/mysql-mmm.spec
SRPM URL: http://5dollarwhitebox.org/tmp/mysql-mmm-1.2.6-3.3.src.rpm

Description: 

This is my first package, and I need a sponsor.

---

MMM (MySQL Multi-Master Replication Manager) is a set of flexible scripts to
perform monitoring/failover and management of MySQL Master-Master replication
configurations (with only one node writable at any time). The toolset also has
the ability to read balance standard master/slave configurations with any 
number of slaves, so you can use it to move virtual IP addresses around a group
of servers depending on whether they are behind in replication.

Comment 1 BJ Dierkes 2009-10-02 15:55:59 UTC
Please note, I am more interested in maintaining this package for Fedora EPEL.  As it is more of a production/server application its more suitable for RHEL+EPEL... that said I have no qualms with maintaining for both EPEL and Fedora Core.

Comment 2 Ruben Kerkhof 2009-10-11 10:58:26 UTC
Hi BJ,

I'll review your package, and I'm willing to sponsor you.

I haven't used mmm before, so I'm trying your packages now. As soon as I got a setup working I'll do a formal review.

Please start with running rpmlint over the packages, there are a lot of errors and warnings.

Comment 3 Itamar Reis Peixoto 2009-10-11 11:17:01 UTC
(In reply to comment #1)

BJ Dierkes

Do you have a fedora account [1] ?

[1] - https://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account


after you got a fedora account, can you join fedora packager group in FAS and try to build it using koji ?


Requires: initscripts --  all machines have it, no need to Requires.


Requires: perl-DBD-MySQL -> replace this with mysql-server


/sbin/chkconfig mmm_agent on ->  fedora packaging rules doesn't allow services enabled by default.

Comment 4 Ruben Kerkhof 2009-10-12 10:22:52 UTC
Itamar,

>> Requires: perl-DBD-MySQL -> replace this with mysql-server

Why? The monitor doesn't need mysql-server installed.

A better idea is to remove the Requires altogether, and let rpm figure it out.

Comment 5 BJ Dierkes 2009-10-12 12:20:32 UTC
(In reply to comment #3)
>
> Do you have a fedora account [1] ?

I do (derks).

 
> 
> after you got a fedora account, can you join fedora packager group in FAS and
> try to build it using koji ?

I have applied to the packager group in FAS and will attempt to build this with koji once that is approved.  I'm already all setup on my mock development machine at work with ssl certs and everything to talk to Koji, and have verified that to work (just haven't submitted anything yet).

 
> 
> Requires: initscripts --  all machines have it, no need to Requires. 
> Requires: perl-DBD-MySQL -> replace this with mysql-server 
> 
> /sbin/chkconfig mmm_agent on ->  fedora packaging rules doesn't allow services
> enabled by default.  

I will make these changes promptly once I'm back in the office.

Thank you for taking the time to work with me on this.  I will admit, the sources kind of limited me to how it is being installed... meaning, the development practice doesn't follow the FHS and expects everything installed to /usr/share/mmm or similar.  I couldn't see any feasible way around that without patching every source file.


>>> Requires: perl-DBD-MySQL -> replace this with mysql-server

> Why? The monitor doesn't need mysql-server installed.

Not sure where this requirement originated from (it may have been before I took over packaging it).  I agree though, mysql-server wouldn't be a good replacement as the monitor generally runs on a stand-alone box that is not one of the database servers.  I'll look into it further.

Comment 6 Ruben Kerkhof 2009-10-12 14:55:56 UTC
I've just approved your sponsorship requests, and removed the FE-NEEDSPONSOR blocker.

Comment 7 BJ Dierkes 2009-10-20 04:26:24 UTC
Per discussion with Ruben, I have scratched 1.2 and start almost complete fresh with the mysql-mmm-2.0 branch.

http://5dollarwhitebox.org/tmp/mysql-mmm-2.0.9-1.src.rpm
http://5dollarwhitebox.org/tmp/mysql-mmm.spec


Installation is much cleaner with 2.x and follows FHS.  I haven't had a chance to fully test it but plan on it in the coming days.  Please review if you like, I will be working to get the build in koji shortly. Additionally, looks as though I will need to maintain perl-Net-ARP as well which is fine with me.

Comment 8 BJ Dierkes 2009-10-20 17:51:05 UTC
New Package CVS Request
=======================
Package Name: mysql-mmm
Short Description: Multi-Master Replication Manager for MySQL
Owners: derks
Branches: F-11 F-12 EL-5
InitialCC:

Comment 9 Itamar Reis Peixoto 2009-10-20 17:53:39 UTC
(In reply to comment #8)

not yet ready for cvs, wait for review flag + from ruben

Comment 10 BJ Dierkes 2009-11-04 00:06:06 UTC
This has been updated due to source change upstream, as well as a number of added patches/changes to clean up a few things:

http://5dollarwhitebox.org/tmp/mysql-mmm.spec
http://5dollarwhitebox.org/tmp/mysql-mmm-2.0.10-1.src.rpm

Comment 11 Ruben Kerkhof 2009-11-07 18:36:38 UTC
Since this are all architecture-independent perl libraries, you have to add BuildArch: noarch.

Comment 12 Ruben Kerkhof 2009-11-10 21:22:39 UTC
I've almost got a master-master configuration running under mysql-mmm. I bumped into a few small things, mainly because I've never used mmm before and the documentation is pretty sparse.

- the monitor subpackage is missing a dependency on dbd::mysql
- starting the agent with the default configuration caused it to die with an error, but it was restarted over and over again. I just got 2GB of emails from the agent :-) We might want to create more sensible defaults for logging in Fedora...
- The upstream init scripts are ok, but a bit verbose. It would be nice if you created a Fedora specific init script, using the functions from /etc/init.d/functions.

I'll do a proper review when I get mysql-mmm running.

Comment 13 BJ Dierkes 2009-11-10 23:00:49 UTC
(In reply to comment #11)
> Since this are all architecture-independent perl libraries, you have to add
> BuildArch: noarch.

Not sure how I missed that, but it has been added.


(In reply to comment #12)
> - the monitor subpackage is missing a dependency on dbd::mysql

Was removed based on comment #4 ;).  Added back in.


> - starting the agent with the default configuration caused it to die with an
> error, but it was restarted over and over again. I just got 2GB of emails from
> the agent :-) We might want to create more sensible defaults for logging in
> Fedora...

I ran into the same issue. I believe setting the IP for db1 in mmm_common.conf to 127.0.0.1 by default will atleast keep the process from dying and causing havoc with logs/email.  I did added comments to the config to encourage the IP be set right, but I know I always try starting the process before anything else.  Regardless, I've reported the following upstream when I first came across it:

https://bugs.launchpad.net/mysql-mmm/+bug/473446


> - The upstream init scripts are ok, but a bit verbose. It would be nice if you
> created a Fedora specific init script, using the functions from
> /etc/init.d/functions.

I will look into rewriting/cleaning up the init scripts.


Thanks Ruben.

Comment 14 Ruben Kerkhof 2009-11-11 15:47:34 UTC
> - the monitor subpackage is missing a dependency on dbd::mysql

>> Was removed based on comment #4 ;).  Added back in.

rpm uses /usr/lib/rpm/perldeps.pl to find perl packages.
That script parses perl code for 'use' statements. It missed dbd::mysql in this case.

> I ran into the same issue. I believe setting the IP for db1 in mmm_common.conf
> to 127.0.0.1 by default will atleast keep the process from dying and causing
> havoc with logs/email.  I did added comments to the config to encourage the IP
> be set right, but I know I always try starting the process before anything
> else.  Regardless, I've reported the following upstream when I first came
> across it:

> https://bugs.launchpad.net/mysql-mmm/+bug/473446

I've added a log4perl config file to my setup:

[ruben@db2 ~]$ sudo cat /etc/mysql-mmm/mmm_agent_log.conf
log4perl.logger = INFO, FileInfo

log4perl.appender.FileInfo = Log::Log4perl::Appender::File
log4perl.appender.FileInfo.Threshold = INFO
log4perl.appender.FileInfo.filename = /var/log/mysql-mmm/mmmd_agent.log
log4perl.appender.FileInfo.recreate = 1
log4perl.appender.FileInfo.layout = PatternLayout
log4perl.appender.FileInfo.layout.ConversionPattern = %d %5p %m%n

This takes care of the mail problem. Something like this might be a nice default for Fedora, but we'll have to rotate that log too.

Regards,

Ruben

Comment 15 BJ Dierkes 2009-11-17 19:44:08 UTC
I have added a patch to lib/Common/Lib.pm to essentially do the same thing your mmm_agent_log.conf does, but makes it the global default for all services (not just agent).  I've also reworked the patch I submitted upstream for the infinit loop on error condition.

http://5dollarwhitebox.org/tmp/mysql-mmm-2.0.10-2.src.rpm
http://5dollarwhitebox.org/tmp/mysql-mmm.spec

Comment 16 Ruben Kerkhof 2009-11-19 10:35:04 UTC
Great, nice work!

Ok, another couple comments:
* package is still x86_64, should be noarch
* mysql-mmm-monitor still needs the DBD::MySQL Requires
* I don't mind the Provides, but use %{version}-%{release}
* The files section contains 2 entries for /var/run/mysql-mmm
* Replace /var/log/mysql-mmm with %{_localstatedir}/log/mysql-mmm

Comment 17 Ruben Kerkhof 2009-11-19 11:02:45 UTC
Oh, and 2 more ;-)

> %post agent
> /sbin/chkconfig --add mysql-mmm-agent
> /sbin/chkconfig mysql-mmm-agent off
> if [ $1 -eq 2 ] ; then
>    /sbin/service mysql-mmm-agent status >/dev/null
>    if [ $? = 0 ]; then
>        /sbin/service mysql-mmm-agent restart
>    fi
>fi

This sets the mysql-mmm-agent service to off at an update of the package.

> %preun agent
> if [ $1 = 0 ]; then

Use -eq here

> %preun monitor
> if [ $1 = 0 ]; then

Same here

Comment 18 BJ Dierkes 2009-11-19 19:57:47 UTC
I apologize for a few of these.  I had a hard drive crash and lost the current stuff I was working with, and forgot to re-implement some of the changes.  These should all be handled.  Note that perl-DBD-MySQL actually provides perl(DBD::mysql) which was added for the monitor subpackage.

I also re-wrote the init scripts for agent/monitor so they are a lot cleaner and use init.d/functions, as well as implement a condrestart that is now used in %post.


http://5dollarwhitebox.org/tmp/mysql-mmm.spec
http://5dollarwhitebox.org/tmp/mysql-mmm-2.0.10-3.src.rpm


%changelog
* Thu Nov 19 2009 BJ Dierkes <wdierkes> - 2.0.10-3
- BuildArch: noarch
- Monitor subpackage Requires: perl(DBD::mysql)
- Provides full version-release for  mmm, mysql-master-master
- Removed redundant /var/run/mysql-mmm entry
- Use _localstatedir macro for /var/log/mysql-mmm file listing 
- Fixed logic in post and postun scripts to properly handle
  install/upgrade conditions.
- Post scripts now perform condrestart
- Added Source3: mysql-mmm-agent.init
- Added Source4: mysql-mmm-monitor.init

Comment 19 BJ Dierkes 2009-12-08 17:16:59 UTC
Hello, Can I get an update on this review?  Just want to make sure nothing is waiting on me.  Thank you.

Comment 20 Ruben Kerkhof 2009-12-10 10:19:13 UTC
Sorry, I was AFK for a while.

Last comments:
- Provides need a full version-release in the subpackages
- rpmlint is not clean:

[ruben@slice SPECS]$ rpmlint ../RPMS/noarch/mysql-mmm-*3*
mysql-mmm.noarch: I: enchant-dictionary-not-found en_US
mysql-mmm.noarch: E: non-standard-dir-perm /var/log/mysql-mmm 0750
mysql-mmm.noarch: E: non-standard-dir-perm /var/lib/mysql-mmm 0750
mysql-mmm.noarch: E: non-readable /etc/mysql-mmm/mmm_common.conf 0640
mysql-mmm.noarch: E: zero-length /usr/share/doc/mysql-mmm-2.0.10/README
mysql-mmm.noarch: E: non-standard-dir-perm /var/run/mysql-mmm 0750
mysql-mmm-agent.noarch: W: no-documentation
mysql-mmm-agent.noarch: E: non-readable /etc/mysql-mmm/mmm_agent.conf 0640
mysql-mmm-agent.noarch: E: subsys-not-used /etc/init.d/mysql-mmm-agent
mysql-mmm-monitor.noarch: W: no-documentation
mysql-mmm-monitor.noarch: E: non-readable /etc/mysql-mmm/mmm_mon.conf 0640
mysql-mmm-monitor.noarch: E: subsys-not-used /etc/init.d/mysql-mmm-monitor
mysql-mmm-tools.noarch: W: no-documentation

Most errors are harmless in my opinion, except for the directory permissions on /var/lib/mysql-mmm and /var/run/mysql-mmm, there's no harm in making them 0755.

Comment 21 BJ Dierkes 2009-12-10 18:38:01 UTC
I've fixed the rpmlint errors except for the following:

$ !rpmli
rpmlint -i RPMS/noarch/mysql-mmm-*4*
mysql-mmm.noarch: E: non-readable /etc/mysql-mmm/mmm_common.conf 0640
The file can't be read by everybody. If this is expected (for security
reasons), contact your rpmlint distributor to get it added to the list of
exceptions for your distro (or add it to your local configuration if you
installed rpmlint from the source tarball).

mysql-mmm-agent.noarch: E: non-readable /etc/mysql-mmm/mmm_agent.conf 0640
The file can't be read by everybody. If this is expected (for security
reasons), contact your rpmlint distributor to get it added to the list of
exceptions for your distro (or add it to your local configuration if you
installed rpmlint from the source tarball).

mysql-mmm-monitor.noarch: E: non-readable /etc/mysql-mmm/mmm_mon.conf 0640
The file can't be read by everybody. If this is expected (for security
reasons), contact your rpmlint distributor to get it added to the list of
exceptions for your distro (or add it to your local configuration if you
installed rpmlint from the source tarball).

4 packages and 0 specfiles checked; 3 errors, 0 warnings.



Is it _really_ preferred that everyone can read all config files, or can I ignore these?  Seems that would be a security issue if config files have sensitive information like passwords.

Comment 22 BJ Dierkes 2009-12-10 19:08:17 UTC
Latest build fixing the above issues, with exception of changing the mode of the config files:

http://5dollarwhitebox.org/tmp/mysql-mmm.spec
http://5dollarwhitebox.org/tmp/mysql-mmm-2.0.10-4.src.rpm

Comment 23 Ruben Kerkhof 2009-12-11 12:59:09 UTC
I think it's safe to ignore the last 3 rpmlint errors since the files contain database passwords.

I've looked at mysql-mmm-2.0.10-4 and see no further issues.

This package is approved.

Comment 24 BJ Dierkes 2009-12-11 17:33:29 UTC
New Package CVS Request
=======================
Package Name: mysql-mmm
Short Description: Multi-Master Replication Manager for MySQL
Owners: derks
Branches: F-11 F-12 EL-5
InitialCC:

Comment 25 Kevin Fenzi 2009-12-14 17:48:44 UTC
cvs done.


Note You need to log in before you can comment on or make changes to this bug.