use Executor class for better subprocess calling on server

18 files changed, 238 insertions, 286 deletions

diff --git a/src/lib/Bcfg2/Server/Admin/Init.py b/src/lib/Bcfg2/Server/Admin/Init.py
index cf4bd4c0c..724da124b 100644
--- a/src/lib/Bcfg2/Server/Admin/Init.py
+++ b/src/lib/Bcfg2/Server/Admin/Init.py
@@ -8,8 +8,7 @@ import random
 import socket
 import string
 import getpass
-import subprocess
-
+from Bcfg2.Utils import Executor
 import Bcfg2.Server.Admin
 import Bcfg2.Server.Plugin
 import Bcfg2.Options
@@ -103,23 +102,26 @@ def gen_password(length):
 
 def create_key(hostname, keypath, certpath, country, state, location):
     """Creates a bcfg2.key at the directory specifed by keypath."""
-    kcstr = ("openssl req -batch -x509 -nodes -subj '/C=%s/ST=%s/L=%s/CN=%s' "
-             "-days 1000 -newkey rsa:2048 -keyout %s -noout" % (country,
-                                                                state,
-                                                                location,
-                                                                hostname,
-                                                                keypath))
-    subprocess.call((kcstr), shell=True)
-    ccstr = ("openssl req -batch -new  -subj '/C=%s/ST=%s/L=%s/CN=%s' -key %s "
-             "| openssl x509 -req -days 1000 -signkey %s -out %s" % (country,
-                                                                     state,
-                                                                     location,
-                                                                     hostname,
-                                                                     keypath,
-                                                                     keypath,
-                                                                     certpath))
-    subprocess.call((ccstr), shell=True)
+    cmd = Executor(timeout=120)
+    subject = "/C=%s/ST=%s/L=%s/CN=%s'" % (country, state, location, hostname)
+    key = cmd.run(["openssl", "req", "-batch", "-x509", "-nodes",
+                   "-subj", subject, "-days", "1000", "-newkey", "rsa:2048",
+                   "-keyout", keypath, "-noout"])
+    if not key.success:
+        print("Error generating key: %s" % key.error)
+        return
     os.chmod(keypath, stat.S_IRUSR | stat.S_IWUSR)  # 0600
+    csr = cmd.run(["openssl", "req", "-batch", "-new", "-subj", subject,
+                   "-key", keypath])
+    if not csr.success:
+        print("Error generating certificate signing request: %s" % csr.error)
+        return
+    cert = cmd.run(["openssl", "x509", "-req", "-days", "1000",
+                    "-signkey", keypath, "-out", certpath],
+                   inputdata=csr.stdout)
+    if not cert.success:
+        print("Error signing certificate: %s" % cert.error)
+        return
 
 
 def create_conf(confpath, confdata):
diff --git a/src/lib/Bcfg2/Server/Admin/Viz.py b/src/lib/Bcfg2/Server/Admin/Viz.py
index b3d317604..cdd8fd0cb 100644
--- a/src/lib/Bcfg2/Server/Admin/Viz.py
+++ b/src/lib/Bcfg2/Server/Admin/Viz.py
@@ -1,10 +1,8 @@
 """ Produce graphviz diagrams of metadata structures """
 
 import getopt
-from subprocess import Popen, PIPE
-import pipes
 import Bcfg2.Server.Admin
-
+from Bcfg2.Utils import Executor
 
 class Viz(Bcfg2.Server.Admin.MetadataCore):
     """ Produce graphviz diagrams of metadata structures """
@@ -72,40 +70,34 @@ class Viz(Bcfg2.Server.Admin.MetadataCore):
         else:
             fmt = 'png'
 
+        exc = Executor()
         cmd = ["dot", "-T", fmt]
         if output:
             cmd.extend(["-o", output])
+        idata = ["digraph groups {",
+                 '\trankdir="LR";',
+                 self.metadata.viz(hosts, bundles,
+                                   key, only_client, self.colors)]
+        if key:
+            idata.extend([
+                    "\tsubgraph cluster_key {",
+                    '\tstyle="filled";',
+                    '\tcolor="lightblue";',
+                    '\tBundle [ shape="septagon" ];',
+                    '\tGroup [shape="ellipse"];',
+                    '\tProfile [style="bold", shape="ellipse"];',
+                    '\tHblock [label="Host1|Host2|Host3",shape="record"];',
+                    '\tlabel="Key";',
+                    "\t}"])
+        idata.append("}")
         try:
-            dotpipe = Popen(cmd, stdin=PIPE, stdout=PIPE, close_fds=True)
+            result = exc.run(cmd, inputdata=idata)
         except OSError:
             # on some systems (RHEL 6), you cannot run dot with
             # shell=True.  on others (Gentoo with Python 2.7), you
             # must.  In yet others (RHEL 5), either way works.  I have
             # no idea what the difference is, but it's kind of a PITA.
-            cmd = ["dot", "-T", pipes.quote(fmt)]
-            if output:
-                cmd.extend(["-o", pipes.quote(output)])
-            dotpipe = Popen(cmd, shell=True,
-                            stdin=PIPE, stdout=PIPE, close_fds=True)
-        try:
-            dotpipe.stdin.write("digraph groups {\n")
-        except:
-            print("write to dot process failed. Is graphviz installed?")
-            raise SystemExit(1)
-        dotpipe.stdin.write('\trankdir="LR";\n')
-        dotpipe.stdin.write(self.metadata.viz(hosts, bundles,
-                                              key, only_client, self.colors))
-        if key:
-            dotpipe.stdin.write("\tsubgraph cluster_key {\n")
-            dotpipe.stdin.write('\tstyle="filled";\n')
-            dotpipe.stdin.write('\tcolor="lightblue";\n')
-            dotpipe.stdin.write('\tBundle [ shape="septagon" ];\n')
-            dotpipe.stdin.write('\tGroup [shape="ellipse"];\n')
-            dotpipe.stdin.write('\tProfile [style="bold", shape="ellipse"];\n')
-            dotpipe.stdin.write('\tHblock [label="Host1|Host2|Host3", '
-                                'shape="record"];\n')
-            dotpipe.stdin.write('\tlabel="Key";\n')
-            dotpipe.stdin.write("\t}\n")
-        dotpipe.stdin.write("}\n")
-        dotpipe.stdin.close()
-        return dotpipe.stdout.read()
+            result = exc.run(cmd, shell=True, inputdata=idata)
+        if not result.success:
+            print("Error running %s: %s" % (cmd, result.error))
+            raise SystemExit(result.retval)
diff --git a/src/lib/Bcfg2/Server/Lint/Validate.py b/src/lib/Bcfg2/Server/Lint/Validate.py
index 7a9d9f877..dd45ac62e 100644
--- a/src/lib/Bcfg2/Server/Lint/Validate.py
+++ b/src/lib/Bcfg2/Server/Lint/Validate.py
@@ -5,8 +5,8 @@ import sys
 import glob
 import fnmatch
 import lxml.etree
