Merge lp:~fwereade/pyjuju/cobbler-shutdown into lp:pyjuju

[William Reade]

destroy_environment now attempts to clear state before killing machines; if it doesn't manage, it ignores the error and kills the machines regardless.

orchestra provider now has a shutdown_machines method, and hence shutdown_machine and destroy_environment.

[William Reade (fwereade)]

Note: includes API fixes suggested in https://code.launchpad.net/~fwereade/ensemble/hide-instances/+merge/71152

[lp:~fwereade/pyjuju/cobbler-shutdown]

328. By William Reade on 2011-08-16

fix base interface, docstring for shutdown_machines

329. By William Reade on 2011-08-16

merge parent

[Gustavo Niemeyer (niemeyer)]

Very nice branch too. Only a few simple things, and +1.

[1]

- def shutdown_machines(self, requested_machines=()):
+ def shutdown_machines(self, requested_machines):

Woohay. :-)

[2]

+ try:
+ yield self.save_state({})
+ except Exception as error:
+ # failing to clear state is a bit bad; failing to kill
+ # the machines would be *really* bad
+ log.error("Failed to clear provider state: %s", error)
+ live_machines = yield self.get_machines()
+ killed_machines = yield self.shutdown_machines(live_machines)

The try/except above will eat NameErrors and the such blindly and
unnecessarily. This can easily try/finally instead:

   try:
       yield self.save_state({})
   finally:
       live_machines = yield self.get_machines()
       killed_machines = yield self.shutdown_machines(live_machines)

[3]

+ @inlineCallbacks
     def destroy_environment(self):
     - return self.shutdown_machines()
     + machines = yield self.get_machines()
     + yield self.shutdown_machines(machines)
     + returnValue(machines)

This looks naturally structured for callbacks (at last!):

d = self.get_machines()
d.addCallback(self.shutdown_machines)
return d

[4]

+ def shutdown_machines(self, requested_machines):
         """Terminate machine resources associated with this provider."""
+ if not requested_machines:
+ returnValue(requested_machines)

s/requested_// would go well here too.

returnValue([]) feels more readable as well, and slightly less error
prone (lists are mutable).

[5]

Same as above for Orchestra version.

[6]

+class StartSystemTest(CobblerClientTestCase, PowerTestsMixin):
+
+ desired_state = "on"
+ method_name = "start_system"
+
+
+class StopSystemTest(CobblerClientTestCase, PowerTestsMixin):
+
+ desired_state = "off"
+ method_name = "stop_system"

Interesting design.

[7]

    def describe_systems(self, *instance_ids):
(...)
+ def shutdown_machines(self, machines):

We have an inconsistency in calling conventions here. Please don't
fix in this branch, but a good candidate for a small trivial branch
later.

review: Approve

[lp:~fwereade/pyjuju/cobbler-shutdown]

330. By William Reade on 2011-08-17

fixes per review

331. By William Reade on 2011-08-17

merge parent

[William Reade (fwereade)]

> [1]

Yeehaw!

> [2]

Good point :)

> [3]
> This looks naturally structured for callbacks (at last!):

It would be, if I hadn't forgotten to call save_state :/. Fixed now.

> [4,5]
> s/requested_// would go well here too.
>
> returnValue([]) feels more readable as well, and slightly less error
> prone (lists are mutable).

Both fixed.

> [6]
> Interesting design.

They're so similar, it seemed a shame to duplicate the effort :). I doubt they'll stay so perfectly similar forever, but for now I think it's a win.

> [7]

Filed lp:827994

[lp:~fwereade/pyjuju/cobbler-shutdown]

332. By William Reade on 2011-08-18

merge trunk

[Gustavo Niemeyer (niemeyer)]

LGTM

[lp:~fwereade/pyjuju/cobbler-shutdown]

333. By William Reade on 2011-08-22

merge trunk

334. By William Reade on 2011-08-23

merge trunk

[Kapil Thangavelu (hazmat)]

[0] Branch implementation looks very clean, and with good testing, nice.

[1] Pep 8 cleanups
 https://pastebin.canonical.com/51598/

review: Approve

[lp:~fwereade/pyjuju/cobbler-shutdown]

335. By William Reade on 2011-08-23

pep8 re review

=== modified file 'ensemble/providers/common/base.py'
--- ensemble/providers/common/base.py	2011-08-17 12:04:56 +0000
+++ ensemble/providers/common/base.py	2011-08-23 21:41:27 +0000
@@ -1,11 +1,13 @@
 import copy
 from operator import itemgetter
 
+from twisted.internet.defer import inlineCallbacks, returnValue
+
 from ensemble.environment.errors import EnvironmentsConfigError
-from ensemble.providers.common.bootstrap import Bootstrap
-from ensemble.providers.common.findzookeepers import find_zookeepers
-from ensemble.providers.common.state import SaveState, LoadState
-from ensemble.providers.common.utils import get_user_authorized_keys
+from .bootstrap import Bootstrap
+from .findzookeepers import find_zookeepers
+from .state import SaveState, LoadState
+from .utils import get_user_authorized_keys
 
 
 class MachineProviderBase(object):
@@ -93,11 +95,10 @@
         """
         raise NotImplementedError()
 
-    def shutdown_machines(self, requested_machines=()):
+    def shutdown_machines(self, machines):
         """Terminate machines associated with this provider.
 
-        @param requested_machines: list of machines to shut down; leave
-            empty to shut down all associated machines.
+        @param machines: list of machines to shut down
         """
         raise NotImplementedError()
 
@@ -152,9 +153,15 @@
         d.addCallback(itemgetter(0))
         return d
 
+    @inlineCallbacks
     def destroy_environment(self):
         """Clear ensemble state and terminate all associated machines"""
-        return self.shutdown_machines()
+        try:
+            yield self.save_state({})
+        finally:
+            live_machines = yield self.get_machines()
+            killed_machines = yield self.shutdown_machines(live_machines)
+            returnValue(killed_machines)
 
     def save_state(self, state):
         """Save state to the provider.
