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/

# HG changeset patch
# User Dejan Muhamedagic <[email protected]>
# Date 1277459481 -7200
# Node ID 042797c1c99d7d47d77bbb3dc111810720911d8e
# Parent  d21398877775eedd59f983e758ccd104c9934657
Low: pgsql: a few fixes for the socketdir handling

diff -r d21398877775 -r 042797c1c99d heartbeat/pgsql
--- a/heartbeat/pgsql	Fri Jun 25 11:51:21 2010 +0200
+++ b/heartbeat/pgsql	Fri Jun 25 11:51:21 2010 +0200
@@ -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
+    if [ -n "$OCF_RESKEY_config" ]; then
+        config=$OCF_RESKEY_config
+    else
         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
+        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
# HG changeset patch
# User Dejan Muhamedagic <[email protected]>
# Date 1277461144 -7200
# Node ID d0dd5fbbd518a735432bec4ef958a2af7fdb3ef7
# Parent  042797c1c99d7d47d77bbb3dc111810720911d8e
Low: pgsql: update the validation handling

diff -r 042797c1c99d -r d0dd5fbbd518 heartbeat/pgsql
--- a/heartbeat/pgsql	Fri Jun 25 11:51:21 2010 +0200
+++ b/heartbeat/pgsql	Fri Jun 25 12:19:04 2010 +0200
@@ -441,11 +441,26 @@
     return $OCF_SUCCESS
 }
 
+check_binary2() {
+    if ! have_binary "$1"; then
+        ocf_log err "Setup problem: couldn't find command: $1"
+        return 1
+    fi
+    return 0
+}
+
 # Validate most critical parameters
 pgsql_validate_all() {
-    check_binary $OCF_RESKEY_pgctl
-    check_binary $OCF_RESKEY_psql
-    check_binary fuser
+    if ! check_binary2 "$OCF_RESKEY_pgctl" ||
+            ! check_binary2 "$OCF_RESKEY_psql" ||
+            ! check_binary2 fuser; then
+        return $OCF_ERR_INSTALLED
+    fi
+
+    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
 
     return $OCF_SUCCESS
 }
@@ -527,13 +542,15 @@
                 exit $?;;
 esac
 
-if ! pgsql_validate_all
+pgsql_validate_all
+rc=$?
+if [ $rc -ne 0 ]
 then
     case "$1" in
         stop)    exit $OCF_SUCCESS;;
         monitor) exit $OCF_NOT_RUNNING;;
         status)  exit $OCF_NOT_RUNNING;;
-        *)       exit $OCF_ERR_INSTALLED;;
+        *)       exit $rc;;
     esac
 fi
 
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Reply via email to