Review: Needs Fixing comments inline
Diff comments: > diff --git a/hooks/hooks.py b/hooks/hooks.py > index 5028988..3623e46 100755 > --- a/hooks/hooks.py > +++ b/hooks/hooks.py > @@ -1,3 +1,86 @@ > #!/usr/bin/python > -import services > -services.manage() > + > +import sys > + > +from charmhelpers.core.services.base import ServiceManager > +from charmhelpers.core.services import helpers > +from charmhelpers.core import hookenv, host > + > +import actions > +import thruk_helpers > + > +config = hookenv.config() > +hooks = hookenv.Hooks() > + > + > [email protected]('nrpe-external-master-relation-{joined,changed,broken,departed}') > [email protected]('thruk-agent-relation-{joined,changed,broken,departed}') > [email protected]('config-changed') > [email protected]('start') > [email protected]('stop') > +def config_changed(): > + manager = ServiceManager([ > + { > + 'service': 'thruk', > + 'provided_data': [ > + thruk_helpers.ThrukAgentRelation() > + ], > + 'required_data': [ > + thruk_helpers.ThrukInfo(), > + thruk_helpers.CheckLivestatusSocket(), > + ], > + 'data_ready': [ > + helpers.render_template( > + source='thruk_local.conf', > + target='/etc/thruk/thruk_local.conf'), > + actions.log_start, > + actions.update_ppa, > + actions.fix_livestatus_perms, > + actions.thruk_set_password, > + actions.notify_thrukmaster_relation, > + actions.update_status, > + ], > + }, > + { > + 'service': 'thruk-monitoring', > + 'required_data': [ > + thruk_helpers.NEMRelation(), > + helpers.RequiredConfig(), > + ], > + 'data_ready': [ > + helpers.render_template( > + source='thruk-nrpe.j2', > + target='/etc/nagios/nrpe.d/check_{}.cfg'.format( > + hookenv.local_unit().replace('/', '-'), > + ) > + ), > + helpers.render_template( > + source='thruk-nagios.j2', > + > target='/var/lib/nagios/export/service__{}-{}.cfg'.format( > + config['nagios_context'], > + hookenv.local_unit().replace('/', '-'), > + ) > + ), > + ], > + }, > + ]) > + manager.manage() > + > + > [email protected]('update-status') > +def update_status(): > + if host.service_running('thruk'): I propose adding a small comment to explain that thruk will never start if livestatus is off, otherwise one may consider wrong not checking livestatus if thruk is running > + hookenv.status_set('active', 'ready') > + else: > + hookenv.status_set('blocked', '"thruk" service is not running.') I propose to rearrange this a little: if thruk is down and livestatus is disabled, this would continuously alternate between blocked - thruk not running, and blocked - nagios livestatus socket missing. Proposal: @hooks.hook('update-status') def update_status(): if host.service_running('thruk'): hookenv.status_set('active', 'ready') elif thruk_helpers.CheckLivestatusSocket(): hookenv.status_set('blocked', '"thruk" service is not running.') # Reconfigure service if nagios has enabled livestatus config_changed() > + > + # Reconfigure service if nagios has enabled livestatus > + if thruk_helpers.CheckLivestatusSocket(): > + config_changed() > + > + > +if __name__ == '__main__': > + try: > + hooks.execute(sys.argv) > + except hookenv.UnregisteredHookError as e: > + hookenv.log('Unknown hook {} - skipping.'.format(e)) -- https://code.launchpad.net/~peppepetra86/thruk-agent-charm/+git/thruk-agent-charm/+merge/378127 Your team Nagios Charm developers is subscribed to branch thruk-agent-charm:master. -- Mailing list: https://launchpad.net/~nagios-charmers Post to : [email protected] Unsubscribe : https://launchpad.net/~nagios-charmers More help : https://help.launchpad.net/ListHelp
