Merge lp:~johnsca/charms/trusty/cf-uaa/port-conflicts into lp:~cf-charmers/charms/trusty/cf-uaa/trunk

[Cory Johns]

Resolve port conflicts for cf-uaa

https://codereview.appspot.com/109790043/

[Cory Johns (johnsca)]

Reviewers: mp+222678_code.launchpad.net,

Message:
Please take a look.

Description:
Resolve port conflicts for cf-uaa

https://code.launchpad.net/~johnsca/charms/trusty/cf-uaa/port-conflicts/+merge/222678

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/109790043/

Affected files (+115, -67 lines):
   A [revision details]
   M config.yaml
   M hooks/charmhelpers/contrib/cloudfoundry/contexts.py
   M hooks/charmhelpers/core/services.py
   M hooks/config.py
   M templates/registrar.yml
   M templates/server.xml

[lp:~johnsca/charms/trusty/cf-uaa/port-conflicts]

38. By Cory Johns on 2014-06-11

Removed port options as they don't add much value to change

[Cory Johns (johnsca)]

Please take a look.

https://codereview.appspot.com/109790043/

[Benjamin Saller (bcsaller)]

LGTM, thanks

https://codereview.appspot.com/109790043/

[Cory Johns (johnsca)]

*** Submitted:

Resolve port conflicts for cf-uaa

R=benjamin.saller
CC=
https://codereview.appspot.com/109790043

https://codereview.appspot.com/109790043/

=== modified file 'hooks/charmhelpers/contrib/cloudfoundry/contexts.py'
--- hooks/charmhelpers/contrib/cloudfoundry/contexts.py	2014-05-27 22:40:17 +0000
+++ hooks/charmhelpers/contrib/cloudfoundry/contexts.py	2014-06-11 22:23:05 +0000
@@ -33,7 +33,7 @@
 
 class NatsRelation(RelationContext):
     interface = 'nats'
-    required_keys = ['nats_port', 'nats_address', 'nats_user', 'nats_password']
+    required_keys = ['address', 'port', 'user', 'password']
 
 
 class MysqlRelation(RelationContext):
@@ -44,9 +44,10 @@
     def get_data(self):
         RelationContext.get_data(self)
         if self.is_ready():
-            if 'port' not in self['db']:
-                self['db']['port'] = '3306'
-            self['db']['dsn'] = self.dsn_template.format(**self['db'])
+            for unit in self['db']:
+                if 'port' not in unit:
+                    unit['port'] = '3306'
+                unit['dsn'] = self.dsn_template.format(**unit)
 
 
 class RouterRelation(RelationContext):
@@ -56,9 +57,19 @@
 
 class LogRouterRelation(RelationContext):
     interface = 'logrouter'
-    required_keys = ['shared-secret', 'logrouter-address']
+    required_keys = ['shared_secret', 'address', 'incoming_port', 'outgoing_port']
 
 
 class LoggregatorRelation(RelationContext):
     interface = 'loggregator'
-    required_keys = ['shared_secret', 'loggregator_address']
+    required_keys = ['address', 'incoming_port', 'outgoing_port']
+
+
+class EtcdRelation(RelationContext):
+    interface = 'etcd'
+    required_keys = ['hostname', 'port']
+
+
+class CloudControllerRelation(RelationContext):
+    interface = 'cc'
+    required_keys = ['hostname', 'port', 'user', 'password']

