Merge ~tilmanbaumann/interface-prometheus:reactive_endpoints into interface-prometheus:master

[Tilman Baumann]

Switch from RelationBase class to Endpoint

I converted the interface to the new Endpoint class because of unfixable bugs in the old method.

The code stays compatible with old charms expecting RelationBase. But I will propose patches to all charms using this interface to make use of the new features.

[Stuart Bishop (stub)]

Yes please. This is going to be a teaching example, as it is simple enough to comprehend and not need any weird stuff, but complex enough to demonstrate most of the Endpoint stuff (including implementing both provider and consumer; this is the first non-trivial Endpoint I've seen that has done that). The most magic thing is mixing to_publish for implicit JSON encoding with to_publish_raw for the legacy fields that need to be unencoded.

So I'd like to see two minor changes needed to support cross model relations fixed in this branch, so that when people inevitably cut&paste the code they get the good stuff. Some other comments inline too, primarily with setting the old flags for backwards compatibility.

review: Needs Fixing

[Tilman Baumann (tilmanbaumann)]

Thanks, Stuart for the great review. I will be more than happy to implement those changes.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Stuart Bishop (stub)]

The updates all look great :)

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision defa8b8ead4b33f58826be29b254fff9c024ac59

diff --git a/provides.py b/provides.py
index b28d2f2..d1f1683 100644
--- a/provides.py
+++ b/provides.py
@@ -1,31 +1,59 @@
-from charms.reactive import RelationBase, scopes, hook
+from charms.reactive import Endpoint
+from charms.reactive import hook, when, when_not
 from charmhelpers.core import hookenv
+from charms.reactive.flags import clear_flag, set_flag
 
 
-class PrometheusProvides(RelationBase):
-    scope = scopes.UNIT
+'''
+This interface is desiged to be compatible with a previous
+implementation based on RelationBase.
 
-    @hook('{provides:prometheus}-relation-{joined,changed}')
-    def changed(self):
-        self.set_state('{relation_name}.available')
+Specifically
+- the `{endpoint_name}.available` flags 
+- the use of `to_publish_raw` to avoid double quoting of the values 
+  as the old interface used plain values here
+'''
 
-    @hook('{provides:prometheus}-relation-{broken,departed}')
-    def broken(self):
-        self.remove_state('{relation_name}.available')
+
+class PrometheusProvides(Endpoint):
 
     def configure(self, port, path='/metrics',
                   scrape_interval=None, scrape_timeout=None, labels={}):
-        unit_name = hookenv.local_unit()
+        """
+        Interface method to set information provided to remote units
+        """
+
+        # Use our unit name if the label isn't provided
         if labels.get('host') is None:
+            unit_name = hookenv.local_unit()
             labels['host'] = unit_name.replace("/", "-")
-        relation_info = {
-            'hostname': hookenv.unit_private_ip(),
-            'port': port,
-            'metrics_path': path,
-            'labels': labels,
-        }
-        if scrape_interval is not None:
-            relation_info['scrape_interval'] = scrape_interval
-        if scrape_timeout is not None:
-            relation_info['scrape_timeout'] = scrape_timeout
-        self.set_remote(**relation_info)
+        
+        for relation in self.relations:
+            relation.to_publish_raw['hostname'] = hookenv.ingress_address(
+                relation.relation_id, hookenv.local_unit()
+            )
+            relation.to_publish_raw['port'] = port
+            relation.to_publish_raw['metrics_path'] = path
+            relation.to_publish['labels'] = labels
+            if scrape_interval is not None:
+                relation.to_publish_raw['scrape_interval'] = scrape_interval
+            if scrape_timeout is not None:
+                relation.to_publish_raw['scrape_timeout'] = scrape_timeout
+
+    @when('endpoint.{endpoint_name}.joined')
+    def available(self):
+        """
+        Raising the available flag when a unit joined
+        """
+        set_flag(self.expand_name('endpoint.{endpoint_name}.available'))
+        set_flag(self.expand_name('{endpoint_name}.available')) #compatibility
+
+
+    @when('endpoint.{endpoint_name}.departed')
+    def broken(self):
+        """
+        Remove the available flag when the last unit has departed
+        """
+        if not self.is_joined:
+            clear_flag(self.expand_name('endpoint.{endpoint_name}.available'))
+            clear_flag(self.expand_name('{endpoint_name}.available')) #compatibility
 \ No newline at end of file
diff --git a/requires.py b/requires.py
index 83cc4f1..a2c10a2 100644
--- a/requires.py
+++ b/requires.py
@@ -1,27 +1,42 @@
-from charms.reactive import hook
-from charms.reactive import RelationBase
-from charms.reactive import scopes
+from charms.reactive import Endpoint
+from charms.reactive import hook, when, when_not
+from charms.reactive.flags import clear_flag, set_flag
+from charmhelpers.core.hookenv import service_name, ingress_address
 
 
-class PrometheusRequires(RelationBase):
-    scope = scopes.UNIT
+'''
+This interface is desiged to be compatible with a previous
+implementation based on RelationBase.
 
-    @hook('{requires:prometheus}-relation-{joined,changed}')
+The old `{endpoint_name}.available` flags are maintained
+'''
+
+class PrometheusRequires(Endpoint):
+
+    @when('endpoint.{endpoint_name}.changed')
     def changed(self):
-        conv = self.conversation()
-        if conv.get_remote('port'):
-            # this unit's conversation has a port, so
-            # it is part of the set of available units
-            conv.set_state('{relation_name}.available')
+        """
+        Raising the availability flag once we have received the port field from a connected unit.
+        It is convention that remote units signal availability this way.
+        """
+        if self.all_joined_units.received['port']:
+            set_flag(self.expand_name('endpoint.{endpoint_name}.available'))
+            set_flag(self.expand_name('{endpoint_name}.available')) #compatibility
+    
 
-    @hook('{requires:prometheus}-relation-{departed,broken}')
+    @when('endpoint.{endpoint_name}.departed')
     def broken(self):
-        conv = self.conversation()
-        conv.remove_state('{relation_name}.available')
+        """
+        Clearing the availability flag once the last unit departed.
+        """
+        if not self.is_joined:
+            clear_flag(self.expand_name('endpoint.{endpoint_name}.available'))
+            clear_flag(self.expand_name('{endpoint_name}.available')) #compatibility
+
 
     def targets(self):
         """
-        Returns a list of available prometheus targets.
+        Interface method returns a list of available prometheus targets.
             [
                 {
                     'job_name': name_of_job,
@@ -35,23 +50,34 @@ class PrometheusRequires(RelationBase):
             ]
         """
         services = {}
-        for conv in self.conversations():
-            service_name = conv.scope.split('/')[0]
-            service = services.setdefault(service_name, {
-                'job_name': service_name,
-                'targets': [],
-            })
-            host = conv.get_remote('hostname') or\
-                conv.get_remote('private-address')
-            port = conv.get_remote('port')
-            if host and port:
-                service['targets'].append('{}:{}'.format(host, port))
-            if conv.get_remote('metrics_path'):
-                service['metrics_path'] = conv.get_remote('metrics_path')
-            if conv.get_remote('scrape_interval'):
-                service['scrape_interval'] = conv.get_remote('scrape_interval')
-            if conv.get_remote('scrape_timeout'):
-                service['scrape_timeout'] = conv.get_remote('scrape_timeout')
-            if conv.get_remote('labels'):
-                service['labels'] = conv.get_remote('labels')
+        for relation in self.relations:
+            service_name = relation.application_name
+            for unit in relation.units:
+                service = services.setdefault(service_name, {
+                    'job_name': service_name,
+                    'targets': [],
+                })
+
+                # If the hostname is not provided we use the informaton from the relation
+                host = (unit.received['hostname'] or
+                    ingress_address(relation.relation_id, unit))
+                port = unit.received['port']
+
+                # Skipping this unit if it isn't ready yet
+                if host and port:
+                    service['targets'].append('{}:{}'.format(host, port))
+                else:
+                    continue
+                
+                if unit.received['metrics_path']:
+                    service['metrics_path'] = unit.received['metrics_path']
+                if unit.received['labels']:
+                    service['labels'] = unit.received['labels']
+                
+                # Optional fields
+                if unit.received['scrape_interval']:
+                    service['scrape_interval'] = unit.received['scrape_interval']
+                if unit.received['scrape_timeout']:
+                    service['scrape_timeout'] = unit.received['scrape_timeout']
         return [s for s in services.values() if s['targets']]
+