-from subprocess import Popen, PIPE, STDOUT
 import Bcfg2.Server.Lint
+from Bcfg2.Utils import Executor
 
 
 class Validate(Bcfg2.Server.Lint.ServerlessPlugin):
@@ -44,6 +44,7 @@ class Validate(Bcfg2.Server.Lint.ServerlessPlugin):
 
         self.filelists = {}
         self.get_filelists()
+        self.cmd = Executor()
 
     def Run(self):
         schemadir = self.config['schema']
@@ -94,11 +95,10 @@ class Validate(Bcfg2.Server.Lint.ServerlessPlugin):
         try:
             datafile = lxml.etree.parse(filename)
         except SyntaxError:
-            lint = Popen(["xmllint", filename], stdout=PIPE, stderr=STDOUT)
+            result = self.cmd.run(["xmllint", filename])
             self.LintError("xml-failed-to-parse",
-                           "%s fails to parse:\n%s" % (filename,
-                                                       lint.communicate()[0]))
-            lint.wait()
+                           "%s fails to parse:\n%s" %
+                           (filename, result.stdout + result.stderr))
             return False
         except IOError:
             self.LintError("xml-failed-to-read",
@@ -110,11 +110,11 @@ class Validate(Bcfg2.Server.Lint.ServerlessPlugin):
             if self.files is None:
                 cmd.append("--xinclude")
             cmd.extend(["--noout", "--schema", schemafile, filename])
-            lint = Popen(cmd, stdout=PIPE, stderr=STDOUT)
-            output = lint.communicate()[0]
-            if lint.wait():
+            result = self.cmd.run(cmd)
+            if not result.success:
                 self.LintError("xml-failed-to-verify",
-                               "%s fails to verify:\n%s" % (filename, output))
+                               "%s fails to verify:\n%s" %
+                               (filename, result.stdout + result.stderr))
                 return False
         return True
 
diff --git a/src/lib/Bcfg2/Server/Plugins/Cfg/CfgExternalCommandVerifier.py b/src/lib/Bcfg2/Server/Plugins/Cfg/CfgExternalCommandVerifier.py
index 313e53ee9..d06b864ac 100644
--- a/src/lib/Bcfg2/Server/Plugins/Cfg/CfgExternalCommandVerifier.py
+++ b/src/lib/Bcfg2/Server/Plugins/Cfg/CfgExternalCommandVerifier.py
@@ -3,8 +3,8 @@
 import os
 import sys
 import shlex
+from Bcfg2.Utils import Executor
 from Bcfg2.Server.Plugin import PluginExecutionError
-from subprocess import Popen, PIPE
 from Bcfg2.Server.Plugins.Cfg import CfgVerifier, CfgVerificationError
 
 
@@ -18,24 +18,16 @@ class CfgExternalCommandVerifier(CfgVerifier):
     def __init__(self, name, specific, encoding):
         CfgVerifier.__init__(self, name, specific, encoding)
         self.cmd = []
+        self.exc = Executor(timeout=30)
     __init__.__doc__ = CfgVerifier.__init__.__doc__
 
     def verify_entry(self, entry, metadata, data):
         try:
-            proc = Popen(self.cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE)
-            out, err = proc.communicate(input=data)
-            rv = proc.wait()
-            if rv != 0:
-                # pylint: disable=E1103
-                raise CfgVerificationError(err.strip() or out.strip() or
-                                           "Non-zero return value %s" % rv)
-                # pylint: enable=E1103
-        except CfgVerificationError:
-            raise
-        except:
-            err = sys.exc_info()[1]
-            raise CfgVerificationError("Error running external command "
-                                       "verifier: %s" % err)
+            result = self.exc.run(self.cmd, inputdata=data)
+            if not result.success:
+                raise CfgVerificationError(result.error)
+        except OSError:
+            raise CfgVerificationError(sys.exc_info()[1])
     verify_entry.__doc__ = CfgVerifier.verify_entry.__doc__
 
     def handle_event(self, event):
diff --git a/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPrivateKeyCreator.py b/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPrivateKeyCreator.py
index 1711cc655..7ebce192c 100644
--- a/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPrivateKeyCreator.py
+++ b/src/lib/Bcfg2/Server/Plugins/Cfg/CfgPrivateKeyCreator.py
@@ -3,7 +3,7 @@
 import os
 import shutil
 import tempfile
-import subprocess
+from Bcfg2.Utils import Executor
 from Bcfg2.Options import get_option_parser
 from Bcfg2.Server.Plugin import StructFile
 from Bcfg2.Server.Plugins.Cfg import CfgCreator, CfgCreationError
@@ -33,6 +33,7 @@ class CfgPrivateKeyCreator(CfgCreator, StructFile):
         pubkey_name = os.path.join(pubkey_path, os.path.basename(pubkey_path))
         self.pubkey_creator = CfgPublicKeyCreator(pubkey_name)
         self.setup = get_option_parser()
+        self.cmd = Executor()
     __init__.__doc__ = CfgCreator.__init__.__doc__
 
     @property
@@ -104,18 +105,17 @@ class CfgPrivateKeyCreator(CfgCreator, StructFile):
                 log_cmd.append("''")
             self.debug_log("Cfg: Generating new SSH key pair: %s" %
                            " ".join(log_cmd))
-            proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
-                                    stderr=subprocess.PIPE)
-            err = proc.communicate()[1]
-            if proc.wait():
+            result = self.cmd.run(cmd)
+            if not result.success:
                 raise CfgCreationError("Cfg: Failed to generate SSH key pair "
                                        "at %s for %s: %s" %
-                                       (filename, metadata.hostname, err))
-            elif err:
+                                       (filename, metadata.hostname,
+                                        result.error))
+            elif result.stderr:
                 self.logger.warning("Cfg: Generated SSH key pair at %s for %s "
                                     "with errors: %s" % (filename,
                                                          metadata.hostname,
-                                                         err))
+                                                         result.stderr))
             return filename
         except:
             shutil.rmtree(tempdir)
