Merge lp:~nuclearbob/utah/exception-overhaul into lp:utah

[Max Brustkern]

I've made a lot of changes to exception handling to attempt to address a number of issues raised in the code review.

[lp:~nuclearbob/utah/exception-overhaul]

833. By Javier Collado on 2013-03-15

Merged changes to add a timeout parameter to the retry function

Instead of calling time.sleep in every function that is called through retry,
now is retry itself that takes care of that using a new parameter.

Source branch: lp:~javier.collado/utah/retry-timeout

834. By Javier Collado on 2013-03-15

Merged changes to add reboot support (LP: #1086450)

Source branch: lp:~javier.collado/utah/reboot-support
Based on: lp:~nuclearbob/utah/reboot-support

[Javier Collado (javier.collado)]

It looks nice.

The only issue I've seen that was partially fixed in another merge proposal
is that this:

try:
    self.external = kw.pop('external')
except KeyError:
    self.external = False

can be replaced with this:

self.external = kw.pop('external', False)

[lp:~nuclearbob/utah/exception-overhaul]

835. By Max Brustkern on 2013-03-15

Rebasing on dev branch

836. By Max Brustkern on 2013-03-15

Resolved merge conflicts

[Max Brustkern (nuclearbob)]

Merge conflicts should be fixed. Javier's suggestion should be addressed. I also went around and added handling for most of the rest of the subprocess calls. I thought about trying to find every OS call, but I think those generally give reasonably clear error on their own. If we think it would be beneficial to wrap those, I can.

review: Needs Resubmitting

[lp:~nuclearbob/utah/exception-overhaul]

837. By Max Brustkern on 2013-03-15

Overhauled exception handling based on code review

[Javier Collado (javier.collado)]

I've tested the changes running the `pass.run` runlist with a desktop amd64
image and everything worked fine.

Anyway, I'd like to make a couple of comments:

- I don't think we should create a new version (0.9ubuntu2) in the changelog.
  Since the old one (0.9ubuntu1) is still unreleased (i.e. not available in the
  stable PPA), what I think we should do is keep on adding entries to the same
  version.

- I'm confused about the commit messages in revs. 835 and 836. If you run `bzr
  rebase /path/to/dev` what you get is all the revisions in your branch
  replayed on top of the new version of the dev branch, that is, including all
  changes that were added since you started working on your own branch. Once the
  rebasing is done, there are not conflicts because otherwise the rebasing
  doesn't succeed, so no merging is needed.

[Max Brustkern (nuclearbob)]

I'll decrement the version. I may have done the rebase incorrectly. I tried running the rebase command and then pushing my local branch to launchpad, and it said that the branches had diverged, so I merged my launchpad branch into my local branch, which generated conflicts. Do I need to just overwrite the launchpad branch when doing that in the future?

[lp:~nuclearbob/utah/exception-overhaul]

838. By Max Brustkern on 2013-03-18

Edited changelod

[Javier Collado (javier.collado)]

Yes, after the rebase you need to overwrite the launchpad branch.

[Javier Collado (javier.collado)]

I think everything looks nice. Thanks.

review: Approve

[lp:~nuclearbob/utah/exception-overhaul]

839. By Max Brustkern on 2013-03-18

Currently, unpacking the initrd sometimes exits with status 512 even if it works. We'll want to address this later, and already have a plan to do so, but for now, we'll just log that so things continue to work

[Max Brustkern (nuclearbob)]

I ran into problems testing this; the pipe to unpack the initrd was exiting with status 512 for a gzipped initrd, even when things unpacked fine. Since we already have a plan to overhaul the use of pipes, I changed the exception to a log message there, and I'm retesting now.

[lp:~nuclearbob/utah/exception-overhaul]

840. By Max Brustkern on 2013-03-18

Added initrd exception handling to BambooFeederMachine

841. By Max Brustkern on 2013-03-18

Added handling on IOError in NFS-editing pipe

[Max Brustkern (nuclearbob)]

I fixed the issue I had in testing and added handling for possible pipe exceptions in other places. I think this should be good to go if nobody has further objections.

review: Needs Resubmitting

[Andy Doan (doanac)]

> I ran into problems testing this; the pipe to unpack the initrd was exiting
> with status 512 for a gzipped initrd, even when things unpacked fine. Since
> we already have a plan to overhaul the use of pipes, I changed the exception
> to a log message there, and I'm retesting now.

It looks like your patch ignores all non-zero codes. That seems a little heavy handed - should we just ensure the exit code is 0 or 512?

[Max Brustkern (nuclearbob)]

Since we're planning to get rid of pipes soon anyway, and we've been ignoring all codes up to this point since we started using pipes, I think just logging any non-zero code is okay. If we want to track other codes, we should check all currently supported images on a couple of different systems and see what they return.

[Andy Doan (doanac)]

On 03/18/2013 10:26 AM, Max Brustkern wrote:
> Since we're planning to get rid of pipes soon anyway, and we've been ignoring all codes up to this point since we started using pipes, I think just logging any non-zero code is okay. If we want to track other codes, we should check all currently supported images on a couple of different systems and see what they return.
>
Good point - your approach limits the chance of a regression.

=== modified file 'debian/changelog'
--- debian/changelog	2013-03-01 14:18:10 +0000
+++ debian/changelog	2013-03-18 14:45:38 +0000
@@ -12,6 +12,7 @@
 
   [ Max Brustkern ]
   * Fixed ProvisionedMachine template support (LP: #1147306)
+  * Overhauled exception handling based on code review
 
 
  -- Max Brustkern <[e-mail]>  Thu, 07 Mar 2013 17:11:12 -0500

=== modified file 'tests/test_rsyslog.py'
--- tests/test_rsyslog.py	2013-03-05 21:09:24 +0000
+++ tests/test_rsyslog.py	2013-03-18 14:45:38 +0000
@@ -202,7 +202,8 @@
         with tempfile.NamedTemporaryFile() as f:
             r = RSyslog("utah-test", '/tmp', f.name)
             threading.Thread(
-                target=self.file_producer, args=(f, messages, truncate)).start()
+                target=self.file_producer,
+                args=(f, messages, truncate)).start()
             r.wait_for_booted(steps)
 
     def test_usefile(self):

=== modified file 'utah/client/exceptions.py'
--- utah/client/exceptions.py	2012-12-03 14:02:18 +0000
+++ utah/client/exceptions.py	2013-03-18 14:45:38 +0000
@@ -27,7 +27,7 @@
 class BadDir(UTAHClientError):
     """
     Raised when some test directory isn't found
-    or and error happens when trying to change to it
+    or an error happens when trying to change to it
     """
 
 

=== modified file 'utah/client/runner.py'
--- utah/client/runner.py	2013-03-15 13:31:22 +0000
+++ utah/client/runner.py	2013-03-18 14:45:38 +0000
@@ -13,41 +13,38 @@
 # You should have received a copy of the GNU General Public License along
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+
 import datetime
+import jsonschema
+import os
+import shutil
+import stat
 import subprocess
+import urllib
 
+from utah.client import exceptions
+from utah.client.battery import battery
 from utah.client.common import (
+    BzrHandler,
     CMD_TC_REBOOT,
     DATE_FORMAT,
+    DEFAULT_STATE_FILE,
     DEFAULT_TSLIST,
     DefaultValidator,
+    DevHandler,
+    GitHandler,
     make_result,
+    MASTER_RUNLIST,
+    parse_yaml_file,
+    ReturnCodes,
+    UTAH_DIR,
 )
 from utah.client.result import Result
+from utah.client.state_agent import StateAgentYAML
 from utah.client.testsuite import TestSuite
-from utah.client.state_agent import StateAgentYAML
-from utah.client import exceptions
 from utah.exceptions import UTAHException
+from utah.retry import retry
 from utah.timeout import timeout
-from utah.retry import retry
-from utah.client.battery import battery
-
-import os
-import shutil
-import stat
-import urllib
-import jsonschema
-
-from utah.client.common import (
-    MASTER_RUNLIST,
-    UTAH_DIR,
-    DEFAULT_STATE_FILE,
-    ReturnCodes,
-    parse_yaml_file,
-    BzrHandler,
-    DevHandler,
-    GitHandler,
-)
 
 RC_LOCAL = '/etc/rc.local'
 RC_LOCAL_BACKUP = '%s-utah.bak' % RC_LOCAL
@@ -167,11 +164,17 @@
 
         # create the testsuite directory
         if not os.path.exists(self.testsuitedir):
-            os.mkdir(self.testsuitedir)
+            try:
+                os.mkdir(self.testsuitedir)
+            except OSError as err:
+                raise exceptions.BadDir(err)
 
         # If no runlist is supplied look for one in the testdir
         if runlist is None:
             runlist = os.path.join(testdir, 'master.run')
+        if not os.path.isfile(runlist):
+            raise exceptions.MissingFile(
+                'Runlist {} does not exist'.format(runlist))
 
         self.master_runlist = runlist
         self.suites = []
@@ -222,8 +225,9 @@
         try:
             os.rename(RC_LOCAL, RC_LOCAL_BACKUP)
         except OSError as e:
-            if e.errno in [13]:
-                pass
+            if e.errno != 13:
+                raise exceptions.UTAHClientError(
+                    'Failed to backup rc.local: {}'.format(e))
 
     def reset_rc_local(self):
         """
@@ -239,8 +243,9 @@
             elif os.path.exists(RC_LOCAL):
                 os.remove(RC_LOCAL)
         except OSError as e:
-            if e.errno in [13]:
-                pass
+            if e.errno != 13:
+                raise exceptions.UTAHClientError(
+                    'Failed to backup rc.local: {}'.format(e))
 
     def setup_rc_local(self, rc_local=RC_LOCAL, runlist=None):
         """
@@ -250,11 +255,14 @@
 
         self.rc_local_content = rc_local_content
 
-        fp = open(rc_local, 'w')
-        fp.write(self.rc_local_content.format(runlist=runlist))
-        fp.close()
-
-        os.chmod(rc_local, stat.S_IRWXU | stat.S_IROTH | stat.S_IRGRP)
+        try:
+            with open(rc_local, 'w') as fp:
+                fp.write(self.rc_local_content.format(runlist=runlist))
+                fp.close()
+                os.fchmod(fp, stat.S_IRWXU | stat.S_IROTH | stat.S_IRGRP)
+        except (IOError, OSError) as err:
+            raise exceptions.UTAHClientError(
+                'Error setting up rc.local: {}'.format(err))
 
     def process_results(self):
         # Add stats to the results
@@ -281,7 +289,10 @@
         self.status = "RUN"
         for suite in self.suites:
             # Start in self.testdir.
-            os.chdir(self.testsuitedir)
+            try:
+                os.chdir(self.testsuitedir)
+            except OSError as err:
+                raise exceptions.BadDir(err)
 
             if not suite.is_done():
                 keep_going = suite.run()
@@ -349,7 +360,12 @@
         if (os.path.exists(
                 self.master_runlist)
                 and self.master_runlist != self.backup_runlist):
-            shutil.copyfile(self.master_runlist, self.backup_runlist)
+            try:
+                shutil.copyfile(self.master_runlist, self.backup_runlist)
+            except OSError as err:
+                raise exceptions.UTAHClientError(
+                    'Failed to backup master runlist from {} to {}: {}'
+                    .format(self.master_runlist, self.backup_runlist, err))
 
         state = {
             'master_runlist': self.backup_runlist,
@@ -397,6 +413,10 @@
             local_filename = urllib.urlretrieve(runlist)[0]
         except IOError:
             raise exceptions.MissingFile(runlist)
+        except urllib.ContentTooShortError as err:
+            raise exceptions.MissingFile(
+                'Error when downloading {} (probably interrupted): {}'
+                .format(runlist, err))
 
         data = parse_yaml_file(local_filename)
         validator = DefaultValidator()
@@ -440,7 +460,10 @@
                 self.process_master_runlist(suite['include'])
                 continue
 
-            os.chdir(orig_dir)
+            try:
+                os.chdir(orig_dir)
+            except OSError as err:
+                raise exceptions.BadDir(err)
             fetch_method = suite['fetch_method']
             fetch_location = suite['fetch_location']
 
@@ -466,7 +489,8 @@
             suite_runlist = suite.get('runlist', DEFAULT_TSLIST)
 
             if name in seen:
-                raise Exception("%s duplicated in runlist" % name)
+                raise exceptions.BadMasterRunlist(
+                    "{} duplicated in runlist".format(name))
 
             # Fetch the testsuite. On resume don't remove the testsuite
             # directory.
@@ -475,9 +499,17 @@
                 # except on failures where it's easier
                 # to find troubleshoot permission problems
                 absolute_name = os.path.abspath(name)
-                shutil.rmtree(absolute_name)
+                try:
+                    shutil.rmtree(absolute_name)
+                except OSError as err:
+                    raise exceptions.UTAHClientError(
+                        'Error removing the testsuite directory {}: {}'
+                        .format(absolute_name, err))
             if not os.path.exists(name):
-                os.mkdir(name)
+                try:
+                    os.mkdir(name)
+                except OSError as err:
+                    raise exceptions.BadDir(err)
 
             def vcs_get_retriable():
                 result = vcs_handler.get(directory=name)
@@ -572,7 +604,11 @@
         self.backup_rc_local()
         self.setup_rc_local(runlist=self.backup_runlist)
 
-        subprocess.call(['shutdown', '-r', 'now'])
+        try:
+            subprocess.call(['shutdown', '-r', 'now'])
+        except OSError as err:
+            raise exceptions.UTAHClientError('Failed to reboot: {}'
+                                             .format(err))
 
         # End of execution
 

=== modified file 'utah/client/testcase.py'
--- utah/client/testcase.py	2013-02-11 19:56:06 +0000
+++ utah/client/testcase.py	2013-03-18 14:45:38 +0000
@@ -16,7 +16,10 @@
 """
 Testcase specific code.
 """
+
+
 import jsonschema
+import os
 
 from utah.client.common import (
     run_cmd,
@@ -125,7 +128,6 @@
             self.save_state_callback = _save_state_callback
 
         control_data = None
-        import os
         if _control_data is None:
             if os.path.exists(self.filename):
                 try:

=== modified file 'utah/client/testsuite.py'
--- utah/client/testsuite.py	2013-02-11 21:47:33 +0000
+++ utah/client/testsuite.py	2013-03-18 14:45:38 +0000
@@ -21,12 +21,18 @@
 import os
 import yaml
 
-from .common import run_cmd, parse_control_file
-from .common import DEFAULT_TSLIST, DEFAULT_TSCONTROL
-from .common import CMD_TS_BUILD, CMD_TS_SETUP, CMD_TS_CLEANUP
-from .common import do_nothing
-from .testcase import TestCase
-from .exceptions import MissingFile, ValidationError, YAMLEmptyFile
+from utah.client import exceptions
+from utah.client.common import (
+    CMD_TS_BUILD,
+    CMD_TS_CLEANUP,
+    CMD_TS_SETUP,
+    DEFAULT_TSCONTROL,
+    DEFAULT_TSLIST,
+    do_nothing,
+    parse_control_file,
+    run_cmd,
+)
+from utah.client.testcase import TestCase
 
 
 def parse_runlist_file(runlist_file):
@@ -94,12 +100,15 @@
         self.timeout = timeout
         self.battery_measurements = battery_measurements
 
-        # work from within the testsuite's directory.
-        # eg. /var/lib/utah/examples
-        if not os.path.exists(self.path):
-            os.mkdir(self.path)
+        try:
+            # work from within the testsuite's directory.
+            # eg. /var/lib/utah/examples
+            if not os.path.exists(self.path):
+                os.mkdir(self.path)
 
-        os.chdir(self.path)
+            os.chdir(self.path)
+        except OSError as err:
+            raise exceptions.BadDir(err)
         # Build a TestSuite from a control file's data.
         control_file = os.path.join(name, control_file)
 
@@ -123,12 +132,12 @@
             try:
                 control_data = parse_control_file(self.control_file,
                                                   self.CONTROL_SCHEMA)
-            except YAMLEmptyFile:
+            except exceptions.YAMLEmptyFile:
                 # Skip schema validation for empty file
                 # Schema allows an empty dictionary, but not None
                 control_data = None
             except jsonschema.ValidationError as exception:
-                raise ValidationError(
+                raise exceptions.ValidationError(
                     '{!r} test suite control file invalid: {!r}\n1'
                     'Detailed information: {}'
                     .format(self.name, self.control_file, exception))
@@ -146,12 +155,13 @@
             try:
                 runlist_data = parse_runlist_file(self.runlist_file)
             except jsonschema.ValidationError as exception:
-                raise ValidationError(
+                raise exceptions.ValidationError(
                     '{!r} test suite runlist invalid: {!r}\n'
                     'Detailed information: {}'
                     .format(self.name, self.runlist_file, exception))
         else:
-            raise MissingFile('File not found: {}'.format(self.runlist_file))
+            raise exceptions.MissingFile(
+                'File not found: {}'.format(self.runlist_file))
 
         for test in runlist_data:
             name = test['test']
@@ -243,7 +253,10 @@
         keep_going = True
 
         # Work from the testsuite directory
-        os.chdir(self.path)
+        try:
+            os.chdir(self.path)
+        except OSError as err:
+            raise exceptions.BadDir(err)
 
         result = self.result
 

=== modified file 'utah/config.py'
--- utah/config.py	2013-03-15 13:31:22 +0000
+++ utah/config.py	2013-03-18 14:45:38 +0000
@@ -42,8 +42,12 @@
     """
     dns = os.path.join('/', 'etc', 'utah', 'dns')
     if os.path.isfile(dns):
-        return (subprocess.Popen([dns], stdout=subprocess.PIPE)
-                .communicate()[0].strip())
+        try:
+            process = subprocess.Popen([dns], stdout=subprocess.PIPE)
+            output = process.communicate()[0].strip()
+            return output
+        except OSError:
+            return 'virbr0'
     else:
         return 'virbr0'
 
@@ -166,7 +170,8 @@
             'pattern': [
                 '.*ubiquity reboot',
                 '.*log-output -t ubiquity umount /target$',
-                '.*finish-install.d/94save-logs',  # needed for server bootspeed
+                # needed for server bootspeed
+                '.*finish-install.d/94save-logs',
                 '.*finish-install: umount',
                 '.*rsyslogd:.*exiting on signal 15.',
             ],
@@ -237,25 +242,28 @@
 # http://stackoverflow.com/questions/699325/
 # suppress-output-in-python-calls-to-executables
 with open(os.devnull, 'w') as fnull:
-    if subprocess.call(['which', 'dpkg-architecture'],
-                       stdout=fnull, stderr=fnull) == 0:
-        # If dpkg-architecture is available, default to current host arch
-        LOCALDEFAULTS['arch'] = subprocess.check_output(
-            ['dpkg-architecture', '-qDEB_HOST_ARCH']).strip()
-    else:
-        # Otherwise, default to i386
-        LOCALDEFAULTS['arch'] = 'i386'
+    # Default to i386
+    LOCALDEFAULTS['arch'] = 'i386'
+    try:
+        if subprocess.call(['which', 'dpkg-architecture'],
+                           stdout=fnull, stderr=fnull) == 0:
+            # If dpkg-architecture is available, default to current host arch
+            LOCALDEFAULTS['arch'] = subprocess.check_output(
+                ['dpkg-architecture', '-qDEB_HOST_ARCH']).strip()
+    except OSError:
+        pass
 
-# I'm not sure lsb_release is always around
-with open(os.devnull, 'w') as fnull:
-    if subprocess.call(['which', 'lsb_release'],
-                       stdout=fnull, stderr=fnull) == 0:
-        # If lsb_release is available, default to current series
-        LOCALDEFAULTS['series'] = subprocess.check_output(
-            ['lsb_release', '-sc']).strip()
-    else:
-        # Otherwise, default to precise
-        LOCALDEFAULTS['series'] = 'precise'
+    # Default to precise
+    LOCALDEFAULTS['series'] = 'precise'
+    # I'm not sure lsb_release is always around
+    try:
+        if subprocess.call(['which', 'lsb_release'],
+                           stdout=fnull, stderr=fnull) == 0:
+            # If lsb_release is available, default to current series
+            LOCALDEFAULTS['series'] = subprocess.check_output(
+                ['lsb_release', '-sc']).strip()
+    except (OSError, subprocess.CalledProcessError):
+        pass
 
 DEFAULTS.update(LOCALDEFAULTS)
 

=== modified file 'utah/exceptions.py'
--- utah/exceptions.py	2013-03-13 16:29:27 +0000
+++ utah/exceptions.py	2013-03-18 14:45:38 +0000
@@ -25,4 +25,5 @@
     """
     def __init__(self, *args, **kw):
         self.retry = kw.pop('retry', False)
+        self.external = kw.pop('external', False)
         super(UTAHException, self).__init__(*args, **kw)

=== modified file 'utah/iso.py'
--- utah/iso.py	2013-01-23 17:17:23 +0000
+++ utah/iso.py	2013-03-18 14:45:38 +0000
@@ -32,6 +32,24 @@
 from utah.exceptions import UTAHException
 
 
+class UTAHISOException(UTAHException):
+    pass
+
+
+def get_resource(method, url, *args, **kw):
+    try:
+        resource = method(url, *args, **kw)
+        return resource
+    except IOError as err:
+        raise UTAHISOException(
+            'IOError when downloading {}: {}'
+            .format(url, err), external=True)
+    except urllib.ContentTooShortError as err:
+        raise UTAHISOException(
+            'Error when downloading {} (probably interrupted): {}'
+            .format(url, err), external=True)
+
+
 class ISO(object):
     """
     Provide a simplified method of interfacing with images.
@@ -64,7 +82,8 @@
 
         self.percent = 0
         self.logger.info('Preparing image: ' + path)
-        self.image = urllib.urlretrieve(path, reporthook=self.dldisplay)[0]
+        self.image = get_resource(urllib.urlretrieve, path,
+                                  reporthook=self.dldisplay)[0]
         self.logger.debug(path + ' is locally available as ' + self.image)
         self.installtype = self.getinstalltype()
         if self.installtype == 'mini':
@@ -178,10 +197,19 @@
         cmd = ['bsdtar', '-t', '-f', self.image]
         self.logger.debug('bsdtar list command: ' + ' '.join(cmd))
         if returnlist:
-            return subprocess.check_output(cmd).strip().split('\n')
+            try:
+                proc = subprocess.check_output(cmd).strip().split('\n')
+            except (OSError, subprocess.CalledProcessError) as err:
+                raise UTAHISOException('Error listing files in ISO: {}'
+                                       .format(err))
         else:
-            return subprocess.Popen(cmd, stdout=subprocess.PIPE,
-                                    stderr=subprocess.PIPE).communicate()
+            try:
+                proc = subprocess.Popen(cmd, stdout=subprocess.PIPE,
+                                        stderr=subprocess.PIPE).communicate()
+            except OSError as err:
+                raise UTAHISOException('Error listing files in ISO: {}'
+                                       .format(err))
+        return proc
 
     def getrealfile(self, filename):
         """Return a command to safely extract a file from an ISO.
@@ -198,17 +226,16 @@
         self.logger.debug('bsdtar list command: ' + ' '.join(cmd))
         try:
             output = subprocess.check_output(cmd)
-        except subprocess.CalledProcessError:
-            raise UTAHException('Cannot list {path} in {image}'
-                                .format(path=filename, image=self.image))
+        except (OSError, subprocess.CalledProcessError) as err:
+            raise UTAHISOException('Cannot list {} in {}: {}'
+                                   .format(filename, self.image, err))
         try:
             columns = output.splitlines()[0].split()
             realfile = columns[-1]
-        except (IndexError, subprocess.CalledProcessError):
-            raise UTAHException('Cannot parse bsdtar list output '
-                                'for {path} in {image}: {output}'
-                                .format(path=filename, image=self.image,
-                                        output=output))
+        except IndexError as err:
+            raise UTAHISOException('Cannot parse bsdtar list output '
+                                   'for {} in {}: {}'
+                                   .format(filename, self.image, output))
         cmd = ['bsdtar', '-x', '-f', self.image, '-O', realfile]
         self.logger.debug('bsdtar extract command: ' + ' '.join(cmd))
         return cmd
@@ -231,9 +258,9 @@
         cmd = self.getrealfile(filename)
         try:
             stdout = subprocess.check_output(cmd, **kw)
-        except subprocess.CalledProcessError:
-            raise UTAHException('Cannot extract {path} from {image}'
-                                .format(path=filename, image=self.image))
+        except (OSError, subprocess.CalledProcessError) as err:
+            raise UTAHISOException('Cannot extract {} from {}: {}'
+                                   .format(filename, self.image, err))
 
         if outfile is None:
             outfile = os.path.join(outdir, filename)
@@ -260,9 +287,9 @@
         cmd = self.getrealfile(filename)
         try:
             stdout = subprocess.check_output(cmd, **kw)
-        except subprocess.CalledProcessError:
-            raise UTAHException('Cannot extract {path} from {image}'
-                                .format(path=filename, image=self.image))
+        except (OSError, subprocess.CalledProcessError) as err:
+            raise UTAHISOException('Cannot extract {} from {}: {}'
+                                   .format(filename, self.image, err))
 
         return stdout
 
@@ -296,26 +323,35 @@
         self.logger.info('Attempting to retrieve ' + filename)
         with open(os.devnull, "w") as fnull:
             # If dlcommand (default dl-ubuntu-test-iso) is available, use it
-            if subprocess.call(['which', config.dlcommand],
-                               stdout=fnull, stderr=fnull) == 0:
-                self.logger.debug('Using ' + config.dlcommand)
-                if installtype == 'server':
-                    flavor = 'ubuntu-server'
-                else:
-                    flavor = 'ubuntu'
-                cmd = [config.dlcommand,
-                       '-q',
-                       '--flavor=' + flavor,
-                       '--release=' + series,
-                       '--variant=' + installtype,
-                       '--arch=' + arch,
-                       '--isoroot=' + config.isodir]
-                self.logger.info('Downloading ISO')
-                self.logger.debug(' '.join(cmd))
-                subprocess.check_call(cmd)
-                path = os.path.join(config.isodir, flavor, filename)
-                if os.path.isfile(path):
-                    return path
+            cmd = ['which', config.dlcommand]
+            try:
+                if subprocess.call(cmd,
+                                   stdout=fnull, stderr=fnull) == 0:
+                    self.logger.debug('Using ' + config.dlcommand)
+                    if installtype == 'server':
+                        flavor = 'ubuntu-server'
+                    else:
+                        flavor = 'ubuntu'
+                    cmd = [config.dlcommand,
+                           '-q',
+                           '--flavor=' + flavor,
+                           '--release=' + series,
+                           '--variant=' + installtype,
+                           '--arch=' + arch,
+                           '--isoroot=' + config.isodir]
+                    self.logger.info('Downloading ISO')
+                    self.logger.debug(' '.join(cmd))
+                    try:
+                        subprocess.check_call(cmd)
+                    except (OSError, subprocess.CalledProcessError) as err:
+                        raise UTAHISOException(
+                            "Failed to download ISO using '{}': {}"
+                            .format(' '.join(cmd), err))
+                    path = os.path.join(config.isodir, flavor, filename)
+                    if os.path.isfile(path):
+                        return path
+            except OSError as err:
+                self.logger.warning('{} failed: {}'.format(cmd, err))
 
         # If we haven't returned, dlcommand didn't give us an image
         # We'll need to use the utah image cache in config.isodir
@@ -339,7 +375,9 @@
 
             # Scan the MD5SUMS file for our file
             servermd5 = None
-            for line in urllib.urlopen(remotepath + '/MD5SUMS'):
+            md5path = '{}/MD5SUMS'.format(remotepath)
+            md5list = get_resource(urllib.urlopen, md5path)
+            for line in md5list:
                 if isopattern in line:
                     servermd5, isofile = line.split()
                     isofile = isofile.strip('*')
@@ -347,8 +385,9 @@
 
             # If we can't find the ISO, raise an exception
             if servermd5 is None:
-                raise UTAHException('Specified ISO: ' + filename
-                                    + ' not found on mirrors.')
+                raise UTAHISOException(
+                    'Specified ISO: {} not found on mirrors.'
+                    .format(filename), external=True)
             self.logger.debug('Server md5 is ' + servermd5)
 
             # If the ISO exists locally and the md5 matches, use it
@@ -360,8 +399,8 @@
             isopath = os.path.join(remotepath, isofile)
             self.percent = 0
             self.logger.info('Attempting to download ' + isopath)
-            temppath = urllib.urlretrieve(isopath,
-                                          reporthook=self.dldisplay)[0]
+            temppath = get_resource(urllib.urlretrieve(isopath,
+                                    reporthook=self.dldisplay))[0]
 
             # Try to copy it into our cache
             self.logger.debug('Copying ' + temppath + ' to ' + path)
@@ -370,8 +409,9 @@
             if os.path.isfile(path) and servermd5 == self.getmd5(path):
                 return path
             else:
-                raise UTAHException('Image failed to download after ' +
-                                    config.dlretries + ' tries')
+                raise UTAHISOException(
+                    'Image failed to download after {} tries'
+                    .format(config.dlretries), external=True)
 
     def kernelpath(self):
         kernelpath = './install/vmlinuz'

=== modified file 'utah/parser.py'
--- utah/parser.py	2012-12-06 16:30:34 +0000
+++ utah/parser.py	2013-03-18 14:45:38 +0000
@@ -164,7 +164,16 @@
         data = None
 
         if logfile.startswith('http'):
-            fp = urllib2.urlopen(logfile)
+            try:
+                fp = urllib2.urlopen(logfile)
+            except IOError as err:
+                raise ParserError(
+                    'IOError when downloading {}: {}'
+                    .format(logfile, err))
+            except urllib2.ContentTooShortError as err:
+                raise ParserError(
+                    'Error when downloading {} (probably interrupted): {}'
+                    .format(logfile, err))
             data = self._parse_stream(fp)
         else:
             with open(logfile, 'r') as fp:

=== modified file 'utah/provisioning/baremetal/bamboofeeder.py'
--- utah/provisioning/baremetal/bamboofeeder.py	2013-03-13 16:36:02 +0000
+++ utah/provisioning/baremetal/bamboofeeder.py	2013-03-18 14:45:38 +0000
@@ -95,10 +95,12 @@
         self.imagedir = os.path.join(self.tmpdir, 'image.d')
         if not os.path.isdir(self.imagedir):
             os.makedirs(self.imagedir)
-        self.sector = int(
-            subprocess.check_output(
-                ['file', self.wwwimage]
-            ).strip().split(';')[1].split(',')[3].split()[1])
+        try:
+            output = subprocess.check_output(['file', self.wwwimage]).strip()
+            self.sector = output.split(';')[1].split(',')[3].split()[1]
+        except (OSError, subprocess.CalledProcessError) as err:
+            raise UTAHBMProvisioningException('Failed to get image info: {}'
+                                              .format(err))
         self.logger.debug('Image start sector is ' + str(self.sector))
         self.cleanfunction(self._umountimage)
         self.logger.debug('Mounting ' + self.wwwimage + ' at ' +
@@ -126,11 +128,15 @@
             os.makedirs(os.path.join(self.tmpdir, 'initrd.d'))
         os.chdir(os.path.join(self.tmpdir, 'initrd.d'))
         filesize = os.path.getsize(self.initrd)
-        for line in subprocess.check_output(
-                ['mkimage', '-l', self.initrd]).splitlines():
-            if 'Data Size:' in line:
-                datasize = int(line.split()[2])
-                break
+        try:
+            for line in subprocess.check_output(
+                    ['mkimage', '-l', self.initrd]).splitlines():
+                if 'Data Size:' in line:
+                    datasize = int(line.split()[2])
+                    break
+        except (OSError, subprocess.CalledProcessError) as err:
+            raise UTAHBMProvisioningException('Failed to get initrd info: {}'
+                                              .format(err))
         headersize = filesize - datasize
         self.logger.debug('uInitrd header size is ' + str(headersize))
         pipe = pipes.Template()
@@ -138,7 +144,12 @@
                      ' 2>/dev/null', 'f-')
         pipe.append('gunzip', '--')
         pipe.append('cpio -ivd 2>/dev/null', '-.')
-        pipe.copy(self.initrd, '/dev/null')
+        exitstatus = pipe.copy(self.initrd, '/dev/null')
+        if exitstatus != 0:
+            # Currently this comes up as 512 when things seem fine
+            # TODO: refactor this and check for codes at all stages
+            self.logger.debug(
+                'Unpacking initrd exited with status {}'.format(exitstatus))
 
     def _repackinitrd(self):
         """
@@ -152,7 +163,10 @@
         pipe.append('gzip -9fc', '--')
         initrd = os.path.join(self.tmpdir, 'initrd.gz')
         self.logger.debug('Repacking compressed initrd')
-        pipe.copy('/dev/null', initrd)
+        exitstatus = pipe.copy('/dev/null', initrd)
+        if exitstatus != 0:
+            raise UTAHBMProvisioningException(
+                'Unpacking initrd exited with status: {}'.format(exitstatus))
         self.logger.debug('Creating uInitrd with mkimage')
         self._runargs(['mkimage',
                        '-A', 'arm',

=== modified file 'utah/provisioning/baremetal/cobbler.py'
--- utah/provisioning/baremetal/cobbler.py	2013-03-13 16:36:46 +0000
+++ utah/provisioning/baremetal/cobbler.py	2013-03-18 14:45:38 +0000
@@ -84,10 +84,8 @@
         # (only if we'll be using cobbler for a while longer)
         machines = self._cobble(['system', 'find'])['output'].splitlines()
         if self.name not in machines:
-            raise UTAHBMProvisioningException('No machine named ' + self.name
-                                              + ' exists in cobbler')
-        else:
-            return True
+            raise UTAHBMProvisioningException(
+                'No machine named {} exists in cobbler'.format(self.name))
 
     def _create(self, checktimeout=config.checktimeout,
                 installtimeout=config.installtimeout):
@@ -165,8 +163,12 @@
             pipe = pipes.Template()
             pipe.append('sudo tee -a ' + str(config.nfsconfigfile) +
                         ' >/dev/null', '-.')
-            pipe.open('/dev/null', 'w').write(self.isodir + ' ' +
-                                              config.nfsoptions + "\n")
+            try:
+                pipe.open('/dev/null', 'w').write(self.isodir + ' ' +
+                                                  config.nfsoptions + "\n")
+            except IOError as err:
+                raise UTAHBMProvisioningException(
+                    'Failed to setup NFS: {}'.format(err))
             self.logger.debug('Reloading NFS config')
             self._runargs(config.nfscommand + ['reload'])
             self.logger.debug('Adding NFS boot options')
@@ -191,7 +193,8 @@
                         for arg in self.machineinfo])
 
         self.restart()
-        self.rsyslog.wait_for_install(config.install_steps, self._disable_netboot)
+        self.rsyslog.wait_for_install(config.install_steps,
+                                      self._disable_netboot)
         self.rsyslog.wait_for_booted(config.boot_steps)
 
         if self.installtype == 'desktop':
@@ -216,11 +219,10 @@
         Pull out all cobbler commands so that we can later support changing
         users, remote login, not sudo, etc.
         """
-        tail_process = \
-            subprocess.Popen(['tail', '-n0', '-f',
-                              '/var/log/cobbler/cobbler.log'],
-                             stdout=subprocess.PIPE)
         try:
+            tail_process = subprocess.Popen(
+                ['tail', '-n0', '-f', '/var/log/cobbler/cobbler.log'],
+                stdout=subprocess.PIPE)
             command_results = self._runargs(['sudo', 'cobbler'] + cmd)
             retcode = command_results['returncode']
             if retcode != 0:
@@ -233,6 +235,8 @@
                     raise UTAHBMProvisioningException(error_msg)
             else:
                 return command_results
+        except (OSError, subprocess.CalledProcessError) as err:
+            self.logger.error('Failed to get cobbler logs: {}'.format(err))
         finally:
             # tail process will wait forever because of the -f/--follow option,
             # so it has to be terminated in order to read from stdout.

=== modified file 'utah/provisioning/inventory/sqlite.py'
--- utah/provisioning/inventory/sqlite.py	2013-01-24 16:47:04 +0000
+++ utah/provisioning/inventory/sqlite.py	2013-03-18 14:45:38 +0000
@@ -32,8 +32,8 @@
         db = os.path.expanduser(db)
         super(SQLiteInventory, self).__init__(*args, uniqueid=db, **kw)
         self.db = db
-        self._connection = sqlite3.connect(self.db,
-                                           config.sqlite3_db_connection_timeout)
+        self._connection = sqlite3.connect(
+            self.db, config.sqlite3_db_connection_timeout)
         self._connection.isolation_level = None
         self._connection.row_factory = sqlite3.Row
 

=== modified file 'utah/provisioning/provisioning.py'
--- utah/provisioning/provisioning.py	2013-03-13 16:56:34 +0000
+++ utah/provisioning/provisioning.py	2013-03-18 14:45:38 +0000
@@ -755,9 +755,13 @@
             initrd = self.initrd
         if tmpdir is None:
             tmpdir = self.tmpdir
-        if not os.path.isdir(os.path.join(tmpdir, 'initrd.d')):
-            os.makedirs(os.path.join(tmpdir, 'initrd.d'))
-        os.chdir(os.path.join(tmpdir, 'initrd.d'))
+        try:
+            if not os.path.isdir(os.path.join(tmpdir, 'initrd.d')):
+                os.makedirs(os.path.join(tmpdir, 'initrd.d'))
+            os.chdir(os.path.join(tmpdir, 'initrd.d'))
+        except OSError as err:
+            raise UTAHProvisioningException(
+                'Error using temp directory {}: {}'.format(tmpdir, err))
         pipe = pipes.Template()
         if os.path.splitext(initrd)[1] == '.gz':
             self.logger.debug('Using gzip based on file extension')
@@ -765,8 +769,17 @@
         elif os.path.splitext(initrd)[1] == '.lz':
             self.logger.debug('Using lzma based on file extension')
             pipe.prepend('lzcat -S .lz $IN', 'f-')
+        else:
+            raise UTAHProvisioningException(
+                'initrd file does have have gz or lz extension: {}'
+                .format(initrd))
         pipe.append('cpio -ivd 2>/dev/null', '-.')
-        pipe.copy(initrd, '/dev/null')
+        exitstatus = pipe.copy(initrd, '/dev/null')
+        if exitstatus != 0:
+            # Currently this comes up as 512 when things seem fine
+            # TODO: refactor this and check for codes at all stages
+            self.logger.debug(
+                'Unpacking initrd exited with status {}'.format(exitstatus))
 
     def _setuplatecommand(self, tmpdir=None):
         """
@@ -943,11 +956,14 @@
         os.chmod(filename, 0755)
 
         orderfilename = os.path.join(casper_dir, 'ORDER')
-        self._runargs(['sed', '-i',
-                       '1i/scripts/casper-bottom/'
-                       'utah\\n'
-                       '[ -e /conf/param.conf ] && . /conf/param.conf',
-                       orderfilename])
+        exitstatus = self._runargs(
+            ['sed', '-i',
+             '1i/scripts/casper-bottom/'
+             'utah\\n'
+             '[ -e /conf/param.conf ] && . /conf/param.conf',
+             orderfilename])['returncode']
+        if exitstatus != 0:
+            raise UTAHProvisioningException('Failed to setup casper script')
 
         casper_file = os.path.join(tmpdir, 'initrd.d', 'etc', 'casper.conf')
         tmpfilename = casper_file + '.tmp'
@@ -1010,7 +1026,8 @@
             self.logger.debug('Using gzip because installtype is not desktop')
             pipe.append('gzip -9fc ', '--')
             initrd = os.path.join(tmpdir, 'initrd.gz')
-        pipe.copy('/dev/null', initrd)
+        if pipe.copy('/dev/null', initrd) != 0:
+            raise UTAHProvisioningException('Failed to repack initrd')
         return initrd
 
     def _cmdlinesetup(self, boot=None):

=== modified file 'utah/provisioning/ssh.py'
--- utah/provisioning/ssh.py	2013-03-15 13:31:22 +0000
+++ utah/provisioning/ssh.py	2013-03-18 14:45:38 +0000
@@ -95,24 +95,37 @@
         # http://od-eon.com/blogs/
         # stefan/automating-remote-commands-over-ssh-paramiko/
         self.ssh_logger.debug('Connecting SSH')
-        self.ssh_client.connect(self.name,
-                                username=user,
-                                key_filename=config.sshprivatekey)
-
-        self.ssh_logger.debug('Opening SSH session')
-        channel = self.ssh_client.get_transport().open_session()
-
-        self.ssh_logger.info('Running command through SSH: ' + commandstring)
-        stdout = channel.makefile('rb')
-        stderr = channel.makefile_stderr('rb')
-        if timeout is None:
-            channel.exec_command(commandstring)
-        else:
-            utah.timeout.timeout(timeout, channel.exec_command, commandstring)
-        retval = channel.recv_exit_status()
-
-        self.ssh_logger.debug('Closing SSH connection')
-        self.ssh_client.close()
+        retval = 'not run'
+        try:
+            self.ssh_client.connect(self.name,
+                                    username=user,
+                                    key_filename=config.sshprivatekey)
+
+            self.ssh_logger.debug('Opening SSH session')
+            channel = self.ssh_client.get_transport().open_session()
+
+            self.ssh_logger.info('Running command through SSH: {}'
+                                 .format(commandstring))
+            stdout = channel.makefile('rb')
+            stderr = channel.makefile_stderr('rb')
+            if timeout is None:
+                channel.exec_command(commandstring)
+            else:
+                utah.timeout.timeout(timeout, channel.exec_command,
+                                     commandstring)
+            retval = channel.recv_exit_status()
+
+        except paramiko.BadHostKeyException as err:
+            raise UTAHProvisioningException(
+                'Host key exception encountered; '
+                'machine may be in use by another process: {}'
+                .format(err), external=True)
+        except (paramiko.AuthenticationException, paramiko.SSHException,
+                socket.error) as err:
+            raise UTAHProvisioningException(err)
+        finally:
+            self.ssh_logger.debug('Closing SSH connection')
+            self.ssh_client.close()
 
         if retval == 0:
             self.ssh_logger.debug('Return code: {}'.format(retval))
@@ -146,12 +159,12 @@
             files = [files]
 
         self.activecheck()
-        self.ssh_client.connect(self.name,
-                                username=config.user,
-                                key_filename=config.sshprivatekey)
-        sftp_client = self.ssh_client.open_sftp()
         failed = []
         try:
+            self.ssh_client.connect(self.name,
+                                    username=config.user,
+                                    key_filename=config.sshprivatekey)
+            sftp_client = self.ssh_client.open_sftp()
             for localpath in files:
                 if os.path.isfile(localpath):
                     self.ssh_logger.info('Uploading {} from the host '
@@ -162,6 +175,14 @@
                     sftp_client.put(localpath, remotepath)
                 else:
                     failed.append(localpath)
+        except paramiko.BadHostKeyException as err:
+            raise UTAHProvisioningException(
+                'Host key exception encountered; '
+                'machine may be in use by another process: {}'
+                .format(err), external=True)
+        except (paramiko.AuthenticationException, paramiko.SSHException,
+                socket.error) as err:
+            raise UTAHProvisioningException(err)
         finally:
             sftp_client.close()
             if len(failed) > 0:
@@ -180,10 +201,6 @@
             files = [files]
 
         self.activecheck()
-        self.ssh_client.connect(self.name,
-                                username=config.user,
-                                key_filename=config.sshprivatekey)
-        sftp_client = self.ssh_client.open_sftp()
         if os.path.isdir(target):
             get_localpath = lambda remotepath: \
                 os.path.join(target, os.path.basename(remotepath))
@@ -191,12 +208,24 @@
             get_localpath = lambda remotepath: target
 
         try:
+            self.ssh_client.connect(self.name,
+                                    username=config.user,
+                                    key_filename=config.sshprivatekey)
+            sftp_client = self.ssh_client.open_sftp()
             for remotepath in files:
                 localpath = get_localpath(remotepath)
                 self.ssh_logger.info('Downloading {} from the machine '
                                      'to {} on the host'
                                      .format(remotepath, target))
                 sftp_client.get(remotepath, localpath)
+        except paramiko.BadHostKeyException as err:
+            raise UTAHProvisioningException(
+                'Host key exception encountered; '
+                'machine may be in use by another process: {}'
+                .format(err), external=True)
+        except (paramiko.AuthenticationException, paramiko.SSHException,
+                socket.error) as err:
+            raise UTAHProvisioningException(err)
         finally:
             sftp_client.close()
 
@@ -270,8 +299,18 @@
             self.ssh_client.connect(self.name,
                                     username=config.user,
                                     key_filename=config.sshprivatekey)
+        except paramiko.BadHostKeyException as err:
+            raise UTAHProvisioningException(
+                'Host key exception encountered; '
+                'machine may be in use by another process: {}'
+                .format(err), external=True)
+        except (paramiko.AuthenticationException,
+                paramiko.SSHException) as err:
+            raise UTAHProvisioningException(err)
         except socket.error as err:
             raise UTAHProvisioningException(str(err), retry=True)
+        finally:
+            self.ssh_client.close()
 
     def sshpoll(self, timeout=None,
                 checktimeout=config.checktimeout, logmethod=None):

=== modified file 'utah/run.py'
--- utah/run.py	2013-03-13 17:13:32 +0000
+++ utah/run.py	2013-03-18 14:45:38 +0000
@@ -151,7 +151,16 @@
     def _run(runlist):
         """Run a single runlist."""
         try:
-            runlist_local_path = urllib.urlretrieve(runlist)[0]
+            try:
+                runlist_local_path = urllib.urlretrieve(runlist)[0]
+            except IOError as err:
+                raise UTAHException(
+                    'IOError when downloading {}: {}'
+                    .format(runlist, err), external=True)
+            except urllib.ContentTooShortError as err:
+                raise UTAHException(
+                    'Error when downloading {} (probably interrupted): {}'
+                    .format(runlist, err), external=True)
             target_dir = os.path.normpath('/tmp')
             machine.uploadfiles([runlist_local_path], target_dir)
 
@@ -205,12 +214,20 @@
                                 suffix=report_type))
         report_local_path = os.path.join(config.logpath, log_filename)
         report_local_path = os.path.normpath(report_local_path)
-        machine.downloadfiles(report_remote_path, report_local_path)
-        logging.info('Test log copied to ' + report_local_path)
-        locallogs.append(report_local_path)
-        if args.dumplogs:
-            with open(report_local_path, 'r') as f:
-                print(f.read())
+        try:
+            machine.downloadfiles(report_remote_path, report_local_path)
+        except UTAHException:
+            logging.error('Test log not retrieved')
+        else:
+            logging.info('Test log copied to ' + report_local_path)
+            locallogs.append(report_local_path)
+            if args.dumplogs:
+                try:
+                    with open(report_local_path, 'r') as f:
+                        print(f.read())
+                except IOError as err:
+                    logging.warning('Test log could not be dumped: {}'
+                                    .format(err))
 
     # Write the machine name to standard out for log gathering
     print('Running on machine: ' + machine.name)
@@ -232,7 +249,7 @@
         if args.files is not None:
             try:
                 locallogs += getfiles(args, machine)
-            except Exception as err:
+            except UTAHException as err:
                 logging.warning('Failed to download files: ' + str(err))
 
     # Provisioned systems have an image already installed
@@ -245,8 +262,13 @@
             logging.debug('Capturing preseed due to config option')
         preseedfile = os.path.join(config.logpath,
                                    machine.name + '-preseed.cfg')
-        shutil.copyfile(machine.finalpreseed, preseedfile)
-        locallogs.append(preseedfile)
+        try:
+            shutil.copyfile(machine.finalpreseed, preseedfile)
+        except (IOError, shutil.Error) as err:
+            logging.warning('Failed to copy preseed file: {}'
+                            .format(err))
+        else:
+            locallogs.append(preseedfile)
 
     return exitstatus, locallogs
 
@@ -257,7 +279,11 @@
     else:
         outdir = args.outdir
     if not os.path.isdir(outdir):
-        os.makedirs(outdir)
+        try:
+            os.makedirs(outdir)
+        except OSError as err:
+            raise UTAHException('Failed to create output directory {}: {}'
+                                .format(outdir, err), external=True)
 
     machine.downloadfilesrecursive(args.files, outdir)
 

=== modified file 'utah/timeout.py'
--- utah/timeout.py	2013-03-04 13:09:18 +0000
+++ utah/timeout.py	2013-03-18 14:45:38 +0000
@@ -17,8 +17,9 @@
 Provide functionality to execute command with timeouts.
 """
 import os
+import signal
 import subprocess
-import signal
+import sys
 
 from utah.exceptions import UTAHException
 from utah.commandstr import commandstr
@@ -141,7 +142,7 @@
         raise UTAHTimeout(msg)
 
 
-def get_process_children(pid):
+def get_process_children(pid, logmethod=sys.stderr.write):
     """Find process children so they can be killed when the timeout expires.
 
     :param pid: Process ID for the parent process
@@ -150,8 +151,13 @@
     :rtype: list(int)
 
     """
-    p = subprocess.Popen('ps --no-headers -o pid --ppid %d' % pid, shell=True,
-                         stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-    stdout, _stderr = p.communicate()
-
-    return [int(pid) for pid in stdout.split()]
+    try:
+        p = subprocess.Popen('ps --no-headers -o pid --ppid %d' % pid,
+                             shell=True, stdout=subprocess.PIPE,
+                             stderr=subprocess.PIPE)
+        stdout, _stderr = p.communicate()
+        pids = [int(pid) for pid in stdout.split()]
+        return pids
+    except OSError as err:
+        logmethod('Could not kill process children: {}'.format(err))
+        return []