=== modified file 'hooks/charmhelpers/core/services.py'
--- hooks/charmhelpers/core/services.py	2014-05-29 17:34:04 +0000
+++ hooks/charmhelpers/core/services.py	2014-06-11 22:23:05 +0000
@@ -11,6 +11,14 @@
         """
         Register a list of services, given their definitions.
 
+        Traditional charm authoring is focused on implementing hooks.  That is,
+        the charm author is thinking in terms of "What hook am I handling; what
+        does this hook need to do?"  However, in most cases, the real question
+        should be "Do I have the information I need to configure and start this
+        piece of software and, if so, what are the steps for doing so."  The
+        ServiceManager framework tries to bring the focus to the data and the
+        setup tasks, in the most declarative way possible.
+
         Service definitions are dicts in the following formats (all keys except
         'service' are optional):
 
@@ -67,28 +75,28 @@
         a mongodb relation and which runs a custom `db_migrate` function prior to
         restarting the service, and a Runit serivce called spadesd.
 
-            >>> manager = services.ServiceManager([
-            ...     {
-            ...         'service': 'bingod',
-            ...         'ports': [80, 443],
-            ...         'required_data': [MongoRelation(), config()],
-            ...         'data_ready': [
-            ...             services.template(source='bingod.conf'),
-            ...             services.template(source='bingod.ini',
-            ...                               target='/etc/bingod.ini',
-            ...                               owner='bingo', perms=0400),
-            ...         ],
-            ...     },
-            ...     {
-            ...         'service': 'spadesd',
-            ...         'data_ready': services.template(source='spadesd_run.j2',
-            ...                                         target='/etc/sv/spadesd/run',
-            ...                                         perms=0555),
-            ...         'start': runit_start,
-            ...         'stop': runit_stop,
-            ...     },
-            ... ])
-            ... manager.manage()
+            manager = services.ServiceManager([
+                {
+                    'service': 'bingod',
+                    'ports': [80, 443],
+                    'required_data': [MongoRelation(), config(), {'my': 'data'}],
+                    'data_ready': [
+                        services.template(source='bingod.conf'),
+                        services.template(source='bingod.ini',
+                                          target='/etc/bingod.ini',
+                                          owner='bingo', perms=0400),
+                    ],
+                },
+                {
+                    'service': 'spadesd',
+                    'data_ready': services.template(source='spadesd_run.j2',
+                                                    target='/etc/sv/spadesd/run',
+                                                    perms=0555),
+                    'start': runit_start,
+                    'stop': runit_stop,
+                },
+            ])
+            manager.manage()
         """
         self.services = {}
         for service in services or []:
@@ -213,55 +221,73 @@
     interface = None
     required_keys = []
 
+    def __init__(self, *args, **kwargs):
+        super(RelationContext, self).__init__(*args, **kwargs)
+        self.get_data()
+
     def __bool__(self):
         """
-        Updates the data and returns True if all of the required_keys are available.
+        Returns True if all of the required_keys are available.
         """
-        self.get_data()
         return self.is_ready()
 
     __nonzero__ = __bool__
 
+    def __repr__(self):
+        return super(RelationContext, self).__repr__()
+
     def is_ready(self):
         """
-        Returns True if all of the required_keys are available.
-        """
-        return set(self.get(self.interface, {}).keys()).issuperset(set(self.required_keys))
+        Returns True if all of the `required_keys` are available from any units.
+        """
+        ready = len(self.get(self.interface, [])) > 0
+        if not ready:
+            hookenv.log('Incomplete relation: {}'.format(self.__class__.__name__), hookenv.DEBUG)
+        return ready
+
+    def _is_ready(self, unit_data):
+        """
+        Helper method that tests a set of relation data and returns True if
+        all of the `required_keys` are present.
+        """
+        return set(unit_data.keys()).issuperset(set(self.required_keys))
 
     def get_data(self):
         """