diff --git a/src/lib/Bcfg2/Server/Plugins/Cvs.py b/src/lib/Bcfg2/Server/Plugins/Cvs.py
index ba1559a1a..0054a8a37 100644
--- a/src/lib/Bcfg2/Server/Plugins/Cvs.py
+++ b/src/lib/Bcfg2/Server/Plugins/Cvs.py
@@ -1,7 +1,7 @@
 """ The Cvs plugin provides a revision interface for Bcfg2 repos using
 cvs. """
 
-from subprocess import Popen, PIPE
+from Bcfg2.Utils import Executor
 import Bcfg2.Server.Plugin
 
 
@@ -13,20 +13,17 @@ class Cvs(Bcfg2.Server.Plugin.Version):
 
     def __init__(self, core, datastore):
         Bcfg2.Server.Plugin.Version.__init__(self, core, datastore)
+        self.cmd = Executor()
         self.logger.debug("Initialized cvs plugin with CVS directory %s" %
                           self.vcs_path)
 
     def get_revision(self):
         """Read cvs revision information for the Bcfg2 repository."""
+        result = self.cmd.run(["env LC_ALL=C", "cvs", "log"],
+                              shell=True, cwd=self.vcs_root)
         try:
-            data = Popen("env LC_ALL=C cvs log",
-                        shell=True,
-                        cwd=self.vcs_root,
-                        stdout=PIPE).stdout.readlines()
-            return data[3].strip('\n')
-        except IndexError:
-            msg = "Failed to read CVS log"
+            return result.stdout.splitlines()[0].strip()
+        except (IndexError, AttributeError):
+            msg = "Failed to read revision from CVS: %s" % result.error
             self.logger.error(msg)
-            self.logger.error('Ran command "cvs log" from directory %s' %
-                              self.vcs_root)
             raise Bcfg2.Server.Plugin.PluginExecutionError(msg)
diff --git a/src/lib/Bcfg2/Server/Plugins/Darcs.py b/src/lib/Bcfg2/Server/Plugins/Darcs.py
index 0033e00f3..2c6dde393 100644
--- a/src/lib/Bcfg2/Server/Plugins/Darcs.py
+++ b/src/lib/Bcfg2/Server/Plugins/Darcs.py
@@ -1,7 +1,7 @@
 """ Darcs is a version plugin for dealing with Bcfg2 repos stored in the
 Darcs VCS. """
 
-from subprocess import Popen, PIPE
+from Bcfg2.Utils import Executor
 import Bcfg2.Server.Plugin
 
 
@@ -13,21 +13,17 @@ class Darcs(Bcfg2.Server.Plugin.Version):
 
     def __init__(self, core, datastore):
         Bcfg2.Server.Plugin.Version.__init__(self, core, datastore)
+        self.cmd = Executor()
         self.logger.debug("Initialized Darcs plugin with darcs directory %s" %
                           self.vcs_path)
 
     def get_revision(self):
         """Read Darcs changeset information for the Bcfg2 repository."""
-        try:
-            data = Popen("env LC_ALL=C darcs changes",
-                        shell=True,
-                        cwd=self.vcs_root,
-                        stdout=PIPE).stdout.readlines()
-            revision = data[0].strip('\n')
-        except:
-            msg = "Failed to read darcs repository"
+        result = self.cmd.run(["env LC_ALL=C", "darcs", "changes"],
+                              shell=True, cwd=self.vcs_root)
+        if result.success:
+            return result.stdout.splitlines()[0].strip()
+        else:
+            msg = "Failed to read revision from darcs: %s" % result.error
             self.logger.error(msg)
-            self.logger.error('Ran command "darcs changes" from directory %s' %
-                              self.vcs_root)
             raise Bcfg2.Server.Plugin.PluginExecutionError(msg)
-        return revision
diff --git a/src/lib/Bcfg2/Server/Plugins/Fossil.py b/src/lib/Bcfg2/Server/Plugins/Fossil.py
index f6735df12..05cf4e5d4 100644
--- a/src/lib/Bcfg2/Server/Plugins/Fossil.py
+++ b/src/lib/Bcfg2/Server/Plugins/Fossil.py
@@ -1,7 +1,7 @@
 """ The Fossil plugin provides a revision interface for Bcfg2 repos
 using fossil."""
 
-from subprocess import Popen, PIPE
+from Bcfg2.Utils import Executor
 import Bcfg2.Server.Plugin
 
 
@@ -13,22 +13,22 @@ class Fossil(Bcfg2.Server.Plugin.Version):
 
     def __init__(self, core, datastore):
         Bcfg2.Server.Plugin.Version.__init__(self, core, datastore)
+        self.cmd = Executor()
         self.logger.debug("Initialized Fossil plugin with fossil directory %s"
                           % self.vcs_path)
 
     def get_revision(self):
         """Read fossil revision information for the Bcfg2 repository."""
+        result = self.cmd.run(["env LC_ALL=C", "fossil", "info"],
+                              shell=True, cwd=self.vcs_root)
         try:
-            data = Popen("env LC_ALL=C fossil info",
-                        shell=True,
-                        cwd=self.vcs_root,
-                        stdout=PIPE).stdout.readlines()
-            revline = [line.split(': ')[1].strip() for line in data if \
-                       line.split(': ')[0].strip() == 'checkout'][-1]
-            return revline.split(' ')[0]
-        except IndexError:
-            msg = "Failed to read fossil info"
+            revision = None
+            for line in result.stdout.splitlines():
+                ldata = line.split(': ')
+                if ldata[0].strip() == 'checkout':
+                    revision = line[1].strip().split(' ')[0]
+            return revision
+        except (IndexError, AttributeError):
+            msg = "Failed to read revision from Fossil: %s" % result.error
             self.logger.error(msg)
-            self.logger.error('Ran command "fossil info" from directory "%s"' %
-                              self.vcs_root)
             raise Bcfg2.Server.Plugin.PluginExecutionError(msg)
