Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux-HA: Dev

Patch for pgsql RA

 

 

Linux-HA dev RSS feed   Index | Next | Previous | View Threaded


sergeyfd at gmail

Jun 21, 2010, 9:19 AM

Post #1 of 24 (1442 views)
Permalink
Patch for pgsql RA

Hello -

Some users reported a need for this patch to cover the situation with
non-default unix socket directory in PostgreSQL configuration.

--
Serge Dubrouski.
Attachments: pgsql.patch (1.87 KB)


dejanmm at fastmail

Jun 22, 2010, 4:16 AM

Post #2 of 24 (1407 views)
Permalink
Re: Patch for pgsql RA [In reply to]

Hi Serge,

On Mon, Jun 21, 2010 at 10:19:13AM -0600, Serge Dubrouski wrote:
> Hello -
>
> Some users reported a need for this patch to cover the situation with
> non-default unix socket directory in PostgreSQL configuration.

Why not set OCF_RESKEY_socketdir_default=/var/run/postgresql (or
whatever is its standard location)? I don't see how things could
go wrong with that.

BTW, how comes that nobody complained about this before? I
understand that cleaning up all directories under /var/run is in
LSB.

Cheers,

Dejan

> --
> Serge Dubrouski.

> --- a/heartbeat/pgsql 2010-06-21 09:41:06.000000000 -0600
> +++ b/heartbeat/pgsql 2010-06-21 10:03:04.000000000 -0600
> @@ -27,6 +27,7 @@
> 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}}
> @@ -39,6 +40,7 @@
> : ${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}}
>
> usage() {
> @@ -166,6 +168,14 @@
> <content type="string" default="${OCF_RESKEY_logfile_default}" />
> </parameter>
>
> +<parameter name="socketdir" unique="0" required="0">
> +<longdesc lang="en">
> +Unix socket directory for PostgeSQL
> +</longdesc>
> +<shortdesc lang="en">socketdir</shortdesc>
> +<content type="string" default="${OCF_RESKEY_socketdir_default}" />
> +</parameter>
> +
> <parameter name="stop_escalate" unique="0" required="0">
> <longdesc lang="en">
> Number of shutdown retries (using -m fast) before resorting to -m immediate
> @@ -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
> + fi
>
> # Set options passed to pg_ctl
> pgctl_options="$OCF_RESKEY_ctl_opt -D $OCF_RESKEY_pgdata -l $OCF_RESKEY_logfile"
> @@ -419,6 +434,17 @@
> return 0
> }
>
> +# Check socket directory
> +check_socket_dir() {
> + if [ ! -d "$1" ]
> + then
> + mkdir $1
> + fi
> +
> + chmod 2775 $1
> + chown $OCF_RESKEY_pgdba:$(getent passwd $OCF_RESKEY_pgdba | cut -d ":" -f 4) $1
> +}
> +
> #
> # 'main' starts here...
> #

> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/

_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


sergeyfd at gmail

Jun 22, 2010, 5:42 AM

Post #3 of 24 (1414 views)
Permalink
Re: Patch for pgsql RA [In reply to]

Hello Dejan -

Setting that parameter to /var/run/posgresql by default isn't correct.
The value of that parameter has to correspond to the value of
unix_socket_directory PostgreSQL configuration parameter. By default
it's /tmp, not /var/run/postgresql. That's why nobody else complained
on this problem. /tmp doesn't have to be created neither it's
ownership changed.

On Tue, Jun 22, 2010 at 5:16 AM, Dejan Muhamedagic <dejanmm [at] fastmail> wrote:
> Hi Serge,
>
> On Mon, Jun 21, 2010 at 10:19:13AM -0600, Serge Dubrouski wrote:
>> Hello -
>>
>> Some users reported a need for this patch to cover the situation with
>> non-default unix socket directory in PostgreSQL configuration.
>
> Why not set OCF_RESKEY_socketdir_default=/var/run/postgresql (or
> whatever is its standard location)? I don't see how things could
> go wrong with that.
>
> BTW, how comes that nobody complained about this before? I
> understand that cleaning up all directories under /var/run is in
> LSB.
>
> Cheers,
>
> Dejan
>
>> --
>> Serge Dubrouski.
>
>> --- a/heartbeat/pgsql 2010-06-21 09:41:06.000000000 -0600
>> +++ b/heartbeat/pgsql 2010-06-21 10:03:04.000000000 -0600
>> @@ -27,6 +27,7 @@
>>  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}}
>> @@ -39,6 +40,7 @@
>>  : ${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}}
>>
>>  usage() {
>> @@ -166,6 +168,14 @@
>>  <content type="string" default="${OCF_RESKEY_logfile_default}" />
>>  </parameter>
>>
>> +<parameter name="socketdir" unique="0" required="0">
>> +<longdesc lang="en">
>> +Unix socket directory for PostgeSQL
>> +</longdesc>
>> +<shortdesc lang="en">socketdir</shortdesc>
>> +<content type="string" default="${OCF_RESKEY_socketdir_default}" />
>> +</parameter>
>> +
>>  <parameter name="stop_escalate" unique="0" required="0">
>>  <longdesc lang="en">
>>  Number of shutdown retries (using -m fast) before resorting to -m immediate
>> @@ -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
>> +    fi
>>
>>      # Set options passed to pg_ctl
>>      pgctl_options="$OCF_RESKEY_ctl_opt -D $OCF_RESKEY_pgdata -l $OCF_RESKEY_logfile"
>> @@ -419,6 +434,17 @@
>>      return 0
>>  }
>>
>> +# Check socket directory
>> +check_socket_dir() {
>> +    if [ ! -d "$1" ]
>> +    then
>> +        mkdir $1
>> +    fi
>> +
>> +    chmod 2775 $1
>> +    chown $OCF_RESKEY_pgdba:$(getent passwd $OCF_RESKEY_pgdba | cut -d ":" -f 4) $1
>> +}
>> +
>>  #
>>  #   'main' starts here...
>>  #
>
>> _______________________________________________________
>> Linux-HA-Dev: Linux-HA-Dev [at] lists
>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>> Home Page: http://linux-ha.org/
>
> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
>



