Merge lp:~free.ekanayaka/landscape-charm/new-data-provider-flow into lp:~landscape/landscape-charm/trunk

[Free Ekanayaka]

This branch syncs the charmhelper copy in the charm and takes advantage of some recent changes in the services framework, which now allow data-providers to be triggered outside relation hooks. We As a workaround we happened to have exactly the same behavior, which can now be dropped.

This branch syncs the charmhelper copy in the charm and takes advantage of some recent changes in the services framework, which now allow data-providers to be triggered outside relation hooks. We As a workaround we happened to have exactly the same behavior, which can now be dropped.

[Björn Tillenius (bjornt)]

+1

review: Approve

[Данило Шеган (danilo)]

review: Approve

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 299
Branch: lp:~free.ekanayaka/landscape-charm/new-data-provider-flow
Jenkins: https://ci.lscape.net/job/latch-test/1172/

review: Approve (test results)

=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2015-05-19 09:33:01 +0000
+++ charmhelpers/core/hookenv.py	2015-06-03 12:34:04 +0000
@@ -364,11 +364,16 @@
     relation_settings = relation_settings if relation_settings else {}
     relation_cmd_line = ['relation-set']
     accepts_file = "--file" in subprocess.check_output(
-        relation_cmd_line + ["--help"])
+        relation_cmd_line + ["--help"], universal_newlines=True)
     if relation_id is not None:
         relation_cmd_line.extend(('-r', relation_id))
     settings = relation_settings.copy()
     settings.update(kwargs)
+    for key, value in settings.items():
+        # Force value to be a string: it always should, but some call
+        # sites pass in things like dicts or numbers.
+        if value is not None:
+            settings[key] = "{}".format(value)
     if accepts_file:
         # --file was introduced in Juju 1.23.2. Use it by default if
         # available, since otherwise we'll break if the relation data is
@@ -390,6 +395,17 @@
     flush(local_unit())
 
 
+def relation_clear(r_id=None):
+    ''' Clears any relation data already set on relation r_id '''
+    settings = relation_get(rid=r_id,
+                            unit=local_unit())
+    for setting in settings:
+        if setting not in ['public-address', 'private-address']:
+            settings[setting] = None
+    relation_set(relation_id=r_id,
+                 **settings)
+
+
 @cached
 def relation_ids(reltype=None):
     """A list of relation_ids"""
@@ -681,3 +697,48 @@
             return 'unknown'
         else:
             raise
+
+
+def translate_exc(from_exc, to_exc):
+    def inner_translate_exc1(f):
+        def inner_translate_exc2(*args, **kwargs):
+            try:
+                return f(*args, **kwargs)
+            except from_exc:
+                raise to_exc
+
+        return inner_translate_exc2
+
+    return inner_translate_exc1
+
+
+@translate_exc(from_exc=OSError, to_exc=NotImplementedError)
+def is_leader():
+    """Does the current unit hold the juju leadership
+
+    Uses juju to determine whether the current unit is the leader of its peers
+    """
+    cmd = ['is-leader', '--format=json']
+    return json.loads(subprocess.check_output(cmd).decode('UTF-8'))
+
+
+@translate_exc(from_exc=OSError, to_exc=NotImplementedError)
+def leader_get(attribute=None):
+    """Juju leader get value(s)"""
+    cmd = ['leader-get', '--format=json'] + [attribute or '-']
+    return json.loads(subprocess.check_output(cmd).decode('UTF-8'))
+
+
+@translate_exc(from_exc=OSError, to_exc=NotImplementedError)
+def leader_set(settings=None, **kwargs):
+    """Juju leader set value(s)"""
+    log("Juju leader-set '%s'" % (settings), level=DEBUG)
+    cmd = ['leader-set']
+    settings = settings or {}
+    settings.update(kwargs)
+    for k, v in settings.iteritems():
+        if v is None:
+            cmd.append('{}='.format(k))
+        else:
+            cmd.append('{}={}'.format(k, v))
+    subprocess.check_call(cmd)

=== modified file 'charmhelpers/core/services/base.py'
--- charmhelpers/core/services/base.py	2015-05-12 14:31:50 +0000
+++ charmhelpers/core/services/base.py	2015-06-03 12:34:04 +0000
@@ -15,8 +15,8 @@
 # along with charm-helpers.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
-import re
 import json
+from inspect import getargspec
 from collections import Iterable, OrderedDict
 
 from charmhelpers.core import host
@@ -132,8 +132,8 @@
         if hook_name == 'stop':
             self.stop_services()
         else:
+            self.reconfigure_services()
             self.provide_data()
-            self.reconfigure_services()
         cfg = hookenv.config()
         if cfg.implicit_save:
             cfg.save()
@@ -145,15 +145,36 @@
         A provider must have a `name` attribute, which indicates which relation
         to set data on, and a `provide_data()` method, which returns a dict of
         data to set.
