Merge lp:~macgreagoir/charms/precise/nrpe-external-master/upstart-critical into lp:charms/nrpe-external-master

[Mick Gregg]

When a service is stopped an exception is thrown when trying to GetInstance() for its job. This needs to be caught to allow the calling block to return CRITICAL.

Related, the length of states[] in the exception block needs to be checked for 0, rather than assuming to use an empty array, where length of zero will equal a running count of zero.

[Adam Israel (aisrael)]

Hi Mick,

I had the opportunity to review this merge proposal today. All of my checks are passing, and I've confirmed that the fix works as intended.

Thanks for your work on this! +1

review: Approve

[Charles Butler (lazypower)]

+1 on this, LGTM

Thanks for the contribution. I've pushed this to the charm store, and it should be available immediately.

review: Approve

=== modified file 'files/check_upstart_job'
--- files/check_upstart_job	2014-11-18 01:23:42 +0000
+++ files/check_upstart_job	2015-03-30 16:08:04 +0000
@@ -18,16 +18,21 @@
         self._bus = dbus.SystemBus()
         self._upstart = self._bus.get_object('com.ubuntu.Upstart',
                                              '/com/ubuntu/Upstart')
+
     def get_job(self, job_name):
         path = self._upstart.GetJobByName(job_name,
                                           dbus_interface='com.ubuntu.Upstart0_6')
         return self._bus.get_object('com.ubuntu.Upstart', path)
 
     def get_properties(self, job):
-        path = job.GetInstance([], dbus_interface='com.ubuntu.Upstart0_6.Job')
-        instance = self._bus.get_object('com.ubuntu.Upstart', path)
-        return instance.GetAll('com.ubuntu.Upstart0_6.Instance',
-                               dbus_interface=dbus.PROPERTIES_IFACE)
+        try:
+            path = job.GetInstance([],
+                                   dbus_interface='com.ubuntu.Upstart0_6.Job')
+            instance = self._bus.get_object('com.ubuntu.Upstart', path)
+            return instance.GetAll('com.ubuntu.Upstart0_6.Instance',
+                                   dbus_interface=dbus.PROPERTIES_IFACE)
+        except:
+            return None
 
     def get_job_instances(self, job_name):
         job = self.get_job(job_name)
@@ -41,10 +46,10 @@
 try:
     upstart = Upstart()
     try:
-        job = upstart.get_job(sys.argv[1])        
+        job = upstart.get_job(sys.argv[1])
         props = upstart.get_properties(job)
 
-        if props['state'] == 'running':
+        if props and props['state'] == 'running':
             print 'OK: %s is running' % sys.argv[1]
             sys.exit(0)
         else:
@@ -55,6 +60,9 @@
         instances = upstart.get_job_instances(sys.argv[1])
         propses = [upstart.get_job_instance_properties(instance) for instance in instances]
         states = dict([(props['name'], props['state']) for props in propses])
+        if len(states) == 0:
+            print 'CRITICAL: 0 instances of %s running' % sys.argv[1]
+            sys.exit(2)
         if len(states) != states.values().count('running'):
             not_running = []
             for name in states.keys():
@@ -69,4 +77,3 @@
 except dbus.DBusException as e:
     print 'CRITICAL: failed to get properties of \'%s\' from upstart' % sys.argv[1]
     sys.exit(2)
-