--
Serge Dubrouski.
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


lars.ellenberg at linbit

Jun 22, 2010, 5:53 AM

Post #4 of 24 (1405 views)
Permalink
Re: Patch for pgsql RA [In reply to]

On Tue, Jun 22, 2010 at 01:16:35PM +0200, Dejan Muhamedagic wrote:
> Hi Serge,
>
> On Mon, Jun 21, 2010 at 10:19:13AM -0600, Serge Dubrouski wrote:
> > Hello -
> >
> > Some users reported a need for this patch to cover the situation with
> > non-default unix socket directory in PostgreSQL configuration.
>
> Why not set OCF_RESKEY_socketdir_default=/var/run/postgresql (or
> whatever is its standard location)? I don't see how things could
> go wrong with that.
>
> BTW, how comes that nobody complained about this before? I
> understand that cleaning up all directories under /var/run is in
> LSB.
>
> Cheers,
>
> Dejan
>
> > --
> > Serge Dubrouski.
>
> > --- a/heartbeat/pgsql 2010-06-21 09:41:06.000000000 -0600
> > +++ b/heartbeat/pgsql 2010-06-21 10:03:04.000000000 -0600
> > @@ -27,6 +27,7 @@
> > OCF_RESKEY_start_opt_default=""
> > OCF_RESKEY_pgdb_default=template1
> > OCF_RESKEY_logfile_default=/dev/null
> > +OCF_RESKEY_socketdir_default=""

could it be parsed from the postgres config file instead?

