Hi,
On Wed, Mar 31, 2010 at 01:12:34AM +0200, Dejan Muhamedagic wrote:
> Hi,
>
> On Tue, Mar 30, 2010 at 11:54:07PM +0200, Helmut Weymann wrote:
> > Hi Dejan,
> >
> > this mail consists of two parts. The first part are the answers for your
> > questions. The second part is another proposal to redesign this plugin.
> > That
> > proposal is based on your changes and some brainwork.
> >
> > Here the answers to your questions and some simple comment:
> >
> > During the review I detected lines 215 to 219, which are superfluous
> > comments.
> > They are still referencing the no longer needed device-name.
>
> OK.
>
> > Am Dienstag, 30. März 2010 schrieb Dejan Muhamedagic:
> > > Hi,
> > >
> > > Sorry for the delay, finally found some time to take a look again
> > > at your plugin.
> > >
> > > - in get_cookie_from_device and get_data_from_device there is:
> > >
> > > if grep -qs "Invalid User name or Password" $MY_PATH/$MY_TEMPFILE
> > > if grep -qs "Cookie Time Out" $MY_PATH/$PORT_STATUS
> > >
> > > Can we turn the logic around and look for a string one would
> > > expect to be found? There could probably be more different
> > > errors.
> >
> > If the device ip-address is OK you will either get the expected page,
> > invalid
> > login or cookie timeout.
> >
> > Whenever the script is called by stonithd the first call is for case branch
> > status).
> >
> > within the case branch status) the procedure get_http_status is called.
> > This call is the final validation of ip-address, username, password and
> > responsiveness of the device. And the response does always match the
> > defined
> > pattern.
>
> Right. There is a slim chance that if something goes wrong
> beforehand the plugin would be able to get the list of ports.
>
> > > - in get_http_status
> > >
> > >
> > > pattern="P60=[01],P61=[01],P62=[01],P63=[01],P64=[01],P65=[01],P66=[01],P67
> > >= [01]"
> > >
> > > Aren't there also 4-port devices? Would they return the same?
> >
> > I have the 4-port version and the procedure works. This adheres to the
> > device
> > documentation: The response does always match the defined pattern.
>
> OK.
>
> > > - on hostlist: How about _always_ checking if hostnames defined
> > > in the device match the provided hostlist. That way, there is
> > > no need to have holes in the list. Since the names seem to be
> > > limited to 16 chars, we should also cut the domain name part
> > > from the node name. I actually implemented that already, the
> > > new script attached.
> >
> > Host names within the cluster (node-names) are always(?) without the
> > domain.
>
> Not always, though that practice is discouraged, sometimes people
> use FQDN names.
>
> > Yes, we are interchanging the meaning of hostname and nodename. The domain-
> > name of cluster-nodes is actually needed nowhere in the CIB. The only
> > reference to a domain could be in the stonith-device-ip-address (ip-address
> > or
> > hostname in the sense of DNS).
>
> In this case the name is just a string which has to match what is
> defined in the device.
>
> > Your debug message
> > $LOG_DEBUG "Got new hostlist ($hostlist) from $deviceip"
> > inside build_device_hostlist() should probably be
> > $LOG_DEBUG "Got new hostlist ($device_hostlist) from $deviceip"
> > or
> > $LOG_DEBUG "Got new port-map ($device_hostlist) from $deviceip"
>
> Yes.
>
> > Proposal for adjusted procedure flow.
> >
> > Reason for the proposed re-design is based on these thoughts and
> > definitions:
> >
> > The pacemaker CRM database needs to know the cluster-nodes which are
> > controlled by the device. That information shall be provided via the
> > parameter
> > "hostlist". This hostlist is a list of cluster-node-names. In order to
> > avoid
> > further mis-interpretations we might want to change the parameter name to
> > "nodelist". Its your decision.
>
> hostlist is, for better or worse, used in all other stonith
> plugins.
>
> > In any case this list must not be empty. The
> > information is needed by pacemaker. It must be in the CIB.
>
> It's not needed by pacemaker. It is needed by stonithd. But
> that's what it finds out by invoking gethosts.
>
> > The device command "IPSetPower" needs the port-number to control the power-
> > outlet. The plugin must know the mapping of node-names to port-numbers. The
> > device allows to assign a 16 character name to each of the power-outlets.
> > These port-names - you used device_hostlist - should define the mapping of
> > cluster nodes to power outlets. For that purpose the device should have
> > exactly one port for each node in the hostlist/nodelist with a matching
> > port-
> > name. An alternative would be to add an additional parameter defining the
> > device-port-number for each node.
> >
> > The plugin command status could conduct two tests: get_http_status() and
> > compare the hostlist/nodelist from the CIB with the port-names from the
> > device. Corresponding messages could report the result as status to
> > pacemaker.
>
> Right.
>
> > From this introduction we have two questions left:
> >
> > Shall we just use the device to get a mapping between ports and
> > cluster-nodes
> > or do we want to allow an independent definition of that mapping via an
> > addditional parameter?
>
> No, we get the mapping from the device since it's already
> available there. Otherwise, the parameter list (the resource
> configuration) is going to get ugly.
>
> > Shall the status command of the plugin validate the hostlist/nodelist
> > against
> > the device-port-names?
>
> Yes. The status is actually part of the resource start operation.
> It should verify that the defined hostlist is a subset of the
> list we get from the device.
>
> > With regard to the flow of the procedure these definitions make the
> > following
> > difference:
> >
> > We have three types of commands $1 for the plugin:
> >
> > 1. Requests for Meta data:
> >
> > getconfignames)
> > getinfo-devid)
> > getinfo-devname)
> > getinfo-devdescr)
> > getinfo-devurl)
> > getinfo-xml)
> > gethosts)
> >
> > These commands dont need any interaction with the actual device.
>
> gethosts does, because the hostlist is optional. Alternatively,
> the hostlist may be required as well. I don't really have a
> preference.
>
> > 2. Call for status
> >
> > status)
> >
> > The device status is fully detected by the call to get_http_status().
> > It is implemented by the documented web-command "IPGetPower".
> > We may want to include the validation of node-names in this call.
>
> Definitely.
>
> > 3. Power control commands
> >
> > on)
> > off)
> > reset)
> >
> > These commands are implemented by the documented web-command "IPSetPower".
> > They require a valid mapping between cluster-nodes and device-ports.
> > stonithd does always send two commands to the device. The first command
> > issued
> > is status which is followed by the actual required control function.
> >
> >
> > Comparing these design principles with the code you did attach, I'm
> > convinced
> > this is exactly what you wanted to achieve. And I think I know your answers
> > for the two questions: The device shall define the mapping and status)
> > shall
> > also validate node-names against the port-names.
>
> Yes.
>
> > The difference between these principles and the current code is the
> > handling
> > of the hostlist.
> >
> > If the plugin does update the CIB with the hostlist it gets
> > from the device, your code does achieve this. But there is a risk in
> > reporting
> > that list as hostlist: What happens if the list includes a port-name which
> > is
> > not a member of the cluster? Might pacemaker get confused?
>
> No, but anyway the non-assigned ports shouldn't be reported.
>
> > procedure name2port() should probably translate hostnames using
> > device_hostlist instead of hostlist.
>
> Right.
>
> I can see now that the code is not right in another place, as you
> noticed above:
>
> hostlist="$device_hostlist"
>
> That should be replaced by a filter which would weed out the
> non-assigned ports.
The attached script should have all issues resolved. I'll push
it to the repository. Can you please test it with your device.
Cheers,
Dejan
> Cheers,
>
> Dejan
>
> > Regards,
> >
> > Helmut
> _______________________________________________________
> Linux-HA-Dev: [email protected]
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
#!/bin/sh
#
# External STONITH module using IP Power 9258 or compatible devices.
#
# Copyright (c) 2010 Helmut Weymann (Helmut (at) h-weymann (dot) de)
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of version 2 of the GNU General Public License as
# published by the Free Software Foundation.
#
# This program is distributed in the hope that it would be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
#
# Further, this software is distributed without any warranty that it is
# free of the rightful claim of any third person regarding infringement
# or the like. Any license provided herein, whether implied or
# otherwise, applies only to this software file. Patent licenses, if
# any, provided herein do not apply to combinations of this program with
# other software, or any other product whatsoever.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write the Free Software Foundation,
# Inc., 59 Temple Place - Suite 330, Boston MA 02111-1307, USA.
#
#
# Basic commands & parameters independent from individual device
DEVICE="IP Power 9258"
IPPowerOn="1"
IPPowerOff="0"
IPGetPower="Set.cmd?CMD=GetPower"
IPSetPower="Set.cmd?CMD=SetPower"
IPPort_name="P"
IPPort0=60
HTTP_COMMAND="wget -q -O - --"
LOG_ERROR="ha_log.sh err"
LOG_WARNING="ha_log.sh warn"
LOG_INFO="ha_log.sh info"
LOG_DEBUG="ha_log.sh debug"
MY_COOKIES="cookies.txt"
MY_TEMPFILE="temp.htm"
PORT_STATUS="iocontrol.htm"
UNDEFINED_HOSTNAME="*not-defined*"
#
# check MY_ROOT_PATH for IP Power 9258 and create it if necessary
#
# in final version:
# MY_ROOT_PATH="@GLUE_STATE_DIR@/heartbeat/rsctmp/ippower9258"
MY_ROOT_PATH="/var/run/heartbeat/rsctmp/ippower9258"
mkdir -p $MY_ROOT_PATH
#
# script functions
#
get_challenge() {
#
# device sends a challenge for md5 encryption of username, password and
challenge
send_web_command - "http://$deviceip/"; | grep Challenge | grep input |
cut -d '"' -f 6
}
get_cookie_from_device(){
# the form on the login page has these fields:
# Username, Password, Challenge, Response, ScreenWidth
#
challenge=`get_challenge`
response=`echo -n "$username$password$challenge" | md5sum | cut -b -32`
postdata="Username=$username&Password=&Challenge=&Response=$response&ScreenWidth=1024"
send_web_command " $MY_PATH/$MY_TEMPFILE --post-data=$postdata"
"http://$deviceip/tgi/login.tgi";
if grep -qs "Invalid User name or Password" $MY_PATH/$MY_TEMPFILE
then
$LOG_ERROR "Login to device $deviceip failed."
$LOG_ERROR "Received Challenge = <<<$challenge>>>."
$LOG_ERROR "Sent postdata = <<<$postdata>>>."
exit 1
fi
}
get_data_from_device() {
# If successful all device info is available in MY_PATH
rm -f "$MY_PATH/$PORT_STATUS"
send_web_command "$MY_PATH/$PORT_STATUS" "http://$deviceip/$PORT_STATUS";
if grep -qs "Cookie Time Out" $MY_PATH/$PORT_STATUS
then
$LOG_ERROR "received no port data from $deviceip (Cookie Time
Out)"
exit 1
fi
}
send_http_request() {
# ececution of http commands supported by the device
$HTTP_COMMAND "http://$username:$passw...@$deviceip/$1";
}
send_web_command(){
# ececution of web commands through the web-interface
WEB_COMMAND="wget -q --keep-session-cookies"
WEB_COMMAND="$WEB_COMMAND --load-cookies $MY_PATH/$MY_COOKIES"
WEB_COMMAND="$WEB_COMMAND --save-cookies $MY_PATH/$MY_COOKIES"
$WEB_COMMAND -O $1 -- $2
}
name2port() {
local name=$1
local i=$IPPort0
for h in $device_hostlist ; do
if [ $h = $name ]; then
echo $IPPort_name$i
return
fi
i=`expr $i + 1`
done
echo "invalid"
}
set_port() {
#
# port status is always set. Even if requested status is current status.
# host status is not considered.
local host=$1
local requested_status=$2 # 0 or 1
local port=`name2port $host`
if [ "$port" = "invalid" ]
then
$LOG_ERROR "Host $host is not in hostlist ($hostlist) for
$deviceip."
exit 1
fi
ret=`send_http_request "$IPSetPower+$port=$requested_status" | cut -b
11`
if [ "$ret" != "$requested_status" ]
then
$LOG_ERROR "$DEVICE at $deviceip responds with wrong status
$ret for host $host at port $port."
exit 1
fi
return 0
}
build_device_hostlist() {
#
# hostnames are available from http://$deviceip/iocontrol.htm";
# check for number of ports
#
device_hostlist=$(
w3m -dump $MY_PATH/$PORT_STATUS | grep 'Power[1-8]' |
sed 's/[^[]*\[//;s/\].*//;s/ *//' |
while read h; do
[ -z "$h" ] &&
echo $UNDEFINED_HOSTNAME ||
echo $h
done
)
if [ x = x"$device_hostlist" ]; then
$LOG_ERROR "cannot get hostlist for $deviceip"
exit 1
fi
$LOG_DEBUG "Got new hostlist ($device_hostlist) from $deviceip"
}
filter_device_hostlist() {
# check the given hostlist against the device hostlist
local host
for host in $device_hostlist; do
[ "$host" != "$UNDEFINED_HOSTNAME" ] &&
echo $host
done
}
check_hostlist() {
# check the given hostlist against the device hostlist
local cnt=`echo "$hostlist" | wc -w`
local cnt2=0
local host
for host in $hostlist; do
if [ `name2port $host` != "invalid" ]; then
cnt2=$((cnt2+1))
else
$LOG_ERROR "host $host not defined at $deviceip"
fi
done
[ $cnt -ne $cnt2 ] &&
exit 1
}
get_http_status() {
pattern="P60=[01],P61=[01],P62=[01],P63=[01],P64=[01],P65=[01],P66=[01],P67=[01]"
ret=`send_http_request "$IPGetPower" | grep $pattern`
if [ "X$ret" = "X" ]
then
$LOG_ERROR "$DEVICE at $deviceip returns invalid or no string."
exit 1
fi
}
hostlist=`echo $hostlist | tr ',' ' '`
case $1 in
gethosts|on|off|reset|status)
# need environment from stonithd
# and device information from individual device
#
# default device username is admin
# IP Power 9258 does not allow user management.
#
if [ "X$username" = "X" ]
then
username="admin"
fi
tmp_path="$deviceip" # ensure a simple unique pathname
MY_PATH="$MY_ROOT_PATH/$tmp_path"
mkdir -p $MY_PATH
get_cookie_from_device
get_data_from_device
build_device_hostlist
if [ "X$hostlist" = "X" ]; then
hostlist="`filter_device_hostlist`"
else
check_hostlist
fi
;;
*)
# the client is asking for meta-data
;;
esac
target=`echo $2 | sed 's/[.].*//'`
# the necessary actions for stonithd
case $1 in
gethosts)
echo $hostlist
;;
on)
set_port $target $IPPowerOn
;;
off)
set_port $target $IPPowerOff
;;
reset)
set_port $target $IPPowerOff
sleep 5
set_port $target $IPPowerOn
;;
status)
# werify http command interface
get_http_status
;;
getconfignames)
# return all the config names
for ipparam in deviceip username password hostlist
do
echo $ipparam
done
;;
getinfo-devid)
echo "IP Power 9258"
;;
getinfo-devname)
echo "IP Power 9258 power switch"
;;
getinfo-devdescr)
echo "Power switch IP Power 9258 with 4 or 8 power outlets."
echo "WARNING: It is different from IP Power 9258 HP"
;;
getinfo-devurl)
echo "http://www.aviosys.com/manual.htm";
;;
getinfo-xml)
cat << IPPOWERXML
<parameters>
<parameter name="deviceip" unique="1" required="1">
<content type="string" />
<shortdesc lang="en">
IP address or hostname of the device.
</shortdesc>
<longdesc lang="en">
The IP Address or the hostname of the device.
</longdesc>
</parameter>
<parameter name="password" unique="0" required="1">
<content type="string" />
<shortdesc lang="en">
Password
</shortdesc>
<longdesc lang="en">
The password to log in with.
</longdesc>
</parameter>
<parameter name="hostlist" unique="0" required="0">
<content type="string" />
<shortdesc lang="en">
Hostlist
</shortdesc>
<longdesc lang="en">
The list of hosts that the device controls.
If you leave this list empty, we will retrieve the hostnames from the device.
</longdesc>
</parameter>
<parameter name="username" unique="0" required="0">
<content type="string" default="admin"/>
<shortdesc lang="en">
Account Name
</shortdesc>
<longdesc lang="en">
The user to log in with.
</longdesc>
</parameter>
</parameters>
IPPOWERXML
;;
*)
$LOG_ERROR "Unexpected command $1 for $DEVICE at $deviceip."
exit 1;
;;
esac
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/
