better Executor class for client tools

1 files changed, 53 insertions, 57 deletions

diff --git a/src/lib/Bcfg2/Client/Tools/launchd.py b/src/lib/Bcfg2/Client/Tools/launchd.py
index 0a587da2e..b0661b26b 100644
--- a/src/lib/Bcfg2/Client/Tools/launchd.py
+++ b/src/lib/Bcfg2/Client/Tools/launchd.py
@@ -1,61 +1,58 @@
 """launchd support for Bcfg2."""
 
 import os
-
 import Bcfg2.Client.Tools
 
 
-class launchd(Bcfg2.Client.Tools.Tool):
-    """Support for Mac OS X launchd services."""
+class launchd(Bcfg2.Client.Tools.Tool):  # pylint: disable=C0103
+    """Support for Mac OS X launchd services.  Currently requires the
+    path to the plist to load/unload, and Name is acually a
+    reverse-fqdn (or the label)."""
     __handles__ = [('Service', 'launchd')]
     __execs__ = ['/bin/launchctl', '/usr/bin/defaults']
-    name = 'launchd'
     __req__ = {'Service': ['name', 'status']}
 
-    '''
-    Currently requires the path to the plist to load/unload,
-    and Name is acually a reverse-fqdn (or the label).
-    '''
-
     def __init__(self, logger, setup, config):
         Bcfg2.Client.Tools.Tool.__init__(self, logger, setup, config)
 
-        '''Locate plist file that provides given reverse-fqdn name
-        /Library/LaunchAgents          Per-user agents provided by the administrator.
-        /Library/LaunchDaemons         System wide daemons provided by the administrator.
-        /System/Library/LaunchAgents   Mac OS X Per-user agents.
-        /System/Library/LaunchDaemons  Mac OS X System wide daemons.'''
-        plistLocations = ["/Library/LaunchDaemons",
-                          "/System/Library/LaunchDaemons"]
-        self.plistMapping = {}
-        for directory in plistLocations:
+        # Locate plist file that provides given reverse-fqdn name:
+        #
+        # * ``/Library/LaunchAgents``: Per-user agents provided by the
+        #   administrator.
+        # * ``/Library/LaunchDaemons``: System-wide daemons provided
+        #   by the administrator.
+        # * ``/System/Library/LaunchAgents``: Mac OS X per-user
+        #   agents.
+        # * ``/System/Library/LaunchDaemons``: Mac OS X system-wide
+        #   daemons.
+        plist_locations = ["/Library/LaunchDaemons",
+                           "/System/Library/LaunchDaemons"]
+        self.plist_mapping = {}
+        for directory in plist_locations:
             for daemon in os.listdir(directory):
-                try:
-                    if daemon.endswith(".plist"):
-                        d = daemon[:-6]
-                    else:
-                        d = daemon
-                    label = self.cmd.run('defaults read %s/%s Label' %
-                                         (directory, d))[1][0]
-                    self.plistMapping[label] = "%s/%s" % (directory, daemon)
-                except KeyError:
-                    self.logger.warning("Could not get label from %s/%s" %
-                                        (directory, daemon))
+                if daemon.endswith(".plist"):
+                    daemon = daemon[:-6]
+                dpath = os.path.join(directory, daemon)
+                rv = self.cmd.run(['defaults', 'read', dpath, 'Label'])
+                if rv.success:
+                    label = rv.stdout.splitlines()[0]
+                    self.plist_mapping[label] = dpath
+                else:
+                    self.logger.warning("Could not get label from %s" % dpath)
 
     def FindPlist(self, entry):
-        return self.plistMapping.get(entry.get('name'), None)
+        """ Find the location of the plist file for the given entry """
+        return self.plist_mapping.get(entry.get('name'), None)
 
     def os_version(self):
-        version = ""
-        try:
-            vers = self.cmd.run('sw_vers')[1]
-        except:
-            return version
-
-        for line in vers:
-            if line.startswith("ProductVersion"):
-                version = line.split()[-1]
-        return version
+        """ Determine the OS version """
+        rv = self.cmd.run('sw_vers')
+        if rv:
+            for line in rv.stdout.splitlines():
+                if line.startswith("ProductVersion"):
+                    return line.split()[-1]
+        else:
+            return ''
 
     def VerifyService(self, entry, _):
         """Verify launchd service entry."""
@@ -63,7 +60,7 @@ class launchd(Bcfg2.Client.Tools.Tool):
             return True
 
         try:
-            services = self.cmd.run("/bin/launchctl list")[1]
+            services = self.cmd.run("/bin/launchctl list").stdout.splitlines()
         except IndexError:
             # happens when no services are running (should be never)
             services = []
@@ -93,15 +90,13 @@ class launchd(Bcfg2.Client.Tools.Tool):
         name = entry.get('name')
         if entry.get('status') == 'on':
             self.logger.error("Installing service %s" % name)
-            cmdrc = self.cmd.run("/bin/launchctl load -w %s" %
-                                 self.FindPlist(entry))
-            cmdrc = self.cmd.run("/bin/launchctl start %s" % name)
+            self.cmd.run("/bin/launchctl load -w %s" % self.FindPlist(entry))
+            return self.cmd.run("/bin/launchctl start %s" % name).success
         else:
             self.logger.error("Uninstalling service %s" % name)
-            cmdrc = self.cmd.run("/bin/launchctl stop %s" % name)
-            cmdrc = self.cmd.run("/bin/launchctl unload -w %s" %
-                                 self.FindPlist(entry))
-        return cmdrc[0] == 0
+            self.cmd.run("/bin/launchctl stop %s" % name)
+            return self.cmd.run("/bin/launchctl unload -w %s" %
+                                self.FindPlist(entry)).success
 
     def Remove(self, svcs):
         """Remove Extra launchd entries."""
@@ -110,23 +105,24 @@ class launchd(Bcfg2.Client.Tools.Tool):
     def FindExtra(self):
         """Find Extra launchd services."""
         try:
-            allsrv = self.cmd.run("/bin/launchctl list")[1]
+            allsrv = self.cmd.run("/bin/launchctl list").stdout.splitlines()
         except IndexError:
             allsrv = []
 
-        [allsrv.remove(svc) for svc in [entry.get("name") for entry
-                                        in self.getSupportedEntries()] if svc in allsrv]
-        return [Bcfg2.Client.XML.Element("Service",
-                                         type='launchd',
-                                         name=name,
-                                         status='on') for name in allsrv]
+        for entry in self.getSupportedEntries():
+            svc = entry.get("name")
+            if svc in allsrv:
+                allsrv.remove(svc)
+        return [Bcfg2.Client.XML.Element("Service", type='launchd', name=name,
+                                         status='on')
+                for name in allsrv]
 
     def BundleUpdated(self, bundle, states):
         """Reload launchd plist."""
         for entry in [entry for entry in bundle if self.handlesEntry(entry)]:
             if not self.canInstall(entry):
-                self.logger.error("Insufficient information to restart service %s" %
-                                  (entry.get('name')))
+                self.logger.error("Insufficient information to restart "
+                                  "service %s" % entry.get('name'))
             else:
                 name = entry.get('name')
                 if entry.get('status') == 'on' and self.FindPlist(entry):