Merge lp:~mwhudson/lava-dispatcher/kill-SerialIO-sio-madness into lp:lava-dispatcher

[Michael Hudson-Doyle]

The dispatcher has had the feature for a while of attempting to attach the serial output from the device as an attachment to the lava test run. I say "attempting" because during the device refactoring some code got duplicated and the serial output got collected into a different place than where the attachment was made from, so these attachments to the lava test run have been 0 bytes for months. And noone apparently noticed apart from me, so let's kill this feature and make space to reimplement something in this area more sensibly...

[Antonio Terceiro (terceiro)]

  review approve

I really like deleting code :-)

--
Antonio Terceiro
Software Engineer - Linaro
http://www.linaro.org

review: Approve

=== modified file 'lava_dispatcher/client/base.py'
--- lava_dispatcher/client/base.py	2013-01-14 16:29:38 +0000
+++ lava_dispatcher/client/base.py	2013-01-16 23:47:22 +0000
@@ -29,14 +29,11 @@
 
 import lava_dispatcher.utils as utils
 
-from cStringIO import StringIO
-
 from lava_dispatcher.errors import (
     GeneralError,
     NetworkError,
     OperationFailed,
 )
-from lava_dispatcher.test_data import create_attachment
 
 
 def wait_for_prompt(connection, prompt_pattern, timeout):
@@ -334,7 +331,6 @@
         self.context = context
         self.config = config
         self.hostname = config.hostname
-        self.sio = SerialIO(sys.stdout)
         self.proc = None
         # used for apt-get in lava-test.py
         self.aptget_cmd = "apt-get"
@@ -425,7 +421,7 @@
 
     def get_test_data_attachments(self):
         '''returns attachments to go in the "lava_results" test run'''
-        return [ create_attachment('serial.log', self.sio.getvalue()) ]
+        return []
 
     def retrieve_results(self, result_disk):
         raise NotImplementedError(self.retrieve_results)
@@ -498,21 +494,3 @@
         session.run('echo 0>/sys/class/android_usb/android0/enable')
 
 
-class SerialIO(file):
-    def __init__(self, logfile):
-        self.serialio = StringIO()
-        self.logfile = logfile
-
-    def write(self, text):
-        self.serialio.write(text)
-        self.logfile.write(text)
-
-    def close(self):
-        self.serialio.close()
-        self.logfile.close()
-
-    def flush(self):
-        self.logfile.flush()
-
-    def getvalue(self):
-        return self.serialio.getvalue()

=== modified file 'lava_dispatcher/context.py'
--- lava_dispatcher/context.py	2012-12-18 19:50:48 +0000
+++ lava_dispatcher/context.py	2013-01-16 23:47:22 +0000
@@ -20,7 +20,7 @@
 
 import atexit
 import os
-import shutil
+import sys
 import tempfile
 
 from lava_dispatcher.config import get_device_config
@@ -33,6 +33,10 @@
     def __init__(self, target, dispatcher_config, oob_file, job_data):
         self.config = dispatcher_config
         self.job_data = job_data
+        # This is the file-like object to send serial output from the device
+        # to.  We just send it to stdout for now, but soon we'll do something
+        # cleverer.
+        self.logfile_read = sys.stdout
         device_config = get_device_config(
             target, dispatcher_config.config_dir)
         self._client = TargetBasedClient(self, device_config)

=== modified file 'lava_dispatcher/default-config/lava-dispatcher/README'
--- lava_dispatcher/default-config/lava-dispatcher/README	2012-10-11 02:51:21 +0000
+++ lava_dispatcher/default-config/lava-dispatcher/README	2013-01-16 23:47:22 +0000
@@ -34,9 +34,6 @@
    are already supported by lava-dispatcher, but if you are working on
    supporting a new class of device, you will need to add a file here.
 
-   Note that the device-type name must match the --dev argument to
-   linaro-media-create.
-
  * devices/
 
    This directory contains a file per device that can be targeted by
@@ -44,7 +41,3 @@
    a line "device_type = <device type>", although other settings can
    be included here.  You will definitely need to tell lava-dispatcher
    about the devices you have!
