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

Proposed by Cory Johns
Status: Merged
Merged at revision: 33
Proposed branch: lp:~johnsca/charms/trusty/cf-nats/port-conflicts
Merge into: lp:~cf-charmers/charms/trusty/cf-nats/trunk
Diff against target: 263 lines (+104/-67)
4 files modified
hooks/charmhelpers/contrib/cloudfoundry/contexts.py (+17/-6)
hooks/charmhelpers/core/services.py (+81/-55)
hooks/config.py (+4/-4)
templates/nats.yml (+2/-2)
To merge this branch: bzr merge lp:~johnsca/charms/trusty/cf-nats/port-conflicts
Reviewer Review Type Date Requested Status
Cloud Foundry Charmers Pending
Review via email: mp+222677@code.launchpad.net

Description of the change

Resolve port conflicts for cf-nats

https://codereview.appspot.com/102270043/

To post a comment you must log in.
Revision history for this message
Cory Johns (johnsca) wrote :

Reviewers: mp+222677_code.launchpad.net,

Message:
Please take a look.

Description:
Resolve port conflicts for cf-nats

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

(do not edit description out of merge proposal)

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

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

Revision history for this message
Benjamin Saller (bcsaller) wrote :

LGTM, thanks for fixing the prefixes

https://codereview.appspot.com/102270043/

Revision history for this message
Cory Johns (johnsca) wrote :

*** Submitted:

Resolve port conflicts for cf-nats

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

