Merge lp:~jimbaker/pyjuju/rel-mgmt-endpoint-factory into lp:pyjuju

[Jim Baker]

Adds ensemble.state.service.ServiceStateManager.get_relation_endpoints. Originally this was to be in ensemble.state.relation, however that would require a circular import dependency (and even doing the standard workaround by putting the import at the bottom breaks for some reason in testing ensemble.control).

This branch is stacked on metadata-relations, which it relies on for testing with respect to formulas.

[Gustavo Niemeyer (niemeyer)]

Looks good overall, thanks Jim. +1, considering the following comments:

[1]

383 + try:
384 + query_service_name, query_relation_name = descriptor.split(":")
385 + except:
386 + query_service_name, query_relation_name = descriptor, None

It's generally a bad idea to use bare excepts unless the intention is really to
catch *all* exceptions, which isn't the case in most situations (e.g. this would
get even NameErrors, for instance).

This alternative approach will avoid any of those issues:

tokens = descriptor.split(":", 1)
if len(tokens) == 2: / else:

[2]

There are two terminology conflicts within Ensemble which would be good to get solved:

a) provides/requires vs. server/client as a role
b) interface vs. relation type

Can you please raise that conversation and fix the conflict using the consensus in
that conversation (or at least agree on how and when it should be fixed as part of
the discussion).

review: Approve

[Gustavo Niemeyer (niemeyer)]

Oh, please note that the other branch this is based on has a minor comment as well.

=== modified file 'ensemble/formula/metadata.py'
--- ensemble/formula/metadata.py	2010-10-29 19:55:50 +0000
+++ ensemble/formula/metadata.py	2010-11-25 05:51:59 +0000
@@ -5,20 +5,111 @@
 from ensemble.errors import (
     EnsembleError, InvalidEnsembleHeaderValue, FileNotFound)
 from ensemble.lib.schema import (
-    SchemaError, KeyDict, Constant, Int, UnicodeOrString)
+    SchemaError, Bool, Constant, Dict, Int,
+    KeyDict, List, OneOf, UnicodeOrString)
 
 
 class MetaDataError(EnsembleError):
     """Raised when an error in the info file of a formula is found."""
 
 
+INTERFACE_SCHEMA = KeyDict({
+        "interface": UnicodeOrString("utf-8"),
+        "limit": OneOf(Constant(None), Int()),
+        "optional": Bool()})
+
+
+class ReshapeInterfaceSpec(object):
+    """Support reshaping of a YAML interface spec to list of interfaces.
+
+    We need this class because our formula shorthand is difficult to
+    work with (unfortunately). So we coerce shorthand and then store
+    the desired format in ZK.
+
+    Supports the following variants::
+
+      provides:
+        server: riak
+        admin: http
+        foobar:
+          interface: blah
+
+      provides:
+        server:
+          interface: mysql
+          limit:
+          optional: false
+
+    In all cases, this is converted to a fully specified
+    representation, as seen in the mysql interface description above
+    (in YAML format)
+    """
+
+    value_converter = UnicodeOrString("utf-8")
+
+    def __init__(self, limit):
+        """Create relation interface reshaper.
+
+        @limit: the limit for this relation. Used to provide defaults
+            for a given kind of relation role (peer, provider, consumer)
+        """
+        self.limit = limit
+
+    def coerce_interface(self, interface_spec, path):
+        """Coerce `interface_spec` from shorthand form.
+
+        Helper method to support each of the variants, either the
+        formula does not specify limit and optional, such as foobar in
+        the above example; or the interface spec is just a string,
+        such as the ``server: riak`` example.
+        """
+        if not isinstance(interface_spec, dict):
+            return {
+                "interface":
+                    self.value_converter.coerce(interface_spec, path),
+                "limit": self.limit,
+                "optional": False}
+        else:
+            # Optional values are context-sensitive and/or have
+            # defaults, which is different than what KeyDict can
+            # readily support. So just do it here first, then
+            # coerce.
+            if "limit" not in interface_spec:
+                interface_spec["limit"] = self.limit
+            if "optional" not in interface_spec:
+                interface_spec["optional"] = False
+            return INTERFACE_SCHEMA.coerce(interface_spec, path)
+
+    def coerce(self, value, path):
+        """Converts an interface specification in shorthand form.
+
+        @value: the value to be coerced
+        @path: used in exception reporting"""
+        if not isinstance(value, dict):
+            raise SchemaError(path, "Invalid relation specification")
+
+        return dict(
+            (self.value_converter.coerce(K, path),
+             self.coerce_interface(V, path))
+            for K, V in value.iteritems())
+
+
+RELATION_SCHEMA = Dict(
+    UnicodeOrString("utf-8"), INTERFACE_SCHEMA)
+
 SCHEMA = KeyDict({
     "ensemble": Constant("formula"),
     "name": UnicodeOrString("utf-8"),
     "revision": Int(),
     "summary": UnicodeOrString("utf-8"),
     "description": UnicodeOrString("utf-8"),
-    })
+    "peers": OneOf(RELATION_SCHEMA,
+                   ReshapeInterfaceSpec(limit=1)),
+    "provides": OneOf(RELATION_SCHEMA,
+                      ReshapeInterfaceSpec(limit=None)),
+    "requires": OneOf(RELATION_SCHEMA,
+                      ReshapeInterfaceSpec(limit=1)),
+    }, optional=set(["provides", "requires", "peers"]))
 
 
 class MetaData(object):
