Hi,

On Wed, Feb 02, 2011 at 05:01:19PM +0100, jer...@intuxicated.org wrote:
> Hi,
> 
> I created a resource agent for slapd, and want to submit it to the project.
> I know I should upload it as a patch, but I was having some difficulties
> with that, so I'm doing it this way instead. If I should use the official
> method or use different steps, please let me know.

It is fine as is. However, the current best practice is to have
resource agents shipped with the software it actually supports
whenever possible. The interesting parts of a resource agent
obviously have to do with the software it manages. Since we here
have only occasionaly some expertise in other areas, the resource
agent would benefit from more qualified critique from people
involved in the respective software project.

> I will happily modify the script or provide additional information to get
> it included.

It would be interesting to have this RA support multi-state.
Last time I looked at openldap, it supported a master-slave setup
where every slave was capable of answering queries, but all
modifications went to the master instance. That fits well with
the OCF multi-state resource concept.

The monitor operation in the agent is very basic. An OCF RA
should do better. Extra points if the query can be made
configurable so that users can monitor whichever directory part
is of most importance.

You should lower the severity to debug of the log message on
success in the monitor operation. Otherwise, it's going to make
to much noise.

Various sed scripts should be consolidated into one function
dedicated to getting information from the slapd configuration.

config_file can be a directory. Better name it just config.

The slapd_validate_all() should return with OCF_ERR_INSTALLED
instead of OCF_ERR_GENERIC.

The big for loop in validation is, well, big and unwieldy. Not
sure if it's absolutely necessary to do this kind of validation.

Not sure, but some bits look bash-specific. Either remove
bashisms (preferred) or change #!/bin/sh to #!/bin/bash.

Though we don't have any particular coding style imposed, could
you perhaps reduce the vertical white space somewhat. Seems like
there are too many empty lines to me.

This is not exhaustive and there'd probably be more, but should
keep you busy for a while :)

BTW, there has been once an openldap resource agent posted to
this list, but unfortunately didn't get timely response (entirely
my fault) and the author never responded to the late RA review.
See http://marc.info/?l=linux-ha&m=123003594213018&w=2 and
http://marc.info/?l=linux-ha&m=124327185429394&w=2

And many thanks for the effort.

Cheers,

Dejan

> Best regards,
> 
> Jeroen Koekkoek


> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/

_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Reply via email to