From 32a67999b682dd518daa128ce985e787d4885b0f Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Wed, 16 Apr 2014 18:04:19 +0100 Subject: [PATCH 1/2] Fix race conditions where a process gets in state "Running" between the restart/start command and the summary command. Refactor to avoid repeating the status, and fail if a given call to monit fails. --- monitoring/monit | 77 +++++++++++++++++++++++++----------------------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/monitoring/monit b/monitoring/monit index 0705b714315..abdfa404d64 100644 --- a/monitoring/monit +++ b/monitoring/monit @@ -47,8 +47,6 @@ EXAMPLES = ''' - monit: name=httpd state=started ''' -import pipes - def main(): arg_spec = dict( name=dict(required=True), @@ -68,8 +66,25 @@ def main(): rc, out, err = module.run_command('%s reload' % MONIT) module.exit_json(changed=True, name=name, state=state) - rc, out, err = module.run_command('%s summary | grep "Process \'%s\'"' % (MONIT, pipes.quote(name)), use_unsafe_shell=True) - present = name in out + def status(): + """Return the status of the process in monit, or None if not present.""" + rc, out, err = module.run_command('%s summary' % MONIT, check_rc=True) + for line in out.split('\n'): + # Sample output lines: + # Process 'name' Running + # Process 'name' Running - restart pending + parts = line.lower().split() + if len(parts) > 2 and parts[0] == 'process' and parts[1] == "'%s'" % name: + return ' '.join(parts[2:]) + else: + return None + + def run_command(command): + """Runs a monit command, and returns the new status.""" + module.run_command('%s %s %s' % (MONIT, command, name), check_rc=True) + return status() + + present = status() is not None if not present and not state == 'present': module.fail_json(msg='%s process not presently configured with monit' % name, name=name, state=state) @@ -78,69 +93,57 @@ def main(): if not present: if module.check_mode: module.exit_json(changed=True) - module.run_command('%s reload' % MONIT, check_rc=True) - rc, out, err = module.run_command('%s summary | grep %s' % (MONIT, pipes.quote(name)), use_unsafe_shell=True) - if name in out: - module.exit_json(changed=True, name=name, state=state) + status = run_command('reload') + if status is None: + module.fail_json(msg=status, name=name, state=state) else: - module.fail_json(msg=out, name=name, state=state) - + module.exit_json(changed=True, name=name, state=state) module.exit_json(changed=False, name=name, state=state) - rc, out, err = module.run_command('%s summary | grep %s' % (MONIT, pipes.quote(name)), use_unsafe_shell=True) - running = 'running' in out.lower() - - if running and (state == 'started' or state == 'monitored'): - module.exit_json(changed=False, name=name, state=state) + running = 'running' in status() - if running and state == 'monitored': + if running and state in ['started', 'monitored']: module.exit_json(changed=False, name=name, state=state) if running and state == 'stopped': if module.check_mode: module.exit_json(changed=True) - module.run_command('%s stop %s' % (MONIT, name)) - rc, out, err = module.run_command('%s summary | grep %s' % (MONIT, pipes.quote(name)), use_unsafe_shell=True) - if 'not monitored' in out.lower() or 'stop pending' in out.lower(): + status = run_command('stop') + if status in ['not monitored'] or 'stop pending' in status: module.exit_json(changed=True, name=name, state=state) - module.fail_json(msg=out) + module.fail_json(msg=status) if running and state == 'unmonitored': if module.check_mode: module.exit_json(changed=True) - module.run_command('%s unmonitor %s' % (MONIT, name)) - # FIXME: DRY FOLKS! - rc, out, err = module.run_command('%s summary | grep %s' % (MONIT, pipes.quote(name)), use_unsafe_shell=True) - if 'not monitored' in out.lower(): + status = run_command('unmonitor') + if status in ['not monitored']: module.exit_json(changed=True, name=name, state=state) - module.fail_json(msg=out) + module.fail_json(msg=status) elif state == 'restarted': if module.check_mode: module.exit_json(changed=True) - module.run_command('%s restart %s' % (MONIT, name)) - rc, out, err = module.run_command('%s summary | grep %s' % (MONIT, name)) - if 'initializing' in out.lower() or 'restart pending' in out.lower(): + status = run_command('restart') + if status in ['initializing', 'running'] or 'restart pending' in status: module.exit_json(changed=True, name=name, state=state) - module.fail_json(msg=out) + module.fail_json(msg=status) elif not running and state == 'started': if module.check_mode: module.exit_json(changed=True) - module.run_command('%s start %s' % (MONIT, name)) - rc, out, err = module.run_command('%s summary | grep %s' % (MONIT, name)) - if 'initializing' in out.lower() or 'start pending' in out.lower(): + status = run_command('start') + if status in ['initializing', 'running'] or 'start pending' in status: module.exit_json(changed=True, name=name, state=state) - module.fail_json(msg=out) + module.fail_json(msg=status) elif not running and state == 'monitored': if module.check_mode: module.exit_json(changed=True) - module.run_command('%s monitor %s' % (MONIT, name)) - rc, out, err = module.run_command('%s summary | grep %s' % (MONIT, name)) - if 'initializing' in out.lower() or 'start pending' in out.lower(): + status = run_command('monitor') + if status() not in ['not monitored']: module.exit_json(changed=True, name=name, state=state) - module.fail_json(msg=out) + module.fail_json(msg=status) module.exit_json(changed=False, name=name, state=state) From 296bf6bd0446211db8fd449b240f990e09ef5654 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Fri, 25 Apr 2014 13:30:19 +0100 Subject: [PATCH 2/2] Use empty string rather than None to avoid TypeError Improve error messages. --- monitoring/monit | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/monitoring/monit b/monitoring/monit index abdfa404d64..d27e0bdaa3f 100644 --- a/monitoring/monit +++ b/monitoring/monit @@ -67,7 +67,7 @@ def main(): module.exit_json(changed=True, name=name, state=state) def status(): - """Return the status of the process in monit, or None if not present.""" + """Return the status of the process in monit, or the empty string if not present.""" rc, out, err = module.run_command('%s summary' % MONIT, check_rc=True) for line in out.split('\n'): # Sample output lines: @@ -77,14 +77,14 @@ def main(): if len(parts) > 2 and parts[0] == 'process' and parts[1] == "'%s'" % name: return ' '.join(parts[2:]) else: - return None + return '' def run_command(command): """Runs a monit command, and returns the new status.""" module.run_command('%s %s %s' % (MONIT, command, name), check_rc=True) return status() - present = status() is not None + present = status() != '' if not present and not state == 'present': module.fail_json(msg='%s process not presently configured with monit' % name, name=name, state=state) @@ -94,8 +94,8 @@ def main(): if module.check_mode: module.exit_json(changed=True) status = run_command('reload') - if status is None: - module.fail_json(msg=status, name=name, state=state) + if status == '': + module.fail_json(msg='%s process not configured with monit' % name, name=name, state=state) else: module.exit_json(changed=True, name=name, state=state) module.exit_json(changed=False, name=name, state=state) @@ -111,7 +111,7 @@ def main(): status = run_command('stop') if status in ['not monitored'] or 'stop pending' in status: module.exit_json(changed=True, name=name, state=state) - module.fail_json(msg=status) + module.fail_json(msg='%s process not stopped' % name, status=status) if running and state == 'unmonitored': if module.check_mode: @@ -119,7 +119,7 @@ def main(): status = run_command('unmonitor') if status in ['not monitored']: module.exit_json(changed=True, name=name, state=state) - module.fail_json(msg=status) + module.fail_json(msg='%s process not unmonitored' % name, status=status) elif state == 'restarted': if module.check_mode: @@ -127,7 +127,7 @@ def main(): status = run_command('restart') if status in ['initializing', 'running'] or 'restart pending' in status: module.exit_json(changed=True, name=name, state=state) - module.fail_json(msg=status) + module.fail_json(msg='%s process not restarted' % name, status=status) elif not running and state == 'started': if module.check_mode: @@ -135,7 +135,7 @@ def main(): status = run_command('start') if status in ['initializing', 'running'] or 'start pending' in status: module.exit_json(changed=True, name=name, state=state) - module.fail_json(msg=status) + module.fail_json(msg='%s process not started' % name, status=status) elif not running and state == 'monitored': if module.check_mode: @@ -143,7 +143,7 @@ def main(): status = run_command('monitor') if status() not in ['not monitored']: module.exit_json(changed=True, name=name, state=state) - module.fail_json(msg=status) + module.fail_json(msg='%s process not monitored' % name, status=status) module.exit_json(changed=False, name=name, state=state)