Hi, On Wed, Feb 02, 2011 at 05:01:19PM +0100, [email protected] 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: [email protected] > http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev > Home Page: http://linux-ha.org/ _______________________________________________________ Linux-HA-Dev: [email protected] http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev Home Page: http://linux-ha.org/