@@ -59,6 +150,21 @@
         """The formula description."""
         return self._data.get("description")
 
+    @property
+    def provides(self):
+        """The formula provides relations."""
+        return self._data.get("provides")
+
+    @property
+    def requires(self):
+        """The formula requires relations."""
+        return self._data.get("requires")
+
+    @property
+    def peers(self):
+        """The formula peers relations."""
+        return self._data.get("peers")
+
     def get_serialization_data(self):
         """Get internal dictionary representing the state of this instance.
 

=== added directory 'ensemble/formula/tests/repository/mysql'
=== added file 'ensemble/formula/tests/repository/mysql/metadata.yaml'
--- ensemble/formula/tests/repository/mysql/metadata.yaml	1970-01-01 00:00:00 +0000
+++ ensemble/formula/tests/repository/mysql/metadata.yaml	2010-11-25 05:51:59 +0000
@@ -0,0 +1,7 @@
+ensemble: formula
+name: mysql
+revision: 1
+summary: "Database engine"
+description: "A pretty popular database"
+provides:
+  server: mysql

=== added directory 'ensemble/formula/tests/repository/riak'
=== added file 'ensemble/formula/tests/repository/riak/metadata.yaml'
--- ensemble/formula/tests/repository/riak/metadata.yaml	1970-01-01 00:00:00 +0000
+++ ensemble/formula/tests/repository/riak/metadata.yaml	2010-11-25 05:51:59 +0000
@@ -0,0 +1,13 @@
+ensemble: formula
+name: riak
+revision: 7
+summary: "K/V storage engine"
+description: "Scalable K/V Store in Erlang with Clocks :-)"
+provides:
+  endpoint:
+    interface: http
+  admin:
+    interface: http
+peers:
+  ring:
+    interface: riak

=== added directory 'ensemble/formula/tests/repository/wordpress'
=== added file 'ensemble/formula/tests/repository/wordpress/metadata.yaml'
--- ensemble/formula/tests/repository/wordpress/metadata.yaml	1970-01-01 00:00:00 +0000
+++ ensemble/formula/tests/repository/wordpress/metadata.yaml	2010-11-25 05:51:59 +0000
@@ -0,0 +1,21 @@
+ensemble: formula
+name: wordpress
+revision: 3
+summary: "Blog engine"
+description: "A pretty popular blog engine"
+provides:
+  url:
+    interface: http
+    limit:
+    optional: false
+requires:
+  db:
+    interface: mysql
+    limit: 1
+    optional: false
+  cache:
+    interface: somecache
+    limit: 2
+    optional: true
+
+

=== modified file 'ensemble/formula/tests/test_metadata.py'
--- ensemble/formula/tests/test_metadata.py	2010-10-29 19:55:50 +0000
+++ ensemble/formula/tests/test_metadata.py	2010-11-25 05:51:59 +0000
@@ -5,14 +5,15 @@
 import inspect
 
 from ensemble.lib.testing import TestCase
-from ensemble.formula.metadata import MetaData, MetaDataError
+from ensemble.formula.metadata import (
+    MetaData, MetaDataError, ReshapeInterfaceSpec, SchemaError)
 from ensemble.errors import InvalidEnsembleHeaderValue, FileNotFound
 from ensemble.formula import tests
 
-sample_path = os.path.join(
+test_repository_path = os.path.join(
     os.path.dirname(inspect.getabsfile(tests)),
-    "repository", "dummy", "metadata.yaml")
-
+    "repository")
+sample_path = os.path.join(test_repository_path, "dummy", "metadata.yaml")
 sample_configuration = open(sample_path).read()
 
 
@@ -195,3 +196,84 @@
         self.assertRaises(AssertionError, self.metadata.get_formula_id, None)
         self.assertRaises(AssertionError, self.metadata.get_formula_id, 1)
         self.assertRaises(AssertionError, self.metadata.get_formula_id, "z:x")
+
+
+class ParseTest(TestCase):
+    """Test the parsing of some well-known sample files"""
+
+    def get_metadata(self, formula_name):
+        metadata = MetaData(os.path.join(
+            test_repository_path, formula_name, "metadata.yaml"))
+        self.assertEqual(metadata.name, formula_name)
+        return metadata
+
+    def test_mysql_sample(self):
+        metadata = self.get_metadata("mysql")
+        self.assertEqual(metadata.peers, None)
+        self.assertEqual(
+            metadata.provides["server"],
+            {"interface": "mysql", "limit": None, "optional": False})
+        self.assertEqual(metadata.requires, None)
+
+    def test_riak_sample(self):
+        metadata = self.get_metadata("riak")
+        self.assertEqual(
+            metadata.peers["ring"],
+            {"interface": "riak", "limit": 1, "optional": False})
+        self.assertEqual(
+            metadata.provides["endpoint"],
+            {"interface": "http", "limit": None, "optional": False})
+        self.assertEqual(
+            metadata.provides["admin"],
+            {"interface": "http", "limit": None, "optional": False})
+        self.assertEqual(metadata.requires, None)
+
+    def test_wordpress_sample(self):
+        metadata = self.get_metadata("wordpress")
+        self.assertEqual(metadata.peers, None)
+        self.assertEqual(
+            metadata.provides["url"],
+            {"interface": "http", "limit": None, "optional": False})
+        self.assertEqual(
+            metadata.requires["db"],
+            {"interface": "mysql", "limit": 1, "optional": False})
+        self.assertEqual(
+            metadata.requires["cache"],
+            {"interface": "somecache", "limit": 2, "optional": True})
+
+    def test_reshape_interface_spec(self):
+        provides_reshaper = ReshapeInterfaceSpec(limit=None)
+        self.assertEqual(
+            provides_reshaper.coerce({"admin": "http"}, ["provides"]),
+            {"admin": {"interface": "http", "limit": None, "optional": False}})
+        self.assertEqual(
+            provides_reshaper.coerce(
+                {"admin": {"interface": "http"}}, ["provides"]),
+            {"admin": {"interface": "http", "limit": None, "optional": False}})
+        self.assertEqual(
+            provides_reshaper.coerce(
+                {"foobar": {"interface": "http", "limit": 2}}, ["provides"]),
+            {"foobar": {"interface": "http", "limit": 2, "optional": False}})
+        self.assertEqual(
+            provides_reshaper.coerce(
+                {"foobar": {"interface": "http", "optional": True}},
+                ["provides"]),
+            {"foobar": {"interface": "http", "limit": None, "optional": True}})
+        self.assertRaises(
+            SchemaError,
+            provides_reshaper.coerce, {"admin": 42}, ["provides"])
+        self.assertRaises(
+            SchemaError,
+            provides_reshaper.coerce,
+            {"admin": {"interface": "http", "optional": None}},
+            ["provides"])
+        self.assertRaises(
+            SchemaError,
+            provides_reshaper.coerce,
+            {"admin": {"interface": "http", "limit": "none, really"}},
+            ["provides"])
+
+        consumer_reshaper = ReshapeInterfaceSpec(limit=1) # just test defaults
+        self.assertEqual(
+            consumer_reshaper.coerce({"admin": "http"}, ["consumes"]),
+            {"admin": {"interface": "http", "limit": 1, "optional": False}})

=== modified file 'ensemble/state/endpoint.py'
--- ensemble/state/endpoint.py	2010-11-24 21:16:25 +0000
+++ ensemble/state/endpoint.py	2010-11-25 05:51:59 +0000
@@ -23,20 +23,19 @@
         """Test whether the `other` endpoint may be used in a common relation.
 
         RelationEndpoints may be related if they share the same relation_type