https://codereview.appspot.com/102270043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'hooks/charmhelpers/contrib/cloudfoundry/contexts.py'
2--- hooks/charmhelpers/contrib/cloudfoundry/contexts.py 2014-05-27 22:33:13 +0000
3+++ hooks/charmhelpers/contrib/cloudfoundry/contexts.py 2014-06-10 16:00:32 +0000
4@@ -33,7 +33,7 @@
5
6 class NatsRelation(RelationContext):
7 interface = 'nats'
8- required_keys = ['nats_port', 'nats_address', 'nats_user', 'nats_password']
9+ required_keys = ['address', 'port', 'user', 'password']
10
11
12 class MysqlRelation(RelationContext):
13@@ -44,9 +44,10 @@
14 def get_data(self):
15 RelationContext.get_data(self)
16 if self.is_ready():
17- if 'port' not in self['db']:
18- self['db']['port'] = '3306'
19- self['db']['dsn'] = self.dsn_template.format(**self['db'])
20+ for unit in self['db']:
21+ if 'port' not in unit:
22+ unit['port'] = '3306'
23+ unit['dsn'] = self.dsn_template.format(**unit)
24
25
26 class RouterRelation(RelationContext):
27@@ -56,9 +57,19 @@
28
29 class LogRouterRelation(RelationContext):
30 interface = 'logrouter'
31- required_keys = ['shared-secret', 'logrouter-address']
32+ required_keys = ['shared_secret', 'address', 'incoming_port', 'outgoing_port']
33
34
35 class LoggregatorRelation(RelationContext):
36 interface = 'loggregator'
37- required_keys = ['shared_secret', 'loggregator_address']
38+ required_keys = ['address', 'incoming_port', 'outgoing_port']
39+
40+
41+class EtcdRelation(RelationContext):
42+ interface = 'etcd'
43+ required_keys = ['hostname', 'port']
44+
45+
46+class CloudControllerRelation(RelationContext):
47+ interface = 'cc'
48+ required_keys = ['hostname', 'port', 'user', 'password']
49
50=== modified file 'hooks/charmhelpers/core/services.py'
51--- hooks/charmhelpers/core/services.py 2014-05-29 17:33:12 +0000
52+++ hooks/charmhelpers/core/services.py 2014-06-10 16:00:32 +0000
53@@ -11,6 +11,14 @@
54 """
55 Register a list of services, given their definitions.
56
57+ Traditional charm authoring is focused on implementing hooks. That is,
58+ the charm author is thinking in terms of "What hook am I handling; what
59+ does this hook need to do?" However, in most cases, the real question
60+ should be "Do I have the information I need to configure and start this
61+ piece of software and, if so, what are the steps for doing so." The
62+ ServiceManager framework tries to bring the focus to the data and the
63+ setup tasks, in the most declarative way possible.
64+
65 Service definitions are dicts in the following formats (all keys except
66 'service' are optional):
67
68@@ -67,28 +75,28 @@
69 a mongodb relation and which runs a custom `db_migrate` function prior to
70 restarting the service, and a Runit serivce called spadesd.
71
72- >>> manager = services.ServiceManager([
73- ... {
74- ... 'service': 'bingod',
75- ... 'ports': [80, 443],
76- ... 'required_data': [MongoRelation(), config()],
77- ... 'data_ready': [
78- ... services.template(source='bingod.conf'),
79- ... services.template(source='bingod.ini',
80- ... target='/etc/bingod.ini',
81- ... owner='bingo', perms=0400),
82- ... ],
83- ... },
84- ... {
85- ... 'service': 'spadesd',
86- ... 'data_ready': services.template(source='spadesd_run.j2',
87- ... target='/etc/sv/spadesd/run',
88- ... perms=0555),
89- ... 'start': runit_start,
90- ... 'stop': runit_stop,
91- ... },
92- ... ])
93- ... manager.manage()
94+ manager = services.ServiceManager([
95+ {
96+ 'service': 'bingod',
97+ 'ports': [80, 443],
98+ 'required_data': [MongoRelation(), config(), {'my': 'data'}],
99+ 'data_ready': [
100+ services.template(source='bingod.conf'),
101+ services.template(source='bingod.ini',
102+ target='/etc/bingod.ini',
103+ owner='bingo', perms=0400),
104+ ],
105+ },
106+ {
107+ 'service': 'spadesd',
108+ 'data_ready': services.template(source='spadesd_run.j2',
109+ target='/etc/sv/spadesd/run',
110+ perms=0555),
111+ 'start': runit_start,
112+ 'stop': runit_stop,
113+ },
114+ ])
115+ manager.manage()
116 """
117 self.services = {}
118 for service in services or []:
119@@ -213,55 +221,73 @@
120 interface = None
121 required_keys = []
122
123+ def __init__(self, *args, **kwargs):
124+ super(RelationContext, self).__init__(*args, **kwargs)
125+ self.get_data()
126+
127 def __bool__(self):
128 """
129- Updates the data and returns True if all of the required_keys are available.
130+ Returns True if all of the required_keys are available.
131 """
132- self.get_data()
133 return self.is_ready()
134
135 __nonzero__ = __bool__
136
137+ def __repr__(self):
138+ return super(RelationContext, self).__repr__()
139+
140 def is_ready(self):
141 """
142- Returns True if all of the required_keys are available.
143- """
144- return set(self.get(self.interface, {}).keys()).issuperset(set(self.required_keys))
145+ Returns True if all of the `required_keys` are available from any units.
146+ """
147+ ready = len(self.get(self.interface, [])) > 0
148+ if not ready:
149+ hookenv.log('Incomplete relation: {}'.format(self.__class__.__name__), hookenv.DEBUG)
150+ return ready
151+
152+ def _is_ready(self, unit_data):
153+ """
154+ Helper method that tests a set of relation data and returns True if
155+ all of the `required_keys` are present.
156+ """
157+ return set(unit_data.keys()).issuperset(set(self.required_keys))
158
159 def get_data(self):
160 """
161- Retrieve the relation data and store it under `self[self.interface]`.
162-
163- If there are more than one units related on the desired interface,
164- then each unit will have its data stored under `self[self.interface][unit_id]`
165- and one of the units with complete information will chosen at random
166- to fill the values at `self[self.interface]`.
167-
168-
169- For example:
170-
171- {
172- 'foo': 'bar',
173- 'unit/0': {
174- 'foo': 'bar',
175- },
176- 'unit/1': {
177- 'foo': 'baz',
178- },
179- }
180+ Retrieve the relation data for each unit involved in a realtion and,
181+ if complete, store it in a list under `self[self.interface]`. This
182+ is automatically called when the RelationContext is instantiated.
183+
184+ The units are sorted lexographically first by the service ID, then by
185+ the unit ID. Thus, if an interface has two other services, 'db:1'
186+ and 'db:2', with 'db:1' having two units, 'wordpress/0' and 'wordpress/1',
187+ and 'db:2' having one unit, 'mediawiki/0', all of which have a complete
188+ set of data, the relation data for the units will be stored in the
189+ order: 'wordpress/0', 'wordpress/1', 'mediawiki/0'.
190+
191+ If you only care about a single unit on the relation, you can just
192+ access it as `{{ interface[0]['key'] }}`. However, if you can at all
193+ support multiple units on a relation, you should iterate over the list,
194+ like:
195+
196+ {% for unit in interface -%}
197+ {{ unit['key'] }}{% if not loop.last %},{% endif %}
198+ {%- endfor %}
199+
200+ Note that since all sets of relation data from all related services and
201+ units are in a single list, if you need to know which service or unit a
202+ set of data came from, you'll need to extend this class to preserve
203+ that information.
204 """
205 if not hookenv.relation_ids(self.interface):
206 return
207
208- ns = self.setdefault(self.interface, {})
209- required = set(self.required_keys)
210- for rid in hookenv.relation_ids(self.interface):
211- for unit in hookenv.related_units(rid):
212+ ns = self.setdefault(self.interface, [])
213+ for rid in sorted(hookenv.relation_ids(self.interface)):
214+ for unit in sorted(hookenv.related_units(rid)):
215 reldata = hookenv.relation_get(rid=rid, unit=unit)
216- unit_ns = ns.setdefault(unit, {})
217- unit_ns.update(reldata)
218- if set(reldata.keys()).issuperset(required):
219- ns.update(reldata)
220+ if self._is_ready(reldata):
221+ ns.append(reldata)
222
223
224 class ManagerCallback(object):
225@@ -316,6 +342,6 @@
226
227
228 # Convenience aliases
229-template = TemplateCallback
230+render_template = template = TemplateCallback
231 open_ports = PortManagerCallback()
232 close_ports = PortManagerCallback()
233
234=== modified file 'hooks/config.py'
235--- hooks/config.py 2014-05-29 17:33:12 +0000
236+++ hooks/config.py 2014-06-10 16:00:32 +0000
237@@ -17,10 +17,10 @@
238
239 NATS_CONTEXT = contexts.StoredContext(
240 'nats_credentials.yml', {
241- 'nats_user': host.pwgen(7),
242- 'nats_password': host.pwgen(7),
243- 'nats_port': 4222,
244- 'nats_address': hookenv.unit_get('private-address').encode('utf-8')})
245+ 'user': host.pwgen(7),
246+ 'password': host.pwgen(7),
247+ 'port': 4222,
248+ 'address': hookenv.unit_get('private-address').encode('utf-8')})
249
250 SERVICES = [
251 {
252
253=== modified file 'templates/nats.yml'
254--- templates/nats.yml 2014-04-01 08:34:31 +0000
255+++ templates/nats.yml 2014-06-10 16:00:32 +0000
256@@ -3,5 +3,5 @@
257 logtime: true
258 authentication:
259 timeout: 15
260- user: {{ nats_user }}
261- password: {{ nats_password }}
262+ user: {{ user }}
263+ password: {{ password }}

Subscribers

People subscribed via source and target branches