Merge lp:~fwereade/pyjuju/find-all-zookeepers into lp:pyjuju

[William Reade]

Fix lp:823511 (we only find one zookeeper, even if several exist).

Did this because forthcoming common zookeeper-connect code is quite neat if it works with multiple machines, and I got tired of seeing the TODO.

[Gustavo Niemeyer (niemeyer)]

William confirmed on IRC that the call sites won't break with more than one machine.

It's not clear what's the goal here, but that's ok.

review: Approve

=== modified file 'ensemble/providers/common/findzookeepers.py'
--- ensemble/providers/common/findzookeepers.py	2011-08-09 20:10:05 +0000
+++ ensemble/providers/common/findzookeepers.py	2011-08-16 11:22:25 +0000
@@ -17,19 +17,20 @@
     instance_ids = state.get("zookeeper-instances")
     _require(instance_ids)
 
-    # TODO we should collect all valid machines, instead of just returning a
-    # list of the first valid one we find. Filed bug #823511.
     pending = False
+    machines = []
     missing_instance_ids = []
     for instance_id in instance_ids:
         try:
             machine = yield get_machine(provider, instance_id)
-            returnValue([machine])
+            machines.append(machine)
         except MachineNotFound as e:
             missing_instance_ids.append(e.instance_id)
         except MachineNotReady:
             pending = True
 
+    if machines:
+        returnValue(machines)
     if pending:
         raise EnvironmentPending("Started machine is not yet initialized.")
     raise EnvironmentNotFound("None of the candidate machines (%s) are either "

=== modified file 'ensemble/providers/common/tests/test_findzookeepers.py'
--- ensemble/providers/common/tests/test_findzookeepers.py	2011-08-03 13:22:35 +0000
+++ ensemble/providers/common/tests/test_findzookeepers.py	2011-08-16 11:22:25 +0000
@@ -93,22 +93,25 @@
         d.addCallback(verify_machine)
         return d
 
-    def test_first_good_always_wins(self):
+    def test_gets_all_good_machines(self):
         provider = DummyProvider(
             {"zookeeper-instances": ["porter", "carter", "miller", "baker"]})
         get_machine = self.mocker.mock()
         get_machine(provider, "porter")
         self.mocker.result(fail(MachineNotFound("porter")))
         get_machine(provider, "carter")
-        self.mocker.result(fail(MachineNotReady("carter")))
+        carter = object()
+        self.mocker.result(succeed(carter))
         get_machine(provider, "miller")
-        machine = object()
-        self.mocker.result(succeed(machine))
+        self.mocker.result(fail(MachineNotReady("miller")))
+        get_machine(provider, "baker")
+        baker = object()
+        self.mocker.result(succeed(baker))
         self.mocker.replay()
 
         d = find_zookeepers(provider, get_machine)
 
         def verify_machine(result):
-            self.assertEquals(result, [machine])
+            self.assertEquals(result, [carter, baker])
         d.addCallback(verify_machine)
         return d

=== modified file 'ensemble/providers/ec2/tests/test_findzookeeper.py'
--- ensemble/providers/ec2/tests/test_findzookeeper.py	2011-08-05 12:43:39 +0000
+++ ensemble/providers/ec2/tests/test_findzookeeper.py	2011-08-16 11:22:25 +0000
@@ -154,12 +154,12 @@
         # Zk instances are checked individually to handle invalid ids correctly
         self.ec2.describe_instances("i-invalid")
         self.mocker.result(fail(_invalid_id_error()))
-
         self.ec2.describe_instances(server1.instance_id)
         self.mocker.result(succeed([server1]))
-
         self.ec2.describe_instances(server2.instance_id)
         self.mocker.result(succeed([server2]))
+        self.ec2.describe_instances(server3.instance_id)
+        self.mocker.result(succeed([server3]))
 
         self.mocker.replay()
 
@@ -167,8 +167,10 @@
         d = provider.get_zookeeper_machines()
 
         def verify_machines(machines):
-            (machine,) = machines
-            self.assertEquals(machine.instance_id, "i-foobaz")
-            self.assertTrue(isinstance(machine, EC2ProviderMachine))
+            (foobaz, foobop) = machines
+            self.assertEquals(foobaz.instance_id, "i-foobaz")
+            self.assertTrue(isinstance(foobaz, EC2ProviderMachine))
+            self.assertEquals(foobop.instance_id, "i-foobop")
+            self.assertTrue(isinstance(foobop, EC2ProviderMachine))
         d.addCallback(verify_machines)
         return d

=== modified file 'ensemble/providers/orchestra/tests/test_findzookeepers.py'
--- ensemble/providers/orchestra/tests/test_findzookeepers.py	2011-07-31 00:54:45 +0000
+++ ensemble/providers/orchestra/tests/test_findzookeepers.py	2011-08-16 11:22:25 +0000
@@ -60,8 +60,8 @@
         return self.assert_no_environment()
 
     def test_eventual_success(self):
-        self.mock_load_state(succeed(dump({"zookeeper-instances": ["bad",
-                                                                   "foo"]})))
+        self.mock_load_state(succeed(dump({
+            "zookeeper-instances": ["bad", "foo", "wrong", "bar"]})))
         proxy_m = self.mocker.mock(Proxy)
         Proxy_m = self.mocker.replace(Proxy, spec=None)
         Proxy_m("http://somewhe.re/cobbler_api")
@@ -69,18 +69,27 @@
         proxy_m.callRemote("find_system", {"uid": "bad"})
         self.mocker.result(succeed([]))
         proxy_m.callRemote("find_system", {"uid": "foo"})
-        self.mocker.result(succeed(["bar"]))
-        proxy_m.callRemote("get_system", "bar")
-        self.mocker.result(succeed({"name": "bar", "uid": "foo"}))
+        self.mocker.result(succeed(["fooname"]))
+        proxy_m.callRemote("get_system", "fooname")
+        self.mocker.result(succeed({"name": "fooname", "uid": "foo"}))
+        proxy_m.callRemote("find_system", {"uid": "wrong"})
+        self.mocker.result(succeed([]))
+        proxy_m.callRemote("find_system", {"uid": "bar"})
+        self.mocker.result(succeed(["barname"]))
+        proxy_m.callRemote("get_system", "barname")
+        self.mocker.result(succeed({"name": "barname", "uid": "bar"}))
         self.mocker.replay()
 
         provider = self.get_provider()
         d = provider.get_zookeeper_machines()
 
         def verify_machine(machines):
-            (machine,) = machines
-            self.assertTrue(isinstance(machine, OrchestraMachine))
-            self.assertEquals(machine.instance_id, "foo")
-            self.assertEquals(machine.dns_name, "bar")
+            (foo, bar) = machines
+            self.assertTrue(isinstance(foo, OrchestraMachine))
+            self.assertEquals(foo.instance_id, "foo")
+            self.assertEquals(foo.dns_name, "fooname")
+            self.assertTrue(isinstance(bar, OrchestraMachine))
+            self.assertEquals(bar.instance_id, "bar")
+            self.assertEquals(bar.dns_name, "barname")
         d.addCallback(verify_machine)
         return d