Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/power-off into lp:maas.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/power-off/+merge/107716
As discussed with Julian, this adds a Celery task power_on. Given how little
the power methods I saw in Cobbler varied between the on/off actions, I chose
to use the same templates for both actions. It does risk adding a bit of
complexity in shell script where it's harder to test, but it avoids the
complexity and consistency hazards of having separate sets of templates for the
two actions.
I made the wake-on-LAN template a bit easier to read along the way, but also, I
made it fail with a (somewhat) helpful error message when you try to use it to
power off a node. We may find other ways to deal with that, or end up punting
to humans, but I felt this was better than a silent no-op.
You'll see two very similar tests. Don't be angry. I think I can explain.
The PowerAction test is a unit test. It tests a corner case: you can't use
wake-on-LAN to shut down a machine. The task test is an integration test: it's
the only really testable behaviour we have right now for the power_off action,
and so that's what I ended up testing.
I have my doubts about what else goes on in that test, actually: where does the
magical hard-coded MAC address come from? Was it chosen to match a hard-coded
address somewhere else? Was it hand-vetted to be safe, so that the tests won't
accidentally mess with one specific machine somewhere in the world? Or is it
an arbitrary address that somebody thought was clearer than
factory.getRandomMACAddress? I don't know. I just copied it in cargo-cult
fashion and hoped that whatever reasons there were for this, will apply to my
branch as well. That's what generally happens in cases like this, in my
experience. I may take some time to find out more and fix things up separately
if appropriate.
Finally, note that an exception in a “delayed” task in a test gets propagated
into the test. So the way to check for one in a test is self.assertRaises, not
a check on the task's success code.
Jeroen
--
https://code.launchpad.net/~jtv/maas/power-off/+merge/107716
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jtv/maas/power-off into lp:maas.
=== modified file 'src/provisioningserver/power/poweraction.py'
--- src/provisioningserver/power/poweraction.py 2012-05-21 15:51:33 +0000
+++ src/provisioningserver/power/poweraction.py 2012-05-29 06:34:21 +0000
@@ -45,7 +45,7 @@
basedir = POWER_TEMPLATES_DIR
self.path = os.path.join(basedir, power_type + ".template")
if not os.path.exists(self.path):
- raise UnknownPowerType
+ raise UnknownPowerType(power_type)
self.power_type = power_type
=== modified file 'src/provisioningserver/power/templates/ether_wake.template'
--- src/provisioningserver/power/templates/ether_wake.template 2012-05-16 06:53:48 +0000
+++ src/provisioningserver/power/templates/ether_wake.template 2012-05-29 06:34:21 +0000
@@ -1,6 +1,16 @@
-set mac = %(mac)s
-if [ -x /usr/bin/wakeonlan ]; then
- /usr/bin/wakeonlan $mac
-elif [ -x /usr/sbin/etherwake ]; then
- /usr/sbin/etherwake $mac
+if [ '%(power_change)s' != 'on' ]
+then
+ echo "There is no way to power down a node through etherwake." >&2
+ exit 1
+elif [ -x /usr/bin/wakeonlan ]
+then
+ /usr/bin/wakeonlan %(mac)s
+elif [ -x /usr/sbin/etherwake ]
+then
+ /usr/sbin/etherwake %(mac)s
+else
+ echo "No wakeonlan or etherwake program found." >&2
+ exit 1
fi
+
+exit 0
=== modified file 'src/provisioningserver/power/tests/test_poweraction.py'
--- src/provisioningserver/power/tests/test_poweraction.py 2012-05-21 15:51:33 +0000
+++ src/provisioningserver/power/tests/test_poweraction.py 2012-05-29 06:34:21 +0000
@@ -104,3 +104,9 @@
exception = self.assertRaises(PowerActionFail, self.run_action, path)
self.assertEqual(
"ether_wake failed with return code 127", exception.message)
+
+ def test_wake_on_lan_cannot_shut_down_node(self):
+ pa = PowerAction(POWER_TYPE.WAKE_ON_LAN)
+ self.assertRaises(
+ PowerActionFail,
+ pa.execute, power_change='off', mac=factory.getRandomMACAddress())
=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py 2012-05-23 14:43:16 +0000
+++ src/provisioningserver/tasks.py 2012-05-29 06:34:21 +0000
@@ -11,7 +11,8 @@
__metaclass__ = type
__all__ = [
- 'power_on'
+ 'power_off',
+ 'power_on',
]
@@ -22,8 +23,17 @@
)
-@task
-def power_on(power_type, **kwargs):
+def issue_power_action(power_type, power_change, **kwargs):
+ """Issue a power action to a node.
+
+ :param power_type: The node's power type. Must have a corresponding
+ power template.
+ :param power_change: The change to request: 'on' or 'off'.
+ :param **kwargs: Keyword arguments are passed on to :class:`PowerAction`.
+ """
+ assert power_change in ('on', 'off'), (
+ "Unknown power change keyword: %s" % power_change)
+ kwargs['power_change'] = power_change
try:
pa = PowerAction(power_type)
pa.execute(**kwargs)
@@ -35,3 +45,15 @@
raise
# TODO: signal to webapp that it worked.
+
+
+@task
+def power_on(power_type, **kwargs):
+ """Turn a node on."""
+ issue_power_action(power_type, 'on', **kwargs)
+
+
+@task
+def power_off(power_type, **kwargs):
+ """Turn a node off."""
+ issue_power_action(power_type, 'off', **kwargs)
=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py 2012-05-23 14:43:16 +0000
+++ src/provisioningserver/tests/test_tasks.py 2012-05-29 06:34:21 +0000
@@ -16,7 +16,10 @@
from maastesting.testcase import TestCase
from provisioningserver.enum import POWER_TYPE
from provisioningserver.power.poweraction import PowerActionFail
-from provisioningserver.tasks import power_on
+from provisioningserver.tasks import (
+ power_off,
+ power_on,
+ )
from testresources import FixtureResource
@@ -37,3 +40,8 @@
mac = "AA:BB:CC:DD:EE:FF"
result = power_on.delay(POWER_TYPE.WAKE_ON_LAN, mac=mac)
self.assertTrue(result.successful())
+
+ def test_ether_wake_does_not_support_power_off(self):
+ mac = "AA:BB:CC:DD:EE:FF"
+ self.assertRaises(
+ PowerActionFail, power_off.delay, POWER_TYPE.WAKE_ON_LAN, mac=mac)
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