@@ -170,4 +177,3 @@
         @return: a dictionary.
         """
         return LoadState(self).run()
-

=== modified file 'ensemble/providers/common/tests/test_base.py'
--- ensemble/providers/common/tests/test_base.py	2011-08-17 12:04:56 +0000
+++ ensemble/providers/common/tests/test_base.py	2011-08-23 21:41:27 +0000
@@ -110,8 +110,12 @@
 
     def test_destroy_environment_error(self):
         provider = DummyProvider()
+        provider.get_machines = self.mocker.mock()
+        provider.get_machines()
+        machines = [object(), object()]
+        self.mocker.result(succeed(machines))
         provider.shutdown_machines = self.mocker.mock()
-        provider.shutdown_machines()
+        provider.shutdown_machines(machines)
         self.mocker.result(fail(SomeError()))
         self.mocker.replay()
 
@@ -121,9 +125,12 @@
 
     def test_destroy_environment_success(self):
         provider = DummyProvider()
+        provider.get_machines = self.mocker.mock()
+        provider.get_machines()
+        machines = [object(), object()]
+        self.mocker.result(succeed(machines))
         provider.shutdown_machines = self.mocker.mock()
-        provider.shutdown_machines()
-        machines = object()
+        provider.shutdown_machines(machines)
         self.mocker.result(succeed(machines))
         self.mocker.replay()
 

=== modified file 'ensemble/providers/dummy.py'
--- ensemble/providers/dummy.py	2011-08-22 23:03:03 +0000
+++ ensemble/providers/dummy.py	2011-08-23 21:41:27 +0000
@@ -85,11 +85,11 @@
         return self.start_machine({"machine-id": 0})
 
     @inlineCallbacks
-    def shutdown_machines(self, requested_machines=()):
+    def shutdown_machines(self, machines):
         """
         Terminate any machine resources associated to the provider.
         """
-        instance_ids = [m.instance_id for m in requested_machines]
+        instance_ids = [m.instance_id for m in machines]
         machines = yield self.get_machines(instance_ids)
         for machine in machines:
             self._machines.remove(machine)
@@ -102,11 +102,15 @@
         for m in self._machines:
             if m.instance_id == machine.instance_id:
                 self._machines.remove(m)
-                return
+                return m
         return fail(ProviderError("Machine not found %r" % machine))
 
+    @inlineCallbacks
     def destroy_environment(self):
-        return self.shutdown_machines()
+        yield self.save_state({})
+        machines = yield self.get_machines()
+        machines = yield self.shutdown_machines(machines)
+        returnValue(machines)
 
     def save_state(self, state):
         """Save the state to the provider."""

=== modified file 'ensemble/providers/ec2/__init__.py'
--- ensemble/providers/ec2/__init__.py	2011-08-17 12:04:56 +0000
+++ ensemble/providers/ec2/__init__.py	2011-08-23 21:41:27 +0000
@@ -98,7 +98,8 @@
             code = error.get_error_codes()
             if code == "InvalidInstanceID.NotFound":
                 message = error.get_error_messages()
-                raise MachinesNotFound(re.findall(r"\bi-[0-9a-f]{3,15}\b", message))
+                raise MachinesNotFound(
+                    re.findall(r"\bi-[0-9a-f]{3,15}\b", message))
             raise ProviderInteractionError(
                 "EC2 error when looking up instances %s: %s"
                 % (", ".join(instance_ids), error.get_error_messages()))
@@ -121,19 +122,25 @@
         returnValue(machines)
 
     @inlineCallbacks
-    def shutdown_machines(self, requested_machines=()):
-        """Terminate machine resources associated with this provider."""
-        for machine in requested_machines:
+    def shutdown_machines(self, machines):
+        """Terminate machines associated with this provider.
+
+        @param machines: list of machines to shut down
+        """
+        if not machines:
+            returnValue([])
+
+        for machine in machines:
             if not isinstance(machine, EC2ProviderMachine):
                 raise ProviderError("Can only shut down EC2ProviderMachines; "
                                     "got a %r" % type(machine))
 
-        requested_ids = [m.instance_id for m in requested_machines]
-        killable_machines = yield self.get_machines(requested_ids)
+        ids = [m.instance_id for m in machines]
+        killable_machines = yield self.get_machines(ids)
         if killable_machines:
             killable_ids = [m.instance_id for m in killable_machines]
             yield self.ec2.terminate_instances(*killable_ids)
-            returnValue(killable_machines)
+        returnValue(killable_machines)
 
     def open_port(self, machine, machine_id, port, protocol="tcp"):
         """Authorizes `port` using `protocol` on EC2 for `machine`."""

=== modified file 'ensemble/providers/ec2/tests/test_shutdown.py'
--- ensemble/providers/ec2/tests/test_shutdown.py	2011-08-12 21:22:07 +0000
+++ ensemble/providers/ec2/tests/test_shutdown.py	2011-08-23 21:41:27 +0000
@@ -1,4 +1,4 @@
-from twisted.internet.defer import succeed
+from twisted.internet.defer import fail, succeed
 
 from ensemble.lib.testing import TestCase
 from ensemble.providers.ec2.tests.common import EC2TestMixin
@@ -9,6 +9,10 @@
 from ensemble.providers.ec2.machine import EC2ProviderMachine
 
 
+class SomeError(Exception):
+    pass
+
+
 class EC2ShutdownMachineTest(EC2TestMixin, TestCase):
 
     def test_shutdown_machine(self):
@@ -67,47 +71,34 @@
         d.addCallback(check_error)
         return d
 
-    def test_shutdown_machines_all(self):
-        self.ec2.describe_instances()
+    def test_shutdown_machines_none(self):
+        self.mocker.replay()
+        provider = self.get_provider()
+        d = provider.shutdown_machines([])
+
+        def verify(result):
+            self.assertEquals(result, [])
+        d.addCallback(verify)
+        return d
+
+    def test_shutdown_machines_some_invalid(self):
+        self.ec2.describe_instances("i-amkillable", "i-amalien", "i-amdead")
         self.mocker.result(succeed([
             self.get_instance("i-amkillable"),
-            self.get_instance("i-amdead", "shutting-down"),
             self.get_instance("i-amalien", groups=["other"]),
-            self.get_instance("i-amkillabletoo")]))
-        self.ec2.terminate_instances("i-amkillable", "i-amkillabletoo")
-        self.mocker.result(succeed([
-            ("i-amkillable", "running", "shutting-down"),
-            ("i-amkillabletoo", "running", "shutting-down")]))
-        self.mocker.replay()
-
-        provider = self.get_provider()
-        d = provider.shutdown_machines()
-
-        def verify(result):
-            (machine_1, machine_2) = result
-            self.assertTrue(isinstance(machine_1, EC2ProviderMachine))
-            self.assertEquals(machine_1.instance_id, "i-amkillable")
-            self.assertTrue(isinstance(machine_2, EC2ProviderMachine))
-            self.assertEquals(machine_2.instance_id, "i-amkillabletoo")
-        d.addCallback(verify)
-        return d
-
-    def test_shutdown_machines_some_invalid(self):
-        self.ec2.describe_instances("i-amkillable", "i-amdead")
-        self.mocker.result(succeed([
-            self.get_instance("i-amkillable"),
             self.get_instance("i-amdead", "shutting-down")]))
         self.mocker.replay()
 
         provider = self.get_provider()
         d = provider.shutdown_machines([
             EC2ProviderMachine("i-amkillable"),
+            EC2ProviderMachine("i-amalien"),
             EC2ProviderMachine("i-amdead")])
         self.failUnlessFailure(d, MachinesNotFound)
 
         def verify(error):
             self.assertEquals(str(error),
-                              "Cannot find machine: i-amdead")
+                              "Cannot find machines: i-amalien, i-amdead")
         d.addCallback(verify)
         return d
 
@@ -144,13 +135,18 @@
         The destroy_environment operation terminates all running and pending
         instances associated to the C{MachineProvider} instance.
         """
-        instances = [self.get_instance("i-canbekilled"),
-                     self.get_instance("i-amdead", state="terminated"),
-                     self.get_instance("i-dontbelong", groups=["unknown"]),
-                     self.get_instance("i-canbekilledtoo", state="pending")]
-
+        self.s3.put_object("moon", "provider-state", "{}\n")
+        self.mocker.result(succeed(None))
         self.ec2.describe_instances()
