Merge lp:~andreserl/maas/fix_ipmi_command_execution into lp:~maas-committers/maas/trunk

[Andres Rodriguez]

Minor improvements to IPMI template

[Andres Rodriguez (andreserl)]

This branch fixes the ipmi command executing as in a particular case it produced an error, due to recent changes I introduced. I'm approving this branch myself as there's no one to review and this gets the issue fixed. It has been tested.

review: Approve

[MAAS Lander (maas-lander)]

No commit message specified.

[Gavin Panella (allenap)]

I think there are a couple of errors in here that need fixing, in a
follow-up branch now this has merged.

[1]

+formulate_power_command() {
+    case $1 in
+    'on') echo '--cycle --on-if-off' ;;
+    'off') echo '--off' ;;
+    *)
+        echo "Got unknown power state from ipmipower: '$2'" >&2

s/$2/$1/

[2]

+    ${ipmipower} -h ${power_address} -u ${power_user} -p ${power_pass} $*

"$@" is almost always a better choice than $*; the latter will cause
the shell to break arguments that contain white-space into multiple
arguments.

[3]

formulate_power_state() {
     case $2 in

I know you didn't change this, but what does $2 refer to? Please add
comments describing the arguments to this function, and to
formulate_power_command().

review: Needs Fixing

[Gavin Panella (allenap)]

review: Approve

[MAAS Lander (maas-lander)]

No commit message specified.

=== modified file 'src/provisioningserver/power/templates/ipmi.template'
--- src/provisioningserver/power/templates/ipmi.template	2012-10-06 04:10:16 +0000
+++ src/provisioningserver/power/templates/ipmi.template	2012-10-06 16:29:23 +0000
@@ -12,16 +12,22 @@
 ipmi_chassis_config={{ipmi_chassis_config}}
 config={{config_dir}}/{{ipmi_config}}
 
+# Determines the power command needed to execute the desired
+# action. This function receives ${power_change} as argument.
 formulate_power_command() {
     case $1 in
     'on') echo '--cycle --on-if-off' ;;
     'off') echo '--off' ;;
     *)
-        echo "Got unknown power state from ipmipower: '$2'" >&2
+        echo "Got unknown power state from ipmipower: '$1'" >&2
         exit 1
     esac
 }
 
+# Determines the current state on which the machine finds itself.
+# The argument passed comes from IPMI's stat command in the form:
+# <ipmi-ip-address>: <on/off>
+# This function evaluates whether it was <on/off>.
 formulate_power_state() {
     case $2 in
     'on') echo 'on' ;;
@@ -38,7 +44,7 @@
     echo workaround |\
     ${ipmi_chassis_config} -h ${power_address} -u ${power_user} -p ${power_pass} --commit --filename ${config}
     echo workaround |\
-    ${ipmipower} -h ${power_address} -u ${power_user} -p ${power_pass} $*
+    ${ipmipower} -h ${power_address} -u ${power_user} -p ${power_pass} "$@"
 }