Merge lp:~johnsca/charms/trusty/cf-loggregator/port-conflicts into lp:~cf-charmers/charms/trusty/cf-loggregator/trunk
- Trusty Tahr (14.04)
- port-conflicts
- Merge into trunk
Proposed by
Cory Johns
Status: | Merged |
---|---|
Merged at revision: | 34 |
Proposed branch: | lp:~johnsca/charms/trusty/cf-loggregator/port-conflicts |
Merge into: | lp:~cf-charmers/charms/trusty/cf-loggregator/trunk |
Diff against target: |
348 lines (+128/-87) 7 files modified
config.yaml (+1/-7) hooks/charmhelpers/contrib/cloudfoundry/contexts.py (+17/-6) hooks/charmhelpers/core/services.py (+81/-55) hooks/config.py (+11/-2) hooks/loggregator-relation-changed (+5/-4) metadata.yaml (+2/-2) templates/loggregator.json (+11/-11) |
To merge this branch: | bzr merge lp:~johnsca/charms/trusty/cf-loggregator/port-conflicts |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Cloud Foundry Charmers | Pending | ||
Review via email: mp+222675@code.launchpad.net |
Commit message
Description of the change
Resolve port conflicts for cf-loggregator
To post a comment you must log in.
Revision history for this message
Cory Johns (johnsca) wrote : | # |
- 40. By Cory Johns
-
Removed port options as they don't add much value to change
Revision history for this message
Cory Johns (johnsca) wrote : | # |
Please take a look.
Revision history for this message
Benjamin Saller (bcsaller) wrote : | # |
LGTM, thanks
https:/
File config.yaml (left):
https:/
config.yaml:11: The shared-secret for log clients to use when publishing
log
thanks :)
- 41. By Cory Johns
-
Fixed missed config-to-static port changes
Revision history for this message
Cory Johns (johnsca) wrote : | # |
Please take a look.
Revision history for this message
Benjamin Saller (bcsaller) wrote : | # |
LGTM pending port confirmation mentioned in the other log charm.
Looks like they still conflict
- 42. By Cory Johns
-
Fixed conflicting ports
Revision history for this message
Cory Johns (johnsca) wrote : | # |
Please take a look.
Revision history for this message
Cory Johns (johnsca) wrote : | # |
*** Submitted:
Resolve port conflicts for cf-loggregator
R=benjamin.saller
CC=
https:/
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'config.yaml' | |||
2 | --- config.yaml 2014-06-03 04:16:58 +0000 | |||
3 | +++ config.yaml 2014-06-12 16:51:24 +0000 | |||
4 | @@ -1,16 +1,10 @@ | |||
5 | 1 | options: | 1 | options: |
7 | 2 | max-retained-logs: | 2 | max_retained_logs: |
8 | 3 | type: int | 3 | type: int |
9 | 4 | default: 20 | 4 | default: 20 |
10 | 5 | description: | | 5 | description: | |
11 | 6 | Max lines of log output to buffer. If you wish to keep logs in a scalable | 6 | Max lines of log output to buffer. If you wish to keep logs in a scalable |
12 | 7 | way sync them to a real logging backend. | 7 | way sync them to a real logging backend. |
13 | 8 | client_secret: | ||
14 | 9 | type: string | ||
15 | 10 | description: | | ||
16 | 11 | The shared-secret for log clients to use when publishing log | ||
17 | 12 | messages. | ||
18 | 13 | default: 17b6cc14-eea8-433e-b8b7-a65ff8941e3b | ||
19 | 14 | source: | 8 | source: |
20 | 15 | type: string | 9 | type: string |
21 | 16 | default: 'ppa:cf-charm/ppa' | 10 | default: 'ppa:cf-charm/ppa' |
22 | 17 | 11 | ||
23 | === modified file 'hooks/charmhelpers/contrib/cloudfoundry/contexts.py' | |||
24 | --- hooks/charmhelpers/contrib/cloudfoundry/contexts.py 2014-05-27 22:03:37 +0000 | |||
25 | +++ hooks/charmhelpers/contrib/cloudfoundry/contexts.py 2014-06-12 16:51:24 +0000 | |||
26 | @@ -33,7 +33,7 @@ | |||
27 | 33 | 33 | ||
28 | 34 | class NatsRelation(RelationContext): | 34 | class NatsRelation(RelationContext): |
29 | 35 | interface = 'nats' | 35 | interface = 'nats' |
31 | 36 | required_keys = ['nats_port', 'nats_address', 'nats_user', 'nats_password'] | 36 | required_keys = ['address', 'port', 'user', 'password'] |
32 | 37 | 37 | ||
33 | 38 | 38 | ||
34 | 39 | class MysqlRelation(RelationContext): | 39 | class MysqlRelation(RelationContext): |
35 | @@ -44,9 +44,10 @@ | |||
36 | 44 | def get_data(self): | 44 | def get_data(self): |
37 | 45 | RelationContext.get_data(self) | 45 | RelationContext.get_data(self) |
38 | 46 | if self.is_ready(): | 46 | if self.is_ready(): |
42 | 47 | if 'port' not in self['db']: | 47 | for unit in self['db']: |
43 | 48 | self['db']['port'] = '3306' | 48 | if 'port' not in unit: |
44 | 49 | self['db']['dsn'] = self.dsn_template.format(**self['db']) | 49 | unit['port'] = '3306' |
45 | 50 | unit['dsn'] = self.dsn_template.format(**unit) | ||
46 | 50 | 51 | ||
47 | 51 | 52 | ||
48 | 52 | class RouterRelation(RelationContext): | 53 | class RouterRelation(RelationContext): |
49 | @@ -56,9 +57,19 @@ | |||
50 | 56 | 57 | ||
51 | 57 | class LogRouterRelation(RelationContext): | 58 | class LogRouterRelation(RelationContext): |
52 | 58 | interface = 'logrouter' | 59 | interface = 'logrouter' |
54 | 59 | required_keys = ['shared-secret', 'logrouter-address'] | 60 | required_keys = ['shared_secret', 'address', 'incoming_port', 'outgoing_port'] |
55 | 60 | 61 | ||
56 | 61 | 62 | ||
57 | 62 | class LoggregatorRelation(RelationContext): | 63 | class LoggregatorRelation(RelationContext): |
58 | 63 | interface = 'loggregator' | 64 | interface = 'loggregator' |
60 | 64 | required_keys = ['shared_secret', 'loggregator_address'] | 65 | required_keys = ['address', 'incoming_port', 'outgoing_port'] |
61 | 66 | |||
62 | 67 | |||
63 | 68 | class EtcdRelation(RelationContext): | ||
64 | 69 | interface = 'etcd' | ||
65 | 70 | required_keys = ['hostname', 'port'] | ||
66 | 71 | |||
67 | 72 | |||
68 | 73 | class CloudControllerRelation(RelationContext): | ||
69 | 74 | interface = 'cc' | ||
70 | 75 | required_keys = ['hostname', 'port', 'user', 'password'] | ||
71 | 65 | 76 | ||
72 | === modified file 'hooks/charmhelpers/core/services.py' | |||
73 | --- hooks/charmhelpers/core/services.py 2014-05-29 17:28:42 +0000 | |||
74 | +++ hooks/charmhelpers/core/services.py 2014-06-12 16:51:24 +0000 | |||
75 | @@ -11,6 +11,14 @@ | |||
76 | 11 | """ | 11 | """ |
77 | 12 | Register a list of services, given their definitions. | 12 | Register a list of services, given their definitions. |
78 | 13 | 13 | ||
79 | 14 | Traditional charm authoring is focused on implementing hooks. That is, | ||
80 | 15 | the charm author is thinking in terms of "What hook am I handling; what | ||
81 | 16 | does this hook need to do?" However, in most cases, the real question | ||
82 | 17 | should be "Do I have the information I need to configure and start this | ||
83 | 18 | piece of software and, if so, what are the steps for doing so." The | ||
84 | 19 | ServiceManager framework tries to bring the focus to the data and the | ||
85 | 20 | setup tasks, in the most declarative way possible. | ||
86 | 21 | |||
87 | 14 | Service definitions are dicts in the following formats (all keys except | 22 | Service definitions are dicts in the following formats (all keys except |
88 | 15 | 'service' are optional): | 23 | 'service' are optional): |
89 | 16 | 24 | ||
90 | @@ -67,28 +75,28 @@ | |||
91 | 67 | a mongodb relation and which runs a custom `db_migrate` function prior to | 75 | a mongodb relation and which runs a custom `db_migrate` function prior to |
92 | 68 | restarting the service, and a Runit serivce called spadesd. | 76 | restarting the service, and a Runit serivce called spadesd. |
93 | 69 | 77 | ||
116 | 70 | >>> manager = services.ServiceManager([ | 78 | manager = services.ServiceManager([ |
117 | 71 | ... { | 79 | { |
118 | 72 | ... 'service': 'bingod', | 80 | 'service': 'bingod', |
119 | 73 | ... 'ports': [80, 443], | 81 | 'ports': [80, 443], |
120 | 74 | ... 'required_data': [MongoRelation(), config()], | 82 | 'required_data': [MongoRelation(), config(), {'my': 'data'}], |
121 | 75 | ... 'data_ready': [ | 83 | 'data_ready': [ |
122 | 76 | ... services.template(source='bingod.conf'), | 84 | services.template(source='bingod.conf'), |
123 | 77 | ... services.template(source='bingod.ini', | 85 | services.template(source='bingod.ini', |
124 | 78 | ... target='/etc/bingod.ini', | 86 | target='/etc/bingod.ini', |
125 | 79 | ... owner='bingo', perms=0400), | 87 | owner='bingo', perms=0400), |
126 | 80 | ... ], | 88 | ], |
127 | 81 | ... }, | 89 | }, |
128 | 82 | ... { | 90 | { |
129 | 83 | ... 'service': 'spadesd', | 91 | 'service': 'spadesd', |
130 | 84 | ... 'data_ready': services.template(source='spadesd_run.j2', | 92 | 'data_ready': services.template(source='spadesd_run.j2', |
131 | 85 | ... target='/etc/sv/spadesd/run', | 93 | target='/etc/sv/spadesd/run', |
132 | 86 | ... perms=0555), | 94 | perms=0555), |
133 | 87 | ... 'start': runit_start, | 95 | 'start': runit_start, |
134 | 88 | ... 'stop': runit_stop, | 96 | 'stop': runit_stop, |
135 | 89 | ... }, | 97 | }, |
136 | 90 | ... ]) | 98 | ]) |
137 | 91 | ... manager.manage() | 99 | manager.manage() |
138 | 92 | """ | 100 | """ |
139 | 93 | self.services = {} | 101 | self.services = {} |
140 | 94 | for service in services or []: | 102 | for service in services or []: |
141 | @@ -213,55 +221,73 @@ | |||
142 | 213 | interface = None | 221 | interface = None |
143 | 214 | required_keys = [] | 222 | required_keys = [] |
144 | 215 | 223 | ||
145 | 224 | def __init__(self, *args, **kwargs): | ||
146 | 225 | super(RelationContext, self).__init__(*args, **kwargs) | ||
147 | 226 | self.get_data() | ||
148 | 227 | |||
149 | 216 | def __bool__(self): | 228 | def __bool__(self): |
150 | 217 | """ | 229 | """ |
152 | 218 | Updates the data and returns True if all of the required_keys are available. | 230 | Returns True if all of the required_keys are available. |
153 | 219 | """ | 231 | """ |
154 | 220 | self.get_data() | ||
155 | 221 | return self.is_ready() | 232 | return self.is_ready() |
156 | 222 | 233 | ||
157 | 223 | __nonzero__ = __bool__ | 234 | __nonzero__ = __bool__ |
158 | 224 | 235 | ||
159 | 236 | def __repr__(self): | ||
160 | 237 | return super(RelationContext, self).__repr__() | ||
161 | 238 | |||
162 | 225 | def is_ready(self): | 239 | def is_ready(self): |
163 | 226 | """ | 240 | """ |
167 | 227 | Returns True if all of the required_keys are available. | 241 | Returns True if all of the `required_keys` are available from any units. |
168 | 228 | """ | 242 | """ |
169 | 229 | return set(self.get(self.interface, {}).keys()).issuperset(set(self.required_keys)) | 243 | ready = len(self.get(self.interface, [])) > 0 |
170 | 244 | if not ready: | ||
171 | 245 | hookenv.log('Incomplete relation: {}'.format(self.__class__.__name__), hookenv.DEBUG) | ||
172 | 246 | return ready | ||
173 | 247 | |||
174 | 248 | def _is_ready(self, unit_data): | ||
175 | 249 | """ | ||
176 | 250 | Helper method that tests a set of relation data and returns True if | ||
177 | 251 | all of the `required_keys` are present. | ||
178 | 252 | """ | ||
179 | 253 | return set(unit_data.keys()).issuperset(set(self.required_keys)) | ||
180 | 230 | 254 | ||
181 | 231 | def get_data(self): | 255 | def get_data(self): |
182 | 232 | """ | 256 | """ |
202 | 233 | Retrieve the relation data and store it under `self[self.interface]`. | 257 | Retrieve the relation data for each unit involved in a realtion and, |
203 | 234 | 258 | if complete, store it in a list under `self[self.interface]`. This | |
204 | 235 | If there are more than one units related on the desired interface, | 259 | is automatically called when the RelationContext is instantiated. |
205 | 236 | then each unit will have its data stored under `self[self.interface][unit_id]` | 260 | |
206 | 237 | and one of the units with complete information will chosen at random | 261 | The units are sorted lexographically first by the service ID, then by |
207 | 238 | to fill the values at `self[self.interface]`. | 262 | the unit ID. Thus, if an interface has two other services, 'db:1' |
208 | 239 | 263 | and 'db:2', with 'db:1' having two units, 'wordpress/0' and 'wordpress/1', | |
209 | 240 | 264 | and 'db:2' having one unit, 'mediawiki/0', all of which have a complete | |
210 | 241 | For example: | 265 | set of data, the relation data for the units will be stored in the |
211 | 242 | 266 | order: 'wordpress/0', 'wordpress/1', 'mediawiki/0'. | |
212 | 243 | { | 267 | |
213 | 244 | 'foo': 'bar', | 268 | If you only care about a single unit on the relation, you can just |
214 | 245 | 'unit/0': { | 269 | access it as `{{ interface[0]['key'] }}`. However, if you can at all |
215 | 246 | 'foo': 'bar', | 270 | support multiple units on a relation, you should iterate over the list, |
216 | 247 | }, | 271 | like: |
217 | 248 | 'unit/1': { | 272 | |
218 | 249 | 'foo': 'baz', | 273 | {% for unit in interface -%} |
219 | 250 | }, | 274 | {{ unit['key'] }}{% if not loop.last %},{% endif %} |
220 | 251 | } | 275 | {%- endfor %} |
221 | 276 | |||
222 | 277 | Note that since all sets of relation data from all related services and | ||
223 | 278 | units are in a single list, if you need to know which service or unit a | ||
224 | 279 | set of data came from, you'll need to extend this class to preserve | ||
225 | 280 | that information. | ||
226 | 252 | """ | 281 | """ |
227 | 253 | if not hookenv.relation_ids(self.interface): | 282 | if not hookenv.relation_ids(self.interface): |
228 | 254 | return | 283 | return |
229 | 255 | 284 | ||
234 | 256 | ns = self.setdefault(self.interface, {}) | 285 | ns = self.setdefault(self.interface, []) |
235 | 257 | required = set(self.required_keys) | 286 | for rid in sorted(hookenv.relation_ids(self.interface)): |
236 | 258 | for rid in hookenv.relation_ids(self.interface): | 287 | for unit in sorted(hookenv.related_units(rid)): |
233 | 259 | for unit in hookenv.related_units(rid): | ||
237 | 260 | reldata = hookenv.relation_get(rid=rid, unit=unit) | 288 | reldata = hookenv.relation_get(rid=rid, unit=unit) |
242 | 261 | unit_ns = ns.setdefault(unit, {}) | 289 | if self._is_ready(reldata): |
243 | 262 | unit_ns.update(reldata) | 290 | ns.append(reldata) |
240 | 263 | if set(reldata.keys()).issuperset(required): | ||
241 | 264 | ns.update(reldata) | ||
244 | 265 | 291 | ||
245 | 266 | 292 | ||
246 | 267 | class ManagerCallback(object): | 293 | class ManagerCallback(object): |
247 | @@ -316,6 +342,6 @@ | |||
248 | 316 | 342 | ||
249 | 317 | 343 | ||
250 | 318 | # Convenience aliases | 344 | # Convenience aliases |
252 | 319 | template = TemplateCallback | 345 | render_template = template = TemplateCallback |
253 | 320 | open_ports = PortManagerCallback() | 346 | open_ports = PortManagerCallback() |
254 | 321 | close_ports = PortManagerCallback() | 347 | close_ports = PortManagerCallback() |
255 | 322 | 348 | ||
256 | === modified file 'hooks/config.py' | |||
257 | --- hooks/config.py 2014-05-29 17:28:42 +0000 | |||
258 | +++ hooks/config.py 2014-06-12 16:51:24 +0000 | |||
259 | @@ -14,14 +14,23 @@ | |||
260 | 14 | LOGGREGATOR_CONFIG_DIR = os.path.join(LOGGREGATOR_DIR, 'config') | 14 | LOGGREGATOR_CONFIG_DIR = os.path.join(LOGGREGATOR_DIR, 'config') |
261 | 15 | LOGGREGATOR_CONFIG_PATH = os.path.join(LOGGREGATOR_CONFIG_DIR, | 15 | LOGGREGATOR_CONFIG_PATH = os.path.join(LOGGREGATOR_CONFIG_DIR, |
262 | 16 | 'loggregator.json') | 16 | 'loggregator.json') |
263 | 17 | INCOMING_PORT = 3457 | ||
264 | 18 | OUTGOING_PORT = 8082 | ||
265 | 19 | VARZ_PORT = 8883 | ||
266 | 17 | 20 | ||
267 | 18 | SERVICES = [ | 21 | SERVICES = [ |
268 | 19 | { | 22 | { |
269 | 20 | 'service': LOGGREGATOR_JOB_NAME, | 23 | 'service': LOGGREGATOR_JOB_NAME, |
270 | 21 | 'required_data': [ | 24 | 'required_data': [ |
271 | 22 | {'service_name': os.environ['JUJU_UNIT_NAME'].split('/')[0]}, | ||
272 | 23 | hookenv.config(), | ||
273 | 24 | contexts.NatsRelation(), | 25 | contexts.NatsRelation(), |
274 | 26 | contexts.LogRouterRelation(), | ||
275 | 27 | { | ||
276 | 28 | 'service_name': os.environ['JUJU_UNIT_NAME'].split('/')[0], | ||
277 | 29 | 'config': hookenv.config(), | ||
278 | 30 | 'incoming_port': INCOMING_PORT, | ||
279 | 31 | 'outgoing_port': OUTGOING_PORT, | ||
280 | 32 | 'varz_port': VARZ_PORT, | ||
281 | 33 | }, | ||
282 | 25 | ], | 34 | ], |
283 | 26 | 'data_ready': [ | 35 | 'data_ready': [ |
284 | 27 | services.template(source='loggregator.conf', | 36 | services.template(source='loggregator.conf', |
285 | 28 | 37 | ||
286 | === modified file 'hooks/loggregator-relation-changed' | |||
287 | --- hooks/loggregator-relation-changed 2014-05-27 22:03:37 +0000 | |||
288 | +++ hooks/loggregator-relation-changed 2014-06-12 16:51:24 +0000 | |||
289 | @@ -1,8 +1,9 @@ | |||
290 | 1 | #!/usr/bin/env python | 1 | #!/usr/bin/env python |
291 | 2 | from charmhelpers.core import hookenv | 2 | from charmhelpers.core import hookenv |
292 | 3 | import config | ||
293 | 3 | 4 | ||
294 | 4 | config = hookenv.config() | ||
295 | 5 | loggregator_address = hookenv.unit_get('private-address').encode('utf-8') | ||
296 | 6 | hookenv.relation_set(None, { | 5 | hookenv.relation_set(None, { |
299 | 7 | 'shared_secret': config['client_secret'], | 6 | 'address': hookenv.unit_get('private-address').encode('utf-8'), |
300 | 8 | 'loggregator_address': loggregator_address}) | 7 | 'incoming_port': config.INCOMING_PORT, |
301 | 8 | 'outgoing_port': config.OUTGOING_PORT, | ||
302 | 9 | }) | ||
303 | 9 | 10 | ||
304 | === modified file 'metadata.yaml' | |||
305 | --- metadata.yaml 2014-05-12 11:12:06 +0000 | |||
306 | +++ metadata.yaml 2014-06-12 16:51:24 +0000 | |||
307 | @@ -12,5 +12,5 @@ | |||
308 | 12 | requires: | 12 | requires: |
309 | 13 | nats: | 13 | nats: |
310 | 14 | interface: nats | 14 | interface: nats |
311 | 15 | # logrouter: | ||
312 | 16 | # interface: logrouter | ||
313 | 17 | \ No newline at end of file | 15 | \ No newline at end of file |
314 | 16 | logrouter: | ||
315 | 17 | interface: logrouter | ||
316 | 18 | 18 | ||
317 | === modified file 'templates/loggregator.json' | |||
318 | --- templates/loggregator.json 2014-04-28 12:57:31 +0000 | |||
319 | +++ templates/loggregator.json 2014-06-12 16:51:24 +0000 | |||
320 | @@ -1,16 +1,16 @@ | |||
321 | 1 | { | 1 | { |
322 | 2 | "IncomingPort": {{ incoming_port }}, | ||
323 | 3 | "OutgoingPort": {{ outgoing_port }}, | ||
324 | 2 | "Index": 0, | 4 | "Index": 0, |
325 | 3 | "NatsHost": "{{ nats['nats_address'] }}", | ||
326 | 4 | "VarzPort": 8888, | ||
327 | 5 | "SkipCertVerify": false, | 5 | "SkipCertVerify": false, |
328 | 6 | "MaxRetainedLogMessages": {{ config['max_retained_logs'] }}, | ||
329 | 7 | "Syslog": "", | ||
330 | 8 | "SharedSecret": "{{ logrouter[0]['shared_secret'] }}", | ||
331 | 6 | "VarzUser": "{{ service_name }}", | 9 | "VarzUser": "{{ service_name }}", |
332 | 7 | "MaxRetainedLogMessages": 20, | ||
333 | 8 | "OutgoingPort": 8080, | ||
334 | 9 | "Syslog": "", | ||
335 | 10 | "VarzPass": "{{ service_name }}", | 10 | "VarzPass": "{{ service_name }}", |
336 | 11 | "NatsUser": "{{ nats['nats_user'] }}", | ||
337 | 12 | "NatsPass": "{{ nats['nats_password'] }}", | ||
338 | 13 | "IncomingPort": 3456, | ||
339 | 14 | "NatsPort": {{ nats['nats_port'] }}, | ||
340 | 15 | "SharedSecret": "{{ client_secret }}" | ||
341 | 16 | } | ||
342 | 17 | \ No newline at end of file | 11 | \ No newline at end of file |
343 | 12 | "VarzPort": {{ varz_port }}, | ||
344 | 13 | "NatsHost": "{{ nats[0]['address'] }}", | ||
345 | 14 | "NatsUser": "{{ nats[0]['user'] }}", | ||
346 | 15 | "NatsPass": "{{ nats[0]['password'] }}", | ||
347 | 16 | "NatsPort": {{ nats[0]['port'] }} | ||
348 | 17 | } |
Reviewers: mp+222675_ code.launchpad. net,
Message:
Please take a look.
Description:
Resolve port conflicts for cf-loggregator
https:/ /code.launchpad .net/~johnsca/ charms/ trusty/ cf-loggregator/ port-conflicts/ +merge/ 222675
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/103250045/
Affected files (+135, -86 lines): ers/contrib/ cloudfoundry/ contexts. py ers/core/ services. py or-relation- changed loggregator. json
A [revision details]
M config.yaml
M hooks/charmhelp
M hooks/charmhelp
M hooks/config.py
M hooks/loggregat
M metadata.yaml
M templates/