Mailing List Archive

Hi Dejan,

2012/1/31 Dejan Muhamedagic <dejan@suse.de>:
> Hi Keisuke-san,
>
> On Tue, Jan 31, 2012 at 09:52:24PM +0900, Keisuke MORI wrote:
>> Hi Dejan
>>
>> 2012/1/31 Dejan Muhamedagic <dejan@suse.de>:
>> > Hi Keisuke-san,
>> (...)
>> > On Tue, Jan 31, 2012 at 08:46:35PM +0900, Keisuke MORI wrote:
>> >> The current RA will try to check the top page (http://localhost:80)
>> >> as the default behavior if you have not enabled server-status in httpd.conf
>> >> and it would fail to start even for the apache's default test page:)
>> >
>> > Hmm, the current RA would produce an error for that URL:
>> >
>> > 488     case $STATUSURL in
>> > 489         http://*/*) ;;
>> > 490         *)
>> > 491         ocf_log err "Invalid STATUSURL $STATUSURL"
>> > 492         exit $OCF_ERR_ARGS ;;
>> > 493     esac
>>
>> Strange. That URL is generated by the RA itself.
>>
>> apache-conf.sh:
>>    119  buildlocalurl() {
>>    120    [ "x$Listen" != "x" ] &&
>>    121          echo "http://${Listen}" ||
>>    122          echo "${LOCALHOST}:${PORT}"
>>
>>
>> Probably we should relax the validation pattern, as just 'http://*' ?
>
> Agreed. I thought that the intention was to always use the status
> page, but obviously people figured out that they could skip that.
> Just as well.

Thank you for your productive comments and discussions!

As the result of the discussion regarding to this topic,
I would suggest two patches as the pull request below:
https://github.com/ClusterLabs/resource-agents/pull/54

Regards,


--
Keisuke MORI
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

On Wed, Feb 01, 2012 at 02:32:27PM +0900, Keisuke MORI wrote:
> Hi Dejan,
>
> 2012/1/31 Dejan Muhamedagic <dejan@suse.de>:
> > Hi Keisuke-san,
> >
> > On Tue, Jan 31, 2012 at 09:52:24PM +0900, Keisuke MORI wrote:
> >> Hi Dejan
> >>
> >> 2012/1/31 Dejan Muhamedagic <dejan@suse.de>:
> >> > Hi Keisuke-san,
> >> (...)
> >> > On Tue, Jan 31, 2012 at 08:46:35PM +0900, Keisuke MORI wrote:
> >> >> The current RA will try to check the top page (http://localhost:80)
> >> >> as the default behavior if you have not enabled server-status in httpd.conf
> >> >> and it would fail to start even for the apache's default test page:)
> >> >
> >> > Hmm, the current RA would produce an error for that URL:
> >> >
> >> > 488     case $STATUSURL in
> >> > 489         http://*/*) ;;
> >> > 490         *)
> >> > 491         ocf_log err "Invalid STATUSURL $STATUSURL"
> >> > 492         exit $OCF_ERR_ARGS ;;
> >> > 493     esac
> >>
> >> Strange. That URL is generated by the RA itself.
> >>
> >> apache-conf.sh:
> >>    119  buildlocalurl() {
> >>    120    [ "x$Listen" != "x" ] &&
> >>    121          echo "http://${Listen}" ||
> >>    122          echo "${LOCALHOST}:${PORT}"
> >>

apache-conf.sh actually does, in GetParams,

StatusURL=`FindLocationForHandler $1 server-status | tail -1`
STATUSURL="`buildlocalurl`$StatusURL"

Where StatusURL, as found by FindLocationForHandler <config file> server-status,
is supposed to end up being /server-status (or similar),
so STATUSURL in fact should match http://*/*

> >> Probably we should relax the validation pattern, as just 'http://*' ?
> >
> > Agreed. I thought that the intention was to always use the status

Yes, I think it is intentional.

> > page, but obviously people figured out that they could skip that.
> > Just as well.
>
> Thank you for your productive comments and discussions!
>
> As the result of the discussion regarding to this topic,
> I would suggest two patches as the pull request below:
> https://github.com/ClusterLabs/resource-agents/pull/54

We can do that anyways, but why?

--
: Lars Ellenberg
: LINBIT | Your Way to High Availability
: DRBD/HA support and consulting http://www.linbit.com
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Hi Lars,

On Wed, Feb 01, 2012 at 12:41:51PM +0100, Lars Ellenberg wrote:
> On Wed, Feb 01, 2012 at 02:32:27PM +0900, Keisuke MORI wrote:
> > Hi Dejan,
> >
> > 2012/1/31 Dejan Muhamedagic <dejan@suse.de>:
> > > Hi Keisuke-san,
> > >
> > > On Tue, Jan 31, 2012 at 09:52:24PM +0900, Keisuke MORI wrote:
> > >> Hi Dejan
> > >>
> > >> 2012/1/31 Dejan Muhamedagic <dejan@suse.de>:
> > >> > Hi Keisuke-san,
> > >> (...)
> > >> > On Tue, Jan 31, 2012 at 08:46:35PM +0900, Keisuke MORI wrote:
> > >> >> The current RA will try to check the top page (http://localhost:80)
> > >> >> as the default behavior if you have not enabled server-status in httpd.conf
> > >> >> and it would fail to start even for the apache's default test page:)
> > >> >
> > >> > Hmm, the current RA would produce an error for that URL:
> > >> >
> > >> > 488     case $STATUSURL in
> > >> > 489         http://*/*) ;;
> > >> > 490         *)
> > >> > 491         ocf_log err "Invalid STATUSURL $STATUSURL"
> > >> > 492         exit $OCF_ERR_ARGS ;;
> > >> > 493     esac
> > >>
> > >> Strange. That URL is generated by the RA itself.
> > >>
> > >> apache-conf.sh:
> > >>    119  buildlocalurl() {
> > >>    120    [ "x$Listen" != "x" ] &&
> > >>    121          echo "http://${Listen}" ||
> > >>    122          echo "${LOCALHOST}:${PORT}"
> > >>
>
> apache-conf.sh actually does, in GetParams,
>
> StatusURL=`FindLocationForHandler $1 server-status | tail -1`
> STATUSURL="`buildlocalurl`$StatusURL"
>
> Where StatusURL, as found by FindLocationForHandler <config file> server-status,
> is supposed to end up being /server-status (or similar),
> so STATUSURL in fact should match http://*/*
>
> > >> Probably we should relax the validation pattern, as just 'http://*' ?
> > >
> > > Agreed. I thought that the intention was to always use the status
>
> Yes, I think it is intentional.
>
> > > page, but obviously people figured out that they could skip that.
> > > Just as well.
> >
> > Thank you for your productive comments and discussions!
> >
> > As the result of the discussion regarding to this topic,
> > I would suggest two patches as the pull request below:
> > https://github.com/ClusterLabs/resource-agents/pull/54
>
> We can do that anyways, but why?

If we relax the checks as proposed, it is going to work for any
start web page. That seems to me to be a good idea. Now, the
problem is that in case the RA finds the server status URL in the
apache configuration, but the status module is not loaded, the
statusurl will still be set to the server status page and it
won't work. That makes the whole busines not exactly
straightforward.

Cheers,

Dejan

> --
> : Lars Ellenberg
> : LINBIT | Your Way to High Availability
> : DRBD/HA support and consulting http://www.linbit.com
> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Hi,

Any update on this?

2012/2/1 Keisuke MORI <keisuke.mori+ha@gmail.com>:
> Hi Dejan,
>
> 2012/1/31 Dejan Muhamedagic <dejan@suse.de>:
>> Hi Keisuke-san,
>>
>> On Tue, Jan 31, 2012 at 09:52:24PM +0900, Keisuke MORI wrote:
>>> Hi Dejan
>>>
>>> 2012/1/31 Dejan Muhamedagic <dejan@suse.de>:
>>> > Hi Keisuke-san,
>>> (...)
>>> > On Tue, Jan 31, 2012 at 08:46:35PM +0900, Keisuke MORI wrote:
>>> >> The current RA will try to check the top page (http://localhost:80)
>>> >> as the default behavior if you have not enabled server-status in httpd.conf
>>> >> and it would fail to start even for the apache's default test page:)
>>> >
>>> > Hmm, the current RA would produce an error for that URL:
>>> >
>>> > 488     case $STATUSURL in
>>> > 489         http://*/*) ;;
>>> > 490         *)
>>> > 491         ocf_log err "Invalid STATUSURL $STATUSURL"
>>> > 492         exit $OCF_ERR_ARGS ;;
>>> > 493     esac
>>>
>>> Strange. That URL is generated by the RA itself.
>>>
>>> apache-conf.sh:
>>>    119  buildlocalurl() {
>>>    120    [ "x$Listen" != "x" ] &&
>>>    121          echo "http://${Listen}" ||
>>>    122          echo "${LOCALHOST}:${PORT}"
>>>
>>>
>>> Probably we should relax the validation pattern, as just 'http://*' ?
>>
>> Agreed. I thought that the intention was to always use the status
>> page, but obviously people figured out that they could skip that.
>> Just as well.
>
> Thank you for your productive comments and discussions!
>
> As the result of the discussion regarding to this topic,
> I would suggest two patches as the pull request below:
> https://github.com/ClusterLabs/resource-agents/pull/54
>
> Regards,
>
>
> --
> Keisuke MORI



--
Keisuke MORI
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Hi Keisuke-san,

On Thu, Mar 01, 2012 at 11:47:50AM +0900, Keisuke MORI wrote:
> Hi,
>
> Any update on this?

No. I was reluctant, but I think that we can pull the patches.

Cheers,

Dejan

> 2012/2/1 Keisuke MORI <keisuke.mori+ha@gmail.com>:
> > Hi Dejan,
> >
> > 2012/1/31 Dejan Muhamedagic <dejan@suse.de>:
> >> Hi Keisuke-san,
> >>
> >> On Tue, Jan 31, 2012 at 09:52:24PM +0900, Keisuke MORI wrote:
> >>> Hi Dejan
> >>>
> >>> 2012/1/31 Dejan Muhamedagic <dejan@suse.de>:
> >>> > Hi Keisuke-san,
> >>> (...)
> >>> > On Tue, Jan 31, 2012 at 08:46:35PM +0900, Keisuke MORI wrote:
> >>> >> The current RA will try to check the top page (http://localhost:80)
> >>> >> as the default behavior if you have not enabled server-status in httpd.conf
> >>> >> and it would fail to start even for the apache's default test page:)
> >>> >
> >>> > Hmm, the current RA would produce an error for that URL:
> >>> >
> >>> > 488     case $STATUSURL in
> >>> > 489         http://*/*) ;;
> >>> > 490         *)
> >>> > 491         ocf_log err "Invalid STATUSURL $STATUSURL"
> >>> > 492         exit $OCF_ERR_ARGS ;;
> >>> > 493     esac
> >>>
> >>> Strange. That URL is generated by the RA itself.
> >>>
> >>> apache-conf.sh:
> >>>    119  buildlocalurl() {
> >>>    120    [ "x$Listen" != "x" ] &&
> >>>    121          echo "http://${Listen}" ||
> >>>    122          echo "${LOCALHOST}:${PORT}"
> >>>
> >>>
> >>> Probably we should relax the validation pattern, as just 'http://*' ?
> >>
> >> Agreed. I thought that the intention was to always use the status
> >> page, but obviously people figured out that they could skip that.
> >> Just as well.
> >
> > Thank you for your productive comments and discussions!
> >
> > As the result of the discussion regarding to this topic,
> > I would suggest two patches as the pull request below:
> > https://github.com/ClusterLabs/resource-agents/pull/54
> >
> > Regards,
> >
> >
> > --
> > Keisuke MORI
>
>
>
> --
> Keisuke MORI
> _______________________________________________________
> Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: Linux-HA-Dev@lists.linux-ha.org
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/