Merge lp:~paulgear/charms/trusty/memcached/detect-nagios-context-changed into lp:charms/trusty/memcached

Proposed by Paul Gear
Status: Work in progress
Proposed branch: lp:~paulgear/charms/trusty/memcached/detect-nagios-context-changed
Merge into: lp:charms/trusty/memcached
Diff against target: 31 lines (+14/-0)
1 file modified
hooks/memcached_hooks.py (+14/-0)
To merge this branch: bzr merge lp:~paulgear/charms/trusty/memcached/detect-nagios-context-changed
Reviewer Review Type Date Requested Status
Adam Israel (community) Needs Fixing
Review Queue (community) automated testing Needs Fixing
charmers Pending
Review via email: mp+250715@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Paul Gear (paulgear) wrote :

This MP seems to cause the test suite some difficulty. This appears to be because it uses features of config() from charmhelpers.core.hookenv that are not accurately emulated by the mocked up version in the test suite.

Revision history for this message
Review Queue (review-queue) wrote :

This items has failed automated testing! Results available here http://reports.vapour.ws/charm-tests/charm-bundle-test-11065-results

review: Needs Fixing (automated testing)
Revision history for this message
Adam Israel (aisrael) wrote :

Hi Paul,

Thanks again for your work on improving the memcached charm. I'm going to have to NACK this merge proposal. The failing tests are due to un-mocked methods. Additionally, there's an undefined variable, local_unit_name, that will fail once the mocking is in place.

Revision history for this message
Adam Israel (aisrael) :
review: Needs Fixing

Unmerged revisions

67. By Paul Gear

Detect changed nagios_context setting; remove obsolete Nagios check if this occurs

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'hooks/memcached_hooks.py'
--- hooks/memcached_hooks.py 2015-02-20 16:15:18 +0000
+++ hooks/memcached_hooks.py 2015-02-24 07:03:16 +0000
@@ -246,6 +246,9 @@
246 # In case port changed, inform consumers246 # In case port changed, inform consumers
247 cache_relation_joined()247 cache_relation_joined()
248248
249 # nagios_context might have changed
250 check_nagios_config_change()
251
249252
250@hooks.hook('munin-relation-changed')253@hooks.hook('munin-relation-changed')
251@restart_on_change(RESTART_MAP)254@restart_on_change(RESTART_MAP)
@@ -288,6 +291,17 @@
288 os.chmod(os.path.join(LOCAL_NAGIOS_PLUGINS, "check_memcache.py"),291 os.chmod(os.path.join(LOCAL_NAGIOS_PLUGINS, "check_memcache.py"),
289 0o555)292 0o555)
290293
294 check_nagios_config_change()
295
296
297def check_nagios_config_change():
298 cfg = config()
299 if cfg.changed('nagios_context'):
300 old_hostname = "{}-{}".format(cfg.previous('nagios_context'),
301 local_unit_name)
302 os.remove(('/var/lib/nagios/export/service__{}_check_'
303 'memcached.cfg').format(old_hostname))
304
291 nagios_hostname = "{}-{}".format(config('nagios_context'),305 nagios_hostname = "{}-{}".format(config('nagios_context'),
292 local_unit().replace('/', '-'))306 local_unit().replace('/', '-'))
293 configs = {'tcp_port': config('tcp_port'),307 configs = {'tcp_port': config('tcp_port'),

Subscribers

People subscribed via source and target branches

to all changes: