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

[Javier Collado]

This branch addresses the issue of truncated report when getting it through the
ssh command standard output.

The issue seems to be related to the buffering in openssh which is set to 64kB.
To fix this, it would be possible to read the output in chunks to prevent the
buffer from becoming full.

The workaround used in this change is just write the file locally in the
machine where the client is being executed and then get the file with sftp
(that automatically handles this problem).

To verify the fix, I've run the ceph test case using a server amd64 image and
got a 199kB file with the complete report.

[Max Brustkern (nuclearbob)]

Did you test with the -j option? It looks to me like if you used that option, extraopts would be:
-f json--install-type desktop
(for a desktop install.) I'm going to try this and see, but it'll take a little bit.

review: Needs Information

[Javier Collado (javier.collado)]

Looking at the code, I think you're right. Let me review and fix it.

[Max Brustkern (nuclearbob)]

Yeah, this is what I saw in the log:
2013-01-10 09:55:24,097 utah-682-precise-amd64 INFO: Running command through SSH: utah -f json--install-type desktop -r /tmp/pass.run -o /tmp/pass.yaml

review: Needs Fixing

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

799. By Javier Collado on 2013-01-10

Fixed get installmachine from machine object

800. By Javier Collado on 2013-01-10

Removed extraopts line that was left

801. By Javier Collado on 2013-01-10

Fixed report type is used to set the extension of the report file

In the past even for json reports the file name had the '.yaml' extension.

[Javier Collado (javier.collado)]

Thanks for pointing out that problem.

Since for the client the -f/--format option can be used to specify either yaml
or json (being yaml the default), as opposed to the server in which the format
by default is yaml and -j/--json means json, I've decided to always set the -f
option when calling the client to make the code more explicit and less prone to
whitespace errors (as it happened to in this change).

I've tested it with/without the -j option in run_utah_tests.py and it worked
fine.

review: Needs Resubmitting

[Max Brustkern (nuclearbob)]

Looks good to me.

review: Approve

=== modified file 'utah/run.py'
--- utah/run.py	2013-01-07 13:00:38 +0000
+++ utah/run.py	2013-01-10 18:29:22 +0000
@@ -110,34 +110,45 @@
     """
     Run some tests and retrieve results.
     """
+    if args.json:
+        report_type = 'json'
+    else:
+        report_type = 'yaml'
+
     def _run(runlist):
         """Run a single runlist."""
-        stdout = None
-
         try:
-            locallist = urllib.urlretrieve(runlist)[0]
-            machine.uploadfiles([locallist], os.path.normpath('/tmp'))
+            runlist_local_path = urllib.urlretrieve(runlist)[0]
+            target_dir = os.path.normpath('/tmp')
+            machine.uploadfiles([runlist_local_path], target_dir)
 
-            options = ' -r /tmp/' + os.path.basename(locallist)
-            utah_command = 'utah' + extraopts + options
-            exitstatus, stdout, _stderr = machine.run(utah_command, root=True)
+            runlist_file = os.path.basename(runlist_local_path)
+            runlist_remote_path = os.path.join(target_dir, runlist_file)
+            runlist_name = os.path.splitext(runlist_file)[0]
+            report_remote_path = \
+                os.path.join(target_dir,
+                             '{}.{}'.format(runlist_name, report_type))
+            options = '-r {} -o {}'.format(runlist_remote_path,
+                                           report_remote_path)
+            utah_command = 'utah {} {}'.format(extraopts, options)
+            exitstatus, _stdout, _stderr = machine.run(utah_command, root=True)
         except UTAHException as error:
             machine.logger.error('Failed to run test: ' + str(error))
             exitstatus = 1
-
-        if stdout is not None:
-            _write(runlist, stdout)
+        else:
+            _write(runlist, report_remote_path)
 
         return exitstatus
 
-    def _write(runlist, report):
+    def _write(runlist, report_remote_path):
         """Write runlist report.
 
         Report will be written using appropriate filename based on the runlist.
 
         :param runlist: Path to the runlist file
         :type runlist: str
-        :param report: Report to be written in any of the supported formats.
+        :param report_remote_path:
+            Path to the report file in the machine being tested
         :type report: str
 
         """
@@ -149,14 +160,13 @@
                                 runlist=listname,
                                 timestamp=timestamp,
                                 suffix=report_type))
-        locallog = os.path.join(config.logpath, log_filename)
-        locallog = os.path.normpath(locallog)
-        with open(locallog, 'w') as f:
-            f.write(report)
-        machine.logger.info('Test log copied to ' + locallog)
-        locallogs.append(locallog)
+        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)
+        machine.logger.info('Test log copied to ' + report_local_path)
+        locallogs.append(report_local_path)
         if args.dumplogs:
-            with open(locallog, 'r') as f:
+            with open(report_local_path, 'r') as f:
                 print(f.read())
 
     # Since we're running from a script, output INFO
@@ -167,14 +177,8 @@
     # Write the machine name to standard out for log gathering
     print('Running on machine: ' + machine.name)
 
-    if args.json:
-        extraopts = ' -f json'
-        report_type = 'json'
-
-    else:
-        extraopts = ''
-        report_type = 'yaml'
-    extraopts += " --install-type " + machine.installtype
+    extraopts = ('-f {} --install-type {}'
+                 .format(report_type, machine.installtype))
 
     locallogs = []
     try: