Merge lp:~javier.collado/utah/returncodes into lp:utah

[Javier Collado]

This branch intend is to clarify the client/server return codes and make it
possible to know from the server return code if a failure happened either at
the client or at the server.

The set of possible return codes is as follows:

- Client
PASS (0) - All test cases where executed and passed
FAIL (1) - All test cases where executed, but at least one of them failed
ERROR (2) - At least one error was detected that prevented a test case from
being executed.

- Server
SUCCESS (0) - No problems found
UTAH_EXCEPTION_ERROR (1) - UTAH exception caught
TIMEOUT_ERROR (2) - Unable to complete before config.jobtimeout seconds
GROUP_ERROR (3) - User isn't in UTAH group
FAIL (101) - All test cases where executed, but at least one of them failed
ERROR (102) - At least one error was detected that prevented a test case from
being executed.

Note that the convention followed in the server is that when the client returns
an error 100 is added to that code to make clear what errors came from the
server and what errors came from the client. This might be useful in jenkins
jobs in which the actions following the test run depend on the test execution
results.

To test this I've used different variations of the pass.run (and its testsuite)
to make sure that the codes above work as expected. Anyway, I haven't tested
some error codes in the server (1-3) as only the return code number might have
changed, but the code is almost the same as before the change.

[Andy Doan (doanac)]

Looks pretty good. It will be nice to consolidate run_* stuff to make changes like this easier :)

One thing I was wondering about: rather than using dictionaries for return codes would it be better to do something like:

class ReturnCode:
     SUCCESS = 0
     UTAH_EXCEPTION_ERROR = 1
...

    @staticmethod
    def client_error(rc):
        return 100 + rc

then there aren't hard-coded strings.

[lp:~javier.collado/utah/returncodes]

837. By Javier Collado on 2013-03-11

Refactored server return codes to use a class instead of a dictionary

Suggested by Andy

838. By Javier Collado on 2013-03-11

Refactored client return codes to use a class instead of a dictionary

Suggested by Andy

[Javier Collado (javier.collado)]

@Andy

I've updated the dictionaries to use classes instead. Thanks for the suggestion.

[Andy Doan (doanac)]

looks good to me.

=== modified file 'debian/changelog'
--- debian/changelog	2013-03-08 16:14:06 +0000
+++ debian/changelog	2013-03-11 10:10:26 +0000
@@ -6,10 +6,13 @@
   * Added installation failure log message (LP: #1126361)
   * Added check to make sure user is in the password database
     (LP: #1071020)
+  * Clarified return code values in server and client
+    (LP: #1025633, LP: #1133227)
 
   [ Max Brustkern ]
   * Fixed ProvisionedMachine template support (LP: #1147306)
 
+
  -- Max Brustkern <[e-mail]>  Thu, 07 Mar 2013 17:11:12 -0500
 
 utah (0.8ubuntu1) quantal; urgency=low

=== modified file 'examples/run_install_test.py'
--- examples/run_install_test.py	2013-02-15 08:36:32 +0000
+++ examples/run_install_test.py	2013-03-11 10:10:26 +0000
@@ -28,6 +28,7 @@
     run_tests,
     virtual_arguments,
     configure_logging,
+    ReturnCodes,
 )
 
 
@@ -52,13 +53,13 @@
 def run_install_test(args=None):
     if not check_user_group():
         print_group_error_message(__file__)
-        sys.exit(3)
+        sys.exit(ReturnCodes.GROUP_ERROR)
 
     if args is None:
         args = get_parser().parse_args()
 
     locallogs = []
-    exitstatus = 0
+    exitstatus = ReturnCodes.SUCCESS
     machine = None
 
     configure_logging(args.debug)
@@ -78,7 +79,7 @@
 
     except UTAHException as error:
         sys.stderr.write('Exception: ' + str(error))
-        exitstatus = 2
+        exitstatus = ReturnCodes.UTAH_EXCEPTION_ERROR
 
     finally:
         if not args.no_destroy and machine is not None:

=== modified file 'examples/run_test_bamboo_feeder.py'
--- examples/run_test_bamboo_feeder.py	2013-02-15 08:36:32 +0000
+++ examples/run_test_bamboo_feeder.py	2013-03-11 10:10:26 +0000
@@ -31,6 +31,7 @@
     name_argument,
     run_tests,
     configure_logging,
+    ReturnCodes,
 )
 
 
@@ -54,13 +55,13 @@
 def run_test_bamboo_feeder(args=None):
     if not check_user_group():
         print_group_error_message(__file__)
-        sys.exit(3)
+        sys.exit(ReturnCodes.GROUP_ERROR)
 
     if args is None:
         args = get_parser().parse_args()
 
     locallogs = []
-    exitstatus = 0
+    exitstatus = ReturnCodes.SUCCESS
     machine = None
 
     configure_logging(args.debug)
@@ -86,7 +87,7 @@
             logging.error(mesg)
         except (AttributeError, NameError):
             sys.stderr.write(mesg)
-        exitstatus = 2
+        exitstatus = ReturnCodes.UTAH_EXCEPTION_ERROR
 
     finally:
         if machine is not None:

=== modified file 'examples/run_test_cobbler.py'
--- examples/run_test_cobbler.py	2013-02-15 08:36:32 +0000
+++ examples/run_test_cobbler.py	2013-03-11 10:10:26 +0000
@@ -30,6 +30,7 @@
     name_argument,
     run_tests,
     configure_logging,
+    ReturnCodes,
 )
 
 
@@ -53,13 +54,13 @@
 def run_test_cobbler(args=None):
     if not check_user_group():
         print_group_error_message(__file__)
-        sys.exit(3)
+        sys.exit(ReturnCodes.GROUP_ERROR)
 
     if args is None:
         args = get_parser().parse_args()
 
     locallogs = []
-    exitstatus = 0
+    exitstatus = ReturnCodes.SUCCESS
     machine = None
 
     configure_logging(args.debug)
@@ -91,7 +92,7 @@
             logging.error(mesg)
         except (AttributeError, NameError):
             sys.stderr.write(mesg)
-        exitstatus = 2
+        exitstatus = ReturnCodes.UTAH_EXCEPTION_ERROR
 
     finally:
         if machine is not None:

=== modified file 'examples/run_test_vm.py'
--- examples/run_test_vm.py	2013-02-15 08:36:32 +0000
+++ examples/run_test_vm.py	2013-03-11 10:10:26 +0000
@@ -25,6 +25,7 @@
     common_arguments,
     run_tests,
     configure_logging,
+    ReturnCodes,
 )
 
 