-        Retrieve the relation data and store it under `self[self.interface]`.
-
-        If there are more than one units related on the desired interface,
-        then each unit will have its data stored under `self[self.interface][unit_id]`
-        and one of the units with complete information will chosen at random
-        to fill the values at `self[self.interface]`.
-
-
-        For example:
-
-            {
-                'foo': 'bar',
-                'unit/0': {
-                    'foo': 'bar',
-                },
-                'unit/1': {
-                    'foo': 'baz',
-                },
-            }
+        Retrieve the relation data for each unit involved in a realtion and,
+        if complete, store it in a list under `self[self.interface]`.  This
+        is automatically called when the RelationContext is instantiated.
+
+        The units are sorted lexographically first by the service ID, then by
+        the unit ID.  Thus, if an interface has two other services, 'db:1'
+        and 'db:2', with 'db:1' having two units, 'wordpress/0' and 'wordpress/1',
+        and 'db:2' having one unit, 'mediawiki/0', all of which have a complete
+        set of data, the relation data for the units will be stored in the
+        order: 'wordpress/0', 'wordpress/1', 'mediawiki/0'.
+
+        If you only care about a single unit on the relation, you can just
+        access it as `{{ interface[0]['key'] }}`.  However, if you can at all
+        support multiple units on a relation, you should iterate over the list,
+        like:
+
+            {% for unit in interface -%}
+                {{ unit['key'] }}{% if not loop.last %},{% endif %}
+            {%- endfor %}
+
+        Note that since all sets of relation data from all related services and
+        units are in a single list, if you need to know which service or unit a
+        set of data came from, you'll need to extend this class to preserve
+        that information.
         """
         if not hookenv.relation_ids(self.interface):
             return
 
-        ns = self.setdefault(self.interface, {})
-        required = set(self.required_keys)
-        for rid in hookenv.relation_ids(self.interface):
-            for unit in hookenv.related_units(rid):
+        ns = self.setdefault(self.interface, [])
+        for rid in sorted(hookenv.relation_ids(self.interface)):
+            for unit in sorted(hookenv.related_units(rid)):
                 reldata = hookenv.relation_get(rid=rid, unit=unit)
-                unit_ns = ns.setdefault(unit, {})
-                unit_ns.update(reldata)
-                if set(reldata.keys()).issuperset(required):
-                    ns.update(reldata)
+                if self._is_ready(reldata):
+                    ns.append(reldata)
 
 
 class ManagerCallback(object):
@@ -316,6 +342,6 @@
 
 
 # Convenience aliases
-template = TemplateCallback
+render_template = template = TemplateCallback
 open_ports = PortManagerCallback()
 close_ports = PortManagerCallback()

=== modified file 'hooks/config.py'
--- hooks/config.py	2014-05-29 17:34:04 +0000
+++ hooks/config.py	2014-06-11 22:23:05 +0000
@@ -49,12 +49,18 @@
         'service': 'cf-registrar',
         'required_data': [contexts.NatsRelation(),
                           contexts.RouterRelation(),
-                          {'uaa_address': UAA_ADDRESS}],
+                          {
+                              'uaa_address': UAA_ADDRESS,
+                              'config': hookenv.config(),
+                              'port': 8080,
+                          }],
         'data_ready': [
             services.template(source='cf-registrar.conf',
                               target='/etc/init/cf-registrar.conf'),
             services.template(source='registrar.yml',
                               target=REGISTRAR_CONFIG_FILE),
+            services.template(source='server.xml',
+                              target=os.path.join(UAA_CONFIG_DIR, 'tomcat/server.xml')),
         ],
     }
 ]

=== modified file 'templates/registrar.yml'
--- templates/registrar.yml	2014-04-03 08:33:51 +0000
+++ templates/registrar.yml	2014-06-11 22:23:05 +0000
@@ -4,15 +4,15 @@
   level: info
 
 message_bus_servers:
-  - nats://{{ nats["nats_user"] }}:{{ nats["nats_password"] }}@{{ nats["nats_address"] }}:{{ nats["nats_port"] }}
+  - nats://{{ nats[0]["user"] }}:{{ nats[0]["password"] }}@{{ nats[0]["address"] }}:{{ nats[0]["port"] }}
 
 # domain is set to domain name that is resolved to router ip.
-uri: 
-  - uaa.{{ router["domain"] }}
+uri:
+  - uaa.{{ router[0]["domain"] }}
 
 host: {{ uaa_address }}   # public IP for UAA service
+port: {{ port }}  # This is port of tomcat UAA.
 index: 1
-port: 8080  # This is port of tomcat UAA.
 tags:
   component: uaa
 varz:

=== renamed file 'files/config/tomcat/server.xml' => 'templates/server.xml'
--- files/config/tomcat/server.xml	2014-03-26 17:52:44 +0000
+++ templates/server.xml	2014-06-11 22:23:05 +0000
@@ -8,7 +8,7 @@
 
   <Service name="Catalina">
 
-    <Connector class="org.apache.coyote.http11.Http11NioProtocol" port="8080" protocol="HTTP/1.1" connectionTimeout="20000"/>
+    <Connector class="org.apache.coyote.http11.Http11NioProtocol" port="{{ port }}" protocol="HTTP/1.1" connectionTimeout="20000"/>
     <Engine name="Catalina" defaultHost="localhost">
 
       <Host name="localhost" appBase="webapps" unpackWARs="true" autoDeploy="false">