-        (which is called an "interface" in formulas) and one is a ``provider``
-        and the other is a ``consumer``; or if both endpoints have a
-        relation_role of ``peer``.
-        
+        (which is called an "interface" in formulas) and one is a ``provides``
+        and the other is a ``requires``; or if both endpoints have a
+        relation_role of ``peers``.
+
         Raises a `TypeError` is `other` is not a `RelationEndpoint`.
         """
 
         if not isinstance(other, RelationEndpoint):
             raise TypeError("Not a RelationEndpoint", other)
         return (self.relation_type == other.relation_type and
-                ((self.relation_role == "provider" and
-                  other.relation_role == "consumer") or
-                 (self.relation_role == "consumer" and
-                  other.relation_role == "provider") or
-                 (self.relation_role == "peer" and
-                  other.relation_role == "peer")))
-
+                ((self.relation_role == "provides" and
+                  other.relation_role == "requires") or
+                 (self.relation_role == "requires" and
+                  other.relation_role == "provides") or
+                 (self.relation_role == "peers" and
+                  other.relation_role == "peers")))

=== modified file 'ensemble/state/relation.py'
--- ensemble/state/relation.py	2010-11-15 21:08:50 +0000
+++ ensemble/state/relation.py	2010-11-25 05:51:59 +0000
@@ -667,3 +667,4 @@
         """
         return [unit_id for unit_id in units \
                 if unit_id != self._watcher_unit.internal_unit_id]