@@ -45,13 +46,13 @@
 def run_test_vm(args=None):
     if not check_user_group():
         print_group_error_message(__file__)
-        sys.exit(3)
+        sys.exit(ReturnCodes.GROUP_ERROR)
 
     if args is None:
         args = get_parser().parse_args()
 
     locallogs = []
-    exitstatus = 0
+    exitstatus = ReturnCodes.SUCCESS
     machine = None
 
     configure_logging(args.debug)
@@ -72,7 +73,7 @@
 
     except UTAHException as error:
         sys.stderr.write('Exception: ' + str(error))
-        exitstatus = 2
+        exitstatus = ReturnCodes.UTAH_EXCEPTION_ERROR
 
     finally:
         if not args.no_destroy and machine is not None:

=== modified file 'examples/run_utah_tests.py'
--- examples/run_utah_tests.py	2013-02-15 08:36:32 +0000
+++ examples/run_utah_tests.py	2013-03-11 10:10:26 +0000
@@ -28,6 +28,7 @@
     virtual_arguments,
     configure_logging,
     run_tests,
+    ReturnCodes,
 )
 from utah.timeout import timeout, UTAHTimeout
 from run_install_test import run_install_test
@@ -67,7 +68,7 @@
 def run_utah_tests(args=None):
     if not check_user_group():
         print_group_error_message(__file__)
-        sys.exit(3)
+        sys.exit(ReturnCodes.GROUP_ERROR)
 
     if args is None:
         args = get_parser().parse_args()
@@ -86,7 +87,7 @@
         def run_provisioned_tests(args):
             """Run test cases in a provisioned machine."""
             locallogs = []
-            exitstatus = 0
+            exitstatus = ReturnCodes.SUCCESS
             try:
                 # TBD: Inventory should be used to verify machine
                 # is not running other tests
@@ -94,7 +95,7 @@
                 exitstatus, locallogs = run_tests(args, machine)
             except UTAHException as error:
                 sys.stderr.write('Exception: ' + str(error))
-                exitstatus = 2
+                exitstatus = ReturnCodes.UTAH_EXCEPTION_ERROR
             finally:
                 if len(locallogs) != 0:
                     print('Test logs copied to the following files:')
@@ -121,7 +122,7 @@
             except UTAHTimeout:
                 sys.stderr.write('Job failed to complete after '
                                  + str(config.jobtimeout) + ' seconds.')
-                sys.exit(3)
+                sys.exit(ReturnCodes.TIMEOUT_ERROR)
         else:
             run_utah_tests()
     except AttributeError:

=== modified file 'utah/client/common.py'
--- utah/client/common.py	2013-03-04 14:31:13 +0000
+++ utah/client/common.py	2013-03-11 10:10:26 +0000
@@ -35,11 +35,6 @@
 
 from utah.client.battery import battery
 