diff --git a/src/lib/Bcfg2/Server/Plugins/Git.py b/src/lib/Bcfg2/Server/Plugins/Git.py
index c8362db41..781413e1a 100644
--- a/src/lib/Bcfg2/Server/Plugins/Git.py
+++ b/src/lib/Bcfg2/Server/Plugins/Git.py
@@ -3,12 +3,12 @@ git. """
 
 import sys
 from Bcfg2.Server.Plugin import Version, PluginExecutionError
-from subprocess import Popen, PIPE
 
 try:
     import git
     HAS_GITPYTHON = True
 except ImportError:
+    from Bcfg2.Utils import Executor
     HAS_GITPYTHON = False
 
 
@@ -24,10 +24,12 @@ class Git(Version):
         Version.__init__(self, core, datastore)
         if HAS_GITPYTHON:
             self.repo = git.Repo(self.vcs_root)
+            self.cmd = None
         else:
             self.logger.debug("Git: GitPython not found, using CLI interface "
                               "to Git")
             self.repo = None
+            self.cmd = Executor()
         self.logger.debug("Initialized git plugin with git directory %s" %
                           self.vcs_path)
 
@@ -45,11 +47,10 @@ class Git(Version):
                 cmd = ["git", "--git-dir", self.vcs_path,
                        "--work-tree", self.vcs_root, "rev-parse", "HEAD"]
                 self.debug_log("Git: Running cmd")
-                proc = Popen(cmd, stdout=PIPE, stderr=PIPE)
-                rv, err = proc.communicate()
-                if proc.wait():
-                    raise Exception(err)
-                return rv
+                result = self.cmd.run(cmd)
+                if not result.success:
+                    raise Exception(result.stderr)
+                return result.stdout
         except:
             raise PluginExecutionError("Git: Error getting revision from %s: "
                                        "%s" % (self.vcs_root,
diff --git a/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py b/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py
index 775caaa08..3799b1723 100644
--- a/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py
+++ b/src/lib/Bcfg2/Server/Plugins/Packages/Yum.py
@@ -58,9 +58,9 @@ import errno
 import socket
 import logging
 import lxml.etree
-from subprocess import Popen, PIPE
 import Bcfg2.Server.FileMonitor
 import Bcfg2.Server.Plugin
+from Bcfg2.Utils import Executor
 from Bcfg2.Options import get_option_parser
 # pylint: disable=W0622
 from Bcfg2.Compat import StringIO, cPickle, HTTPError, URLError, \
@@ -105,10 +105,6 @@ FL = '{http://linux.duke.edu/metadata/filelists}'
 PULPSERVER = None
 PULPCONFIG = None
 
-#: The path to bcfg2-yum-helper
-HELPER = None
-
-
 def _setup_pulp():
     """ Connect to a Pulp server and pass authentication credentials.
     This only needs to be called once, but multiple calls won't hurt
@@ -278,6 +274,7 @@ class YumCollection(Collection):
                             debug=debug)
         self.keypath = os.path.join(self.cachepath, "keys")
 
+        self._helper = None
         if self.use_yum:
             #: Define a unique cache file for this collection to use
             #: for cached yum metadata
@@ -290,8 +287,10 @@ class YumCollection(Collection):
             #: resolving packages with the Python yum libraries
             self.cfgfile = os.path.join(self.cachefile, "yum.conf")
             self.write_config()
+            self.cmd = Executor()
         else:
             self.cachefile = None
+            self.cmd = None
 
         if HAS_PULP and self.has_pulp_sources:
             _setup_pulp()
