launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/maas/remove-ipmi-optimisation into lp:maas.

Commit message:
Formally revert to old IPMI power control behaviour: do not check the current state before issuing a power command.

Requested reviews:
  Andres Rodriguez (andreserl)
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1171418 in MAAS: "MAAS fails to power up machines when trying to install nodes"
  https://bugs.launchpad.net/maas/+bug/1171418

For more details, see:
https://code.launchpad.net/~allenap/maas/remove-ipmi-optimisation/+merge/160146
-- 
https://code.launchpad.net/~allenap/maas/remove-ipmi-optimisation/+merge/160146
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/remove-ipmi-optimisation into lp:maas.

=== modified file 'src/provisioningserver/power/templates/ipmi.template'
--- src/provisioningserver/power/templates/ipmi.template	2013-04-18 19:30:26 +0000
+++ src/provisioningserver/power/templates/ipmi.template	2013-04-22 15:52:29 +0000
@@ -45,7 +45,7 @@
     driver_option=""
     if [ -n "$power_driver" ]
         then
-          driver_option="--driver-type=$power_driver"
+          driver_option="--driver-type=${power_driver}"
     fi
 
     echo workaround |\
@@ -54,16 +54,7 @@
     ${ipmipower} ${driver_option} -h ${power_address} -u ${power_user} -p ${power_pass} "$@"
 }
 
-
-# Get the given system's power state: 'on' or 'off'.
-get_power_state() {
-    ipmi_state=$(issue_ipmi_command --stat)
-    formulate_power_state ${ipmi_state}
-}
-
-
-if [ "$(get_power_state)" != "${power_change}" ]
-then
-    power_command=$(formulate_power_command ${power_change})
-    issue_ipmi_command ${power_command}
-fi
+# This script used to get the current power state, then only send a power
+# command if a change was needed. See bug 1171418 for an explanation.
+power_command=$(formulate_power_command ${power_change})
+issue_ipmi_command ${power_command}

=== modified file 'src/provisioningserver/power/tests/test_poweraction.py'
--- src/provisioningserver/power/tests/test_poweraction.py	2013-03-15 11:47:04 +0000
+++ src/provisioningserver/power/tests/test_poweraction.py	2013-04-22 15:52:29 +0000
@@ -180,16 +180,6 @@
         output = action.run_shell(script)
         self.assertIn("Got unknown power state from fence_cdu", output)
 
-    def test_ipmi_checks_state(self):
-        action = PowerAction(POWER_TYPE.IPMI)
-        script = action.render_template(
-            action.get_template(), power_change='on',
-            power_address='mystystem', power_user='me', power_pass='me',
-            ipmipower='echo', ipmi_chassis_config='echo', config_dir='dir',
-            ipmi_config='file.conf', power_driver='LAN')
-        output = action.run_shell(script)
-        self.assertIn("Got unknown power state from ipmipower", output)
-
     def configure_power_config_dir(self, path=None):
         """Configure POWER_CONFIG_DIR to `path`."""
         self.patch(