-        self.mocker.result(succeed(instances))
+        self.mocker.result(succeed([
+            self.get_instance("i-canbekilled"),
+            self.get_instance("i-amdead", state="terminated"),
+            self.get_instance("i-dontbelong", groups=["unknown"]),
+            self.get_instance("i-canbekilledtoo", state="pending")]))
+        self.ec2.describe_instances("i-canbekilled", "i-canbekilledtoo")
+        self.mocker.result(succeed([
+            self.get_instance("i-canbekilled"),
+            self.get_instance("i-canbekilledtoo", state="pending")]))
         self.ec2.terminate_instances("i-canbekilled", "i-canbekilledtoo")
         self.mocker.result(succeed([
             ("i-canbekilled", "running", "shutting-down"),
@@ -169,11 +165,36 @@
         d.addCallback(verify)
         return d
 
+    def test_s3_failure(self):
+        """Failing to store empty state should not stop us killing machines"""
+        self.s3.put_object("moon", "provider-state", "{}\n")
+        self.mocker.result(fail(SomeError()))
+        self.ec2.describe_instances()
+        self.mocker.result(succeed([self.get_instance("i-canbekilled")]))
+        self.ec2.describe_instances("i-canbekilled")
+        self.mocker.result(succeed([self.get_instance("i-canbekilled")]))
+        self.ec2.terminate_instances("i-canbekilled")
+        self.mocker.result(succeed([
+            ("i-canbekilled", "running", "shutting-down")]))
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        d = provider.destroy_environment()
+
+        def verify(result):
+            (machine,) = result
+            self.assertTrue(isinstance(machine, EC2ProviderMachine))
+            self.assertEquals(machine.instance_id, "i-canbekilled")
+        d.addCallback(verify)
+        return d
+
     def test_shutdown_no_instances(self):
         """
         If there are no instances to shutdown, running the destroy_environment
         operation does nothing.
         """
+        self.s3.put_object("moon", "provider-state", "{}\n")
+        self.mocker.result(succeed(None))
         self.ec2.describe_instances()
         self.mocker.result(succeed([]))
         self.mocker.replay()
@@ -182,6 +203,6 @@
         d = provider.destroy_environment()
 
         def verify(result):
-            self.assertEquals(result, None)
+            self.assertEquals(result, [])
         d.addCallback(verify)
         return d

=== modified file 'ensemble/providers/orchestra/__init__.py'
--- ensemble/providers/orchestra/__init__.py	2011-08-17 12:04:56 +0000
+++ ensemble/providers/orchestra/__init__.py	2011-08-23 21:41:27 +0000
@@ -1,11 +1,12 @@
 from twisted.internet.defer import inlineCallbacks, returnValue
 
+from ensemble.errors import ProviderError
 from ensemble.providers.common.base import MachineProviderBase
 
 from .cobbler import CobblerClient
 from .files import FileStorage
 from .launch import OrchestraLaunchMachine
-from .machine import machine_from_dict
+from .machine import machine_from_dict, OrchestraMachine
 
 
 class MachineProvider(MachineProviderBase):
@@ -45,3 +46,24 @@
         """
         instances = yield self.cobbler.describe_systems(*instance_ids)
         returnValue([machine_from_dict(i) for i in instances])
+
+    @inlineCallbacks
+    def shutdown_machines(self, machines):
+        """Terminate machines associated with this provider.
+
+        @param machines: list of machines to shut down
+        """
+        if not machines:
+            returnValue([])
+
+        for machine in machines:
+            if not isinstance(machine, OrchestraMachine):
+                raise ProviderError("Can only shut down OrchestraMachines; "
+                                    "got a %r" % type(machine))
+
+        ids = [m.instance_id for m in machines]
+        killable_machines = yield self.get_machines(ids)
+        for machine in killable_machines:
+            yield self.cobbler.stop_system(machine.instance_id)
+            yield self.cobbler.release_system(machine.instance_id)
+        returnValue(killable_machines)

=== modified file 'ensemble/providers/orchestra/cobbler.py'
--- ensemble/providers/orchestra/cobbler.py	2011-08-19 19:43:33 +0000
+++ ensemble/providers/orchestra/cobbler.py	2011-08-23 21:41:27 +0000
@@ -121,10 +121,13 @@
     def _class_swapper(self, class_):
         if class_ == self._available_class:
             return self._acquired_class
+        if class_ == self._acquired_class:
+            return self._available_class
         return class_
 
     @inlineCallbacks
     def _get_available_system(self):
+        # TODO it would be simpler just to use get_systems here...
         names = yield self._caller.call(
             "find_system", ({"mgmt_classes": self._available_class,
                              "netboot_enabled": "true"},))
@@ -164,18 +167,12 @@
         returnValue(instance_id)
 
     @inlineCallbacks
-    def describe_system(self, instance_id):
-        """Get all available information about a system.
-
-        @return: a dictionary
-
-        @raise: MachinesNotFound if the machine doesn't exist
-        """
-        name = yield self._get_name(instance_id)
-        info = yield self._caller.call("get_system", (name,))
-        if info == "~":
-            raise MachinesNotFound([instance_id])
-        returnValue(info)
+    def release_system(self, instance_id):
+        """Take a system marked as acquired, and make it available again"""
+        (system,) = yield self.describe_systems(instance_id)
+        new_classes = map(self._class_swapper, system["mgmt_classes"])
+        yield self.set_on_system(instance_id, "mgmt_classes", new_classes)
+        returnValue(True)
 
     @inlineCallbacks
     def describe_systems(self, *instance_ids):
@@ -204,10 +201,21 @@
             raise MachinesNotFound(missing_instance_ids)
         returnValue(result_systems)
 
+    def _power_call(self, operation, names):
+        return self._caller.call("background_power_system",
+                                 ({"power": operation, "systems": names},),
+                                 auth=True)
     @inlineCallbacks
     def start_system(self, instance_id):
         """Launch a cobbler system."""
         name = yield self.set_on_system(instance_id, "netboot_enabled", True)
-        yield self._caller.call("background_power_system",
-                                ({"power": "on", "systems": [name]},),
-                                auth=True)
+        yield self._power_call("on", [name])
+
+    @inlineCallbacks
+    def stop_system(self, instance_id):
+        """Shut down a cobbler system.
+
+        Also reenables netboot for next time.
+        """
+        name = yield self.set_on_system(instance_id, "netboot_enabled", True)
+        yield self._power_call("off", [name])

=== modified file 'ensemble/providers/orchestra/launch.py'
--- ensemble/providers/orchestra/launch.py	2011-08-10 18:16:51 +0000
+++ ensemble/providers/orchestra/launch.py	2011-08-23 21:41:27 +0000
@@ -68,7 +68,7 @@
         ks_meta = self._build_ks_meta(instance_id, variables)
         yield cobbler.set_on_system(instance_id, "ks_meta", ks_meta)
         yield cobbler.start_system(instance_id)
-        info = yield cobbler.describe_system(instance_id)
+        (info,) = yield cobbler.describe_systems(instance_id)
         returnValue([machine_from_dict(info)])
 
     def get_instance_id_command(self):

=== modified file 'ensemble/providers/orchestra/tests/common.py'
--- ensemble/providers/orchestra/tests/common.py	2011-08-11 05:59:30 +0000
+++ ensemble/providers/orchestra/tests/common.py	2011-08-23 21:41:27 +0000
@@ -150,12 +150,6 @@
             "TOKEN")
         self.mocker.result(succeed("[some-timestamp]_power"))
 
-    def mock_describe_system(self, uid="winston-uid", name="winston"):
-        self.proxy_m.callRemote("find_system", {"uid": uid})
-        self.mocker.result(succeed([name]))
-        self.proxy_m.callRemote("get_system", name)
-        self.mocker.result(succeed({"name": name, "uid": uid}))
-
     def mock_describe_systems(self, result):
         self.proxy_m.callRemote("get_systems")
         self.mocker.result(result)

=== modified file 'ensemble/providers/orchestra/tests/test_bootstrap.py'
--- ensemble/providers/orchestra/tests/test_bootstrap.py	2011-08-11 05:59:30 +0000
+++ ensemble/providers/orchestra/tests/test_bootstrap.py	2011-08-23 21:41:27 +0000
@@ -87,7 +87,10 @@
         self.mock_set_ks_meta(
             self.get_verify_ks_meta(0, "bootstrap_late_command"))
         self.mock_start_system()
-        self.mock_describe_system()
+        self.mock_describe_systems(succeed([{
+            "uid": "winston-uid",
+            "name": "winston",
+            "mgmt_classes": ["acquired"]}]))
         self.mock_save_state()
         self.mocker.replay()
 
@@ -95,6 +98,7 @@
             (machine,) = machines
             self.assertTrue(isinstance(machine, OrchestraMachine))
             self.assertEquals(machine.instance_id, "winston-uid")
+            self.assertEquals(machine.dns_name, "winston")
         d = self.get_provider().bootstrap()
         d.addCallback(verify_machines)
         return d

=== modified file 'ensemble/providers/orchestra/tests/test_cobbler.py'
--- ensemble/providers/orchestra/tests/test_cobbler.py	2011-08-12 19:28:37 +0000
+++ ensemble/providers/orchestra/tests/test_cobbler.py	2011-08-23 21:41:27 +0000
@@ -187,7 +187,7 @@
         return d
 
 
-class CobblerClientTest(TestCase):
+class CobblerClientTestCase(TestCase):
 
     def setup_mock(self):
         self.proxy_m = self.mocker.mock(Proxy)
@@ -221,19 +221,19 @@
                                                 "netboot_enabled": "true"})
         self.mocker.result(result)
 
-    def mock_power_system(self, result):
+    def mock_power_system(self, state, result):
         self.proxy_m.callRemote("background_power_system",
-                                {"power": "on", "systems": ["some-name"]}, "")
+                                {"power": state, "systems": ["some-name"]}, "")
         self.mocker.result(result)
 
     def mock_get_systems(self, result):
         self.proxy_m.callRemote("get_systems")
         self.mocker.result(result)
 
-    def call_cobbler_method(self, method_name, *args, **kwargs):
+    def call_cobbler_method(self, method_name, *args):
         cobbler = CobblerClient(_CONFIG)
         method = getattr(cobbler, method_name)
-        return method(*args, **kwargs)
+        return method(*args)
 
     def check_not_found(self, d, uids=["some-uid"]):
         self.assertFailure(d, MachinesNotFound)
@@ -251,18 +251,16 @@
         d.addCallback(verify)
         return d
 
-    def check_get_name_failure(self, method_name, *args, **kwargs):
-        self.setup_mock()
+    def check_get_name_failure(self, method_name, *args):
         self.mock_get_name(succeed([]))
         self.mocker.replay()
-        d = self.call_cobbler_method(method_name, *args, **kwargs)
+        d = self.call_cobbler_method(method_name, *args)
         return self.check_not_found(d)
 
-    def check_get_name_insane(self, method_name, *args, **kwargs):
-        self.setup_mock()
+    def check_get_name_insane(self, method_name, *args):
         self.mock_get_name(succeed(["some-name", "another-name"]))
         self.mocker.replay()
-        d = self.call_cobbler_method(method_name, *args, **kwargs)
+        d = self.call_cobbler_method(method_name, *args)
         self.assertFailure(d, ProviderInteractionError)
 
         def check_error(error):
@@ -271,137 +269,127 @@
         d.addCallback(check_error)
         return d
 
-    def check_get_name_error(self, method_name, *args, **kwargs):
-        self.setup_mock()
+    def check_get_name_error(self, method_name, *args):
         self.mock_get_name(fail(SomeError()))
         self.mocker.replay()
-        d = self.call_cobbler_method(method_name, *args, **kwargs)
+        d = self.call_cobbler_method(method_name, *args)
         self.assertFailure(d, SomeError)
         return d
 
-    def test_describe_system_get_name_failure(self):
-        return self.check_get_name_failure("describe_system", "some-uid")
-
-    def test_describe_system_get_name_insane(self):
-        return self.check_get_name_insane("describe_system", "some-uid")
-
-    def test_describe_system_get_name_error(self):
-        return self.check_get_name_error("describe_system", "some-uid")
-
-    def test_describe_system_get_system_failure(self):
-        self.setup_mock()
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system(succeed("~"))
+    def check_get_system_handle_failure(self, method_name, *args):
+        self.mock_get_system_handle(fail(
+            Fault("blah", "blah unknown system name blah")))
         self.mocker.replay()
-        d = self.call_cobbler_method("describe_system", "some-uid")
+        d = self.call_cobbler_method(method_name, *args)
         return self.check_not_found(d)
 
-    def test_describe_system_get_system_error(self):
-        self.setup_mock()
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system(fail(SomeError()))
-        self.mocker.replay()
-        d = self.call_cobbler_method("describe_system", "some-uid")
-        self.assertFailure(d, SomeError)
-        return d
-
-    def test_describe_system_success(self):
-        self.setup_mock()
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system(succeed(_SOME_SYSTEM))
-        self.mocker.replay()
-        d = self.call_cobbler_method("describe_system", "some-uid")
-
-        def verify_info(info):
-            self.assertEquals(info, _SOME_SYSTEM)
-        d.addCallback(verify_info)
-        return d
+    def check_get_system_handle_fault(self, method_name, *args):
+        self.mock_get_system_handle(fail(Fault("blah", "blah")))
+        self.mocker.replay()
+        d = self.call_cobbler_method(method_name, *args)
+        self.assertFailure(d, Fault)
+        return d
+
+    def check_get_system_handle_error(self, method_name, *args):
+        self.mock_get_system_handle(fail(SomeError()))
+        self.mocker.replay()
+        d = self.call_cobbler_method(method_name, *args)
+        self.assertFailure(d, SomeError)
+        return d
+
+    def check_modify_system_failure(self, key, value, method_name, *args):
+        self.mock_modify_system(succeed(False), key, value)
+        self.mocker.replay()
+        d = self.call_cobbler_method(method_name, *args)
+        return self.check_bad_call(d, "modify_system")
+
+    def check_modify_system_error(self, key, value, method_name, *args):
+        self.mock_modify_system(fail(SomeError()), key, value)
+        self.mocker.replay()
+        d = self.call_cobbler_method(method_name, *args)
+        self.assertFailure(d, SomeError)
+        return d
+
+    def check_save_system_failure(self, method_name, *args):
+        self.mock_save_system(succeed(False))
+        self.mocker.replay()
+        d = self.call_cobbler_method(method_name, *args)
+        return self.check_bad_call(d, "save_system")
+
+    def check_save_system_error(self, method_name, *args):
+        self.mock_save_system(fail(SomeError()))
+        self.mocker.replay()
+        d = self.call_cobbler_method(method_name, *args)
+        self.assertFailure(d, SomeError)
+        return d
+
+
+class SetOnSystemTest(CobblerClientTestCase):
 
     def test_set_on_system_get_name_failure(self):
+        self.setup_mock()
         return self.check_get_name_failure(
             "set_on_system", "some-uid", "don't", "care")
 
     def test_set_on_system_get_name_insane(self):
+        self.setup_mock()
         return self.check_get_name_insane(
             "set_on_system", "some-uid", "don't", "care")
 
     def test_set_on_system_get_name_error(self):
+        self.setup_mock()
         return self.check_get_name_error(
             "set_on_system", "some-uid", "don't", "care")
 
     def test_set_on_system_get_system_handle_failure(self):
         self.setup_mock()
         self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(fail(
-            Fault("blah", "blah unknown system name blah")))
-        self.mocker.replay()
-        d = self.call_cobbler_method(
+        return self.check_get_system_handle_failure(
             "set_on_system", "some-uid", "don't", "care")
-        return self.check_not_found(d)
 
     def test_set_on_system_get_system_handle_unknown_fault(self):
         self.setup_mock()
         self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(fail(Fault("blah", "blah")))
-        self.mocker.replay()
-        d = self.call_cobbler_method(
+        return self.check_get_system_handle_fault(
             "set_on_system", "some-uid", "don't", "care")
-        self.assertFailure(d, Fault)
-        return d
 
     def test_set_on_system_get_system_handle_error(self):
         self.setup_mock()
         self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(fail(SomeError()))
-        self.mocker.replay()
-        d = self.call_cobbler_method(
+        return self.check_get_system_handle_error(
             "set_on_system", "some-uid", "don't", "care")
-        self.assertFailure(d, SomeError)
-        return d
 
     def test_set_on_system_modify_system_failure(self):
         self.setup_mock()
         self.mock_get_name(succeed(["some-name"]))
         self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(succeed(False))
-        self.mocker.replay()
-        d = self.call_cobbler_method(
+        return self.check_modify_system_failure(
+            "some-key", "some-value",
             "set_on_system", "some-uid", "some-key", "some-value")
-        return self.check_bad_call(d, "modify_system")
 
     def test_set_on_system_modify_system_error(self):
         self.setup_mock()
         self.mock_get_name(succeed(["some-name"]))
         self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(fail(SomeError()))
-        self.mocker.replay()
-        d = self.call_cobbler_method(
+        return self.check_modify_system_error(
+            "some-key", "some-value",
             "set_on_system", "some-uid", "some-key", "some-value")
-        self.assertFailure(d, SomeError)
-        return d
 
     def test_set_on_system_save_system_failure(self):
         self.setup_mock()
         self.mock_get_name(succeed(["some-name"]))
         self.mock_get_system_handle(succeed("some-handle"))
         self.mock_modify_system(succeed(True))
-        self.mock_save_system(succeed(False))
-        self.mocker.replay()
-        d = self.call_cobbler_method(
+        return self.check_save_system_failure(
             "set_on_system", "some-uid", "some-key", "some-value")
-        return self.check_bad_call(d, "save_system")
 
     def test_set_on_system_save_system_error(self):
         self.setup_mock()
         self.mock_get_name(succeed(["some-name"]))
         self.mock_get_system_handle(succeed("some-handle"))
         self.mock_modify_system(succeed(True))
-        self.mock_save_system(fail(SomeError()))
-        self.mocker.replay()
-        d = self.call_cobbler_method(
+        return self.check_save_system_error(
             "set_on_system", "some-uid", "some-key", "some-value")
-        self.assertFailure(d, SomeError)
-        return d
 
     def test_set_on_system_success(self):
         self.setup_mock()
@@ -418,7 +406,10 @@
         d.addCallback(verify)
         return d
 
-    def test_acquire_system_find_error(self):
+
+class AcquireSystemTest(CobblerClientTestCase):
+
+    def test_find_error(self):
         self.setup_mock()
         self.mock_find_system(fail(SomeError()))
         self.mocker.replay()
@@ -426,7 +417,7 @@
         self.assertFailure(d, SomeError)
         return d
 
-    def test_acquire_system_find_failure(self):
+    def test_find_failure(self):
         self.setup_mock()
         self.mock_find_system(succeed([]))
         self.mocker.replay()
@@ -440,7 +431,7 @@
         d.addCallback(verify)
         return d
 
-    def test_acquire_system_get_error(self):
+    def test_get_system_error(self):
         self.setup_mock()
         self.mock_find_system(succeed(["some-name", "other-name"]))
         self.mock_get_system(fail(SomeError()))
@@ -449,7 +440,7 @@
         self.assertFailure(d, SomeError)
         return d
 
-    def test_acquire_system_find_all_insane(self):
+    def test_find_all_insane(self):
         self.setup_mock()
         self.mock_find_system(succeed(["some-name", "other-name"]))
         self.mock_get_system(succeed(_crazy_system("some-uid")))
@@ -466,119 +457,88 @@
         d.addCallback(verify)
         return d
 
-    def test_acquire_system_get_name_failure(self):
-        self.setup_mock()
-        self.mock_find_system(succeed(["some-name"]))
-        self.mock_get_system(succeed(_SOME_SYSTEM))
-        self.mock_get_name(succeed([]))
-        self.mocker.replay()
-        d = self.call_cobbler_method("acquire_system")
-        return self.check_not_found(d)
-
-    def test_acquire_system_get_name_error(self):
-        self.setup_mock()
-        self.mock_find_system(succeed(["some-name"]))
-        self.mock_get_system(succeed(_SOME_SYSTEM))
-        self.mock_get_name(fail(SomeError()))
-        self.mocker.replay()
-        d = self.call_cobbler_method("acquire_system")
-        self.assertFailure(d, SomeError)
-        return d
-
-    def test_acquire_system_get_system_handle_failure(self):
-        self.setup_mock()
-        self.mock_find_system(succeed(["some-name"]))
-        self.mock_get_system(succeed(_SOME_SYSTEM))
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(fail(
-            Fault("blah", "blah unknown system name blah")))
-        self.mocker.replay()
-        d = self.call_cobbler_method("acquire_system")
-        return self.check_not_found(d)
-
-    def test_acquire_system_get_system_handle_unknown_fault(self):
-        self.setup_mock()
-        self.mock_find_system(succeed(["some-name"]))
-        self.mock_get_system(succeed(_SOME_SYSTEM))
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(fail(Fault("blah", "blah")))
-        self.mocker.replay()
-        d = self.call_cobbler_method("acquire_system")
-        self.assertFailure(d, Fault)
-        return d
-
-    def test_acquire_system_get_system_handle_error(self):
-        self.setup_mock()
-        self.mock_find_system(succeed(["some-name"]))
-        self.mock_get_system(succeed(_SOME_SYSTEM))
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(fail(SomeError()))
-        self.mocker.replay()
-        d = self.call_cobbler_method("acquire_system")
-        self.assertFailure(d, SomeError)
-        return d
-
-    def test_acquire_system_modify_system_failure(self):
-        self.setup_mock()
-        self.mock_find_system(succeed(["some-name"]))
-        self.mock_get_system(succeed(_SOME_SYSTEM))
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(succeed(False),
-                                "mgmt_classes", ["preserve_me", "acquired"])
-        self.mocker.replay()
-        d = self.call_cobbler_method("acquire_system")
-        return self.check_bad_call(d, "modify_system")
-
-    def test_acquire_system_modify_system_error(self):
-        self.setup_mock()
-        self.mock_find_system(succeed(["some-name"]))
-        self.mock_get_system(succeed(_SOME_SYSTEM))
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(fail(SomeError()),
-                                "mgmt_classes", ["preserve_me", "acquired"])
-        self.mocker.replay()
-        d = self.call_cobbler_method("acquire_system")
-        self.assertFailure(d, SomeError)
-        return d
-
-    def test_acquire_system_save_system_failure(self):
-        self.setup_mock()
-        self.mock_find_system(succeed(["some-name"]))
-        self.mock_get_system(succeed(_SOME_SYSTEM))
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(succeed(True),
-                                "mgmt_classes", ["preserve_me", "acquired"])
-        self.mock_save_system(succeed(False))
-        self.mocker.replay()
-        d = self.call_cobbler_method("acquire_system")
-        return self.check_bad_call(d, "save_system")
-
-    def test_acquire_system_save_system_error(self):
-        self.setup_mock()
-        self.mock_find_system(succeed(["some-name"]))
-        self.mock_get_system(succeed(_SOME_SYSTEM))
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(succeed(True),
-                                "mgmt_classes", ["preserve_me", "acquired"])
-        self.mock_save_system(fail(SomeError()))
-        self.mocker.replay()
-        d = self.call_cobbler_method("acquire_system")
-        self.assertFailure(d, SomeError)
-        return d
-
-    def test_acquire_system_eventual_success(self):
+    def test_get_name_failure(self):
+        self.setup_mock()
+        self.mock_find_system(succeed(["some-name"]))
+        self.mock_get_system(succeed(_SOME_SYSTEM))
+        return self.check_get_name_failure("acquire_system")
+
+    def test_get_name_error(self):
+        self.setup_mock()
+        self.mock_find_system(succeed(["some-name"]))
+        self.mock_get_system(succeed(_SOME_SYSTEM))
+        return self.check_get_name_error("acquire_system")
+
+    def test_get_system_handle_failure(self):
+        self.setup_mock()
+        self.mock_find_system(succeed(["some-name"]))
+        self.mock_get_system(succeed(_SOME_SYSTEM))
+        self.mock_get_name(succeed(["some-name"]))
+        return self.check_get_system_handle_failure("acquire_system")
+
+    def test_get_system_handle_unknown_fault(self):
+        self.setup_mock()
+        self.mock_find_system(succeed(["some-name"]))
+        self.mock_get_system(succeed(_SOME_SYSTEM))
+        self.mock_get_name(succeed(["some-name"]))
+        return self.check_get_system_handle_fault("acquire_system")
+
+    def test_get_system_handle_error(self):
+        self.setup_mock()
+        self.mock_find_system(succeed(["some-name"]))
+        self.mock_get_system(succeed(_SOME_SYSTEM))
+        self.mock_get_name(succeed(["some-name"]))
+        return self.check_get_system_handle_error("acquire_system")
+
+    def test_modify_system_failure(self):
+        self.setup_mock()
+        self.mock_find_system(succeed(["some-name"]))
+        self.mock_get_system(succeed(_SOME_SYSTEM))
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        return self.check_modify_system_failure(
+            "mgmt_classes", ["preserve_me", "acquired"],
+            "acquire_system")
+
+    def test_modify_system_error(self):
+        self.setup_mock()
+        self.mock_find_system(succeed(["some-name"]))
+        self.mock_get_system(succeed(_SOME_SYSTEM))
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        return self.check_modify_system_error(
+            "mgmt_classes", ["preserve_me", "acquired"],
+            "acquire_system")
+
+    def test_save_system_failure(self):
+        self.setup_mock()
+        self.mock_find_system(succeed(["some-name"]))
+        self.mock_get_system(succeed(_SOME_SYSTEM))
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        self.mock_modify_system(
+            succeed(True), "mgmt_classes", ["preserve_me", "acquired"])
+        return self.check_save_system_failure("acquire_system")
+
+    def test_save_system_error(self):
+        self.setup_mock()
+        self.mock_find_system(succeed(["some-name"]))
+        self.mock_get_system(succeed(_SOME_SYSTEM))
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        self.mock_modify_system(
+            succeed(True), "mgmt_classes", ["preserve_me", "acquired"])
+        return self.check_save_system_error("acquire_system")
+
+    def test_eventual_success(self):
         self.setup_mock()
         self.mock_find_system(succeed(["bad-name", "some-name"]))
         self.mock_get_system(succeed("~"), name="bad-name")
         self.mock_get_system(succeed(_SOME_SYSTEM))
         self.mock_get_name(succeed(["some-name"]))
         self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(succeed(True),
-                                "mgmt_classes", ["preserve_me", "acquired"])
+        self.mock_modify_system(
+            succeed(True), "mgmt_classes", ["preserve_me", "acquired"])
         self.mock_save_system(succeed(True))
         self.mocker.replay()
         d = self.call_cobbler_method("acquire_system")
@@ -588,106 +548,213 @@
         d.addCallback(verify)
         return d
 
-    def test_start_system_get_name_failure(self):
-        return self.check_get_name_failure("start_system", "some-uid")
-
-    def test_start_system_get_name_insane(self):
-        return self.check_get_name_insane("start_system", "some-uid")
-
-    def test_start_system_get_name_error(self):
-        return self.check_get_name_error("start_system", "some-uid")
-
-    def test_start_system_get_system_handle_failure(self):
-        self.setup_mock()
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(fail(
-            Fault("blah", "blah unknown system name blah")))
-        self.mocker.replay()
-        d = self.call_cobbler_method("start_system", "some-uid")
+
+class ReleaseSystemTest(CobblerClientTestCase):
+
+    def test_get_systems_error(self):
+        self.setup_mock()
+        self.mock_get_systems(fail(SomeError()))
+        self.mocker.replay()
+
+        d = self.call_cobbler_method("release_system", "some-uid")
+        self.assertFailure(d, SomeError)
+        return d
+
+    def test_get_systems_failure(self):
+        self.setup_mock()
+        self.mock_get_systems(succeed([]))
+        self.mocker.replay()
+
+        d = self.call_cobbler_method("release_system", "some-uid")
         return self.check_not_found(d)
 
-    def test_start_system_get_system_handle_unknown_fault(self):
-        self.setup_mock()
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(fail(Fault("blah", "blah")))
-        self.mocker.replay()
-        d = self.call_cobbler_method("start_system", "some-uid")
-        self.assertFailure(d, Fault)
-        return d
-
-    def test_start_system_get_system_handle_error(self):
-        self.setup_mock()
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(fail(SomeError()))
-        self.mocker.replay()
-        d = self.call_cobbler_method("start_system", "some-uid")
-        self.assertFailure(d, SomeError)
-        return d
-
-    def test_start_system_modify_system_failure(self):
-        self.setup_mock()
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(succeed(False), "netboot_enabled", True)
-        self.mocker.replay()
-        d = self.call_cobbler_method("start_system", "some-uid")
-        return self.check_bad_call(d, "modify_system")
-
-    def test_start_system_modify_system_error(self):
-        self.setup_mock()
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(fail(SomeError()), "netboot_enabled", True)
-        self.mocker.replay()
-        d = self.call_cobbler_method("start_system", "some-uid")
-        self.assertFailure(d, SomeError)
-        return d
-
-    def test_start_system_save_system_failure(self):
-        self.setup_mock()
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(succeed(True), "netboot_enabled", True)
-        self.mock_save_system(succeed(False))
-        self.mocker.replay()
-        d = self.call_cobbler_method("start_system", "some-uid")
-        return self.check_bad_call(d, "save_system")
-
-    def test_start_system_save_system_error(self):
-        self.setup_mock()
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(succeed(True), "netboot_enabled", True)
-        self.mock_save_system(fail(SomeError()))
-        self.mocker.replay()
-        d = self.call_cobbler_method("start_system", "some-uid")
-        self.assertFailure(d, SomeError)
-        return d
-
-    def test_start_system_power_error(self):
-        self.setup_mock()
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(succeed(True), "netboot_enabled", True)
-        self.mock_save_system(succeed(True))
-        self.mock_power_system(fail(SomeError()))
-        self.mocker.replay()
-        d = self.call_cobbler_method("start_system", "some-uid")
-        self.assertFailure(d, SomeError)
-        return d
-
-    def test_start_system_power_probably_success_cant_tell(self):
-        self.setup_mock()
-        self.mock_get_name(succeed(["some-name"]))
-        self.mock_get_system_handle(succeed("some-handle"))
-        self.mock_modify_system(succeed(True), "netboot_enabled", True)
-        self.mock_save_system(succeed(True))
-        self.mock_power_system(succeed("ignored"))
-        self.mocker.replay()
-        d = self.call_cobbler_method("start_system", "some-uid")
-        return d
-
-    def test_describe_systems_error(self):
+    def test_get_name_error(self):
+        self.setup_mock()
+        self.mock_get_systems(succeed([
+            {"uid": "some-uid", "mgmt_classes": ["acquired"]}]))
+        return self.check_get_name_error("release_system", "some-uid")
+
+    def test_get_name_failure(self):
+        self.setup_mock()
+        self.mock_get_systems(succeed([
+            {"uid": "some-uid", "mgmt_classes": ["acquired"]}]))
+        return self.check_get_name_failure("release_system", "some-uid")
+
+    def test_get_system_handle_error(self):
+        self.setup_mock()
+        self.mock_get_systems(succeed([
+            {"uid": "some-uid", "mgmt_classes": ["acquired"]}]))
+        self.mock_get_name(succeed(["some-name"]))
+        return self.check_get_system_handle_error("release_system", "some-uid")
+
+    def test_get_system_handle_fault(self):
+        self.setup_mock()
+        self.mock_get_systems(succeed([
+            {"uid": "some-uid", "mgmt_classes": ["acquired"]}]))
+        self.mock_get_name(succeed(["some-name"]))
+        return self.check_get_system_handle_fault("release_system", "some-uid")
+
+    def test_get_system_handle_failure(self):
+        self.setup_mock()
+        self.mock_get_systems(succeed([
+            {"uid": "some-uid", "mgmt_classes": ["acquired"]}]))
+        self.mock_get_name(succeed(["some-name"]))
+        return self.check_get_system_handle_failure(
+            "release_system", "some-uid")
+
+    def test_modify_system_error(self):
+        self.setup_mock()
+        self.mock_get_systems(succeed([
+            {"uid": "some-uid", "mgmt_classes": ["preserve_me", "acquired"]}]))
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        return self.check_modify_system_error(
+            "mgmt_classes", ["preserve_me", "available"],
+            "release_system", "some-uid")
+
+    def test_modify_system_failure(self):
+        self.setup_mock()
+        self.mock_get_systems(succeed([
+            {"uid": "some-uid", "mgmt_classes": ["preserve_me", "acquired"]}]))
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        return self.check_modify_system_failure(
+            "mgmt_classes", ["preserve_me", "available"],
+            "release_system", "some-uid")
+
+    def test_save_system_error(self):
+        self.setup_mock()
+        self.mock_get_systems(succeed([
+            {"uid": "some-uid", "mgmt_classes": ["preserve_me", "acquired"]}]))
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        self.mock_modify_system(
+            succeed(True), "mgmt_classes", ["preserve_me", "available"])
+        return self.check_save_system_error("release_system", "some-uid")
+
+    def test_save_system_failure(self):
+        self.setup_mock()
+        self.mock_get_systems(succeed([
+            {"uid": "some-uid", "mgmt_classes": ["preserve_me", "acquired"]}]))
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        self.mock_modify_system(
+            succeed(True), "mgmt_classes", ["preserve_me", "available"])
+        return self.check_save_system_failure("release_system", "some-uid")
+
+    def test_success(self):
+        self.setup_mock()
+        self.mock_get_systems(succeed([
+            {"uid": "some-uid", "mgmt_classes": ["preserve_me", "acquired"]}]))
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        self.mock_modify_system(
+            succeed(True), "mgmt_classes", ["preserve_me", "available"])
+        self.mock_save_system(succeed(True))
+        self.mocker.replay()
+        return self.call_cobbler_method("release_system", "some-uid")
+
+
+class PowerTestsMixin(object):
+
+    def test_get_name_failure(self):
+        self.setup_mock()
+        return self.check_get_name_failure(self.method_name, "some-uid")
+
+    def test_get_name_insane(self):
+        self.setup_mock()
+        return self.check_get_name_insane(self.method_name, "some-uid")
+
+    def test_get_name_error(self):
+        self.setup_mock()
+        return self.check_get_name_error(self.method_name, "some-uid")
+
+    def test_get_system_handle_failure(self):
+        self.setup_mock()
+        self.mock_get_name(succeed(["some-name"]))
+        return self.check_get_system_handle_failure(
+            self.method_name, "some-uid")
+
+    def test_get_system_handle_unknown_fault(self):
+        self.setup_mock()
+        self.mock_get_name(succeed(["some-name"]))
+        return self.check_get_system_handle_fault(
+            self.method_name, "some-uid")
+
+    def test_get_system_handle_error(self):
+        self.setup_mock()
+        self.mock_get_name(succeed(["some-name"]))
+        return self.check_get_system_handle_error(
+            self.method_name, "some-uid")
+
+    def test_modify_system_failure(self):
+        self.setup_mock()
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        return self.check_modify_system_failure(
+            "netboot_enabled", True,
+            self.method_name, "some-uid")
+
+    def test_modify_system_error(self):
+        self.setup_mock()
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        return self.check_modify_system_error(
+            "netboot_enabled", True,
+            self.method_name, "some-uid")
+
+    def test_save_system_failure(self):
+        self.setup_mock()
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        self.mock_modify_system(succeed(True), "netboot_enabled", True)
+        return self.check_save_system_failure(self.method_name, "some-uid")
+
+    def test_save_system_error(self):
+        self.setup_mock()
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        self.mock_modify_system(succeed(True), "netboot_enabled", True)
+        return self.check_save_system_error(self.method_name, "some-uid")
+
+    def test_power_error(self):
+        self.setup_mock()
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        self.mock_modify_system(succeed(True), "netboot_enabled", True)
+        self.mock_save_system(succeed(True))
+        self.mock_power_system(self.desired_state, fail(SomeError()))
+        self.mocker.replay()
+        d = self.call_cobbler_method(self.method_name, "some-uid")
+        self.assertFailure(d, SomeError)
+        return d
+
+    def test_power_probably_success_cant_tell(self):
+        self.setup_mock()
+        self.mock_get_name(succeed(["some-name"]))
+        self.mock_get_system_handle(succeed("some-handle"))
+        self.mock_modify_system(succeed(True), "netboot_enabled", True)
+        self.mock_save_system(succeed(True))
+        self.mock_power_system(self.desired_state, succeed("ignored"))
+        self.mocker.replay()
+        return self.call_cobbler_method(self.method_name, "some-uid")
+
+
+class StartSystemTest(CobblerClientTestCase, PowerTestsMixin):
+
+    desired_state = "on"
+    method_name = "start_system"
+
+
+class StopSystemTest(CobblerClientTestCase, PowerTestsMixin):
+
+    desired_state = "off"
+    method_name = "stop_system"
+
+
+class DescribeSystemsTest(CobblerClientTestCase):
+
+    def test_error(self):
         self.setup_mock()
         self.mock_get_systems(fail(SomeError()))
         self.mocker.replay()
@@ -695,7 +762,7 @@
         self.assertFailure(d, SomeError)
         return d
 
-    def test_describe_systems_failure(self):
+    def test_failure(self):
         self.setup_mock()
         self.mock_get_systems(succeed([
             {"uid": "something-else", "mgmt_classes": "acquired"}]))
@@ -703,7 +770,7 @@
         d = self.call_cobbler_method("describe_systems", "something")
         return self.check_not_found(d, ["something"])
 
-    def test_describe_systems_success_all(self):
+    def test_success_all(self):
         self.setup_mock()
         self.mock_get_systems(succeed([
             {"uid": "bar", "mgmt_classes": ["acquired"]},
@@ -719,7 +786,7 @@
         d.addCallback(verify)
         return d
 
-    def test_describe_systems_not_acquired_some(self):
+    def test_not_acquired_some(self):
         self.setup_mock()
         self.mock_get_systems(succeed([
             {"uid": "bar", "mgmt_classes": ["irrelevant"]},
@@ -729,7 +796,7 @@
         d = self.call_cobbler_method("describe_systems", "foo", "baz")
         return self.check_not_found(d, ["foo"])
 
-    def test_describe_systems_success_some(self):
+    def test_success_some(self):
         self.setup_mock()
         self.mock_get_systems(succeed([
             {"uid": "bar", "mgmt_classes": ["ignored"]},
@@ -745,7 +812,7 @@
         d.addCallback(verify)
         return d
 
-    def test_describe_systems_success_none(self):
+    def test_success_none(self):
         self.setup_mock()
         self.mock_get_systems(succeed([]))
         self.mocker.replay()

=== modified file 'ensemble/providers/orchestra/tests/test_getmachines.py'
--- ensemble/providers/orchestra/tests/test_getmachines.py	2011-08-12 19:28:37 +0000
+++ ensemble/providers/orchestra/tests/test_getmachines.py	2011-08-23 21:41:27 +0000
@@ -97,4 +97,3 @@
         d = provider.get_machine("foo")
         self.assertFailure(d, SomeError)
         return d
-

=== modified file 'ensemble/providers/orchestra/tests/test_launch.py'
--- ensemble/providers/orchestra/tests/test_launch.py	2011-08-11 04:00:26 +0000
+++ ensemble/providers/orchestra/tests/test_launch.py	2011-08-23 21:41:27 +0000
@@ -1,3 +1,5 @@
+from twisted.internet.defer import succeed
+
 from ensemble.errors import EnvironmentNotFound, ProviderError
 from ensemble.lib.testing import TestCase
 from ensemble.providers.orchestra.machine import OrchestraMachine
@@ -54,13 +56,17 @@
         self.mock_set_ks_meta(
             self.get_verify_ks_meta(42, "launch_late_command"))
         self.mock_start_system()
-        self.mock_describe_system()
+        self.mock_describe_systems(succeed([{
+            "uid": "winston-uid",
+            "name": "winston",
+            "mgmt_classes": ["acquired"]}]))
         self.mocker.replay()
 
         def verify_machines(machines):
             (machine,) = machines
             self.assertTrue(isinstance(machine, OrchestraMachine))
             self.assertEquals(machine.instance_id, "winston-uid")
+            self.assertEquals(machine.dns_name, "winston")
         d = self.get_provider().start_machine({"machine-id": "42"})
         d.addCallback(verify_machines)
         return d

=== added file 'ensemble/providers/orchestra/tests/test_shutdown.py'
--- ensemble/providers/orchestra/tests/test_shutdown.py	1970-01-01 00:00:00 +0000
+++ ensemble/providers/orchestra/tests/test_shutdown.py	2011-08-23 21:41:27 +0000
@@ -0,0 +1,195 @@
+from twisted.internet.defer import fail, succeed
+
+from ensemble.errors import MachinesNotFound
+from ensemble.lib.testing import TestCase
+from ensemble.providers.orchestra.machine import OrchestraMachine
+
+from .common import OrchestraTestMixin
+
+
+class SomeError(Exception):
+    pass
+
+
+class ShutdownTest(OrchestraTestMixin, TestCase):
+
+    def assert_machine(self, machine, uid, name):
+        self.assertTrue(isinstance(machine, OrchestraMachine))
+        self.assertEquals(machine.instance_id, uid)
+        self.assertEquals(machine.dns_name, name)
+
+    def assert_missing(self, d, instance_ids):
+        self.assertFailure(d, MachinesNotFound)
+
+        def verify(error):
+            self.assertEquals(error.instance_ids, instance_ids)
+        d.addCallback(verify)
+        return d
+
+    def mock_get_systems(self, *systems):
+        self.proxy_m.callRemote("get_systems")
+        self.mocker.result(succeed([
+            {"uid": uid, "name": name, "mgmt_classes": ["keep", cls]}
+            for (uid, name, cls) in systems]))
+
+    def mock_shutdown_release(self, uid, name):
+        self.proxy_m.callRemote("find_system", {"uid": uid})
+        self.mocker.result(succeed([name]))
+        self.proxy_m.callRemote("get_system_handle", name, "")
+        self.mocker.result(succeed("some-handle"))
+        self.proxy_m.callRemote(
+            "modify_system", "some-handle", "netboot_enabled", True, "")
+        self.mocker.result(succeed(True))
+        self.proxy_m.callRemote("save_system", "some-handle", "")
+        self.mocker.result(succeed(True))
+        self.proxy_m.callRemote(
+            "background_power_system", {"power": "off", "systems": [name]}, "")
+        self.mocker.result(succeed("ignored"))
+        self.mock_get_systems((uid, name, "acquired"))
+        self.proxy_m.callRemote("find_system", {"uid": uid})
+        self.mocker.result(succeed([name]))
+        self.proxy_m.callRemote("get_system_handle", name, "")
+        self.mocker.result(succeed("some-handle"))
+        self.proxy_m.callRemote(
+            "modify_system", "some-handle",
+            "mgmt_classes", ["keep", "available"], "")
+        self.mocker.result(succeed(True))
+        self.proxy_m.callRemote("save_system", "some-handle", "")
+        self.mocker.result(succeed(True))
+
+    def check_shutdown_all(self, method_name):
+        self.mock_get_systems(
+            ("i-amok", "ok", "acquired"),
+            ("i-amalien", "alien", "available"),
+            ("i-amoktoo", "oktoo", "acquired"))
+        self.mock_get_systems(
+            ("i-amok", "ok", "acquired"),
+            ("i-amalien", "alien", "available"),
+            ("i-amoktoo", "oktoo", "acquired"))
+        self.mock_shutdown_release("i-amok", "ok")
+        self.mock_shutdown_release("i-amoktoo", "oktoo")
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        method = getattr(provider, method_name)
+        d = method()
+
+        def verify(machines):
+            (ok, oktoo) = machines
+            self.assert_machine(ok, "i-amok", "ok")
+            self.assert_machine(oktoo, "i-amoktoo", "oktoo")
+        d.addCallback(verify)
+        return d
+
+    def test_shutdown_machine(self):
+        self.setup_mocks()
+        self.mock_get_systems(("i-amhere", "blah", "acquired"))
+        self.mock_shutdown_release("i-amhere", "blah")
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        d = provider.shutdown_machine(OrchestraMachine("i-amhere"))
+        d.addCallback(self.assert_machine, "i-amhere", "blah")
+        return d
+
+    def test_shutdown_machine_missing(self):
+        self.setup_mocks()
+        self.mock_get_systems(("i-amirrelevant", "blah", "acquired"))
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        d = provider.shutdown_machine(OrchestraMachine("i-ammissing"))
+        return self.assert_missing(d, ["i-ammissing"])
+
+    def test_shutdown_machine_unowned(self):
+        self.setup_mocks()
+        self.mock_get_systems(("i-amalien", "blah", "available"))
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        d = provider.shutdown_machine(OrchestraMachine("i-amalien"))
+        return self.assert_missing(d, ["i-amalien"])
+
+    def test_shutdown_machines_none(self):
+        self.mocker.replay()
+        provider = self.get_provider()
+        d = provider.shutdown_machines([])
+        d.addCallback(self.assertEquals, [])
+        return d
+
+    def test_shutdown_machines_some_good(self):
+        self.setup_mocks()
+        self.mock_get_systems(
+            ("i-amok", "ok", "acquired"),
+            ("i-amalien", "alien", "available"),
+            ("i-amoktoo", "oktoo", "acquired"))
+        self.mock_shutdown_release("i-amok", "ok")
+        self.mock_shutdown_release("i-amoktoo", "oktoo")
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        d = provider.shutdown_machines([
+            OrchestraMachine("i-amok"),
+            OrchestraMachine("i-amoktoo")])
+
+        def verify(machines):
+            (ok, oktoo) = machines
+            self.assert_machine(ok, "i-amok", "ok")
+            self.assert_machine(oktoo, "i-amoktoo", "oktoo")
+        d.addCallback(verify)
+        return d
+
+    def test_shutdown_machines_some_missing(self):
+        self.setup_mocks()
+        self.mock_get_systems(
+            ("i-amok", "ok", "acquired"),
+            ("i-amalien", "alien", "available"),
+            ("i-amoktoo", "oktoo", "acquired"))
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        d = provider.shutdown_machines([
+            OrchestraMachine("i-amok"),
+            OrchestraMachine("i-ammissing")])
+        return self.assert_missing(d, ["i-ammissing"])
+
+    def test_shutdown_machines_some_unowned(self):
+        self.setup_mocks()
+        self.mock_get_systems(
+            ("i-amok", "ok", "acquired"),
+            ("i-amalien", "alien", "available"),
+            ("i-amoktoo", "oktoo", "acquired"))
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        d = provider.shutdown_machines([
+            OrchestraMachine("i-amok"),
+            OrchestraMachine("i-amalien")])
+        return self.assert_missing(d, ["i-amalien"])
+
+    def test_destroy_environment(self):
+        self.setup_mocks()
+        self.getPage("http://somewhe.re/webdav/provider-state",
+                     method="PUT", postdata="{}\n")
+        self.mocker.result(succeed(True))
+        return self.check_shutdown_all("destroy_environment")
+
+    def test_destroy_environment_no_machines(self):
+        self.setup_mocks()
+        self.getPage("http://somewhe.re/webdav/provider-state",
+                     method="PUT", postdata="{}\n")
+        self.mocker.result(succeed(True))
+        self.mock_get_systems()
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        d = provider.destroy_environment()
+        d.addCallback(self.assertEquals, [])
+        return d
+
+    def test_destroy_environment_unwritable(self):
+        self.setup_mocks()
+        self.getPage("http://somewhe.re/webdav/provider-state",
+                     method="PUT", postdata="{}\n")
+        self.mocker.result(fail(SomeError()))
+        return self.check_shutdown_all("destroy_environment")