-
-* logging.conf
-
-   This file defines settings for Python logging

=== modified file 'lava_dispatcher/device/fastmodel.py'
--- lava_dispatcher/device/fastmodel.py	2012-11-22 02:14:51 +0000
+++ lava_dispatcher/device/fastmodel.py	2013-01-16 23:47:22 +0000
@@ -200,7 +200,7 @@
         logging.info('launching fastmodel with command %r' % sim_cmd)
         self._sim_proc = logging_spawn(
             sim_cmd,
-            logfile=self.sio,
+            logfile=self.client.context.logfile_read,
             timeout=1200)
         self._sim_proc.expect(self.PORT_PATTERN, timeout=300)
         self._serial_port = self._sim_proc.match.groups()[0]
@@ -216,7 +216,8 @@
         logging.info('simulator is started connecting to serial port')
         self.proc = logging_spawn(
             'telnet localhost %s' % self._serial_port,
-            logfile=self._create_rtsm_ostream(self.sio),
+            logfile=self._create_rtsm_ostream(
+                self.client.context.logfile_read),
             timeout=1200)
         return self.proc
 

=== modified file 'lava_dispatcher/device/master.py'
--- lava_dispatcher/device/master.py	2012-12-29 03:50:52 +0000
+++ lava_dispatcher/device/master.py	2013-01-16 23:47:22 +0000
@@ -23,12 +23,11 @@
 import logging
 import os
 import time
-import traceback
 
 import pexpect
 
-import lava_dispatcher.device.boot_options as boot_options
-import lava_dispatcher.tarballcache as tarballcache
+from lava_dispatcher.device import boot_options
+from lava_dispatcher import tarballcache
 
 from lava_dispatcher.client.base import (
     NetworkCommandRunner,
@@ -85,7 +84,7 @@
         if config.pre_connect_command:
             logging_system(config.pre_connect_command)
 
-        self.proc = connect_to_serial(config, self.sio)
+        self.proc = connect_to_serial(config, self.context.logfile_read)
 
     def get_device_version(self):
         return self.device_version
@@ -164,9 +163,7 @@
                 _deploy_linaro_rootfs(master, root_url)
                 _deploy_linaro_bootfs(master, boot_url)
             except:
-                logging.error("Deployment failed")
-                tb = traceback.format_exc()
-                self.sio.write(tb)
+                logging.exception("Deployment failed")
                 raise CriticalError("Deployment failed")
 
     def _format_testpartition(self, runner, fstype):
@@ -185,9 +182,7 @@
             _extract_partition(image_file, self.config.boot_part, boot_tgz)
             _extract_partition(image_file, self.config.root_part, root_tgz)
         except:
-            logging.error("Failed to generate tarballs")
-            tb = traceback.format_exc()
-            self.sio.write(tb)
+            logging.exception("Failed to generate tarballs")
             raise
 
         # we need to associate the deployment data with these so that we
@@ -452,9 +447,7 @@
         try:
             self.wait_network_up(timeout=20)
         except NetworkError:
-            msg = "Unable to reach LAVA server"
-            logging.error(msg)
-            self._client.sio.write(traceback.format_exc())
+            logging.exception("Unable to reach LAVA server")
             raise
 
         pattern1 = "<(\d?\d?\d?\.\d?\d?\d?\.\d?\d?\d?\.\d?\d?\d?)>"

=== modified file 'lava_dispatcher/device/qemu.py'
--- lava_dispatcher/device/qemu.py	2012-11-20 13:34:19 +0000
+++ lava_dispatcher/device/qemu.py	2013-01-16 23:47:22 +0000
@@ -79,7 +79,10 @@
             self.config.qemu_drive_interface,
             self._sd_image)
         logging.info('launching qemu with command %r' % qemu_cmd)
-        proc = logging_spawn(qemu_cmd, logfile=self.sio, timeout=1200)
+        proc = logging_spawn(
+            qemu_cmd,
+            logfile=self.context.logfile_read,
+            timeout=1200)
         return proc
 
     def get_device_version(self):

