Merge lp:~dpb/juju-deployer/lxc-placement-fix into lp:juju-deployer

[David Britton]

I'm pretty sure a simple variable error/typo caused this bug. The fall-through behavior of the routine was correct, but an early exit was checking the wrong variable making the default placement algorithm take over too soon.

I changed the variable being tested and cleaned up the warning message a bit. I also added a unit test for this case (which runs and fails on the old code)

[lp:~dpb/juju-deployer/lxc-placement-fix]

125. By David Britton on 2014-08-15

typo in log print

[Kapil Thangavelu (hazmat)]

Thanks, merged

review: Approve

=== modified file 'deployer/service.py'
--- deployer/service.py	2014-06-19 12:30:44 +0000
+++ deployer/service.py	2014-08-15 21:05:19 +0000
@@ -139,10 +139,11 @@
             return None
 
         svc_units = with_service['units']
-        if len(svc_units) <= unit_number:
+        if int(u_idx) >= len(svc_units):
             self.deployment.log.warning(
-                "Service:%s deploy-with Service:%s, but no with unit found",
-                svc.name, placement)
+                "Service:%s, Deploy-with-service:%s, Requested-unit-index=%s, "
+                "Cannot solve, falling back to default placement",
+                svc.name, placement, u_idx)
             return None
         unit_names = svc_units.keys()
         unit_names.sort()
 
=== modified file 'deployer/tests/test_data/stack-placement.yaml'
--- deployer/tests/test_data/stack-placement.yaml	2013-10-31 19:19:36 +0000
+++ deployer/tests/test_data/stack-placement.yaml	2014-08-15 21:05:19 +0000
@@ -16,3 +16,7 @@
       to: lxc:nova-compute=2
     semper:
       to: nova-compute=2
+    lxc-service:
+      # Ensure more than nova-compute, catches lp:1357196
+      num_units: 5
+      to: [ "lxc:nova-compute=1", "lxc:nova-compute=2", "lxc:nova-compute=0", "lxc:nova-compute=0", "lxc:nova-compute=2" ]
 
=== modified file 'deployer/tests/test_deployment.py'
--- deployer/tests/test_deployment.py	2014-02-18 14:33:13 +0000
+++ deployer/tests/test_deployment.py	2014-08-15 21:05:19 +0000
@@ -119,6 +119,13 @@
         placement = d.get_unit_placement('semper', status)
         self.assertEqual(placement.get(0), '3')
 
+        placement = d.get_unit_placement('lxc-service', status)
+        self.assertEqual(placement.get(0), 'lxc:2')
+        self.assertEqual(placement.get(1), 'lxc:3')
+        self.assertEqual(placement.get(2), 'lxc:1')
+        self.assertEqual(placement.get(3), 'lxc:1')
+        self.assertEqual(placement.get(4), 'lxc:3')
+
     def test_multiple_relations_no_weight(self):
         data = {"relations": {"wordpress": {"consumes": ["mysql"]},
                               "nginx": {"consumes": ["wordpress"]}}}
@@ -154,5 +161,5 @@
         deployment = self.get_named_deployment("stack-placement.yaml", "stack")
         service_names = deployment.get_service_names()
         expected_service_names = [
-            'ceph', 'mysql', 'nova-compute', 'quantum', 'semper', 'verity']
+            'ceph', 'mysql', 'nova-compute', 'quantum', 'semper', 'verity', 'lxc-service']
         self.assertEqual(set(expected_service_names), set(service_names))