Merge lp:~fwereade/pyjuju/cobbler-instance-ids into lp:pyjuju

[William Reade]

Long-term, cobbler system names can change, and are therefore a bad choice for ensemble instance IDs. This uses cobbler UIDs as instance IDs instead, and should therefore be more reliable in general (we'll still have problems if system names change while we're manipulating them, but the window of opportunity is much smaller)

[Gustavo Niemeyer (niemeyer)]

Looks good!

review: Approve

[lp:~fwereade/pyjuju/cobbler-instance-ids]

316. By William Reade on 2011-07-27

merge parent

317. By William Reade on 2011-07-30

merge parent

318. By William Reade on 2011-08-02

merge parent

[Kapil Thangavelu (hazmat)]

LGTM, +1

review: Approve

[lp:~fwereade/pyjuju/cobbler-instance-ids]

319. By William Reade on 2011-08-03

merge latest trunk

=== modified file 'ensemble/providers/orchestra/cobbler.py'
--- ensemble/providers/orchestra/cobbler.py	2011-07-30 11:50:10 +0000
+++ ensemble/providers/orchestra/cobbler.py	2011-08-03 06:19:24 +0000
@@ -75,33 +75,35 @@
         return call
 
     @inlineCallbacks
-    def _set_on_system(self, name, key, value):
+    def _set_on_system(self, instance_id, key, value):
+        (name,) = yield self._call("find_system", ({"uid": instance_id},))
         handle = yield self._call("get_system_handle", (name,), auth=True)
         yield self._check_call("modify_system", (handle, key, value),
                                auth=True, expect=True)
         yield self._check_call("save_system", (handle,), auth=True, expect=True)
+        returnValue(name)
 
     @inlineCallbacks
     def acquire_system(self):
         """Find a system marked as available and mark it as acquired.
 
-        @return: the name of the acquired system.
+        @return: the instance id (cobbler uid) of the acquired system.
 
         @raise: ProviderError if no suitable system can be found.
         """
         systems = yield self._call("get_systems")
         system = _acceptable_system(systems, self._available_class)
-        name = system["name"]
+        instance_id = system["uid"]
         # TODO surely we should preserve unrelated management classes?
-        yield self._set_on_system(name, "mgmt_classes", [self._acquired_class])
-        returnValue(name)
+        yield self._set_on_system(instance_id, "mgmt_classes", [self._acquired_class])
+        returnValue(instance_id)
 
-    def set_system_ks_meta(self, name, ks_meta):
-        return self._set_on_system(name, "ks_meta", ks_meta)
+    def set_system_ks_meta(self, instance_id, ks_meta):
+        return self._set_on_system(instance_id, "ks_meta", ks_meta)
 
     @inlineCallbacks
-    def start_system(self, name):
-        yield self._set_on_system(name, "netboot_enabled", True)
+    def start_system(self, instance_id):
+        name = yield self._set_on_system(instance_id, "netboot_enabled", True)
         yield self._call("background_power_system",
                          ({"power": "on", "systems": [name]},),
                          auth=True)

=== modified file 'ensemble/providers/orchestra/launch.py'
--- ensemble/providers/orchestra/launch.py	2011-07-20 23:20:23 +0000
+++ ensemble/providers/orchestra/launch.py	2011-08-03 06:19:24 +0000
@@ -11,10 +11,10 @@
 
     @inlineCallbacks
     def start_machine(self, variables):
-        name = yield self._provider.cobbler.acquire_system()
+        instance_id = yield self._provider.cobbler.acquire_system()
         ks_meta = 'ENSEMBLE_LATE_COMMAND="true"'
 
-        yield self._provider.cobbler.set_system_ks_meta(name, ks_meta)
-        yield self._provider.cobbler.start_system(name)
+        yield self._provider.cobbler.set_system_ks_meta(instance_id, ks_meta)
+        yield self._provider.cobbler.start_system(instance_id)
 
-        returnValue([OrchestraMachine(name, name)])
+        returnValue([OrchestraMachine(instance_id, instance_id)])

=== modified file 'ensemble/providers/orchestra/tests/test_bootstrap.py'
--- ensemble/providers/orchestra/tests/test_bootstrap.py	2011-08-01 20:02:18 +0000
+++ ensemble/providers/orchestra/tests/test_bootstrap.py	2011-08-03 06:19:24 +0000
@@ -34,12 +34,14 @@
             systems.append(
                 {"mgmt_classes": ["available"], # relevant
                  "netboot_enabled": True,
-                 "name": "winston"})
+                 "uid": "winston-uid"})
 
         self.proxy_m.callRemote("get_systems")
         self.mocker.result(succeed(systems))
 
     def mock_acquire_system(self, unexpected_auth_error=None):
+        self.proxy_m.callRemote("find_system", {"uid": "winston-uid"})
+        self.mocker.result(succeed(["winston"]))
         self.proxy_m.callRemote("get_system_handle", "winston", "")
         if unexpected_auth_error is not None:
             self.mocker.result(fail(unexpected_auth_error))
@@ -56,6 +58,8 @@
         self.mocker.result(succeed(True))
 
     def mock_set_ks_meta(self, fail_modify=False, fail_save=False):
+        self.proxy_m.callRemote("find_system", {"uid": "winston-uid"})
+        self.mocker.result(succeed(["winston"]))
         self.proxy_m.callRemote("get_system_handle", "winston", "TOKEN")
         self.mocker.result(succeed("smith"))
         # TODO: parameterise this match?
@@ -73,6 +77,8 @@
         self.mocker.result(succeed(True))
 
     def mock_start_system(self):
+        self.proxy_m.callRemote("find_system", {"uid": "winston-uid"})
+        self.mocker.result(succeed(["winston"]))
         self.proxy_m.callRemote("get_system_handle", "winston", "TOKEN")
         self.mocker.result(succeed("smith"))
         self.proxy_m.callRemote(
@@ -138,6 +144,6 @@
         def verify_machines(machines):
             (machine,) = machines
             self.assertTrue(isinstance(machine, OrchestraMachine))
-            self.assertEquals(machine.instance_id, "winston")
+            self.assertEquals(machine.instance_id, "winston-uid")
         d.addCallback(verify_machines)
         return d