> > OCF_RESKEY_stop_escalate_default=30
> >
> > : ${OCF_RESKEY_pgctl=${OCF_RESKEY_pgctl_default}}
> > @@ -39,6 +40,7 @@
> > : ${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}}
> >
> > usage() {
> > @@ -166,6 +168,14 @@
> > <content type="string" default="${OCF_RESKEY_logfile_default}" />
> > </parameter>
> >
> > +<parameter name="socketdir" unique="0" required="0">
> > +<longdesc lang="en">
> > +Unix socket directory for PostgeSQL
> > +</longdesc>
> > +<shortdesc lang="en">socketdir</shortdesc>
> > +<content type="string" default="${OCF_RESKEY_socketdir_default}" />
> > +</parameter>
> > +
> > <parameter name="stop_escalate" unique="0" required="0">
> > <longdesc lang="en">
> > Number of shutdown retries (using -m fast) before resorting to -m immediate
> > @@ -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
> > + fi
> >
> > # Set options passed to pg_ctl
> > pgctl_options="$OCF_RESKEY_ctl_opt -D $OCF_RESKEY_pgdata -l $OCF_RESKEY_logfile"
> > @@ -419,6 +434,17 @@
> > return 0
> > }
> >
> > +# Check socket directory
> > +check_socket_dir() {
> > + if [ ! -d "$1" ]
> > + then
> > + mkdir $1

please mkdir "$1"
and if mkdir fails, fail the script.

> > + fi
> > +
> > + chmod 2775 $1

"$1", add error check

> > + chown $OCF_RESKEY_pgdba:$(getent passwd $OCF_RESKEY_pgdba | cut -d ":" -f 4) $1

"$1", add error check
btw, I'd first chown, then chmod.

--
: 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 [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


sergeyfd at gmail

Jun 22, 2010, 9:05 AM

Post #5 of 24 (1408 views)
Permalink
Re: Patch for pgsql RA [In reply to]

Yes it is possible to parse it from postgresql.conf of from provided
config file. But I'd rather not do that. One can set unix_socket_dir
parameter to "/tmp" or "/var/tmp" or some other directory that
shouldn't be owned by PostgreSQL user. I mead this parameter has to be
used very carefully. There are some bug reports for PostgreSQL
indicating that not all binary tools tolerate changing of that
parameter. So I'd rather have a system/database administrator
completely understanding what he/she is doing.

Attached is an improved patch with double quotes and error checks.

On Tue, Jun 22, 2010 at 6:53 AM, Lars Ellenberg
<lars.ellenberg [at] linbit> wrote:
> On Tue, Jun 22, 2010 at 01:16:35PM +0200, Dejan Muhamedagic wrote:
>> Hi Serge,
>>
>> On Mon, Jun 21, 2010 at 10:19:13AM -0600, Serge Dubrouski wrote:
>> > Hello -
>> >
>> > Some users reported a need for this patch to cover the situation with
>> > non-default unix socket directory in PostgreSQL configuration.
>>
>> Why not set OCF_RESKEY_socketdir_default=/var/run/postgresql (or
>> whatever is its standard location)? I don't see how things could
>> go wrong with that.
>>
>> BTW, how comes that nobody complained about this before? I
>> understand that cleaning up all directories under /var/run is in
>> LSB.
>>
>> Cheers,
>>
>> Dejan
>>
>> > --
>> > Serge Dubrouski.
>>
>> > --- a/heartbeat/pgsql       2010-06-21 09:41:06.000000000 -0600
>> > +++ b/heartbeat/pgsql       2010-06-21 10:03:04.000000000 -0600
>> > @@ -27,6 +27,7 @@
>> >  OCF_RESKEY_start_opt_default=""
>> >  OCF_RESKEY_pgdb_default=template1
>> >  OCF_RESKEY_logfile_default=/dev/null
>> > +OCF_RESKEY_socketdir_default=""
>
> could it be parsed from the postgres config file instead?
>
>> >  OCF_RESKEY_stop_escalate_default=30
>> >
>> >  : ${OCF_RESKEY_pgctl=${OCF_RESKEY_pgctl_default}}
>> > @@ -39,6 +40,7 @@
>> >  : ${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}}
>> >
>> >  usage() {
>> > @@ -166,6 +168,14 @@
>> >  <content type="string" default="${OCF_RESKEY_logfile_default}" />
>> >  </parameter>
>> >
>> > +<parameter name="socketdir" unique="0" required="0">
>> > +<longdesc lang="en">
>> > +Unix socket directory for PostgeSQL
>> > +</longdesc>
>> > +<shortdesc lang="en">socketdir</shortdesc>
>> > +<content type="string" default="${OCF_RESKEY_socketdir_default}" />
>> > +</parameter>
>> > +
>> >  <parameter name="stop_escalate" unique="0" required="0">
>> >  <longdesc lang="en">
>> >  Number of shutdown retries (using -m fast) before resorting to -m immediate
>> > @@ -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
>> > +    fi
>> >
>> >      # Set options passed to pg_ctl
>> >      pgctl_options="$OCF_RESKEY_ctl_opt -D $OCF_RESKEY_pgdata -l $OCF_RESKEY_logfile"
>> > @@ -419,6 +434,17 @@
>> >      return 0
>> >  }
>> >
>> > +# Check socket directory
>> > +check_socket_dir() {
>> > +    if [ ! -d "$1" ]
>> > +    then
>> > +        mkdir $1
>
> please mkdir "$1"
> and if mkdir  fails, fail the script.
>
>> > +    fi
>> > +
>> > +    chmod 2775 $1
>
> "$1", add error check
>
>> > +    chown $OCF_RESKEY_pgdba:$(getent passwd $OCF_RESKEY_pgdba | cut -d ":" -f 4) $1
>
> "$1", add error check
> btw, I'd first chown, then chmod.
>
> --
> : 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 [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
>



--
Serge Dubrouski.
Attachments: pgsql.patch (2.21 KB)


dejanmm at fastmail

Jun 23, 2010, 4:04 AM

Post #6 of 24 (1395 views)
Permalink
Re: Patch for pgsql RA [In reply to]

Hi,

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. But I'd rather not do that. One can set unix_socket_dir
> parameter to "/tmp" or "/var/tmp" or some other directory that
> shouldn't be owned by PostgreSQL user. I mead this parameter has to be
> used very carefully. There are some bug reports for PostgreSQL
> indicating that not all binary tools tolerate changing of that
> parameter. So I'd rather have a system/database administrator
> completely understanding what he/she is doing.
>
> Attached is an improved patch with double quotes and error checks.

Patch applied.

Cheers,

Dejan

> On Tue, Jun 22, 2010 at 6:53 AM, Lars Ellenberg
> <lars.ellenberg [at] linbit> wrote:
> > On Tue, Jun 22, 2010 at 01:16:35PM +0200, Dejan Muhamedagic wrote:
> >> Hi Serge,
> >>
> >> On Mon, Jun 21, 2010 at 10:19:13AM -0600, Serge Dubrouski wrote:
> >> > Hello -
> >> >
> >> > Some users reported a need for this patch to cover the situation with
> >> > non-default unix socket directory in PostgreSQL configuration.
> >>
> >> Why not set OCF_RESKEY_socketdir_default=/var/run/postgresql (or
> >> whatever is its standard location)? I don't see how things could
> >> go wrong with that.
> >>
> >> BTW, how comes that nobody complained about this before? I
> >> understand that cleaning up all directories under /var/run is in
> >> LSB.
> >>
> >> Cheers,
> >>
> >> Dejan
> >>
> >> > --
> >> > Serge Dubrouski.
> >>
> >> > --- a/heartbeat/pgsql       2010-06-21 09:41:06.000000000 -0600
> >> > +++ b/heartbeat/pgsql       2010-06-21 10:03:04.000000000 -0600
> >> > @@ -27,6 +27,7 @@
> >> >  OCF_RESKEY_start_opt_default=""
> >> >  OCF_RESKEY_pgdb_default=template1
> >> >  OCF_RESKEY_logfile_default=/dev/null
> >> > +OCF_RESKEY_socketdir_default=""
> >
> > could it be parsed from the postgres config file instead?
> >
> >> >  OCF_RESKEY_stop_escalate_default=30
> >> >
> >> >  : ${OCF_RESKEY_pgctl=${OCF_RESKEY_pgctl_default}}
> >> > @@ -39,6 +40,7 @@
> >> >  : ${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}}
> >> >
> >> >  usage() {
> >> > @@ -166,6 +168,14 @@
> >> >  <content type="string" default="${OCF_RESKEY_logfile_default}" />
> >> >  </parameter>
> >> >
> >> > +<parameter name="socketdir" unique="0" required="0">
> >> > +<longdesc lang="en">
> >> > +Unix socket directory for PostgeSQL
> >> > +</longdesc>
> >> > +<shortdesc lang="en">socketdir</shortdesc>
> >> > +<content type="string" default="${OCF_RESKEY_socketdir_default}" />
> >> > +</parameter>
> >> > +
> >> >  <parameter name="stop_escalate" unique="0" required="0">
> >> >  <longdesc lang="en">
> >> >  Number of shutdown retries (using -m fast) before resorting to -m immediate
> >> > @@ -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
> >> > +    fi
> >> >
> >> >      # Set options passed to pg_ctl
> >> >      pgctl_options="$OCF_RESKEY_ctl_opt -D $OCF_RESKEY_pgdata -l $OCF_RESKEY_logfile"
> >> > @@ -419,6 +434,17 @@
> >> >      return 0
> >> >  }
> >> >
> >> > +# Check socket directory
> >> > +check_socket_dir() {
> >> > +    if [ ! -d "$1" ]
> >> > +    then
> >> > +        mkdir $1
> >
> > please mkdir "$1"
> > and if mkdir  fails, fail the script.
> >
> >> > +    fi
> >> > +
> >> > +    chmod 2775 $1
> >
> > "$1", add error check
> >
> >> > +    chown $OCF_RESKEY_pgdba:$(getent passwd $OCF_RESKEY_pgdba | cut -d ":" -f 4) $1
> >
> > "$1", add error check
> > btw, I'd first chown, then chmod.
> >
> > --
> > : 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 [at] lists
> > http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> > Home Page: http://linux-ha.org/
> >
>
>
>
> --
> Serge Dubrouski.

> --- a/heartbeat/pgsql 2010-06-22 08:58:33.000000000 -0600
> +++ b/heartbeat/pgsql 2010-06-22 09:51:48.000000000 -0600
> @@ -27,6 +27,7 @@
> 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}}
> @@ -39,6 +40,7 @@
> : ${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}}
>
> usage() {
> @@ -166,6 +168,14 @@
> <content type="string" default="${OCF_RESKEY_logfile_default}" />
> </parameter>
>
> +<parameter name="socketdir" unique="0" required="0">
> +<longdesc lang="en">
> +Unix socket directory for PostgeSQL
> +</longdesc>
> +<shortdesc lang="en">socketdir</shortdesc>
> +<content type="string" default="${OCF_RESKEY_socketdir_default}" />
> +</parameter>
> +
> <parameter name="stop_escalate" unique="0" required="0">
> <longdesc lang="en">
> Number of shutdown retries (using -m fast) before resorting to -m immediate
> @@ -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
> + fi
>
> # Set options passed to pg_ctl
> pgctl_options="$OCF_RESKEY_ctl_opt -D $OCF_RESKEY_pgdata -l $OCF_RESKEY_logfile"
> @@ -419,6 +434,30 @@
> return 0
> }
>
> +# Check socket directory
> +check_socket_dir() {
> + if [ ! -d "$1" ]
> + then
> + if ! mkdir "$1"
> + then
> + ocf_log err "Cannot create directory $1"
> + 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
> +
> + if ! chmod 2775 "$1"
> + then
> + ocf_log err "Cannot change permissions for $1"
> + exit $OCF_ERR_GENERIC
> + fi
> +}
> +
> #
> # 'main' starts here...
> #

> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/

_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


lars.ellenberg at linbit

Jun 23, 2010, 5:07 AM

Post #7 of 24 (1393 views)
Permalink
Re: Patch for pgsql RA [In reply to]

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.

> 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 [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


dejanmm at fastmail

Jun 24, 2010, 7:33 AM

Post #8 of 24 (1364 views)
Permalink
Re: Patch for pgsql RA [In reply to]

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 [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


sergeyfd at gmail

Jun 24, 2010, 7:37 AM

Post #9 of 24 (1361 views)
Permalink
Re: Patch for pgsql RA [In reply to]

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 <dejanmm [at] fastmail> 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 [at] lists
>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>> Home Page: http://linux-ha.org/
> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
>



--
Serge Dubrouski.
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


sergeyfd at gmail

Jun 24, 2010, 10:59 AM

Post #10 of 24 (1357 views)
Permalink
Re: Patch for pgsql RA [In reply to]

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.

On Thu, Jun 24, 2010 at 8:37 AM, Serge Dubrouski <sergeyfd [at] gmail> 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 <dejanmm [at] fastmail> 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 [at] lists
>>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>>> Home Page: http://linux-ha.org/
>> _______________________________________________________
>> Linux-HA-Dev: Linux-HA-Dev [at] lists
>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>> Home Page: http://linux-ha.org/
>>
>
>
>
> --
> Serge Dubrouski.
>



--
Serge Dubrouski.
Attachments: pgsql.patch (4.42 KB)


dejanmm at fastmail

Jun 25, 2010, 3:22 AM

Post #11 of 24 (1350 views)
Permalink
Re: Patch for pgsql RA [In reply to]

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 <sergeyfd [at] gmail> 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 <dejanmm [at] fastmail> 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 [at] lists
> >>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >>> Home Page: http://linux-ha.org/
> >> _______________________________________________________
> >> Linux-HA-Dev: Linux-HA-Dev [at] lists
> >> 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 [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
Attachments: pgsql2.patch (1.20 KB)
  pgsql3.patch (1.50 KB)


lars.ellenberg at linbit

Jun 25, 2010, 3:58 AM

Post #12 of 24 (1355 views)
Permalink
Re: Patch for pgsql RA [In reply to]

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)

sed -n -e 's/#.*$//; s/[[:space:]]*$//;' \
-e 's/^[[:space:]]*'"$parameter"'[[:space:]=]\+[[:space:]]*//p;' postgresql.conf |
tail -n1
should do it, too.

or head -n1?
if it is mentioned two times, which occurence wins?


> 2. It creates OCF_RESKEY_socketdir if it doesn't exist otherwise
> checks if PostgreSQL DBA use can create files in that directory.

Actually I don't exactly get it why it was the job of the RA
to create that directory. If the admin puts it into the
postgresql.conf, he can create the directory just as well.
The RA does not create the data dir, either ;-)

But if it does make sense for some setups, fine.

--
: 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 [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


dejanmm at fastmail

Jun 25, 2010, 4:49 AM

Post #13 of 24 (1355 views)
Permalink
Re: Patch for pgsql RA [In reply to]

On Fri, Jun 25, 2010 at 12:58:15PM +0200, Lars Ellenberg wrote:
> 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)
>
> sed -n -e 's/#.*$//; s/[[:space:]]*$//;' \
> -e 's/^[[:space:]]*'"$parameter"'[[:space:]=]\+[[:space:]]*//p;' postgresql.conf |
> tail -n1
> should do it, too.

Isn't then the last s command enough?

> or head -n1?
> if it is mentioned two times, which occurence wins?

>
> > 2. It creates OCF_RESKEY_socketdir if it doesn't exist otherwise
> > checks if PostgreSQL DBA use can create files in that directory.
>
> Actually I don't exactly get it why it was the job of the RA
> to create that directory. If the admin puts it into the
> postgresql.conf, he can create the directory just as well.
> The RA does not create the data dir, either ;-)
>
> But if it does make sense for some setups, fine.