-
-PASS = 0
-FAIL = 1
-ERROR_TIMEOUT = -9
-
 CONFIG = {
     'DEBUG': False,
     'TEST_DIR': '/var/lib/utah',
@@ -57,7 +52,18 @@
 CLIENT_CONFIG = os.path.join(UTAH_DIR, 'config', 'client.json')
 DEFAULT_STATE_FILE = os.path.join(UTAH_DIR, "state.yaml")
 
-RETURN_CODES = {'PASS': 0, 'ERROR': -1, 'FAIL': 1}
+
+# UTAH client return codes
+# PASS: All test cases where executed and passed
+# FAIL: All test cases where executed, but at least one of them failed
+# ERROR: At least one error was detected that prevented a test case from being
+# executed. Examples of situations that are considered an error are:
+# - Fetch command failure
+# - Setup command failure
+class ReturnCodes:
+    PASS = 0
+    FAIL = 1
+    ERROR = 2
 
 CMD_TC_BUILD = 'testcase_build'
 CMD_TC_SETUP = 'testcase_setup'
@@ -114,7 +120,7 @@
         except KeyError:
             return make_result(
                 command,
-                FAIL,
+                1,  # sudo return value for configuration/permission problem
                 stderr=('{!r} user not found in the password database'
                         .format(run_as)))
 
@@ -164,13 +170,14 @@
             # so wrap to avoid OSError: no such process
             try:
                 os.kill(pid, signal.SIGKILL)
+                p.communicate()
             except OSError:
                 pass
 
         time_delta = datetime.datetime.now() - start_time
 
         return make_result(command=command,
-                           retcode=ERROR_TIMEOUT,
+                           retcode=p.returncode,
                            start_time=start_time.strftime(DATE_FORMAT),
                            time_delta=str(time_delta),
                            cmd_type=cmd_type,

=== modified file 'utah/client/result.py'
--- utah/client/result.py	2013-02-06 17:42:00 +0000
+++ utah/client/result.py	2013-03-11 10:10:26 +0000
@@ -112,12 +112,9 @@
 
         self.results.append(result)
 
-        if result['returncode'] == 1 and self.status != 'ERROR':
+        if result['returncode'] != 0 and self.status != 'ERROR':
             self.status = 'FAIL'
 
-        if result['returncode'] not in [0, 1]:
-            self.status = 'ERROR'
-
     def result(self, verbose=False):
         """
         Output a text based result.

=== modified file 'utah/client/runner.py'
--- utah/client/runner.py	2013-02-27 08:32:22 +0000
+++ utah/client/runner.py	2013-03-11 10:10:26 +0000
@@ -37,7 +37,7 @@
     MASTER_RUNLIST,
     UTAH_DIR,
     DEFAULT_STATE_FILE,
-    RETURN_CODES,
+    ReturnCodes,
     parse_yaml_file,
     BzrHandler,
     DevHandler,
@@ -557,11 +557,11 @@
 
     def returncode(self):
         if self.errors > 0 or self.fetch_errors > 0:
-            return RETURN_CODES['ERROR']
+            return ReturnCodes.ERROR
         elif self.failures > 0:
-            return RETURN_CODES['FAIL']
+            return ReturnCodes.FAIL
         else:
-            return RETURN_CODES['PASS']
+            return ReturnCodes.PASS
 
     def report(self):
         tests_run = self.passes + self.errors + self.failures

=== modified file 'utah/client/tests/test_runner.py'
--- utah/client/tests/test_runner.py	2012-12-11 09:47:51 +0000
+++ utah/client/tests/test_runner.py	2013-03-11 10:10:26 +0000
@@ -21,7 +21,7 @@
 from utah.client.result import ResultYAML
 from utah.client.state_agent import StateAgentYAML
 from utah.client.runner import Runner
-from utah.client.common import RETURN_CODES
+from utah.client.common import ReturnCodes
 from utah.client import exceptions
 
 from .common import (  # NOQA
@@ -99,7 +99,7 @@
         for suite in self.runner.suites:
             self.assertTrue(suite.is_done())
 
-        self.assertEqual(retcode, RETURN_CODES['ERROR'])
+        self.assertEqual(retcode, ReturnCodes.ERROR)
 
     def test_missing_testdir(self):
         """

=== modified file 'utah/run.py'
--- utah/run.py	2013-02-15 08:36:32 +0000
+++ utah/run.py	2013-03-11 10:10:26 +0000
@@ -30,6 +30,30 @@
 from utah.provisioning.ssh import ProvisionedMachine
 
 
+# Return codes for the server
+class ReturnCodes:
+    SUCCESS = 0  # No problems found
+    UTAH_EXCEPTION_ERROR = 1  # UTAH exception caught
+    TIMEOUT_ERROR = 2  # Unable to complete before config.jobtimeout seconds
+    GROUP_ERROR = 3  # User isn't in UTAH group
+
+    @staticmethod
+    def client_error(returncode):
+        """Add offset to client error to avoid overlapping.
+
+        This is useful to be able to know when a failure happened in the server
+        or in the client at the shell level.
+
+        :param returncode: The code returned by the client
+        :type returncode: int
+        :returns: The code to be returned by the server
+        :rtype: int
+
+        """
+        offset = 100
+        return returncode + offset
+
+
 def common_arguments(parser):
     parser.add_argument('runlist', metavar='runlist',
                         type=url_argument,
@@ -137,9 +161,13 @@
                                            report_remote_path)
             utah_command = 'utah {} {}'.format(extraopts, options)
             exitstatus, _stdout, _stderr = machine.run(utah_command, root=True)
+            # Make sure that it's possible to know when the server failed
+            # and when the client failed just checking the return code
+            if exitstatus != 0:
+                exitstatus = ReturnCodes.client_error(exitstatus)
         except UTAHException as error:
             logging.error('Failed to run test: ' + str(error))
-            exitstatus = 1
+            exitstatus = ReturnCodes.UTAH_EXCEPTION_ERROR
         else:
             _write(runlist, report_remote_path)