=== modified file 'lava_dispatcher/device/sdmux.py'
--- lava_dispatcher/device/sdmux.py	2013-01-04 19:26:51 +0000
+++ lava_dispatcher/device/sdmux.py	2013-01-16 23:47:22 +0000
@@ -22,7 +22,6 @@
 import logging
 import os
 import subprocess
-import sys
 import time
 
 from lava_dispatcher.errors import (
@@ -208,7 +207,8 @@
         logging_system(self.config.power_off_cmd)
 
     def power_on(self):
-        self.proc = connect_to_serial(self.config, self.sio)
+        self.proc = connect_to_serial(
+            self.config, self.context.logfile_read)
 
         logging.info('powering on')
         logging_system(self.config.power_on_cmd)

=== modified file 'lava_dispatcher/device/target.py'
--- lava_dispatcher/device/target.py	2012-11-15 21:23:20 +0000
+++ lava_dispatcher/device/target.py	2013-01-16 23:47:22 +0000
@@ -19,17 +19,13 @@
 # with this program; if not, see <http://www.gnu.org/licenses>.
 
 import contextlib
-import logging
 import os
-import sys
 
 from lava_dispatcher.client.lmc_utils import (
     image_partition_mounted,
     )
 import lava_dispatcher.utils as utils
 
-from cStringIO import StringIO
-
 
 def get_target(context, device_config):
     ipath = 'lava_dispatcher.device.%s' % device_config.client_type
@@ -67,7 +63,6 @@
         self.context = context
         self.config = device_config
         self.deployment_data = None
-        self.sio = SerialIO(sys.stdout)
 
         self.boot_options = []
         self._scratch_dir = None
@@ -175,23 +170,3 @@
                 # because we are doing pretty standard linux stuff, just
                 # just no upstart or dash assumptions
                 self._customize_oe(mnt)
-
-
-class SerialIO(file):
-    def __init__(self, logfile):
-        self.serialio = StringIO()
-        self.logfile = logfile
-
-    def write(self, text):
-        self.serialio.write(text)
-        self.logfile.write(text)
-
-    def close(self):
-        self.serialio.close()
-        self.logfile.close()
-
-    def flush(self):
-        self.logfile.flush()
-
-    def getvalue(self):
-        return self.serialio.getvalue()

=== modified file 'lava_dispatcher/job.py'
--- lava_dispatcher/job.py	2012-12-07 19:59:19 +0000
+++ lava_dispatcher/job.py	2013-01-16 23:47:22 +0000
@@ -206,6 +206,8 @@
                 finally:
                     err_msg = ""
                     if status == 'fail':
+                        # XXX mwhudson, 2013-01-17: I have no idea what this
+                        # code is doing.
                         logging.warning(
                             "[ACTION-E] %s is finished with error (%s)." % (
                                     cmd['command'], err))
@@ -217,8 +219,7 @@
                             err_msg += "Lava failed on test: %s" % \
                                        params.get('test_name', "Unknown")
                         err_msg = err_msg + traceback.format_exc()
-                        # output to both serial log and logfile
-                        self.context.client.sio.write(err_msg)
+                        print err_msg
                     else:
                         logging.info(
                             "[ACTION-E] %s is finished successfully." % (

=== modified file 'lava_dispatcher/utils.py'
--- lava_dispatcher/utils.py	2013-01-04 00:03:43 +0000
+++ lava_dispatcher/utils.py	2013-01-16 23:47:22 +0000
@@ -214,7 +214,7 @@
                 timeout=1, lava_no_logging=1)
 
 
-def connect_to_serial(device_config, sio):
+def connect_to_serial(device_config, logfile_read):
     """
     Attempts to connect to a serial console server like conmux or cyclades
     """
@@ -238,7 +238,7 @@
 
     while retry_count < retry_limit:
         proc = logging_spawn(device_config.connection_command, timeout=1200)
-        proc.logfile_read = sio
+        proc.logfile_read = logfile_read
         logging.info('Attempting to connect to device')
         match = proc.expect(patterns, timeout=10)
         result = results[match]