Merge lp:~hazmat/pyjuju/status-w-peers into lp:pyjuju

[Kapil Thangavelu]

Fixes ensemble status to work with peer relations.

[Benjamin Saller (bcsaller)]

[1]

        for relation in relations:
            rel_services = yield relation.get_service_states()
            # filter out 'self' if its nots a peer relation
            if len(rel_services) > 1:
                rel_services = [
                    rsn for rsn in rel_services if rsn.service_name !=
                    service.service_name]
            if len(rel_services) > 1:
                raise ValueError("Unexpected relationship with more "
                                 "than 2 endpoints")

This does fix it, I'd like to expand the comment such that its a bit more obvious whats happening there. To be clear this is my fault not yours, but a series of len(x) > 1 checks don't really related to how that handles peer relations or not. Something like:

# Singular rel_services imply a peer relation. Two rel_services imply a provides/requires relationship. In the later case we omit the local size of the relation in reporting.

[Gustavo Niemeyer (niemeyer)]

Looks good, thanks!

[1]

22 + rel_services = [
23 + rsn for rsn in rel_services if rsn.service_name !=
24 + service.service_name]

The way this is formatted feels more unreadable than it ought to be.

Here is a suggested style:

                rel_services = [rsn for rsn in rel_services
                                if rsn.service_name != service.service_name]

review: Approve

=== modified file 'ensemble/control/status.py'
--- ensemble/control/status.py	2011-06-28 09:40:28 +0000
+++ ensemble/control/status.py	2011-07-01 14:41:25 +0000
@@ -192,7 +192,7 @@
                     # but can happen in test circumstances. In
                     # particular, it will happen with a misconfigured
                     # provider, which exercises this codepath.
-                    continue # should not occur, but status should not fail
+                    continue  # should not occur, but status should not fail
                 relation_workflow_client = WorkflowStateClient(
                     client, relation_unit)
                 relation_workflow_state = \
@@ -209,9 +209,11 @@
 
         for relation in relations:
             rel_services = yield relation.get_service_states()
-            # filter out 'self'
-            rel_services = [rsn for rsn in rel_services if rsn.service_name !=
-                            service.service_name]
+            # filter out 'self' if its nots a peer relation
+            if len(rel_services) > 1:
+                rel_services = [
+                    rsn for rsn in rel_services if rsn.service_name !=
+                    service.service_name]
             if len(rel_services) > 1:
                 raise ValueError("Unexpected relationship with more "
                                  "than 2 endpoints")
@@ -295,7 +297,10 @@
     """
     return name.replace("-", "_")
 
-def render_dot(data, filelike, environment, format="dot", style=DEFAULT_STYLE):
+def render_dot(
+    data, filelike, environment, format="dot", style=DEFAULT_STYLE):
+    """Render a graphiz output of the status information.
+    """
     try:
         import pydot
     except ImportError:

=== modified file 'ensemble/control/tests/test_status.py'
--- ensemble/control/tests/test_status.py	2011-06-28 09:40:28 +0000
+++ ensemble/control/tests/test_status.py	2011-07-01 14:41:25 +0000
@@ -19,6 +19,7 @@
 sample_path = os.path.join(tests_path, "sample_cluster.yaml")
 sample_cluster = yaml.load(open(sample_path, "r"))
 
+
 def dump_stringio(stringio, filename):
     """Debug utility to dump a StringIO to a filename."""
     fp = open(filename, "w")
@@ -61,7 +62,8 @@
         for unit_state, state in zip(unit_states, states):
             relation_unit_state = yield relation_state.add_unit_state(
                 unit_state)
-            workflow_client = ZookeeperWorkflowState(self.client, relation_unit_state)
+            workflow_client = ZookeeperWorkflowState(
+                self.client, relation_unit_state)
             yield workflow_client.set_state(state)
 
     @inlineCallbacks
@@ -97,7 +99,6 @@
         m7 = yield self.machine_state_manager.add_machine_state()
 
         # inform the provider about the machine
-
         yield self.provider.start_machine({"machine-id": 0,
                                            "dns-name": "steamcloud-1.com"})
         yield self.provider.start_machine({"machine-id": 1,
@@ -135,7 +136,7 @@
         state["services"] = dict(wordpress=wordpress, mysql=mysql,
                                  varnish=varnish, memcache=memcache)
 
-        # add unit states to services and assign to machines
+         # add unit states to services and assign to machines
         wpu = yield wordpress.add_unit_state()
         yield wpu.assign_to_machine(m1)
 
@@ -207,6 +208,49 @@
 
 
 class StatusTest(StatusTestBase):
+
+    @inlineCallbacks
+    def test_peer_relation(self):
+        """Verify status works with peer relations.
+        """
+        m1 = yield self.machine_state_manager.add_machine_state()
+        m2 = yield self.machine_state_manager.add_machine_state()
+        yield self.provider.start_machine({"machine-id": 0,
+                                           "dns-name": "steamcloud-1.com"})
+        yield self.provider.start_machine({"machine-id": 1,
+                                           "dns-name": "steamcloud-2.com"})
+        yield m1.set_instance_id(0)
+        yield m2.set_instance_id(1)
+
+        riak = yield self.add_service_from_formula("riak")
+        riak_u1 = yield riak.add_unit_state()
+        riak_u2 = yield riak.add_unit_state()
+
+        yield riak_u1.assign_to_machine(m1)
+        yield riak_u2.assign_to_machine(m2)
+        yield self.set_unit_state(riak_u1, "started")
+        yield self.set_unit_state(riak_u2, "started")
+
+        _, (peer_rel,) = yield self.relation_state_manager.add_relation_state(
+            RelationEndpoint("riak", "peer", "ring", "peer"))
+
+        yield ZookeeperWorkflowState(
+            self.client,
+            (yield peer_rel.add_unit_state(riak_u1))).set_state("up")
+        yield peer_rel.add_unit_state(riak_u2)
+
+        state = yield status.collect("riak", self.provider, self.client, None)
+        self.assertEqual(
+            state["services"]["riak"],
+            {'formula': 'namespace:riak-7',
+             'relations': {'ring': 'riak'},
+             'units': {'riak/0': {'machine': 0,
+                                  'relations': {'ring': {'state': 'up'}},
+                                  'state': 'started'},
+                       'riak/1': {'machine': 1,
+                                  'relations': {'ring': {'state': None}},
+                                  'state': 'started'}}})
+
     @inlineCallbacks
     def test_collect(self):
         yield self.build_topology()
@@ -254,7 +298,8 @@
         yield self.build_topology()
 
         # collect by service name
-        state = yield status.collect("wordpress", self.provider, self.client, None)
+        state = yield status.collect(
+            "wordpress", self.provider, self.client, None)
         # Validate that only the expected service is present
         # in the state
         self.assertEqual(state["machines"].keys(), [0])
@@ -299,7 +344,8 @@
                             "relations": {"proxy": {"state": "up"}}}}}})
 
         # filter a missing service
-        state = yield status.collect("cluehammer", self.provider, self.client, None)
+        state = yield status.collect(
+            "cluehammer", self.provider, self.client, None)
         self.assertEqual(set(state["machines"].keys()), set([]))
         self.assertEqual(set(state["services"].keys()), set([]))