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

[Javier Collado]

When an ssh command returns a failure code, it's logged as a warning (we might want to ignore it). However, when the client fails, then an error message is printed to sys.stderr and run_utah_tests.py exits. If we decide to ignore some client return codes and continue for some other return codes, this can be easily updated though.

Example output:
INFO: Running command through SSH: doesnotexist -r /tmp/master.run -o /var/log/utah/utah-36-quantal-amd64_master.run_tmp
WARNING: Return code: 127
utah failed with return code: 127
INFO: Running command through SSH: rm -f /var/log/utah/utah-36-quantal-amd64_master.run_tmp
utah@xps8300:/home/javi/code/bzr/utah/bug1035227$ PYTHONPATH=. examples/run_utah_tests.py utah/client/examples/master.run --image

Note that the error message isn't the last one printed to the console because the 'rm' command is executed later in a finally clause.

[Max Brustkern (nuclearbob)]

I think I'd like to get that error into a log rather than just using sys.stderr.print. How about using machine.logger.error for that? I think this is definitely a good change either way, since it'll provide more useful error info. If you agree with changing the logging method, you can either change the branch, or just let me know and I'll make the change when I do the merge.

review: Needs Information

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

625. By Javier Collado on 2012-08-14

Replaced sys.stderr.write with machine.logger.error

[Javier Collado (javier.collado)]

Applied changes as suggested by the review comment.

review: Needs Resubmitting

[Javier Collado (javier.collado)]

I'm completely fine with using machine.logger, so that those error messages are also available in the log file. However, I think the code isn't consistent and it uses "print" statements when it could use "machine.logger" for example in utah/run.

What do you think about updating them in a separate merge?

[Javier Collado (javier.collado)]

I've created a merge proposal with a few changes to use "machine.logger" where I think it makes sense:
https://code.launchpad.net/~javier.collado/utah/use_machine_logger/+merge/119479

[Max Brustkern (nuclearbob)]

All right, I'll take a look at that. Some of the print statements are used to let something monitoring the output know which files it needs to downloads, but it probably makes sense to change other ones.

review: Approve

=== modified file 'utah/provisioning/provisioning.py'
--- utah/provisioning/provisioning.py	2012-08-13 21:14:04 +0000
+++ utah/provisioning/provisioning.py	2012-08-14 07:15:22 +0000
@@ -471,6 +471,10 @@
         retval = channel.recv_exit_status()
         self.logger.debug('Closing SSH connection')
         self.ssh_client.close()
+        if retval == 0:
+            self.logger.debug('Return code: {}'.format(retval))
+        else:
+            self.logger.warning('Return code: {}'.format(retval))
         self.logger.debug('Standard output follows:')
         for line in stdout:
             self.logger.debug(line.strip())

=== modified file 'utah/run.py'
--- utah/run.py	2012-08-13 20:37:09 +0000
+++ utah/run.py	2012-08-14 07:15:22 +0000
@@ -42,6 +42,7 @@
             machine.uploadfiles([locallist], os.path.normpath('/tmp'))
             options = (' -r /tmp/' + os.path.basename(locallist)
                        + ' -o ' + remotelog)
+<<<<<<< TREE
             try:
                 machine.run('utah' + extraopts + options, root=True)
             except UTAHException as error:
@@ -56,6 +57,20 @@
             else:
                 print('Test log copied to ' + locallog)
                 locallogs.append(locallog)
+=======
+            returncode = machine.run('utah' + extraopts + options, root=True)
+            if returncode != 0:
+                machine.logger.error('utah failed with return code: {}\n'
+                                     .format(returncode))
+                exitstatus = 1
+                break
+            machine.downloadfiles([remotelog], locallog)
+            print('Test log copied to ' + locallog)
+            locallogs.append(locallog)
+        except UTAHException as error:
+            print('Failed to run test: ' + str(error))
+            exitstatus = 1
+>>>>>>> MERGE-SOURCE
         finally:
             try:
                 machine.run('rm -f ' + remotelog)