+
+        The `provide_data()` method can optionally accept two parameters:
+
+          * ``remote_service`` The name of the remote service that the data will
+            be provided to.  The `provide_data()` method will be called once
+            for each connected service (not unit).  This allows the method to
+            tailor its data to the given service.
+          * ``service_ready`` Whether or not the service definition had all of
+            its requirements met, and thus the ``data_ready`` callbacks run.
+
+        Note that the ``provided_data`` methods are now called **after** the
+        ``data_ready`` callbacks are run.  This gives the ``data_ready`` callbacks
+        a chance to generate any data necessary for the providing to the remote
+        services.
         """
-        hook_name = hookenv.hook_name()
-        for service in self.services.values():
+        for service_name, service in self.services.items():
+            service_ready = self.is_ready(service_name)
             for provider in service.get('provided_data', []):
-                if re.match(r'{}-relation-(joined|changed)'.format(provider.name), hook_name):
-                    data = provider.provide_data()
-                    _ready = provider._is_ready(data) if hasattr(provider, '_is_ready') else data
-                    if _ready:
-                        hookenv.relation_set(None, data)
+                for relid in hookenv.relation_ids(provider.name):
+                    units = hookenv.related_units(relid)
+                    if not units:
+                        continue
+                    remote_service = units[0].split('/')[0]
+                    argspec = getargspec(provider.provide_data)
+                    if len(argspec.args) > 1:
+                        data = provider.provide_data(remote_service, service_ready)
+                    else:
+                        data = provider.provide_data()
+                    if data:
+                        hookenv.relation_set(relid, data)
 
     def reconfigure_services(self, *service_names):
         """

=== modified file 'charmhelpers/fetch/giturl.py'
--- charmhelpers/fetch/giturl.py	2015-03-12 11:42:26 +0000
+++ charmhelpers/fetch/giturl.py	2015-06-03 12:34:04 +0000
@@ -45,14 +45,16 @@
         else:
             return True
 
-    def clone(self, source, dest, branch):
+    def clone(self, source, dest, branch, depth=None):
         if not self.can_handle(source):
             raise UnhandledSource("Cannot handle {}".format(source))
 
-        repo = Repo.clone_from(source, dest)
-        repo.git.checkout(branch)
+        if depth:
+            Repo.clone_from(source, dest, branch=branch, depth=depth)
+        else:
+            Repo.clone_from(source, dest, branch=branch)
 
-    def install(self, source, branch="master", dest=None):
+    def install(self, source, branch="master", dest=None, depth=None):
         url_parts = self.parse_url(source)
         branch_name = url_parts.path.strip("/").split("/")[-1]
         if dest:
@@ -63,7 +65,7 @@
         if not os.path.exists(dest_dir):
             mkdir(dest_dir, perms=0o755)
         try:
-            self.clone(source, dest_dir, branch)
+            self.clone(source, dest_dir, branch, depth)
         except GitCommandError as e:
             raise UnhandledSource(e.message)
         except OSError as e:

=== modified file 'lib/services.py'
--- lib/services.py	2015-06-02 09:21:44 +0000
+++ lib/services.py	2015-06-03 12:34:04 +0000
@@ -53,15 +53,13 @@
             leader_context = LandscapeLeaderContext(
                 host=self._host, hookenv=self._hookenv)
 
-        haproxy_provider = HAProxyProvider(
-            SERVICE_COUNTS, paths=self._paths, is_leader=is_leader)
-
         manager = ServiceManager(services=[{
             "service": "landscape",
             "ports": [],
             "provided_data": [
                 LandscapeProvider(leader_context),
-                haproxy_provider,
+                HAProxyProvider(
+                    SERVICE_COUNTS, paths=self._paths, is_leader=is_leader),
                 RabbitMQProvider(),
             ],
             # Required data is available to the render_template calls below.
@@ -94,16 +92,4 @@
             "start": LSCtl(subprocess=self._subprocess, hookenv=self._hookenv),
         }])
 
-        # XXX The services framework only triggers data providers within the
-        #     context of relation joined/changed hooks, however we also
-        #     want to trigger the haproxy provider if the SSL certificate
-        #     has changed.
-        if self._hookenv.hook_name() == "config-changed":
-            config = self._hookenv.config()
-            if config.changed("ssl-cert") or config.changed("ssl-key"):
-                relation_ids = self._hookenv.relation_ids(HAProxyProvider.name)
-                data = haproxy_provider.provide_data()
-                for relation_id in relation_ids:
-                    self._hookenv.relation_set(relation_id, data)
-
         manager.manage()

=== modified file 'lib/tests/test_services.py'
--- lib/tests/test_services.py	2015-05-26 12:05:02 +0000
+++ lib/tests/test_services.py	2015-06-03 12:34:04 +0000
@@ -77,12 +77,8 @@
         self.hook()
         # Assert that the HAProxyProvider has run by checking that it set the
         # relation with the dict returned by HAProxyProvider.provide_data (the
-        # only key of that dict is 'services'). The ID of relation being set
-        # is None because we're running in the website-relation-joined hook
-        # and are using the default relation ID (which in a real-world
-        # relation-set run will resolve to the relation for the http
-        # interface).
-        self.assertIn("services", self.hookenv.relations[None])
+        # only key of that dict is 'services').
+        self.assertIn("services", self.hookenv.relations["website:1"])
 
     def test_amqp_relation_not_ready(self):
         """