@@ -326,20 +325,18 @@ class YumCollection(Collection):
         a call to it; I wish there was a way to do this without
         forking, but apparently not); finally we check in /usr/sbin,
         the default location. """
-        global HELPER
-        if not HELPER:
+        if not self._helper:
             try:
-                HELPER = self.setup.cfp.get("packages:yum", "helper")
+                self._helper = self.setup.cfp.get("packages:yum", "helper")
             except (ConfigParser.NoOptionError, ConfigParser.NoSectionError):
                 # first see if bcfg2-yum-helper is in PATH
                 try:
                     self.debug_log("Checking for bcfg2-yum-helper in $PATH")
-                    Popen(['bcfg2-yum-helper'],
-                          stdin=PIPE, stdout=PIPE, stderr=PIPE).wait()
-                    HELPER = 'bcfg2-yum-helper'
+                    self.cmd.run(['bcfg2-yum-helper'])
+                    self._helper = 'bcfg2-yum-helper'
                 except OSError:
-                    HELPER = "/usr/sbin/bcfg2-yum-helper"
-        return HELPER
+                    self._helper = "/usr/sbin/bcfg2-yum-helper"
+        return self._helper
 
     @property
     def use_yum(self):
@@ -879,28 +876,18 @@ class YumCollection(Collection):
             cmd.append("-v")
         cmd.append(command)
         self.debug_log("Packages: running %s" % " ".join(cmd), flag=verbose)
-        try:
-            helper = Popen(cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE)
-        except OSError:
-            err = sys.exc_info()[1]
-            self.logger.error("Packages: Failed to execute %s: %s" %
-                              (" ".join(cmd), err))
-            return None
-
         if inputdata:
-            idata = json.dumps(inputdata)
-            (stdout, stderr) = helper.communicate(idata)
-        else:
-            (stdout, stderr) = helper.communicate()
-        rv = helper.wait()
-        if rv:
-            self.logger.error("Packages: error running bcfg2-yum-helper "
-                              "(returned %d): %s" % (rv, stderr))
+            result = self.cmd.run(cmd, inputdata=json.dumps(inputdata))
         else:
+            result = self.cmd.run(cmd)
+        if not result.success:
+            self.logger.error("Packages: error running bcfg2-yum-helper: %s" %
+                              result.error)
+        elif result.stderr:
             self.debug_log("Packages: debug info from bcfg2-yum-helper: %s" %
-                           stderr, flag=verbose)
+                           result.stderr, flag=verbose)
         try:
-            return json.loads(stdout)
+            return json.loads(result.stdout)
         except ValueError:
             err = sys.exc_info()[1]
             self.logger.error("Packages: error reading bcfg2-yum-helper "
diff --git a/src/lib/Bcfg2/Server/Plugins/PuppetENC.py b/src/lib/Bcfg2/Server/Plugins/PuppetENC.py
index ce625ea86..3b367573b 100644
--- a/src/lib/Bcfg2/Server/Plugins/PuppetENC.py
+++ b/src/lib/Bcfg2/Server/Plugins/PuppetENC.py
@@ -4,7 +4,7 @@ import os
 import sys
 import Bcfg2.Server
 import Bcfg2.Server.Plugin
-from subprocess import Popen, PIPE
+from Bcfg2.Utils import Executor
 
 try:
     from syck import load as yaml_load, error as yaml_error
@@ -36,6 +36,7 @@ class PuppetENC(Bcfg2.Server.Plugin.Plugin,
         Bcfg2.Server.Plugin.ClientRunHooks.__init__(self)
         Bcfg2.Server.Plugin.DirectoryBacked.__init__(self, self.data)
         self.cache = dict()
+        self.cmd = Executor()
 
     def _run_encs(self, metadata):
         """ Run all Puppet ENCs """
@@ -44,20 +45,17 @@ class PuppetENC(Bcfg2.Server.Plugin.Plugin,
             epath = os.path.join(self.data, enc)
             self.debug_log("PuppetENC: Running ENC %s for %s" %
                            (enc, metadata.hostname))
-            proc = Popen([epath, metadata.hostname], stdin=PIPE, stdout=PIPE,
-                         stderr=PIPE)
-            (out, err) = proc.communicate()
-            rv = proc.wait()
-            if rv != 0:
-                msg = "PuppetENC: Error running ENC %s for %s (%s): %s" % \
-                    (enc, metadata.hostname, rv, err)
+            result = self.cmd.run([epath, metadata.hostname])
+            if not result.success:
+                msg = "PuppetENC: Error running ENC %s for %s: %s" % \
+                    (enc, metadata.hostname, result.error)
                 self.logger.error(msg)
                 raise Bcfg2.Server.Plugin.PluginExecutionError(msg)
-            if err:
-                self.debug_log("ENC Error: %s" % err)
+            if result.stderr:
+                self.debug_log("ENC Error: %s" % result.stderr)
 
             try:
-                yaml = yaml_load(out)
+                yaml = yaml_load(result.stdout)
                 self.debug_log("Loaded data from %s for %s: %s" %
                                (enc, metadata.hostname, yaml))
             except yaml_error:
@@ -67,13 +65,7 @@ class PuppetENC(Bcfg2.Server.Plugin.Plugin,
                 self.logger.error(msg)
                 raise Bcfg2.Server.Plugin.PluginExecutionError(msg)
 
-            groups = dict()
-            if "classes" in yaml:
-                # stock Puppet ENC output format
-                groups = yaml['classes']
-            elif "groups" in yaml:
-                # more Bcfg2-ish output format
-                groups = yaml['groups']
+            groups = yaml.get("classes", yaml.get("groups", dict()))
             if groups:
                 if isinstance(groups, list):
                     self.debug_log("ENC %s adding groups to %s: %s" %
diff --git a/src/lib/Bcfg2/Server/Plugins/SSHbase.py b/src/lib/Bcfg2/Server/Plugins/SSHbase.py
index 7b6c9c418..fb5bd50bf 100644
--- a/src/lib/Bcfg2/Server/Plugins/SSHbase.py
+++ b/src/lib/Bcfg2/Server/Plugins/SSHbase.py
@@ -8,8 +8,8 @@ import shutil
 import logging
 import tempfile
 from itertools import chain
-from subprocess import Popen, PIPE
 import Bcfg2.Server.Plugin
+from Bcfg2.Utils import Executor
 from Bcfg2.Server.Plugin import PluginExecutionError
 from Bcfg2.Compat import any, u_str, b64encode  # pylint: disable=W0622
 
@@ -20,9 +20,7 @@ class KeyData(Bcfg2.Server.Plugin.SpecificData):
     """ class to handle key data for HostKeyEntrySet """
 
     def __init__(self, name, specific, encoding):
-        Bcfg2.Server.Plugin.SpecificData.__init__(self,
-                                                  name,
-                                                  specific,
+        Bcfg2.Server.Plugin.SpecificData.__init__(self, name, specific,
                                                   encoding)
         self.encoding = encoding
 
@@ -150,6 +148,8 @@ class SSHbase(Bcfg2.Server.Plugin.Plugin,
                 HostKeyEntrySet(keypattern, self.data)
             self.Entries['Path']["/etc/ssh/" + keypattern] = self.build_hk
 
+        self.cmd = Executor()
+
     def get_skn(self):
         """Build memory cache of the ssh known hosts file."""
         if not self.__skn:
@@ -174,7 +174,7 @@ class SSHbase(Bcfg2.Server.Plugin.Plugin,
                     newnames.add(name.split('.')[0])
                     try:
                         newips.add(self.get_ipcache_entry(name)[0])
-                    except:  # pylint: disable=W0702
+                    except PluginExecutionError:
                         continue
                 names[cmeta.hostname].update(newnames)
                 names[cmeta.hostname].update(cmeta.addresses)
@@ -279,12 +279,13 @@ class SSHbase(Bcfg2.Server.Plugin.Plugin,
                          (event.filename, action))
 
     def get_ipcache_entry(self, client):
-        """Build a cache of dns results."""
+        """ Build a cache of dns results. """
         if client in self.ipcache:
             if self.ipcache[client]:
                 return self.ipcache[client]
             else:
-                raise socket.gaierror
+                raise PluginExecutionError("No cached IP address for %s" %
+                                           client)
         else:
             # need to add entry
             try:
@@ -292,14 +293,17 @@ class SSHbase(Bcfg2.Server.Plugin.Plugin,
                 self.ipcache[client] = (ipaddr, client)
                 return (ipaddr, client)
             except socket.gaierror:
-                ipaddr = Popen(["getent", "hosts", client],
-                               stdout=PIPE).stdout.read().strip().split()
-                if ipaddr:
-                    self.ipcache[client] = (ipaddr, client)
-                    return (ipaddr, client)
+                result = self.cmd.run(["getent", "hosts", client])
+                if result.success:
+                    ipaddr = result.stdout.strip().split()
+                    if ipaddr:
+                        self.ipcache[client] = (ipaddr, client)
+                        return (ipaddr, client)
                 self.ipcache[client] = False
-                self.logger.error("Failed to find IP address for %s" % client)
-                raise socket.gaierror
+                msg = "Failed to find IP address for %s: %s" % (client,
+                                                                result.error)
+                self.logger(msg)
+                raise PluginExecutionError(msg)
 
     def get_namecache_entry(self, cip):
         """Build a cache of name lookups from client IP addresses."""
@@ -398,11 +402,10 @@ class SSHbase(Bcfg2.Server.Plugin.Plugin,
         cmd = ["ssh-keygen", "-q", "-f", temploc, "-N", "",
                "-t", keytype, "-C", "root@%s" % client]
         self.debug_log("SSHbase: Running: %s" % " ".join(cmd))
-        proc = Popen(cmd, stdout=PIPE, stdin=PIPE)
-        err = proc.communicate()[1]
-        if proc.wait():
+        result = self.cmd.run(cmd)
+        if not result.success:
             raise PluginExecutionError("SSHbase: Error running ssh-keygen: %s"
-                                       % err)
+                                       % result.error)
 
         try:
             shutil.copy(temploc, fileloc)
diff --git a/src/lib/Bcfg2/Server/Plugins/SSLCA.py b/src/lib/Bcfg2/Server/Plugins/SSLCA.py
index ab2f80552..d52d9325c 100644
--- a/src/lib/Bcfg2/Server/Plugins/SSLCA.py
+++ b/src/lib/Bcfg2/Server/Plugins/SSLCA.py
@@ -6,9 +6,9 @@ import sys
 import logging
 import tempfile
 import lxml.etree
-from subprocess import Popen, PIPE, STDOUT
 import Bcfg2.Options
 import Bcfg2.Server.Plugin
+from Bcfg2.Utils import Executor
 from Bcfg2.Compat import ConfigParser
 from Bcfg2.Server.Plugin import PluginExecutionError
 
@@ -90,6 +90,7 @@ class SSLCAEntrySet(Bcfg2.Server.Plugin.EntrySet):
         self.parent = parent
         self.key = None
         self.cert = None
+        self.cmd = Executor(timeout=120)
 
     def handle_event(self, event):
         action = event.code2str()
@@ -123,14 +124,14 @@ class SSLCAEntrySet(Bcfg2.Server.Plugin.EntrySet):
         elif ktype == 'dsa':
             cmd = ["openssl", "dsaparam", "-noout", "-genkey", bits]
         self.debug_log("SSLCA: Generating new key: %s" % " ".join(cmd))
-        proc = Popen(cmd, stdout=PIPE, stderr=PIPE)
-        key, err = proc.communicate()
-        if proc.wait():
+        result = self.cmd.run(cmd)
+        if not result.success:
             raise PluginExecutionError("SSLCA: Failed to generate key %s for "
                                        "%s: %s" % (entry.get("name"),
-                                                   metadata.hostname, err))
-        open(os.path.join(self.path, filename), 'w').write(key)
-        return key
+                                                   metadata.hostname,
+                                                   result.error))
+        open(os.path.join(self.path, filename), 'w').write(result.stdout)
+        return result.stdout
 
     def build_cert(self, entry, metadata, keyfile):
         """ generate a new cert """
@@ -163,13 +164,10 @@ class SSLCAEntrySet(Bcfg2.Server.Plugin.EntrySet):
 
             self.debug_log("SSLCA: Generating new certificate: %s" %
                            " ".join(_scrub_pass(a) for a in cmd))
-            proc = Popen(cmd, stdin=PIPE, stdout=PIPE, stderr=PIPE)
-            (cert, err) = proc.communicate()
-            if proc.wait():
-                # pylint: disable=E1103
+            result = self.cmd.run(cmd)
+            if not result.success:
                 raise PluginExecutionError("SSLCA: Failed to generate cert: %s"
-                                           % err.splitlines()[-1])
-                # pylint: enable=E1103
+                                           % result.error)
         finally:
             try:
                 if req_config and os.path.exists(req_config):
@@ -179,6 +177,7 @@ class SSLCAEntrySet(Bcfg2.Server.Plugin.EntrySet):
             except OSError:
                 self.logger.error("SSLCA: Failed to unlink temporary files: %s"
                                   % sys.exc_info()[1])
+        cert = result.stdout
         if cert_spec['append_chain'] and 'chaincert' in ca:
             cert += open(ca['chaincert']).read()
 
@@ -242,11 +241,10 @@ class SSLCAEntrySet(Bcfg2.Server.Plugin.EntrySet):
         cmd = ["openssl", "req", "-new", "-config", req_config,
                "-days", days, "-key", keyfile, "-text", "-out", req]
         self.debug_log("SSLCA: Generating new CSR: %s" % " ".join(cmd))
-        proc = Popen(cmd, stdout=PIPE, stderr=PIPE)
-        err = proc.communicate()[1]
-        if proc.wait():
+        result = self.cmd.run(cmd)
+        if not result.success:
             raise PluginExecutionError("SSLCA: Failed to generate CSR: %s" %
-                                       err)
+                                       result.error)
         return req
 
     def verify_cert(self, filename, keyfile, entry, metadata):
@@ -277,34 +275,34 @@ class SSLCAEntrySet(Bcfg2.Server.Plugin.EntrySet):
         cmd.extend([chaincert, cert])
         self.debug_log("SSLCA: Verifying %s against CA: %s" %
                        (entry.get("name"), " ".join(cmd)))
-        res = Popen(cmd, stdout=PIPE, stderr=STDOUT).stdout.read()
-        if res == cert + ": OK\n":
+        result = self.cmd.run(cmd)
+        if result.stdout == cert + ": OK\n":
             self.debug_log("SSLCA: %s verified successfully against CA" %
                            entry.get("name"))
             return True
         self.logger.warning("SSLCA: %s failed verification against CA: %s" %
-                            (entry.get("name"), res))
+                            (entry.get("name"), result.error))
         return False
 
+    def _get_modulus(self, fname, ftype="x509"):
+        """ get the modulus from the given file """
+        cmd = ["openssl", ftype, "-noout", "-modulus", "-in", fname]
+        self.debug_log("SSLCA: Getting modulus of %s for verification: %s" %
+                       (fname, " ".join(cmd)))
+        result = self.cmd.run(cmd)
+        if not result.success:
+            self.logger.warning("SSLCA: Failed to get modulus of %s: %s" %
+                                (fname, result.error))
+        return result.stdout.strip()
+
     def verify_cert_against_key(self, filename, keyfile):
         """
         check that a certificate validates against its private key.
         """
-        def _modulus(fname, ftype="x509"):
-            """ get the modulus from the given file """
-            cmd = ["openssl", ftype, "-noout", "-modulus", "-in", fname]
-            self.debug_log("SSLCA: Getting modulus of %s for verification: %s"
-                           % (fname, " ".join(cmd)))
-            proc = Popen(cmd, stdout=PIPE, stderr=PIPE)
-            rv, err = proc.communicate()
-            if proc.wait():
-                self.logger.warning("SSLCA: Failed to get modulus of %s: %s" %
-                                    (fname, err))
-            return rv.strip()  # pylint: disable=E1103
 
         certfile = os.path.join(self.path, filename)
-        cert = _modulus(certfile)
-        key = _modulus(keyfile, ftype="rsa")
+        cert = self._get_modulus(certfile)
+        key = self._get_modulus(keyfile, ftype="rsa")
         if cert == key:
             self.debug_log("SSLCA: %s verified successfully against key %s" %
                            (filename, keyfile))
diff --git a/src/lib/Bcfg2/Server/Plugins/Svn.py b/src/lib/Bcfg2/Server/Plugins/Svn.py
index 51f44c52d..34a6e89e0 100644
--- a/src/lib/Bcfg2/Server/Plugins/Svn.py
+++ b/src/lib/Bcfg2/Server/Plugins/Svn.py
@@ -10,8 +10,7 @@ try:
     import pysvn
     HAS_SVN = True
 except ImportError:
-    import pipes
-    from subprocess import Popen, PIPE
+    from Bcfg2.Utils import Executor
     HAS_SVN = False
 
 
@@ -29,10 +28,12 @@ class Svn(Bcfg2.Server.Plugin.Version):
 
         self.revision = None
         self.svn_root = None
+        self.client = None
+        self.cmd = None
         if not HAS_SVN:
             self.logger.debug("Svn: PySvn not found, using CLI interface to "
                               "SVN")
-            self.client = None
+            self.cmd = Executor()
         else:
             self.client = pysvn.Client()
             # pylint: disable=E1101
@@ -84,15 +85,16 @@ class Svn(Bcfg2.Server.Plugin.Version):
             except pysvn.ClientError:  # pylint: disable=E1101
                 msg = "Svn: Failed to get revision: %s" % sys.exc_info()[1]
         else:
-            try:
-                data = Popen("env LC_ALL=C svn info %s" %
-                             pipes.quote(self.vcs_root), shell=True,
-                             stdout=PIPE).communicate()[0].split('\n')
-                return [line.split(': ')[1] for line in data
-                        if line[:9] == 'Revision:'][-1]
-            except IndexError:
-                msg = "Failed to read svn info"
-                self.logger.error('Ran command "svn info %s"' % self.vcs_root)
+            result = self.cmd.run(["env LC_ALL=C", "svn", "info",
+                                   self.vcs_root],
+                                  shell=True)
+            if result.success:
+                self.revision = [line.split(': ')[1]
+                                 for line in result.stdout.splitlines()
+                                 if line.startswith('Revision:')][-1]
+                return self.revision
+            else:
+                msg = "Failed to read svn info: %s" % result.error
         self.revision = None
         raise Bcfg2.Server.Plugin.PluginExecutionError(msg)
 
diff --git a/src/lib/Bcfg2/Server/Plugins/Trigger.py b/src/lib/Bcfg2/Server/Plugins/Trigger.py
index 878bf9cea..a1b79a8c5 100644
--- a/src/lib/Bcfg2/Server/Plugins/Trigger.py
+++ b/src/lib/Bcfg2/Server/Plugins/Trigger.py
@@ -3,18 +3,14 @@
 import os
 import pipes
 import Bcfg2.Server.Plugin
-from subprocess import Popen, PIPE
+from Bcfg2.Utils import Executor
 
 
 class TriggerFile(Bcfg2.Server.Plugin.FileBacked):
     """ Representation of a trigger script file """
-
     def HandleEvent(self, event=None):
         return
 
-    def __str__(self):
-        return "%s: %s" % (self.__class__.__name__, self.name)
-
 
 class Trigger(Bcfg2.Server.Plugin.Plugin,
               Bcfg2.Server.Plugin.ClientRunHooks,
@@ -26,6 +22,7 @@ class Trigger(Bcfg2.Server.Plugin.Plugin,
         Bcfg2.Server.Plugin.Plugin.__init__(self, core, datastore)
         Bcfg2.Server.Plugin.ClientRunHooks.__init__(self)
         Bcfg2.Server.Plugin.DirectoryBacked.__init__(self, self.data)
+        self.cmd = Executor()
 
     def async_run(self, args):
         """ Run the trigger script asynchronously in a forked process
