Dejan -
After a second thought I think that your way of handling checking
binaries is more appropriate since it allows that "case" statement
down the script work properly. I'd like to add a small change to
patch2 though. See attached.
pgsql3.patch has a mistypo:
+ if [ -n "$OCF_RESKEY_config" -a ! -f "$OCF_RESKEY_config" ]; then
+ ocf_log err "the configuration file $OCF_RESKEY_config doesn't exist"
+ return $OCF_ERR_INSTALLED
+ if
Should be
+ if [ -n "$OCF_RESKEY_config" -a ! -f "$OCF_RESKEY_config" ]; then
+ ocf_log err "the configuration file $OCF_RESKEY_config doesn't exist"
+ return $OCF_ERR_INSTALLED
+ fi
On Fri, Jun 25, 2010 at 8:28 AM, Serge Dubrouski <[email protected]> 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?
>
>
> 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.
>
--
Serge Dubrouski.
--- a/heartbeat/pgsql 2010-06-25 09:22:42.000000000 -0600
+++ b/heartbeat/pgsql 2010-06-25 09:22:29.000000000 -0600
@@ -22,20 +22,19 @@
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
+ if [ -n "$OCF_RESKEY_config" ]; then
config=$OCF_RESKEY_config
+ else
+ config=$OCF_RESKEY_pgdata/postgresql.conf
fi
if [ ! -f "$config" ]; then
ocf_log err "Cannot find configuration file $config"
- exit $OCF_ERR_GENERIC
+ return
fi
perl_code="if (/^\s*$param_name[\s=]+\s*(.*)$/) {
@@ -44,8 +43,7 @@
\$dir =~ s/^'(\S*)'/\$1/;
print \$dir;}"
- param_value=`cat $config | perl -ne "$perl_code"`
- echo "$param_value"
+ perl -ne "$perl_code" < $config
}
# Defaults
@@ -74,8 +72,7 @@
: ${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}}
+: ${OCF_RESKEY_socketdir=`get_pgsql_param unix_socket_directory`}
usage() {
cat <<EOF
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
