Merge lp:~johnsca/charms/trusty/cf-cloud-controller/services-callback-fu into lp:~cf-charmers/charms/trusty/cf-cloud-controller/trunk
- Trusty Tahr (14.04)
- services-callback-fu
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 29 |
Proposed branch: | lp:~johnsca/charms/trusty/cf-cloud-controller/services-callback-fu |
Merge into: | lp:~cf-charmers/charms/trusty/cf-cloud-controller/trunk |
Diff against target: |
950 lines (+493/-337) 12 files modified
hooks/charmhelpers/contrib/cloudfoundry/contexts.py (+41/-32) hooks/charmhelpers/core/host.py (+5/-0) hooks/charmhelpers/core/services.py (+316/-79) hooks/charmhelpers/core/templating.py (+38/-145) hooks/config-changed (+9/-0) hooks/config.py (+39/-0) hooks/db-relation-changed (+9/-0) hooks/hooks.py (+0/-81) hooks/nats-relation-changed (+9/-0) hooks/router-relation-changed (+9/-0) hooks/stop (+9/-0) hooks/upgrade-charm (+9/-0) |
To merge this branch: | bzr merge lp:~johnsca/charms/trusty/cf-cloud-controller/services-callback-fu |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Cloud Foundry Charmers | Pending | ||
Review via email: mp+220741@code.launchpad.net |
Commit message
Description of the change
Fixed relation ordering issue
Fixed an relation hook ordering dependency by refactoring to use
callbacks in charmhelpers.
Cory Johns (johnsca) wrote : | # |
Benjamin Saller (bcsaller) wrote : | # |
Thanks I like this, I'll approve it pending the answer to the one
question
https:/
File hooks/hooks.py (right):
https:/
hooks/hooks.py:18: def register():
why the containing method? it seems fine to register with hooks on
import
https:/
hooks/hooks.py:41:
'router-
The other pattern is more familiar but I think we could get people used
to this and it has the advantage of being very concise. :)
- 30. By Cory Johns
-
Use refactored services API and remove symlink hooks
Cory Johns (johnsca) wrote : | # |
Please take a look.
Whit Morriss (whitmo) wrote : | # |
LGTM. nice and concise.
https:/
File hooks/charmhelp
https:/
hooks/charmhelp
As discussed with Cory: we are abusing module scope and globals to
emulate object behavior. I feel the code will age and extend better if
we sacrificed a tiny bit of api convenience and used idiomatic python
classes here.
- 31. By Cory Johns
-
Updated charm-helpers and switched to class API for ServiceManager
Cory Johns (johnsca) wrote : | # |
Please take a look.
Benjamin Saller (bcsaller) wrote : | # |
So this is reviewed in the context of a charm, but its nice to see the
top level singleton thing play out.
I have some more bikeshedding suggestions, but they are only that. Feel
free to land this and the helpers changes. They
LGTM +1
Thanks
https:/
File hooks/charmhelp
https:/
hooks/charmhelp
Even though we only use this as a Singleton this is almost certainly the
proper way to evolve the code. Better to do this now (because its a
minor api change) than later, thanks.
https:/
hooks/charmhelp
callbacks>,
Not sure I love data_ready/
shedding and whit already reviewed this and was ok with the naming so
I'll pass if you don't like these better
'requires': [],
'complete': []
'incomplete': []
another option is to keep with the naming we use in relations though
I'll admit 'joined' is a little hard to understand in this context, so
maybe
'requires', 'changed', 'broken'
I do think 'broken' is a good name, but it needs something that fits
conceptually with it. At this point though I do think what you have is
understandable and nicely documented.
https:/
File hooks/config.py (right):
https:/
hooks/config.py:8:
Yeah, not bad.
Cory Johns (johnsca) wrote : | # |
On 2014/05/27 18:06:56, benjamin.saller wrote:
> Not sure I love data_ready/
shedding and
> whit already reviewed this and was ok with the naming so I'll pass if
you don't
> like these better
> 'requires': [],
> 'complete': []
> 'incomplete': []
> another option is to keep with the naming we use in relations though
I'll admit
> 'joined' is a little hard to understand in this context, so maybe
> 'requires', 'changed', 'broken'
> I do think 'broken' is a good name, but it needs something that fits
> conceptually with it. At this point though I do think what you have is
> understandable and nicely documented.
Yeah, we talked about nomenclature quite a bit on Friday. Whit's point
was that our "dependencies" list was overloaded and actually serves two
purposes:
1) Define what the service needs to operate that it can't get for
itself (relations, config data, etc)
2) Collect all of that information into a unified view
Naming it "dependencies" or "requirements" makes that second aspect
non-obvious, and it would be best for the API to be, as much as
possible, self-documenting. Given that, "required_data" was the best we
could come up with, as it conveys both that this is a list of required
things, and that those things provide data somehow. This also plays
into the ability to, instead of having a semi-opaque StaticContext
class, you can now just drop a plain dict (or the results of config())
in there directly and it will do what you expect.
Then, I also realized that I wanted to separate the "ready" (or
"complete," or whatever) event from the "start" event, so that the
charms can just worry about defining what happens between everything the
service needs being available and spinning up the service itself,
without having to worry about the details of spinning it up by manually
specifying host.service_start every time, since 90% of charms should be
using Upstart or some other SysV-compatible service manager.
So, then we end up with the events of interest:
* All of the requirements have been satisfied and the data is
available to configure and start the service
* Something changed, and we no longer have complete information for
running the service
The names we chose, "data_ready" and "data_lost" seemed to capture those
ideas while matching up well to "required_data." Additionally,
"data_lost" specially calls out the fact that it doesn't need to be
triggered in the case that we're still working to get set up and we've
never had the service set up, so nothing needs to be done to tear it
down.
I'm not opposed to "complete" and "incomplete," or "complete" and
"broken." But I also think that it will not be difficult to change the
keys later in a backwards-
a set of keys, and it would be all encapsulated in fire_event).
So, my inclination is to leave it as-is until / if we get more feedback
that "required_data," "data_ready," and "data_lost" are confusing.
- 32. By Cory Johns
-
Updated charm-helpers and removed implicit target from templates
Cory Johns (johnsca) wrote : | # |
Please take a look.
Cory Johns (johnsca) wrote : | # |
Please take a look.
Cory Johns (johnsca) wrote : | # |
*** Submitted:
Fixed relation ordering issue
Fixed an relation hook ordering dependency by refactoring to use
callbacks in charmhelpers.
R=benjamin.saller, whit.morriss
CC=
https:/
Preview Diff
1 | === modified file 'hooks/charmhelpers/contrib/cloudfoundry/contexts.py' |
2 | --- hooks/charmhelpers/contrib/cloudfoundry/contexts.py 2014-05-16 23:33:25 +0000 |
3 | +++ hooks/charmhelpers/contrib/cloudfoundry/contexts.py 2014-05-29 17:26:14 +0000 |
4 | @@ -1,55 +1,64 @@ |
5 | import os |
6 | - |
7 | -from charmhelpers.core.templating import ( |
8 | - ContextGenerator, |
9 | - RelationContext, |
10 | - StorableContext, |
11 | -) |
12 | - |
13 | - |
14 | -# Stores `config_data` hash into yaml file with `file_name` as a name |
15 | -# if `file_name` already exists, then it loads data from `file_name`. |
16 | -class StoredContext(ContextGenerator, StorableContext): |
17 | +import yaml |
18 | + |
19 | +from charmhelpers.core.services import RelationContext |
20 | + |
21 | + |
22 | +class StoredContext(dict): |
23 | + """ |
24 | + A data context that always returns the data that it was first created with. |
25 | + """ |
26 | def __init__(self, file_name, config_data): |
27 | + """ |
28 | + If the file exists, populate `self` with the data from the file. |
29 | + Otherwise, populate with the given data and persist it to the file. |
30 | + """ |
31 | if os.path.exists(file_name): |
32 | - self.data = self.read_context(file_name) |
33 | + self.update(self.read_context(file_name)) |
34 | else: |
35 | self.store_context(file_name, config_data) |
36 | - self.data = config_data |
37 | - |
38 | - def __call__(self): |
39 | - return self.data |
40 | - |
41 | - |
42 | -class NatsContext(RelationContext): |
43 | + self.update(config_data) |
44 | + |
45 | + def store_context(self, file_name, config_data): |
46 | + with open(file_name, 'w') as file_stream: |
47 | + yaml.dump(config_data, file_stream) |
48 | + |
49 | + def read_context(self, file_name): |
50 | + with open(file_name, 'r') as file_stream: |
51 | + data = yaml.load(file_stream) |
52 | + if not data: |
53 | + raise OSError("%s is empty" % file_name) |
54 | + return data |
55 | + |
56 | + |
57 | +class NatsRelation(RelationContext): |
58 | interface = 'nats' |
59 | required_keys = ['nats_port', 'nats_address', 'nats_user', 'nats_password'] |
60 | |
61 | |
62 | -class MysqlDSNContext(RelationContext): |
63 | +class MysqlRelation(RelationContext): |
64 | interface = 'db' |
65 | required_keys = ['user', 'password', 'host', 'database'] |
66 | dsn_template = "mysql2://{user}:{password}@{host}:{port}/{database}" |
67 | |
68 | - def __call__(self): |
69 | - ctx = RelationContext.__call__(self) |
70 | - if ctx: |
71 | - if 'port' not in ctx: |
72 | - ctx['db']['port'] = '3306' |
73 | - ctx['db']['dsn'] = self.dsn_template.format(**ctx['db']) |
74 | - return ctx |
75 | - |
76 | - |
77 | -class RouterContext(RelationContext): |
78 | + def get_data(self): |
79 | + RelationContext.get_data(self) |
80 | + if self.is_ready(): |
81 | + if 'port' not in self['db']: |
82 | + self['db']['port'] = '3306' |
83 | + self['db']['dsn'] = self.dsn_template.format(**self['db']) |
84 | + |
85 | + |
86 | +class RouterRelation(RelationContext): |
87 | interface = 'router' |
88 | required_keys = ['domain'] |
89 | |
90 | |
91 | -class LogRouterContext(RelationContext): |
92 | +class LogRouterRelation(RelationContext): |
93 | interface = 'logrouter' |
94 | required_keys = ['shared-secret', 'logrouter-address'] |
95 | |
96 | |
97 | -class LoggregatorContext(RelationContext): |
98 | +class LoggregatorRelation(RelationContext): |
99 | interface = 'loggregator' |
100 | required_keys = ['shared_secret', 'loggregator_address'] |
101 | |
102 | === modified file 'hooks/charmhelpers/core/host.py' |
103 | --- hooks/charmhelpers/core/host.py 2014-05-20 19:43:29 +0000 |
104 | +++ hooks/charmhelpers/core/host.py 2014-05-29 17:26:14 +0000 |
105 | @@ -63,6 +63,11 @@ |
106 | return False |
107 | |
108 | |
109 | +def service_available(service_name): |
110 | + """Determine whether a system service is available""" |
111 | + return service('status', service_name) |
112 | + |
113 | + |
114 | def adduser(username, password=None, shell='/bin/bash', system_user=False): |
115 | """Add a user to the system""" |
116 | try: |
117 | |
118 | === modified file 'hooks/charmhelpers/core/services.py' |
119 | --- hooks/charmhelpers/core/services.py 2014-05-16 22:44:17 +0000 |
120 | +++ hooks/charmhelpers/core/services.py 2014-05-29 17:26:14 +0000 |
121 | @@ -1,84 +1,321 @@ |
122 | +import os |
123 | +import sys |
124 | +from collections import Iterable |
125 | from charmhelpers.core import templating |
126 | from charmhelpers.core import host |
127 | - |
128 | - |
129 | -SERVICES = {} |
130 | - |
131 | - |
132 | -def register(services, templates_dir=None): |
133 | - """ |
134 | - Register a list of service configs. |
135 | - |
136 | - Service Configs are dicts in the following formats: |
137 | - |
138 | - { |
139 | - "service": <service name>, |
140 | - "templates": [ { |
141 | - 'target': <render target of template>, |
142 | - 'source': <optional name of template in passed in templates_dir> |
143 | - 'file_properties': <optional dict taking owner and octal mode> |
144 | - 'contexts': [ context generators, see contexts.py ] |
145 | - } |
146 | - ] } |
147 | - |
148 | - Either `source` or `target` must be provided. |
149 | - |
150 | - If 'source' is not provided for a template the templates_dir will |
151 | - be consulted for ``basename(target).j2``. |
152 | - |
153 | - If `target` is not provided, it will be assumed to be |
154 | - ``/etc/init/<service name>.conf``. |
155 | - """ |
156 | - for service in services: |
157 | - service.setdefault('templates_dir', templates_dir) |
158 | - SERVICES[service['service']] = service |
159 | - |
160 | - |
161 | -def reconfigure_services(restart=True): |
162 | - """ |
163 | - Update all files for all services and optionally restart them, if ready. |
164 | - """ |
165 | - for service_name in SERVICES.keys(): |
166 | - reconfigure_service(service_name, restart=restart) |
167 | - |
168 | - |
169 | -def reconfigure_service(service_name, restart=True): |
170 | - """ |
171 | - Update all files for a single service and optionally restart it, if ready. |
172 | - """ |
173 | - service = SERVICES.get(service_name) |
174 | - if not service or service['service'] != service_name: |
175 | - raise KeyError('Service not registered: %s' % service_name) |
176 | - |
177 | - manager_type = service.get('type', UpstartService) |
178 | - manager_type(service).reconfigure(restart) |
179 | - |
180 | - |
181 | -def stop_services(): |
182 | - for service_name in SERVICES.keys(): |
183 | - if host.service_running(service_name): |
184 | - host.service_stop(service_name) |
185 | - |
186 | - |
187 | -class ServiceTypeManager(object): |
188 | - def __init__(self, service_definition): |
189 | - self.service_name = service_definition['service'] |
190 | - self.templates = service_definition['templates'] |
191 | - self.templates_dir = service_definition['templates_dir'] |
192 | - |
193 | - def reconfigure(self, restart=True): |
194 | +from charmhelpers.core import hookenv |
195 | + |
196 | + |
197 | +class ServiceManager(object): |
198 | + def __init__(self, services=None): |
199 | + """ |
200 | + Register a list of services, given their definitions. |
201 | + |
202 | + Service definitions are dicts in the following formats (all keys except |
203 | + 'service' are optional): |
204 | + |
205 | + { |
206 | + "service": <service name>, |
207 | + "required_data": <list of required data contexts>, |
208 | + "data_ready": <one or more callbacks>, |
209 | + "data_lost": <one or more callbacks>, |
210 | + "start": <one or more callbacks>, |
211 | + "stop": <one or more callbacks>, |
212 | + "ports": <list of ports to manage>, |
213 | + } |
214 | + |
215 | + The 'required_data' list should contain dicts of required data (or |
216 | + dependency managers that act like dicts and know how to collect the data). |
217 | + Only when all items in the 'required_data' list are populated are the list |
218 | + of 'data_ready' and 'start' callbacks executed. See `is_ready()` for more |
219 | + information. |
220 | + |
221 | + The 'data_ready' value should be either a single callback, or a list of |
222 | + callbacks, to be called when all items in 'required_data' pass `is_ready()`. |
223 | + Each callback will be called with the service name as the only parameter. |
224 | + After these all of the 'data_ready' callbacks are called, the 'start' |
225 | + callbacks are fired. |
226 | + |
227 | + The 'data_lost' value should be either a single callback, or a list of |
228 | + callbacks, to be called when a 'required_data' item no longer passes |
229 | + `is_ready()`. Each callback will be called with the service name as the |
230 | + only parameter. After these all of the 'data_ready' callbacks are called, |
231 | + the 'stop' callbacks are fired. |
232 | + |
233 | + The 'start' value should be either a single callback, or a list of |
234 | + callbacks, to be called when starting the service, after the 'data_ready' |
235 | + callbacks are complete. Each callback will be called with the service |
236 | + name as the only parameter. This defaults to |
237 | + `[host.service_start, services.open_ports]`. |
238 | + |
239 | + The 'stop' value should be either a single callback, or a list of |
240 | + callbacks, to be called when stopping the service. If the service is |
241 | + being stopped because it no longer has all of its 'required_data', this |
242 | + will be called after all of the 'data_lost' callbacks are complete. |
243 | + Each callback will be called with the service name as the only parameter. |
244 | + This defaults to `[services.close_ports, host.service_stop]`. |
245 | + |
246 | + The 'ports' value should be a list of ports to manage. The default |
247 | + 'start' handler will open the ports after the service is started, |
248 | + and the default 'stop' handler will close the ports prior to stopping |
249 | + the service. |
250 | + |
251 | + |
252 | + Examples: |
253 | + |
254 | + The following registers an Upstart service called bingod that depends on |
255 | + a mongodb relation and which runs a custom `db_migrate` function prior to |
256 | + restarting the service, and a Runit serivce called spadesd. |
257 | + |
258 | + >>> manager = services.ServiceManager([ |
259 | + ... { |
260 | + ... 'service': 'bingod', |
261 | + ... 'ports': [80, 443], |
262 | + ... 'required_data': [MongoRelation(), config()], |
263 | + ... 'data_ready': [ |
264 | + ... services.template(source='bingod.conf'), |
265 | + ... services.template(source='bingod.ini', |
266 | + ... target='/etc/bingod.ini', |
267 | + ... owner='bingo', perms=0400), |
268 | + ... ], |
269 | + ... }, |
270 | + ... { |
271 | + ... 'service': 'spadesd', |
272 | + ... 'data_ready': services.template(source='spadesd_run.j2', |
273 | + ... target='/etc/sv/spadesd/run', |
274 | + ... perms=0555), |
275 | + ... 'start': runit_start, |
276 | + ... 'stop': runit_stop, |
277 | + ... }, |
278 | + ... ]) |
279 | + ... manager.manage() |
280 | + """ |
281 | + self.services = {} |
282 | + for service in services or []: |
283 | + service_name = service['service'] |
284 | + self.services[service_name] = service |
285 | + |
286 | + def manage(self): |
287 | + """ |
288 | + Handle the current hook by doing The Right Thing with the registered services. |
289 | + """ |
290 | + hook_name = os.path.basename(sys.argv[0]) |
291 | + if hook_name == 'stop': |
292 | + self.stop_services() |
293 | + else: |
294 | + self.reconfigure_services() |
295 | + |
296 | + def reconfigure_services(self, *service_names): |
297 | + """ |
298 | + Update all files for one or more registered services, and, |
299 | + if ready, optionally restart them. |
300 | + |
301 | + If no service names are given, reconfigures all registered services. |
302 | + """ |
303 | + for service_name in service_names or self.services.keys(): |
304 | + if self.is_ready(service_name): |
305 | + self.fire_event('data_ready', service_name) |
306 | + self.fire_event('start', service_name, default=[ |
307 | + host.service_restart, |
308 | + open_ports]) |
309 | + self.save_ready(service_name) |
310 | + else: |
311 | + if self.was_ready(service_name): |
312 | + self.fire_event('data_lost', service_name) |
313 | + self.fire_event('stop', service_name, default=[ |
314 | + close_ports, |
315 | + host.service_stop]) |
316 | + self.save_lost(service_name) |
317 | + |
318 | + def stop_services(self, *service_names): |
319 | + """ |
320 | + Stop one or more registered services, by name. |
321 | + |
322 | + If no service names are given, stops all registered services. |
323 | + """ |
324 | + for service_name in service_names or self.services.keys(): |
325 | + self.fire_event('stop', service_name, default=[ |
326 | + close_ports, |
327 | + host.service_stop]) |
328 | + |
329 | + def get_service(self, service_name): |
330 | + """ |
331 | + Given the name of a registered service, return its service definition. |
332 | + """ |
333 | + service = self.services.get(service_name) |
334 | + if not service: |
335 | + raise KeyError('Service not registered: %s' % service_name) |
336 | + return service |
337 | + |
338 | + def fire_event(self, event_name, service_name, default=None): |
339 | + """ |
340 | + Fire a data_ready, data_lost, start, or stop event on a given service. |
341 | + """ |
342 | + service = self.get_service(service_name) |
343 | + callbacks = service.get(event_name, default) |
344 | + if not callbacks: |
345 | + return |
346 | + if not isinstance(callbacks, Iterable): |
347 | + callbacks = [callbacks] |
348 | + for callback in callbacks: |
349 | + if isinstance(callback, ManagerCallback): |
350 | + callback(self, service_name, event_name) |
351 | + else: |
352 | + callback(service_name) |
353 | + |
354 | + def is_ready(self, service_name): |
355 | + """ |
356 | + Determine if a registered service is ready, by checking its 'required_data'. |
357 | + |
358 | + A 'required_data' item can be any mapping type, and is considered ready |
359 | + if `bool(item)` evaluates as True. |
360 | + """ |
361 | + service = self.get_service(service_name) |
362 | + reqs = service.get('required_data', []) |
363 | + return all(bool(req) for req in reqs) |
364 | + |
365 | + def save_ready(self, service_name): |
366 | + """ |
367 | + Save an indicator that the given service is now data_ready. |
368 | + """ |
369 | + ready_file = '{}/.ready.{}'.format(hookenv.charm_dir(), service_name) |
370 | + with open(ready_file, 'a'): |
371 | + pass |
372 | + |
373 | + def save_lost(self, service_name): |
374 | + """ |
375 | + Save an indicator that the given service is no longer data_ready. |
376 | + """ |
377 | + ready_file = '{}/.ready.{}'.format(hookenv.charm_dir(), service_name) |
378 | + if os.path.exists(ready_file): |
379 | + os.remove(ready_file) |
380 | + |
381 | + def was_ready(self, service_name): |
382 | + """ |
383 | + Determine if the given service was previously data_ready. |
384 | + """ |
385 | + ready_file = '{}/.ready.{}'.format(hookenv.charm_dir(), service_name) |
386 | + return os.path.exists(ready_file) |
387 | + |
388 | + |
389 | +class RelationContext(dict): |
390 | + """ |
391 | + Base class for a context generator that gets relation data from juju. |
392 | + |
393 | + Subclasses must provide `interface`, which is the interface type of interest, |
394 | + and `required_keys`, which is the set of keys required for the relation to |
395 | + be considered complete. The first relation for the interface that is complete |
396 | + will be used to populate the data for template. |
397 | + |
398 | + The generated context will be namespaced under the interface type, to prevent |
399 | + potential naming conflicts. |
400 | + """ |
401 | + interface = None |
402 | + required_keys = [] |
403 | + |
404 | + def __bool__(self): |
405 | + """ |
406 | + Updates the data and returns True if all of the required_keys are available. |
407 | + """ |
408 | + self.get_data() |
409 | + return self.is_ready() |
410 | + |
411 | + __nonzero__ = __bool__ |
412 | + |
413 | + def is_ready(self): |
414 | + """ |
415 | + Returns True if all of the required_keys are available. |
416 | + """ |
417 | + return set(self.get(self.interface, {}).keys()).issuperset(set(self.required_keys)) |
418 | + |
419 | + def get_data(self): |
420 | + """ |
421 | + Retrieve the relation data and store it under `self[self.interface]`. |
422 | + |
423 | + If there are more than one units related on the desired interface, |
424 | + then each unit will have its data stored under `self[self.interface][unit_id]` |
425 | + and one of the units with complete information will chosen at random |
426 | + to fill the values at `self[self.interface]`. |
427 | + |
428 | + |
429 | + For example: |
430 | + |
431 | + { |
432 | + 'foo': 'bar', |
433 | + 'unit/0': { |
434 | + 'foo': 'bar', |
435 | + }, |
436 | + 'unit/1': { |
437 | + 'foo': 'baz', |
438 | + }, |
439 | + } |
440 | + """ |
441 | + if not hookenv.relation_ids(self.interface): |
442 | + return |
443 | + |
444 | + ns = self.setdefault(self.interface, {}) |
445 | + required = set(self.required_keys) |
446 | + for rid in hookenv.relation_ids(self.interface): |
447 | + for unit in hookenv.related_units(rid): |
448 | + reldata = hookenv.relation_get(rid=rid, unit=unit) |
449 | + unit_ns = ns.setdefault(unit, {}) |
450 | + unit_ns.update(reldata) |
451 | + if set(reldata.keys()).issuperset(required): |
452 | + ns.update(reldata) |
453 | + |
454 | + |
455 | +class ManagerCallback(object): |
456 | + """ |
457 | + Special case of a callback that takes the `ServiceManager` instance |
458 | + in addition to the service name. |
459 | + |
460 | + Subclasses should implement `__call__` which should accept two parameters: |
461 | + |
462 | + * `manager` The `ServiceManager` instance |
463 | + * `service_name` The name of the service it's being triggered for |
464 | + * `event_name` The name of the event that this callback is handling |
465 | + """ |
466 | + def __call__(self, manager, service_name, event_name): |
467 | raise NotImplementedError() |
468 | |
469 | |
470 | -class UpstartService(ServiceTypeManager): |
471 | - def __init__(self, service_definition): |
472 | - super(UpstartService, self).__init__(service_definition) |
473 | - for tmpl in self.templates: |
474 | - if 'target' not in tmpl: |
475 | - tmpl['target'] = '/etc/init/%s.conf' % self.service_name |
476 | - |
477 | - def reconfigure(self, restart): |
478 | - complete = templating.render(self.templates, self.templates_dir) |
479 | - |
480 | - if restart and complete: |
481 | - host.service_restart(self.service_name) |
482 | +class TemplateCallback(ManagerCallback): |
483 | + """ |
484 | + Callback class that will render a template, for use as a ready action. |
485 | + |
486 | + The `target` param, if omitted, will default to `/etc/init/<service name>`. |
487 | + """ |
488 | + def __init__(self, source, target, owner='root', group='root', perms=0444): |
489 | + self.source = source |
490 | + self.target = target |
491 | + self.owner = owner |
492 | + self.group = group |
493 | + self.perms = perms |
494 | + |
495 | + def __call__(self, manager, service_name, event_name): |
496 | + service = manager.get_service(service_name) |
497 | + context = {} |
498 | + for ctx in service.get('required_data', []): |
499 | + context.update(ctx) |
500 | + templating.render(self.source, self.target, context, |
501 | + self.owner, self.group, self.perms) |
502 | + |
503 | + |
504 | +class PortManagerCallback(ManagerCallback): |
505 | + """ |
506 | + Callback class that will open or close ports, for use as either |
507 | + a start or stop action. |
508 | + """ |
509 | + def __call__(self, manager, service_name, event_name): |
510 | + service = manager.get_service(service_name) |
511 | + for port in service.get('ports', []): |
512 | + if event_name == 'start': |
513 | + hookenv.open_port(port) |
514 | + elif event_name == 'stop': |
515 | + hookenv.close_port(port) |
516 | + |
517 | + |
518 | +# Convenience aliases |
519 | +template = TemplateCallback |
520 | +open_ports = PortManagerCallback() |
521 | +close_ports = PortManagerCallback() |
522 | |
523 | === modified file 'hooks/charmhelpers/core/templating.py' |
524 | --- hooks/charmhelpers/core/templating.py 2014-05-20 19:43:29 +0000 |
525 | +++ hooks/charmhelpers/core/templating.py 2014-05-29 17:26:14 +0000 |
526 | @@ -1,158 +1,51 @@ |
527 | import os |
528 | -import yaml |
529 | |
530 | from charmhelpers.core import host |
531 | from charmhelpers.core import hookenv |
532 | |
533 | |
534 | -class ContextGenerator(object): |
535 | - """ |
536 | - Base interface for template context container generators. |
537 | - |
538 | - A template context is a dictionary that contains data needed to populate |
539 | - the template. The generator instance should produce the context when |
540 | - called (without arguments) by collecting information from juju (config-get, |
541 | - relation-get, etc), the system, or whatever other sources are appropriate. |
542 | - |
543 | - A context generator should only return any values if it has enough information |
544 | - to provide all of its values. Any context that is missing data is considered |
545 | - incomplete and will cause that template to not render until it has all of its |
546 | - necessary data. |
547 | - |
548 | - The template may receive several contexts, which will be merged together, |
549 | - so care should be taken in the key names. |
550 | - """ |
551 | - def __call__(self): |
552 | - raise NotImplementedError |
553 | - |
554 | - |
555 | -class StorableContext(object): |
556 | - """ |
557 | - A mixin for persisting a context to disk. |
558 | - """ |
559 | - def store_context(self, file_name, config_data): |
560 | - with open(file_name, 'w') as file_stream: |
561 | - yaml.dump(config_data, file_stream) |
562 | - |
563 | - def read_context(self, file_name): |
564 | - with open(file_name, 'r') as file_stream: |
565 | - data = yaml.load(file_stream) |
566 | - if not data: |
567 | - raise OSError("%s is empty" % file_name) |
568 | - return data |
569 | - |
570 | - |
571 | -class ConfigContext(ContextGenerator): |
572 | - """ |
573 | - A context generator that generates a context containing all of the |
574 | - juju config values. |
575 | - """ |
576 | - def __call__(self): |
577 | - return hookenv.config() |
578 | - |
579 | - |
580 | -class RelationContext(ContextGenerator): |
581 | - """ |
582 | - Base class for a context generator that gets relation data from juju. |
583 | - |
584 | - Subclasses must provide `interface`, which is the interface type of interest, |
585 | - and `required_keys`, which is the set of keys required for the relation to |
586 | - be considered complete. The first relation for the interface that is complete |
587 | - will be used to populate the data for template. |
588 | - |
589 | - The generated context will be namespaced under the interface type, to prevent |
590 | - potential naming conflicts. |
591 | - """ |
592 | - interface = None |
593 | - required_keys = [] |
594 | - |
595 | - def __call__(self): |
596 | - if not hookenv.relation_ids(self.interface): |
597 | - return {} |
598 | - |
599 | - ctx = {} |
600 | - for rid in hookenv.relation_ids(self.interface): |
601 | - for unit in hookenv.related_units(rid): |
602 | - reldata = hookenv.relation_get(rid=rid, unit=unit) |
603 | - required = set(self.required_keys) |
604 | - if set(reldata.keys()).issuperset(required): |
605 | - ns = ctx.setdefault(self.interface, {}) |
606 | - for k, v in reldata.items(): |
607 | - ns[k] = v |
608 | - return ctx |
609 | - |
610 | - return {} |
611 | - |
612 | - |
613 | -class StaticContext(ContextGenerator): |
614 | - def __init__(self, data): |
615 | - self.data = data |
616 | - |
617 | - def __call__(self): |
618 | - return self.data |
619 | - |
620 | - |
621 | -def _collect_contexts(context_providers): |
622 | - """ |
623 | - Helper function to collect and merge contexts from a list of providers. |
624 | - |
625 | - If any of the contexts are incomplete (i.e., they return an empty dict), |
626 | - the template is considered incomplete and will not render. |
627 | - """ |
628 | - ctx = {} |
629 | - for provider in context_providers: |
630 | - c = provider() |
631 | - if not c: |
632 | - return False |
633 | - ctx.update(c) |
634 | - return ctx |
635 | - |
636 | - |
637 | -def render(template_definitions, templates_dir=None): |
638 | - """ |
639 | - Render one or more templates, given a list of template definitions. |
640 | - |
641 | - The template definitions should be dicts with the keys: `source`, `target`, |
642 | - `file_properties`, and `contexts`. |
643 | - |
644 | - The `source` path, if not absolute, is relative to the `templates_dir` |
645 | - given when the rendered was created. If `source` is not provided |
646 | - for a template the `template_dir` will be consulted for |
647 | - ``basename(target).j2``. |
648 | +def render(source, target, context, owner='root', group='root', perms=0444, templates_dir=None): |
649 | + """ |
650 | + Render a template. |
651 | + |
652 | + The `source` path, if not absolute, is relative to the `templates_dir`. |
653 | |
654 | The `target` path should be absolute. |
655 | |
656 | - The `file_properties` should be a dict optionally containing |
657 | - `owner`, `group`, or `perms` options, to be passed to `write_file`. |
658 | - |
659 | - The `contexts` should be a list containing zero or more ContextGenerators. |
660 | - |
661 | - The `template_dir` defaults to `$CHARM_DIR/templates` |
662 | - |
663 | - Returns True if all of the templates were "complete" (i.e., the context |
664 | - generators were able to collect the information needed to render the |
665 | - template) and were rendered. |
666 | + The context should be a dict containing the values to be replaced in the |
667 | + template. |
668 | + |
669 | + The `owner`, `group`, and `perms` options will be passed to `write_file`. |
670 | + |
671 | + If omitted, `templates_dir` defaults to the `templates` folder in the charm. |
672 | + |
673 | + Note: Using this requires python-jinja2; if it is not installed, calling |
674 | + this will attempt to use charmhelpers.fetch.apt_install to install it. |
675 | """ |
676 | - # lazy import jinja2 in case templating is needed in install hook |
677 | - from jinja2 import FileSystemLoader, Environment, exceptions |
678 | - all_complete = True |
679 | + try: |
680 | + from jinja2 import FileSystemLoader, Environment, exceptions |
681 | + except ImportError: |
682 | + try: |
683 | + from charmhelpers.fetch import apt_install |
684 | + except ImportError: |
685 | + hookenv.log('Could not import jinja2, and could not import ' |
686 | + 'charmhelpers.fetch to install it', |
687 | + level=hookenv.ERROR) |
688 | + raise |
689 | + apt_install('python-jinja2', fatal=True) |
690 | + from jinja2 import FileSystemLoader, Environment, exceptions |
691 | + |
692 | if templates_dir is None: |
693 | templates_dir = os.path.join(hookenv.charm_dir(), 'templates') |
694 | loader = Environment(loader=FileSystemLoader(templates_dir)) |
695 | - for tmpl in template_definitions: |
696 | - ctx = _collect_contexts(tmpl.get('contexts', [])) |
697 | - if ctx is False: |
698 | - all_complete = False |
699 | - continue |
700 | - try: |
701 | - source = tmpl.get('source', os.path.basename(tmpl['target'])+'.j2') |
702 | - template = loader.get_template(source) |
703 | - except exceptions.TemplateNotFound as e: |
704 | - hookenv.log('Could not load template %s from %s.' % |
705 | - (tmpl['source'], templates_dir), |
706 | - level=hookenv.ERROR) |
707 | - raise e |
708 | - content = template.render(ctx) |
709 | - host.mkdir(os.path.dirname(tmpl['target'])) |
710 | - host.write_file(tmpl['target'], content, **tmpl.get('file_properties', {})) |
711 | - return all_complete |
712 | + try: |
713 | + source = source |
714 | + template = loader.get_template(source) |
715 | + except exceptions.TemplateNotFound as e: |
716 | + hookenv.log('Could not load template %s from %s.' % |
717 | + (source, templates_dir), |
718 | + level=hookenv.ERROR) |
719 | + raise e |
720 | + content = template.render(context) |
721 | + host.mkdir(os.path.dirname(target)) |
722 | + host.write_file(target, content, owner, group, perms) |
723 | |
724 | === modified symlink 'hooks/config-changed' (properties changed: -x to +x) |
725 | === target was u'hooks.py' |
726 | --- hooks/config-changed 1970-01-01 00:00:00 +0000 |
727 | +++ hooks/config-changed 2014-05-29 17:26:14 +0000 |
728 | @@ -0,0 +1,9 @@ |
729 | +#!/usr/bin/env python |
730 | +# vim: et ai ts=4 sw=4: |
731 | +from charmhelpers.core import services |
732 | + |
733 | +import config |
734 | + |
735 | + |
736 | +manager = services.ServiceManager(config.SERVICES) |
737 | +manager.manage() |
738 | |
739 | === modified file 'hooks/config.py' |
740 | --- hooks/config.py 2014-05-20 19:50:35 +0000 |
741 | +++ hooks/config.py 2014-05-29 17:26:14 +0000 |
742 | @@ -1,4 +1,10 @@ |
743 | import os |
744 | +import subprocess |
745 | + |
746 | +from charmhelpers.core import host |
747 | +from charmhelpers.core import hookenv |
748 | +from charmhelpers.core import services |
749 | +from charmhelpers.contrib.cloudfoundry import contexts |
750 | |
751 | |
752 | __all__ = ['CF_DIR', 'CC_PACKAGES', 'CC_DIR', 'CC_CONFIG_DIR', |
753 | @@ -23,3 +29,36 @@ |
754 | NGINX_RUN_DIR = '/var/vcap/sys/run/nginx_ccng' |
755 | NGINX_LOG_DIR = '/var/vcap/sys/log/nginx_ccng' |
756 | FOG_CONNECTION = '/var/vcap/nfs/store' |
757 | + |
758 | + |
759 | +def db_migrate(service_name): |
760 | + hookenv.log("Starting db:migrate...", hookenv.DEBUG) |
761 | + with host.chdir(CC_DIR): |
762 | + subprocess.check_call([ |
763 | + 'sudo', '-u', 'vcap', '-g', 'vcap', |
764 | + 'CLOUD_CONTROLLER_NG_CONFIG={}'.format(CC_CONFIG_FILE), |
765 | + 'bundle', 'exec', 'rake', 'db:migrate']) |
766 | + hookenv.log("Finished db:migrate", hookenv.DEBUG) |
767 | + |
768 | + |
769 | +SERVICES = [ |
770 | + { |
771 | + 'service': 'cf-cloudcontroller', |
772 | + 'required_data': [contexts.NatsRelation(), |
773 | + contexts.RouterRelation(), |
774 | + contexts.MysqlRelation()], |
775 | + 'data_ready': [ |
776 | + services.template(source='cf-cloudcontroller.conf', |
777 | + target='/etc/init/cf-cloudcontroller.conf'), |
778 | + services.template(source='cloud_controller.yml', |
779 | + target=CC_CONFIG_FILE, |
780 | + owner='vcap'), |
781 | + db_migrate, |
782 | + ], |
783 | + }, |
784 | + { |
785 | + 'service': 'cf-cloudcontroller-job', |
786 | + 'data_ready': services.template(source='cf-cloudcontroller-job.conf', |
787 | + target='/etc/init/cf-cloudcontroller-job.conf'), |
788 | + }, |
789 | +] |
790 | |
791 | === modified symlink 'hooks/db-relation-changed' (properties changed: -x to +x) |
792 | === target was u'hooks.py' |
793 | --- hooks/db-relation-changed 1970-01-01 00:00:00 +0000 |
794 | +++ hooks/db-relation-changed 2014-05-29 17:26:14 +0000 |
795 | @@ -0,0 +1,9 @@ |
796 | +#!/usr/bin/env python |
797 | +# vim: et ai ts=4 sw=4: |
798 | +from charmhelpers.core import services |
799 | + |
800 | +import config |
801 | + |
802 | + |
803 | +manager = services.ServiceManager(config.SERVICES) |
804 | +manager.manage() |
805 | |
806 | === removed file 'hooks/hooks.py' |
807 | --- hooks/hooks.py 2014-05-20 19:50:35 +0000 |
808 | +++ hooks/hooks.py 1970-01-01 00:00:00 +0000 |
809 | @@ -1,81 +0,0 @@ |
810 | -#!/usr/bin/env python |
811 | -# vim: et ai ts=4 sw=4: |
812 | -import os |
813 | -import sys |
814 | -import subprocess |
815 | - |
816 | -from charmhelpers.core import host |
817 | -from charmhelpers.core import hookenv |
818 | -from charmhelpers.core.hookenv import log |
819 | -from charmhelpers.core import services |
820 | -from charmhelpers.contrib.cloudfoundry import contexts |
821 | - |
822 | -import config |
823 | - |
824 | -hooks = hookenv.Hooks() |
825 | -fileproperties = {'owner': 'vcap'} |
826 | - |
827 | -services.register([ |
828 | - { |
829 | - 'service': 'cf-cloudcontroller', |
830 | - 'templates': [ |
831 | - {'source': 'cf-cloudcontroller.conf'}, |
832 | - {'source': 'cloud_controller.yml', |
833 | - 'target': config.CC_CONFIG_FILE, |
834 | - 'file_properties': fileproperties, |
835 | - 'contexts': [contexts.NatsContext(), |
836 | - contexts.RouterContext(), |
837 | - contexts.MysqlDSNContext()]} |
838 | - ], |
839 | - }, |
840 | - { |
841 | - 'service': 'cf-cloudcontroller-job', |
842 | - 'templates': [{'source': 'cf-cloudcontroller-job.conf'}], |
843 | - }, |
844 | -]) |
845 | - |
846 | - |
847 | -@hooks.hook('upgrade-charm') |
848 | -def upgrade_charm(): |
849 | - pass |
850 | - |
851 | - |
852 | -@hooks.hook("config-changed") |
853 | -def config_changed(): |
854 | - services.reconfigure_services() |
855 | - |
856 | - |
857 | -@hooks.hook() |
858 | -def stop(): |
859 | - services.stop_services() |
860 | - |
861 | - |
862 | -@hooks.hook('db-relation-changed') |
863 | -def db_relation_changed(): |
864 | - services.reconfigure_services() |
865 | - hookenv.log("Starting db:migrate...", hookenv.DEBUG) |
866 | - with host.chdir(config.CC_DIR) as dir: |
867 | - subprocess.check_call([ |
868 | - 'sudo', '-u', 'vcap', '-g', 'vcap', |
869 | - 'CLOUD_CONTROLLER_NG_CONFIG={}'.format(config.CC_CONFIG_FILE), |
870 | - 'bundle', 'exec', 'rake', 'db:migrate']) |
871 | - hookenv.log("Finished db:migrate in %s." % (dir)) |
872 | - |
873 | - |
874 | -@hooks.hook('nats-relation-changed') |
875 | -def nats_relation_changed(): |
876 | - services.reconfigure_services() |
877 | - |
878 | - |
879 | -@hooks.hook('router-relation-changed') |
880 | -def router_relation_changed(): |
881 | - services.reconfigure_services() |
882 | - |
883 | - |
884 | -if __name__ == '__main__': |
885 | - hook_name = os.path.basename(sys.argv[0]) |
886 | - log("Running {} hook".format(hook_name)) |
887 | - if hookenv.relation_id(): |
888 | - log("Relation {} with {}".format( |
889 | - hookenv.relation_id(), hookenv.remote_unit())) |
890 | - hooks.execute(sys.argv) |
891 | |
892 | === modified symlink 'hooks/nats-relation-changed' (properties changed: -x to +x) |
893 | === target was u'hooks.py' |
894 | --- hooks/nats-relation-changed 1970-01-01 00:00:00 +0000 |
895 | +++ hooks/nats-relation-changed 2014-05-29 17:26:14 +0000 |
896 | @@ -0,0 +1,9 @@ |
897 | +#!/usr/bin/env python |
898 | +# vim: et ai ts=4 sw=4: |
899 | +from charmhelpers.core import services |
900 | + |
901 | +import config |
902 | + |
903 | + |
904 | +manager = services.ServiceManager(config.SERVICES) |
905 | +manager.manage() |
906 | |
907 | === modified symlink 'hooks/router-relation-changed' (properties changed: -x to +x) |
908 | === target was u'hooks.py' |
909 | --- hooks/router-relation-changed 1970-01-01 00:00:00 +0000 |
910 | +++ hooks/router-relation-changed 2014-05-29 17:26:14 +0000 |
911 | @@ -0,0 +1,9 @@ |
912 | +#!/usr/bin/env python |
913 | +# vim: et ai ts=4 sw=4: |
914 | +from charmhelpers.core import services |
915 | + |
916 | +import config |
917 | + |
918 | + |
919 | +manager = services.ServiceManager(config.SERVICES) |
920 | +manager.manage() |
921 | |
922 | === modified symlink 'hooks/stop' (properties changed: -x to +x) |
923 | === target was u'hooks.py' |
924 | --- hooks/stop 1970-01-01 00:00:00 +0000 |
925 | +++ hooks/stop 2014-05-29 17:26:14 +0000 |
926 | @@ -0,0 +1,9 @@ |
927 | +#!/usr/bin/env python |
928 | +# vim: et ai ts=4 sw=4: |
929 | +from charmhelpers.core import services |
930 | + |
931 | +import config |
932 | + |
933 | + |
934 | +manager = services.ServiceManager(config.SERVICES) |
935 | +manager.manage() |
936 | |
937 | === modified symlink 'hooks/upgrade-charm' (properties changed: -x to +x) |
938 | === target was u'hooks.py' |
939 | --- hooks/upgrade-charm 1970-01-01 00:00:00 +0000 |
940 | +++ hooks/upgrade-charm 2014-05-29 17:26:14 +0000 |
941 | @@ -0,0 +1,9 @@ |
942 | +#!/usr/bin/env python |
943 | +# vim: et ai ts=4 sw=4: |
944 | +from charmhelpers.core import services |
945 | + |
946 | +import config |
947 | + |
948 | + |
949 | +manager = services.ServiceManager(config.SERVICES) |
950 | +manager.manage() |
Reviewers: mp+220741_ code.launchpad. net,
Message:
Please take a look.
Description:
Fixed relation ordering issue
Fixed an relation hook ordering dependency by refactoring to use core.services
callbacks in charmhelpers.
https:/ /code.launchpad .net/~johnsca/ charms/ trusty/ cf-cloud- controller/ services- callback- fu/+merge/ 220741
(do not edit description out of merge proposal)
Please review this at https:/ /codereview. appspot. com/100680044/
Affected files (+134, -112 lines): ers/core/ host.py ers/core/ services. py ers/core/ templating. py
A [revision details]
M hooks/charmhelp
M hooks/charmhelp
M hooks/charmhelp
M hooks/hooks.py