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?
On Fri, Jun 25, 2010 at 4:22 AM, Dejan Muhamedagic <deja...@fastmail.fm> 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 <serge...@gmail.com> 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 <deja...@fastmail.fm>
>> > 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: 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/
>> >>
>> >
>> >
>> >
>> > --
>> > 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: 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/
>
>
--
Serge Dubrouski.
_______________________________________________________
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/