I believe that it was in case the directory was in /var/run/
which is cleaned on boot.

Cheers,

Dejan

> --
> : 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 [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


lars.ellenberg at linbit

Jun 25, 2010, 4:56 AM

Post #14 of 24 (1350 views)
Permalink
Re: Patch for pgsql RA [In reply to]

On Fri, Jun 25, 2010 at 01:49:32PM +0200, Dejan Muhamedagic wrote:
> On Fri, Jun 25, 2010 at 12:58:15PM +0200, Lars Ellenberg wrote:
> > 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)
> >
> > sed -n -e 's/#.*$//; s/[[:space:]]*$//;' \
> > -e 's/^[[:space:]]*'"$parameter"'[[:space:]=]\+[[:space:]]*//p;' postgresql.conf |
> > tail -n1
> > should do it, too.
>
> Isn't then the last s command enough?

First strip optional inline comments and possibly trailing whitespace.
Yes, you could put it in one s///, with s/...()(|)/\1/p, but
that won't get more readable.

> I believe that it was in case the directory was in /var/run/
> which is cleaned on boot.

Bad choice, then ;-)
Choose differently, or re-create it on boot.
Ah well, or patch the resource agent.
Whatever works best.

--
: 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 [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


sergeyfd at gmail

Jun 25, 2010, 7:16 AM

Post #15 of 24 (1345 views)
Permalink
Re: Patch for pgsql RA [In reply to]

On Fri, Jun 25, 2010 at 4:58 AM, Lars Ellenberg
<lars.ellenberg [at] linbit> wrote:
> 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)
>
> sed -n  -e 's/#.*$//; s/[[:space:]]*$//;' \
>        -e 's/^[[:space:]]*'"$parameter"'[[:space:]=]\+[[:space:]]*//p;' postgresql.conf |
>        tail -n1
> should do it, too.
>
> or head -n1?
> if it is mentioned two times, which occurence wins?
>
>
>> 2. It creates OCF_RESKEY_socketdir if it doesn't exist otherwise
>> checks if PostgreSQL DBA use can create files in that directory.
>
> Actually I don't exactly get it why it was the job of the RA
> to create that directory.  If the admin puts it into the
> postgresql.conf, he can create the directory just as well.
> The RA does not create the data dir, either ;-)