@@ -38,14 +35,12 @@ class Trigger(Bcfg2.Server.Plugin.Plugin,
             if not dpid:
                 self.debug_log("Running %s" % " ".join(pipes.quote(a)
                                                        for a in args))
-                proc = Popen(args, stdin=PIPE, stdout=PIPE, stderr=PIPE)
-                err = proc.communicate()[1]
-                rv = proc.wait()
-                if rv != 0:
-                    self.logger.error("Trigger: Error running %s (%s): %s" %
-                                      (args[0], rv, err))
-                elif err:
-                    self.debug_log("Trigger: Error: %s" % err)
+                result = self.cmd.run(args)
+                if not result.success:
+                    self.logger.error("Trigger: Error running %s: %s" %
+                                      (args[0], result.error))
+                elif result.stderr:
+                    self.debug_log("Trigger: Error: %s" % result.stderr)
             os._exit(0)  # pylint: disable=W0212
 
     def end_client_run(self, metadata):
diff --git a/src/lib/Bcfg2/Utils.py b/src/lib/Bcfg2/Utils.py
index 3b1559528..29c27257a 100644
--- a/src/lib/Bcfg2/Utils.py
+++ b/src/lib/Bcfg2/Utils.py
@@ -183,9 +183,10 @@ class Executor(object):
 
         return _timeout
 
-    def run(self, command, inputdata=None, shell=False, timeout=None):
+    def run(self, command, inputdata=None, timeout=None, **kwargs):
         """ Run a command, given as a list, optionally giving it the
-        specified input data.
+        specified input data.  All additional keyword arguments are
+        passed through to :class:`subprocess.Popen`.
 
         :param command: The command to run, as a list (preferred) or
                         as a string.  See :class:`subprocess.Popen` for
@@ -193,8 +194,6 @@ class Executor(object):
         :type command: list or string
         :param inputdata: Data to pass to the command on stdin
         :type inputdata: string
-        :param shell: Run the given command in a shell (not recommended)
-        :type shell: bool
         :param timeout: Kill the command if it runs longer than this
                         many seconds.  Set to 0 or -1 to explicitly
                         override a default timeout.
@@ -206,9 +205,11 @@ class Executor(object):
         else:
             cmdstr = " ".join(command)
         self.logger.debug("Running: %s" % cmdstr)
-        proc = subprocess.Popen(command, shell=shell, bufsize=16384,
-                                stdin=subprocess.PIPE, stdout=subprocess.PIPE,
-                                stderr=subprocess.PIPE, close_fds=True)
+        args = dict(shell=False, bufsize=16384, close_fds=True)
+        args.update(kwargs)
+        args.update(stdin=subprocess.PIPE, stdout=subprocess.PIPE,
+                    stderr=subprocess.PIPE)
+        proc = subprocess.Popen(command, **args)
         if timeout is None:
             timeout = self.timeout
         if timeout is not None:
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgExternalCommandVerifier.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgExternalCommandVerifier.py
index 0f369113b..7ceedb7c2 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgExternalCommandVerifier.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgExternalCommandVerifier.py
@@ -21,35 +21,32 @@ from TestServer.TestPlugins.TestCfg.Test_init import TestCfgVerifier
 class TestCfgExternalCommandVerifier(TestCfgVerifier):
     test_obj = CfgExternalCommandVerifier
 
-    @patch("Bcfg2.Server.Plugins.Cfg.CfgExternalCommandVerifier.Popen")
-    def test_verify_entry(self, mock_Popen):
-        proc = Mock()
-        mock_Popen.return_value = proc
-        proc.wait.return_value = 0
-        proc.communicate.return_value = ("stdout", "stderr")
+    def test_verify_entry(self):
         entry = lxml.etree.Element("Path", name="/test.txt")
         metadata = Mock()
 
         ecv = self.get_obj()
         ecv.cmd = ["/bin/bash", "-x", "foo"]
+        ecv.exc = Mock()
+        ecv.exc.run.return_value = Mock()
+        ecv.exc.run.return_value.success = True
+
         ecv.verify_entry(entry, metadata, "data")
-        self.assertEqual(mock_Popen.call_args[0], (ecv.cmd,))
-        proc.communicate.assert_called_with(input="data")
-        proc.wait.assert_called_with()
+        ecv.exc.run.assert_called_with(ecv.cmd, inputdata="data")
 
-        mock_Popen.reset_mock()
-        proc.wait.return_value = 13
+        ecv.exc.reset_mock()
+        ecv.exc.run.return_value.success = False
         self.assertRaises(CfgVerificationError,
                           ecv.verify_entry, entry, metadata, "data")
-        self.assertEqual(mock_Popen.call_args[0], (ecv.cmd,))
-        proc.communicate.assert_called_with(input="data")
-        proc.wait.assert_called_with()
+        ecv.exc.run.assert_called_with(ecv.cmd, inputdata="data")
+
+        ecv.exc.reset_mock()
 
-        mock_Popen.reset_mock()
-        mock_Popen.side_effect = OSError
+        ecv.exc.reset_mock()
+        ecv.exc.run.side_effect = OSError
         self.assertRaises(CfgVerificationError,
                           ecv.verify_entry, entry, metadata, "data")
-        self.assertEqual(mock_Popen.call_args[0], (ecv.cmd,))
+        ecv.exc.run.assert_called_with(ecv.cmd, inputdata="data")
 
     @patch("os.access")
     def test_handle_event(self, mock_access):
diff --git a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py
index 2982825a9..6cfd2f666 100644
--- a/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py
+++ b/testsuite/Testsrc/Testlib/TestServer/TestPlugins/TestCfg/TestCfgPrivateKeyCreator.py
@@ -98,24 +98,23 @@ class TestCfgPrivateKeyCreator(TestCfgCreator, TestStructFile):
 
     @patch("shutil.rmtree")
     @patch("tempfile.mkdtemp")
-    @patch("subprocess.Popen")
-    def test__gen_keypair(self, mock_Popen, mock_mkdtemp, mock_rmtree):
+    def test__gen_keypair(self, mock_mkdtemp, mock_rmtree):
         pkc = self.get_obj()
+        pkc.cmd = Mock()
         pkc.XMLMatch = Mock()
         mock_mkdtemp.return_value = datastore
         metadata = Mock()
 
-        proc = Mock()
-        proc.wait.return_value = 0
-        proc.communicate.return_value = MagicMock()
-        mock_Popen.return_value = proc
+        exc = Mock()
+        exc.success = True
+        pkc.cmd.run.return_value = exc
 
         spec = lxml.etree.Element("PrivateKey")
         pkc.XMLMatch.return_value = spec
 
         def reset():
             pkc.XMLMatch.reset_mock()
-            mock_Popen.reset_mock()
+            pkc.cmd.reset_mock()
             mock_mkdtemp.reset_mock()
             mock_rmtree.reset_mock()
 
@@ -123,10 +122,9 @@ class TestCfgPrivateKeyCreator(TestCfgCreator, TestStructFile):
                          os.path.join(datastore, "privkey"))
         pkc.XMLMatch.assert_called_with(metadata)
         mock_mkdtemp.assert_called_with()
-        self.assertItemsEqual(mock_Popen.call_args[0][0],
-                              ["ssh-keygen", "-f",
-                               os.path.join(datastore, "privkey"),
-                               "-t", "rsa", "-N", ""])
+        pkc.cmd.run.assert_called_with(["ssh-keygen", "-f",
+                                        os.path.join(datastore, "privkey"),
+                                        "-t", "rsa", "-N", ""])
 
         reset()
         lxml.etree.SubElement(spec, "Params", bits="768", type="dsa")
@@ -137,13 +135,12 @@ class TestCfgPrivateKeyCreator(TestCfgCreator, TestStructFile):
                          os.path.join(datastore, "privkey"))
         pkc.XMLMatch.assert_called_with(metadata)
         mock_mkdtemp.assert_called_with()
-        self.assertItemsEqual(mock_Popen.call_args[0][0],
-                              ["ssh-keygen", "-f",
-                               os.path.join(datastore, "privkey"),
-                               "-t", "dsa", "-b", "768", "-N", "foo"])
+        pkc.cmd.run.assert_called_with(["ssh-keygen", "-f",
+                                        os.path.join(datastore, "privkey"),
+                                        "-t", "dsa", "-b", "768", "-N", "foo"])
 
         reset()
-        proc.wait.return_value = 1
+        pkc.cmd.run.return_value.success = False
         self.assertRaises(CfgCreationError, pkc._gen_keypair, metadata)
         mock_rmtree.assert_called_with(datastore)