On Fri, Jun 25, 2010 at 11:48:31AM -0600, Serge Dubrouski wrote: > That change is Ok. Attached is a patch to fix meta-data.
Applied. Thanks! Dejan > On Fri, Jun 25, 2010 at 11:31 AM, Dejan Muhamedagic <[email protected]> > wrote: > > Hi, > > > > On Fri, Jun 25, 2010 at 08:28:54AM -0600, Serge Dubrouski wrote: > >> Hi, Dejan - > >> > >> What's the point of introducing check_binary2 ? The old version of > >> script used to use have_binary for checking binary tools but then > >> Florian replaced it with check_binary and threw away "if" statements. > >> Now you kind of reversing it. Why? > > > > Didn't see that changeset, but using check_binary here is wrong, > > because it immediately exits instead of letting the caller decide > > what to do. The earlier version was correct, Florian probably > > misunderstood parts of the code. > > > > That changeset contains an unrelated change: > > > > -OCF_RESKEY_start_opt_default="-p $OCF_RESKEY_pgport_default" > > +OCF_RESKEY_start_opt_default="" > > > > Looks OK, but can you please check too. Oh, I see now that > > start_opt is also repeated twice in the metadata, can you please > > choose one and send a patch. > > > > Thanks, > > > > Dejan > > > >> On Fri, Jun 25, 2010 at 4:22 AM, Dejan Muhamedagic <[email protected]> > >> wrote: > >> > Hi, > >> > > >> > On Thu, Jun 24, 2010 at 11:59:55AM -0600, Serge Dubrouski wrote: > >> >> Attached is an improved patch. > >> >> > >> >> 1. It parses unix_socket_directory out from PostgreSQL configuration > >> >> file and used its value as a default value of OCF_RESKEY_socketdir. > >> >> (Perl is used for that) > >> >> > >> >> 2. It creates OCF_RESKEY_socketdir if it doesn't exist otherwise > >> >> checks if PostgreSQL DBA use can create files in that directory. > >> >> > >> >> 3. If OCF_RESKEY_socketdir is set bug OCF_RESKEY_pghost isn't it uses > >> >> OCF_RESKEY_socketdir as -h parameter for psql command. Otherwise > >> >> monitor function could fail because psql doesn't tolerate non-default > >> >> socket directory and doesn't use posgresql.conf. > >> >> > >> >> 4. It replaced $(command) syntax with `command` syntax in 2 places > >> >> just to make it consistent with the rest of the script. > >> >> > >> >> Perl parsing script may seem ugly but the rules of that config file > >> >> are pretty wide open: equal sign is optional, spaces are optional, > >> >> in-line comments are allowed. > >> > > >> > Thanks for the patch. I amended some parts of it (in > >> > pgsql2.patch) and updated the validate action (pgsql3.patch). Can > >> > you please test these. > >> > > >> > Thanks, > >> > > >> > Dejan > >> > > >> >> On Thu, Jun 24, 2010 at 8:37 AM, Serge Dubrouski <[email protected]> > >> >> wrote: > >> >> > That was I though to do with one exception: > >> >> > > >> >> > check_socket_dir() { > >> >> > if [ ! -d "$1" ] > >> >> > then > >> >> > mkdir > >> >> > chmod > >> >> > chown > >> >> > else > >> >> > if ! touch $OCF_RESKEY_socketdir/test_file > >> >> > then > >> >> > ocf_log err "Can't create files in > >> >> > $OCF_RESKEY_socketdir!" > >> >> > exit > >> >> > else > >> >> > rm $OCF_RESKEY_socketdir/test_file > >> >> > fi > >> >> > fi > >> >> > > >> >> > I'll provide an appropriate patch a bit later. > >> >> > > >> >> > On Thu, Jun 24, 2010 at 8:33 AM, Dejan Muhamedagic > >> >> > <[email protected]> wrote: > >> >> >> On Wed, Jun 23, 2010 at 02:07:09PM +0200, Lars Ellenberg wrote: > >> >> >>> On Tue, Jun 22, 2010 at 10:05:10AM -0600, Serge Dubrouski wrote: > >> >> >>> > Yes it is possible to parse it from postgresql.conf of from > >> >> >>> > provided > >> >> >>> > config file. > >> >> >>> > >> >> >>> Then that should be done, IMO. > >> >> >>> Otherwise it has to be changed in two places again, > >> >> >>> and it will be forgotten in one or the other, > >> >> >>> which will lead to very hard to debug misbehaviour. > >> >> >> > >> >> >> It would indeed be better to fetch it from the configuration > >> >> >> file. However, there's also the concern that Serge mentioned on > >> >> >> directories such as /tmp which should be handled differently. > >> >> >> > >> >> >> I'd suggest to drop the parameter and get it from the > >> >> >> configuration file and create/modify the directory _only_ in case > >> >> >> the directory doesn't exist. Codewise: > >> >> >> > >> >> >> check_socket_dir() { > >> >> >> if [ ! -d "$1" ] > >> >> >> then > >> >> >> mkdir > >> >> >> chmod > >> >> >> chown > >> >> >> fi > >> >> >> } > >> >> >> > >> >> >> I think that this should be safe enough. > >> >> >> > >> >> >> Thanks, > >> >> >> > >> >> >> Dejan > >> >> >> > >> >> >>> > Attached is an improved patch with double quotes and error checks. > >> >> >>> > >> > @@ -238,6 +248,11 @@ > >> >> >>> > >> > ocf_log err "PostgreSQL can't write to the log > >> >> >>> > >> > file: $OCF_RESKEY_logfile" > >> >> >>> > >> > return $OCF_ERR_GENERIC > >> >> >>> > >> > fi > >> >> >>> > >> > + # Check if we need to create a socket directory > >> >> >>> > >> > + if [ -n "$OCF_RESKEY_socketdir" ] > >> >> >>> > >> > + then > >> >> >>> > >> > + check_socket_dir $OCF_RESKEY_socketdir > >> >> >>> > >> >> >>> double quotes missing here already, btw. > >> >> >>> and as OCF_RESKEY_socketdir is a "global" anyways, > >> >> >>> why pass it as argument to subfunctions at all? > >> >> >>> > >> >> >>> -- > >> >> >>> : Lars Ellenberg > >> >> >>> : LINBIT | Your Way to High Availability > >> >> >>> : DRBD/HA support and consulting http://www.linbit.com > >> >> >>> > >> >> >>> DRBD® and LINBIT® are registered trademarks of LINBIT, Austria. > >> >> >>> _______________________________________________________ > >> >> >>> 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/ > >> >> >> > >> >> > > >> >> > > >> >> > > >> >> > -- > >> >> > Serge Dubrouski. > >> >> > > >> >> > >> >> > >> >> > >> >> -- > >> >> Serge Dubrouski. > >> > > >> >> --- a/heartbeat/pgsql 2010-06-24 08:40:18.000000000 -0600 > >> >> +++ b/heartbeat/pgsql 2010-06-24 11:44:11.000000000 -0600 > >> >> @@ -16,6 +16,38 @@ > >> >> : ${OCF_FUNCTIONS_DIR=${OCF_ROOT}/resource.d/heartbeat} > >> >> . ${OCF_FUNCTIONS_DIR}/.ocf-shellfuncs > >> >> > >> >> +# > >> >> +# Get PostgreSQL Configuration parameter > >> >> +# > >> >> +get_pgsql_param() { > >> >> + local config_file > >> >> + local param_name > >> >> + local param_value > >> >> + > >> >> + param_name=$1 > >> >> + > >> >> + #Check that config file exists > >> >> + if [ -n $OCF_RESKEY_config ]; then > >> >> + config=$OCF_RESKEY_pgdata/postgresql.conf > >> >> + else > >> >> + config=$OCF_RESKEY_config > >> >> + fi > >> >> + > >> >> + if [ ! -f "$config" ]; then > >> >> + ocf_log err "Cannot find configuration file $config" > >> >> + exit $OCF_ERR_GENERIC > >> >> + fi > >> >> + > >> >> + perl_code="if (/^\s*$param_name[\s=]+\s*(.*)$/) { > >> >> + \$dir=\$1; > >> >> + \$dir =~ s/\s*\#.*//; > >> >> + \$dir =~ s/^'(\S*)'/\$1/; > >> >> + print \$dir;}" > >> >> + > >> >> + param_value=`cat $config | perl -ne "$perl_code"` > >> >> + echo "$param_value" > >> >> +} > >> >> + > >> >> # Defaults > >> >> OCF_RESKEY_pgctl_default=/usr/bin/pg_ctl > >> >> OCF_RESKEY_psql_default=/usr/bin/psql > >> >> @@ -27,7 +59,6 @@ > >> >> OCF_RESKEY_start_opt_default="" > >> >> OCF_RESKEY_pgdb_default=template1 > >> >> OCF_RESKEY_logfile_default=/dev/null > >> >> -OCF_RESKEY_socketdir_default="" > >> >> OCF_RESKEY_stop_escalate_default=30 > >> >> > >> >> : ${OCF_RESKEY_pgctl=${OCF_RESKEY_pgctl_default}} > >> >> @@ -40,9 +71,12 @@ > >> >> : ${OCF_RESKEY_start_opt=${OCF_RESKEY_start_opt_default}} > >> >> : ${OCF_RESKEY_pgdb=${OCF_RESKEY_pgdb_default}} > >> >> : ${OCF_RESKEY_logfile=${OCF_RESKEY_logfile_default}} > >> >> -: ${OCF_RESKEY_socketdir=${OCF_RESKEY_socketdir_default}} > >> >> : ${OCF_RESKEY_stop_escalate=${OCF_RESKEY_stop_escalate_default}} > >> >> > >> >> +# $OCF_RESKEY_pgdata has to be initialized at this momemnt > >> >> +OCF_RESKEY_socketdir_default=`get_pgsql_param unix_socket_directory` > >> >> +: ${OCF_RESKEY_socketdir=${OCF_RESKEY_socketdir_default}} > >> >> + > >> >> usage() { > >> >> cat <<EOF > >> >> usage: $0 start|stop|status|monitor|meta-data|validate-all|methods > >> >> @@ -248,10 +282,11 @@ > >> >> ocf_log err "PostgreSQL can't write to the log file: > >> >> $OCF_RESKEY_logfile" > >> >> return $OCF_ERR_GENERIC > >> >> fi > >> >> - # Check if we need to create a socket directory > >> >> + > >> >> + # Check socket directory > >> >> if [ -n "$OCF_RESKEY_socketdir" ] > >> >> then > >> >> - check_socket_dir $OCF_RESKEY_socketdir > >> >> + check_socket_dir > >> >> fi > >> >> > >> >> # Set options passed to pg_ctl > >> >> @@ -385,6 +420,10 @@ > >> >> psql_options="-p $OCF_RESKEY_pgport -U $OCF_RESKEY_pgdba > >> >> $OCF_RESKEY_pgdb" > >> >> if [ -n "$OCF_RESKEY_pghost" ]; then > >> >> psql_options="$psql_options -h $OCF_RESKEY_pghost" > >> >> + else > >> >> + if [ -n "$OCF_RESKEY_socketdir" ]; then > >> >> + psql_options="$psql_options -h $OCF_RESKEY_socketdir" > >> >> + fi > >> >> fi > >> >> runasowner "$OCF_RESKEY_psql $psql_options -c 'select now();'" > >> >> > >> >> @@ -422,7 +461,7 @@ > >> >> if [ ! -f "$1" ] > >> >> then > >> >> touch $1 > /dev/null 2>&1 > >> >> - chown $OCF_RESKEY_pgdba:$(getent passwd $OCF_RESKEY_pgdba | > >> >> cut -d ":" -f 4) $1 > >> >> + chown $OCF_RESKEY_pgdba:`getent passwd $OCF_RESKEY_pgdba | cut > >> >> -d ":" -f 4` $1 > >> >> fi > >> >> > >> >> #Check if $OCF_RESKEY_pgdba can write to the log file > >> >> @@ -434,27 +473,33 @@ > >> >> return 0 > >> >> } > >> >> > >> >> +# > >> >> # Check socket directory > >> >> +# > >> >> check_socket_dir() { > >> >> - if [ ! -d "$1" ] > >> >> - then > >> >> - if ! mkdir "$1" > >> >> + if [ ! -d "$OCF_RESKEY_socketdir" ]; then > >> >> + if ! mkdir "$OCF_RESKEY_socketdir"; then > >> >> + ocf_log err "Cannot create directory $OCF_RESKEY_socketdir" > >> >> + exit $OCF_ERR_GENERIC > >> >> + fi > >> >> + > >> >> + if ! chown $OCF_RESKEY_pgdba:`getent passwd \ > >> >> + $OCF_RESKEY_pgdba | cut -d ":" -f 4` > >> >> "$OCF_RESKEY_socketdir" > >> >> then > >> >> - ocf_log err "Cannot create directory $1" > >> >> + ocf_log err "Cannot change ownership for > >> >> $OCF_RESKEY_socketdir" > >> >> exit $OCF_ERR_GENERIC > >> >> - fi > >> >> - fi > >> >> - > >> >> - if ! chown $OCF_RESKEY_pgdba:$(getent passwd $OCF_RESKEY_pgdba | > >> >> cut -d ":" -f 4) "$1" > >> >> - then > >> >> - ocf_log err "Cannot change ownership for $1" > >> >> - exit $OCF_ERR_GENERIC > >> >> - fi > >> >> + fi > >> >> > >> >> - if ! chmod 2775 "$1" > >> >> - then > >> >> - ocf_log err "Cannot change permissions for $1" > >> >> - exit $OCF_ERR_GENERIC > >> >> + if ! chmod 2775 "$OCF_RESKEY_socketdir"; then > >> >> + ocf_log err "Cannot change permissions for > >> >> $OCF_RESKEY_socketdir" > >> >> + exit $OCF_ERR_GENERIC > >> >> + fi > >> >> + else > >> >> + if ! runasowner "touch $OCF_RESKEY_socketdir/test.$$"; then > >> >> + ocf_log err "$OCF_RESKEY_pgdba cannot create files in > >> >> $OCF_RESKEY_socketdir" > >> >> + exit $OCF_ERR_GENERIC > >> >> + fi > >> >> + rm $OCF_RESKEY_socketdir/test.$$ > >> >> fi > >> >> } > >> >> > >> > > >> >> _______________________________________________________ > >> >> 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/ > >> > > >> > > >> > >> > >> > >> -- > >> Serge Dubrouski. > >> _______________________________________________________ > >> 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/ > > > > > > -- > Serge Dubrouski. > --- a/heartbeat/pgsql 2010-06-25 11:38:56.000000000 -0600 > +++ b/heartbeat/pgsql 2010-06-25 11:44:24.000000000 -0600 > @@ -175,14 +175,6 @@ > <content type="integer" default="${OCF_RESKEY_config_default}" /> > </parameter> > > -<parameter name="start_opt" unique="0" required="0"> > -<longdesc lang="en"> > -Additional options passed to the PostgreSQL server daemon. > -</longdesc> > -<shortdesc lang="en">Additional PostgreSQL server options</shortdesc> > -<content type="string" default="${OCF_RESKEY_start_opt_default}" /> > -</parameter> > - > <parameter name="pgdb" unique="0" required="0"> > <longdesc lang="en"> > Database that will be used for monitoring. > > _______________________________________________________ > 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/