The problem exists in Ubuntu distro. As far as I understand developers
of that distro desided to change default value for
unix_socket_directory from "/tmp" for "/var/run/postgresql".
"/var/run" there is tempfs type and gets cleaned after each reboot. I
think that lsb script takes care of the situation there for regular
non-clustered setup.
>
> But if it does make sense for some setups, fine.
>
> --
> : 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 [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
>



--
Serge Dubrouski.
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


sergeyfd at gmail

Jun 25, 2010, 7:22 AM

Post #16 of 24 (1342 views)
Permalink
Re: Patch for pgsql RA [In reply to]

On Fri, Jun 25, 2010 at 5:56 AM, Lars Ellenberg
<lars.ellenberg [at] linbit> wrote:
> On Fri, Jun 25, 2010 at 01:49:32PM +0200, Dejan Muhamedagic wrote:
>> On Fri, Jun 25, 2010 at 12:58:15PM +0200, Lars Ellenberg wrote:
>> > 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)
>> >
>> > sed -n  -e 's/#.*$//; s/[[:space:]]*$//;' \
>> >     -e 's/^[[:space:]]*'"$parameter"'[[:space:]=]\+[[:space:]]*//p;' postgresql.conf |
>> >     tail -n1
>> > should do it, too.
>>
>> Isn't then the last s command enough?
>
> First strip optional inline comments and possibly trailing whitespace.
> Yes, you could put it in one s///, with s/...()(|)/\1/p, but
> that won't get more readable.
>
>> I believe that it was in case the directory was in /var/run/
>> which is cleaned on boot.
>
> Bad choice, then ;-)
> Choose differently, or re-create it on boot.
> Ah well, or patch the resource agent.
> Whatever works best.

It's not a users choice I'm afraid. Changing that parameter in
postgresql.conf from it's default value creates more problems for DBA
than gives advantages since psql tool doesn't tolerate such change and
one have to provide the "non-default" value as a command line
parameter. So as I said earlier I think that Ubuntu developers for
some reason made that choice for their users. Now our user will have
to take care of that bu setting additional OCF parameter since. Since
it's a default value for that distro its value isn't explicitly set in
postgresql.conf and our parsing mechanism won't help.

