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

[Javier Collado]

This branch uses that return code from the utah client as the return code for
the server. This way, failures in the client can be detected easily in jenkins.

Aside from this, a few pep8 warnings and a refactoring on the client log
filename has been committed (look into the changes for further detail).

[Max Brustkern (nuclearbob)]

The way the scripts are currently setup, we can execute more than one runlist. If the first runlist fails and the second runlist passes, I think exitstatus will be 0 with this merge proposal.

[Javier Collado (javier.collado)]

Yes, you're right, exitstatus will be overwritten with the result of machine.run
for every runlist.

What I can do is keep the return code for all runs and do something like this:
retuncode = int(any(returncodes))

this way if the client returns a failure in any runlist, then the server will
return 1. Otherwise, it will return 0.

Would that be fine?

[Max Brustkern (nuclearbob)]

Yep, that should work, as long as you also capture the bit under the except: that sets the exit status to 1 when we can't run a runlist (line 28 of the merge.)

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

761. By Javier Collado on 2012-11-27

Collect exitstatus for each runlist and return 0 only if all runs were successful

Moved code inside runlist loop to a function so that the returned value from
the function can be used as the returned value from the client.

[Javier Collado (javier.collado)]

Code updated to get the exitstatus for each runlist and return 0 only if the
execution of all runlists was successful.

review: Needs Resubmitting

[Max Brustkern (nuclearbob)]

Line 57 of the merge skips over this:

        machine.logger.info('Test log copied to ' + locallog)
        locallogs.append(locallog)
        if args.dumplogs:

I don't think locallogs is in scope for your new _run function, so I don't think any client logs will be returned at the end of the run_tests function (line 72 of the merge.)

review: Needs Fixing

[Javier Collado (javier.collado)]

_run is defined inside run_test, not as a separate function, so all the
variables in the latter are available are available when looking in the outer
scope.

I've executed a simple runlist and didn't get any error, but if there's some
other way to better exercise the code, let me know and I'll run the test.

[Max Brustkern (nuclearbob)]

Okay, I guess I need to brush up on my variable scoping. If you can execute two runlists and it returns logs for both of them, then we should be in good shape.

review: Approve

[Javier Collado (javier.collado)]

Yes, I've executed the pass runlist twice and it has worked fine (maybe utah
should have detected the duplicated runlist, but for the purpose of the test,
it was fine).

=== modified file 'utah/run.py'
--- utah/run.py	2012-11-16 21:17:32 +0000
+++ utah/run.py	2012-11-27 11:58:36 +0000
@@ -21,38 +21,39 @@
 
     if args.json:
         extraopts = ' -f json'
-        suffix = '.json'
+        suffix = 'json'
     else:
         extraopts = ''
-        suffix = '.yaml'
+        suffix = 'yaml'
 
     locallogs = []
-    exitstatus = 0
 
     machine.installclient()
 
     extraopts += " --install-type " + machine.installtype
 
-    for runlist in args.runlists:
+    def _run(runlist):
+        """Run a single runlist."""
         locallist = urllib.urlretrieve(runlist)[0]
         machine.uploadfiles([locallist], os.path.normpath('/tmp'))
 
         options = ' -r /tmp/' + os.path.basename(locallist)
         utah_command = 'utah' + extraopts + options
         try:
-            _returncode, stdout, _stderr = machine.run(utah_command, root=True)
-            # TODO: Decide which returncode means utah client failure
-            # and which one means test case failure
+            # Server will return same code as client
+            exitstatus, stdout, _stderr = machine.run(utah_command, root=True)
         except UTAHException as error:
             machine.logger.error('Failed to run test: ' + str(error))
-            exitstatus = 1
-            break
+            return 1
 
         listname = os.path.basename(runlist)
-        locallog = os.path.join(config.logpath,
-                machine.name + '_' + listname
-                + time.strftime('_%Y-%m-%d_%H-%m-%S', time.gmtime())
-                + suffix)
+        timestamp = time.strftime('%Y-%m-%d_%H-%m-%S', time.gmtime())
+        log_filename = ('{machine}_{runlist}_{timestamp}.{suffix}'
+                        .format(machine=machine.name,
+                                runlist=listname,
+                                timestamp=timestamp,
+                                suffix=suffix))
+        locallog = os.path.join(config.logpath, log_filename)
         locallog = os.path.normpath(locallog)
         with open(locallog, 'w') as f:
             f.write(stdout)
@@ -62,6 +63,11 @@
             with open(locallog, 'r') as f:
                 print(f.read())
 
+        return exitstatus
+
+    exitstatuses = [_run(runlist) for runlist in args.runlists]
+    exitstatus = int(any(exitstatuses))
+
     if args.files is not None:
         try:
             locallogs += getfiles(args, machine)
@@ -69,8 +75,8 @@
             machine.logger.warning('Failed to download files: ' + str(err))
 
     return exitstatus, locallogs
-    
-    
+
+
 def getfiles(args, machine):
     if args.outdir is None:
         outdir = os.path.join(config.logpath, machine.name)
@@ -80,13 +86,12 @@
         os.makedirs(outdir)
 
     machine.downloadfilesrecursive(args.files, outdir)
-    
+
     localfiles = []
-    
+
     for mydir, dirs, files in os.walk(outdir):
         myfiles = [os.path.join(mydir, x) for x in files]
         for myfile in myfiles:
             localfiles.append(myfile)
 
     return localfiles
-