+

=== modified file 'ensemble/state/service.py'
--- ensemble/state/service.py	2010-11-18 23:54:49 +0000
+++ ensemble/state/service.py	2010-11-25 05:51:59 +0000
@@ -5,11 +5,13 @@
 from twisted.internet.defer import inlineCallbacks, returnValue
 
 from ensemble.state.agent import AgentStateMixin
+from ensemble.state.base import StateBase
+from ensemble.state.endpoint import RelationEndpoint
 from ensemble.state.errors import (
     StateChanged, ServiceStateNotFound, ServiceUnitStateNotFound,
     ServiceUnitStateMachineAlreadyAssigned, ServiceStateNameInUse,
     BadServiceStateName)
-from ensemble.state.base import StateBase
+from ensemble.state.formula import FormulaStateManager
 from ensemble.state.relation import ServiceRelationState
 
 
@@ -56,6 +58,40 @@
             raise ServiceStateNotFound(service_name)
         returnValue(ServiceState(self._client, internal_id, service_name))
 
+    @inlineCallbacks
+    def get_relation_endpoints(self, descriptor):
+        """Get all relation endpoints for `descriptor`.
+
+        A `descriptor` is of the form <service name>[:<relation name>].
+        Returns an empty set if there are no endpoints matching
+        the `descriptor`.
+        """
+        try:
+            query_service_name, query_relation_name = descriptor.split(":")
+        except:
+            query_service_name, query_relation_name = descriptor, None
+        formula_state_manager = FormulaStateManager(self._client)
+        service_state = yield self.get_service_state(
+            query_service_name)
+        formula_id = yield service_state.get_formula_id()
+        formula_state = yield formula_state_manager.get_formula_state(
+            formula_id)
+        formula_metadata = yield formula_state.get_metadata()
+        endpoints = set()
+        for relation_role in ("peers", "provides", "requires"):
+            relations = getattr(formula_metadata, relation_role)
+            if relations:
+                for relation_name, spec in relations.iteritems():
+                    if (query_relation_name is None or
+                        query_relation_name == relation_name):
+                        endpoints.add(RelationEndpoint(
+                            service_name=query_service_name,
+                            relation_type=spec["interface"],
+                            relation_name=relation_name,
+                            relation_role=relation_role))
+        returnValue(endpoints)
+
+
 
 class ServiceState(StateBase):
     """State of a service registered in an environment.

=== modified file 'ensemble/state/tests/test_endpoint.py'
--- ensemble/state/tests/test_endpoint.py	2010-11-24 21:25:58 +0000
+++ ensemble/state/tests/test_endpoint.py	2010-11-25 05:51:59 +0000
@@ -1,11 +1,12 @@
 from ensemble.lib.testing import TestCase
 from ensemble.state.endpoint import RelationEndpoint
 
+
 class RelationEndpointTest(TestCase):
     def test_may_relate_to(self):
-        mysql_ep = RelationEndpoint("mysqldb", "mysql", "db", "provider")
-        blog_ep = RelationEndpoint("blog", "mysql", "mysql", "consumer")
-        pg_ep = RelationEndpoint("postgres", "postgres", "db", "provider")
+        mysql_ep = RelationEndpoint("mysqldb", "mysql", "db", "provides")
+        blog_ep = RelationEndpoint("blog", "mysql", "mysql", "requires")
+        pg_ep = RelationEndpoint("postgres", "postgres", "db", "provides")
 
         self.assertRaises(TypeError, mysql_ep.may_relate_to, 42)
 
@@ -20,15 +21,15 @@
         # antireflexive om relation_role -
         # must be consumer AND provider or vice versa
         self.assertFalse(blog_ep.may_relate_to(
-            RelationEndpoint("foo", "mysql", "db", "consumer")))
+            RelationEndpoint("foo", "mysql", "db", "requires")))
         self.assertFalse(mysql_ep.may_relate_to(
-            RelationEndpoint("foo", "mysql", "db", "provider")))
+            RelationEndpoint("foo", "mysql", "db", "provides")))
 
-        # irreflexive for provider/consumer
+        # irreflexive for provides/requires
         self.assertFalse(mysql_ep.may_relate_to(mysql_ep))
         self.assertFalse(blog_ep.may_relate_to(blog_ep))
         self.assertFalse(pg_ep.may_relate_to(pg_ep))
 
         # but reflexive for peers
-        riak_ep = RelationEndpoint("riak", "riak", "riak", "peer")
+        riak_ep = RelationEndpoint("riak", "riak", "riak", "peers")
         self.assert_(riak_ep.may_relate_to(riak_ep))

=== modified file 'ensemble/state/tests/test_relation.py'
--- ensemble/state/tests/test_relation.py	2010-11-16 22:37:44 +0000
+++ ensemble/state/tests/test_relation.py	2010-11-25 05:51:59 +0000
@@ -1,10 +1,10 @@
 import functools
+
 import yaml
 import zookeeper
 
 from twisted.internet.defer import (
     inlineCallbacks, returnValue, Deferred, fail, succeed)
-
 from txzookeeper import ZookeeperClient
 
 from ensemble.state.formula import FormulaStateManager

=== modified file 'ensemble/state/tests/test_service.py'
--- ensemble/state/tests/test_service.py	2010-11-18 23:54:49 +0000
+++ ensemble/state/tests/test_service.py	2010-11-25 05:51:59 +0000
@@ -1,7 +1,12 @@
+import os.path
+
 import yaml
 
 from twisted.internet.defer import inlineCallbacks, Deferred, returnValue
 
+from ensemble.formula.directory import FormulaDirectory
+from ensemble.formula.tests.test_metadata import test_repository_path
+from ensemble.state.endpoint import RelationEndpoint
 from ensemble.state.formula import FormulaStateManager
 from ensemble.state.service import ServiceStateManager
 from ensemble.state.machine import MachineStateManager
@@ -624,3 +629,38 @@
 
         self.assertEqual([r.internal_relation_id for r in new_relations],
                          [relation_state2.internal_id])
+
+    @inlineCallbacks
+    def add_service_from_formula(self, service_name):
+        formula_dir = FormulaDirectory(os.path.join(
+            test_repository_path, service_name))
+        formula_state = yield self.formula_state_manager.add_formula_state(
+            "namespace", formula_dir)
+        service_state = yield self.service_state_manager.add_service_state(
+            service_name, formula_state)
+        returnValue(service_state)
+
+    @inlineCallbacks
+    def test_get_relation_endpoints(self):
+        yield self.add_service_from_formula("wordpress")
+        wp_endpoints = yield self.service_state_manager.get_relation_endpoints(
+            "wordpress")
+        self.assertEqual(
+            wp_endpoints,
+            set([RelationEndpoint("wordpress", "http", "url", "provides"),
+                 RelationEndpoint("wordpress", "mysql", "db", "requires"),
+                 RelationEndpoint("wordpress", "somecache", "cache",
+                                  "requires")]))
+        yield self.add_service_from_formula("riak")
+        riak_endpoints = yield self.service_state_manager.get_relation_endpoints(
+            "riak:ring")
+        self.assertEqual(
+            riak_endpoints,
+            set([RelationEndpoint("riak", "riak", "ring", "peers")]))
+        yield self.add_service("nosuch") # blank formula!
+        nosuch_endpoints = yield self.service_state_manager.get_relation_endpoints(
+            "nosuch")
+        self.assertEqual(nosuch_endpoints, set())
+        nosuch_endpoints2 = yield self.service_state_manager.get_relation_endpoints(
+            "nosuch:nonsense")
+        self.assertEqual(nosuch_endpoints2, set())