>
> --
> : 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 [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
>



--
Serge Dubrouski.
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


sergeyfd at gmail

Jun 25, 2010, 7:28 AM

Post #17 of 24 (1343 views)
Permalink
Re: Patch for pgsql RA [In reply to]

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 <dejanmm [at] fastmail> 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 <sergeyfd [at] gmail> 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 <dejanmm [at] fastmail> 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 [at] lists
>> >>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>> >>> Home Page: http://linux-ha.org/
>> >> _______________________________________________________
>> >> Linux-HA-Dev: Linux-HA-Dev [at] lists
>> >> 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 [at] lists
>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>> Home Page: http://linux-ha.org/
>
>
> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
>
>



--
Serge Dubrouski.
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


sergeyfd at gmail

Jun 25, 2010, 8:32 AM

Post #18 of 24 (1332 views)
Permalink
Re: Patch for pgsql RA [In reply to]

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 <sergeyfd [at] gmail> 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 <dejanmm [at] fastmail> 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 <sergeyfd [at] gmail> 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 <dejanmm [at] fastmail> 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 [at] lists
>>> >>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>>> >>> Home Page: http://linux-ha.org/
>>> >> _______________________________________________________
>>> >> Linux-HA-Dev: Linux-HA-Dev [at] lists
>>> >> 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 [at] lists
>>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>>> Home Page: http://linux-ha.org/
>>
>>
>> _______________________________________________________
>> Linux-HA-Dev: Linux-HA-Dev [at] lists
>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>> Home Page: http://linux-ha.org/
>>
>>
>
>
>
> --
> Serge Dubrouski.
>



--
Serge Dubrouski.
Attachments: pgsql2.patch (1.29 KB)


dejanmm at fastmail

Jun 25, 2010, 10:31 AM

Post #19 of 24 (1328 views)
Permalink
Re: Patch for pgsql RA [In reply to]

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 <dejanmm [at] fastmail> 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 <sergeyfd [at] gmail> 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 <dejanmm [at] fastmail> 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 [at] lists
> >> >>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >> >>> Home Page: http://linux-ha.org/
> >> >> _______________________________________________________
> >> >> Linux-HA-Dev: Linux-HA-Dev [at] lists
> >> >> 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 [at] lists
> >> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >> Home Page: http://linux-ha.org/
> >
> >
> > _______________________________________________________
> > Linux-HA-Dev: Linux-HA-Dev [at] lists
> > http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> > Home Page: http://linux-ha.org/
> >
> >
>
>
>
> --
> Serge Dubrouski.
> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


dejanmm at fastmail

Jun 25, 2010, 10:46 AM

Post #20 of 24 (1329 views)
Permalink
Re: Patch for pgsql RA [In reply to]

On Fri, Jun 25, 2010 at 09:32:40AM -0600, Serge Dubrouski wrote:
> 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.

I reintroduced the _default again, since it's referenced in the
metadata. Everything pushed to the repository now. If you find
more issues please send the patch against the repository.

> 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

Thanks! It took me like 5 minutes to figure out where's the typo.

Cheers,

Dejan

> On Fri, Jun 25, 2010 at 8:28 AM, Serge Dubrouski <sergeyfd [at] gmail> 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 <dejanmm [at] fastmail> 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 <sergeyfd [at] gmail> 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 <dejanmm [at] fastmail> 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 [at] lists
> >>> >>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >>> >>> Home Page: http://linux-ha.org/
> >>> >> _______________________________________________________
> >>> >> Linux-HA-Dev: Linux-HA-Dev [at] lists
> >>> >> 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 [at] lists
> >>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >>> Home Page: http://linux-ha.org/
> >>
> >>
> >> _______________________________________________________
> >> Linux-HA-Dev: Linux-HA-Dev [at] lists
> >> 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: Linux-HA-Dev [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/

_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


sergeyfd at gmail

Jun 25, 2010, 10:48 AM

Post #21 of 24 (1326 views)
Permalink
Re: Patch for pgsql RA [In reply to]

That change is Ok. Attached is a patch to fix meta-data.

On Fri, Jun 25, 2010 at 11:31 AM, Dejan Muhamedagic <dejanmm [at] fastmail> 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 <dejanmm [at] fastmail> 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 <sergeyfd [at] gmail> 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 <dejanmm [at] fastmail> 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 [at] lists
>> >> >>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>> >> >>> Home Page: http://linux-ha.org/
>> >> >> _______________________________________________________
>> >> >> Linux-HA-Dev: Linux-HA-Dev [at] lists
>> >> >> 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 [at] lists
>> >> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>> >> Home Page: http://linux-ha.org/
>> >
>> >
>> > _______________________________________________________
>> > Linux-HA-Dev: Linux-HA-Dev [at] lists
>> > http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>> > Home Page: http://linux-ha.org/
>> >
>> >
>>
>>
>>
>> --
>> Serge Dubrouski.
>> _______________________________________________________
>> Linux-HA-Dev: Linux-HA-Dev [at] lists
>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
>> Home Page: http://linux-ha.org/
> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
>



--
Serge Dubrouski.
Attachments: pgsql4.patch (0.63 KB)


dejanmm at fastmail

Jun 25, 2010, 10:54 AM

Post #22 of 24 (1328 views)
Permalink
Re: Patch for pgsql RA [In reply to]

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 <dejanmm [at] fastmail> 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 <dejanmm [at] fastmail> 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 <sergeyfd [at] gmail> 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 <dejanmm [at] fastmail> 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 [at] lists
> >> >> >>> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >> >> >>> Home Page: http://linux-ha.org/
> >> >> >> _______________________________________________________
> >> >> >> Linux-HA-Dev: Linux-HA-Dev [at] lists
> >> >> >> 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 [at] lists
> >> >> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >> >> Home Page: http://linux-ha.org/
> >> >
> >> >
> >> > _______________________________________________________
> >> > Linux-HA-Dev: Linux-HA-Dev [at] lists
> >> > http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >> > Home Page: http://linux-ha.org/
> >> >
> >> >
> >>
> >>
> >>
> >> --
> >> Serge Dubrouski.
> >> _______________________________________________________
> >> Linux-HA-Dev: Linux-HA-Dev [at] lists
> >> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> >> Home Page: http://linux-ha.org/
> > _______________________________________________________
> > Linux-HA-Dev: Linux-HA-Dev [at] lists
> > 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: Linux-HA-Dev [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/

_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


lars.ellenberg at linbit

Jun 28, 2010, 1:41 AM

Post #23 of 24 (1299 views)
Permalink
Re: Patch for pgsql RA [In reply to]

On Fri, Jun 25, 2010 at 08:22:35AM -0600, Serge Dubrouski wrote:
> On Fri, Jun 25, 2010 at 5:56 AM, Lars Ellenberg
> <lars.ellenberg [at] linbit> wrote:
> > On Fri, Jun 25, 2010 at 01:49:32PM +0200, Dejan Muhamedagic wrote:
> >> On Fri, Jun 25, 2010 at 12:58:15PM +0200, Lars Ellenberg wrote:
> >> > 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)
> >> >
> >> > sed -n  -e 's/#.*$//; s/[[:space:]]*$//;' \
> >> >     -e 's/^[[:space:]]*'"$parameter"'[[:space:]=]\+[[:space:]]*//p;' postgresql.conf |
> >> >     tail -n1
> >> > should do it, too.
> >>
> >> Isn't then the last s command enough?
> >
> > First strip optional inline comments and possibly trailing whitespace.
> > Yes, you could put it in one s///, with s/...()(|)/\1/p, but
> > that won't get more readable.
> >
> >> I believe that it was in case the directory was in /var/run/
> >> which is cleaned on boot.
> >
> > Bad choice, then ;-)
> > Choose differently, or re-create it on boot.
> > Ah well, or patch the resource agent.
> > Whatever works best.
>
> It's not a users choice I'm afraid. Changing that parameter in
> postgresql.conf from it's default value creates more problems for DBA
> than gives advantages since psql tool doesn't tolerate such change and
> one have to provide the "non-default" value as a command line
> parameter. So as I said earlier I think that Ubuntu developers for
> some reason made that choice for their users. Now our user will have
> to take care of that bu setting additional OCF parameter since. Since
> it's a default value for that distro its value isn't explicitly set in
> postgresql.conf and our parsing mechanism won't help.

FUBAR :(
any way to ask postgres about its hardcoded defaults?
sort of like "postconf" for postfix?

Still I don't think it belongs into the RA.
This is needed once after reboot,
not for every start/stop/monitor of the RA.
Rather put a line into rc.local,
or into /etc/default/pacemaker (yes, that's shell code,
and it is sourced from the heartbeat and corosync init scripts).

--
: 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 [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/


dejanmm at fastmail

Jun 28, 2010, 2:15 AM

Post #24 of 24 (1296 views)
Permalink
Re: Patch for pgsql RA [In reply to]

Hi,

On Mon, Jun 28, 2010 at 10:41:47AM +0200, Lars Ellenberg wrote:
> On Fri, Jun 25, 2010 at 08:22:35AM -0600, Serge Dubrouski wrote:
> > On Fri, Jun 25, 2010 at 5:56 AM, Lars Ellenberg
> > <lars.ellenberg [at] linbit> wrote:
> > > On Fri, Jun 25, 2010 at 01:49:32PM +0200, Dejan Muhamedagic wrote:
> > >> On Fri, Jun 25, 2010 at 12:58:15PM +0200, Lars Ellenberg wrote:
> > >> > 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)
> > >> >
> > >> > sed -n  -e 's/#.*$//; s/[[:space:]]*$//;' \
> > >> >     -e 's/^[[:space:]]*'"$parameter"'[[:space:]=]\+[[:space:]]*//p;' postgresql.conf |
> > >> >     tail -n1
> > >> > should do it, too.
> > >>
> > >> Isn't then the last s command enough?
> > >
> > > First strip optional inline comments and possibly trailing whitespace.
> > > Yes, you could put it in one s///, with s/...()(|)/\1/p, but
> > > that won't get more readable.
> > >
> > >> I believe that it was in case the directory was in /var/run/
> > >> which is cleaned on boot.
> > >
> > > Bad choice, then ;-)
> > > Choose differently, or re-create it on boot.
> > > Ah well, or patch the resource agent.
> > > Whatever works best.
> >
> > It's not a users choice I'm afraid. Changing that parameter in
> > postgresql.conf from it's default value creates more problems for DBA
> > than gives advantages since psql tool doesn't tolerate such change and
> > one have to provide the "non-default" value as a command line
> > parameter. So as I said earlier I think that Ubuntu developers for
> > some reason made that choice for their users. Now our user will have
> > to take care of that bu setting additional OCF parameter since. Since
> > it's a default value for that distro its value isn't explicitly set in
> > postgresql.conf and our parsing mechanism won't help.
>
> FUBAR :(
> any way to ask postgres about its hardcoded defaults?
> sort of like "postconf" for postfix?
>
> Still I don't think it belongs into the RA.
> This is needed once after reboot,
> not for every start/stop/monitor of the RA.

It should be only on start.

> Rather put a line into rc.local,
> or into /etc/default/pacemaker (yes, that's shell code,
> and it is sourced from the heartbeat and corosync init scripts).

It is more logical to do that, but requires less management
effort to have it in the RA. Updating files in a cluster requires
discipline of a kind which is usually lacking.

Cheers,

Dejan

> --
> : 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 [at] lists
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev [at] lists
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Linux-HA dev RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.