That change is Ok. Attached is a patch to fix meta-data.
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/
