Merge lp:~terceiro/lava-dispatcher/device-version into lp:lava-dispatcher

[Antonio Terceiro]

This is the dispatcher part of the device version support implementation.

You will notice that the master image part is not done, since I could not get my board to work yet. In the current state of the code, master image devices will always return 'unknown' as their device versions.

[Michael Hudson-Doyle (mwhudson)]

Antonio Terceiro <email address hidden> writes:

> This is the dispatcher part of the device version support implementation.
>
> You will notice that the master image part is not done, since I could
> not get my board to work yet. In the current state of the code, master
> image devices will always return 'unknown' as their device versions.

This looks mostly fine, thanks! Just a couple of comments.

> === modified file 'lava_dispatcher/device/qemu.py' ---
> lava_dispatcher/device/qemu.py 2012-10-17 16:52:05 +0000 +++
> lava_dispatcher/device/qemu.py 2012-10-18 17:49:26 +0000 @@ -20,6
> +20,7 @@
>
> import contextlib
> import logging
> +import subprocess
>
> from lava_dispatcher.device.target import (
> Target
> @@ -76,4 +77,10 @@
> proc = logging_spawn(qemu_cmd, logfile=self.sio, timeout=1200)
> return proc
>
> + def get_device_version(self):
> + try:
> + return subprocess.check_output(["dpkg-query", "--showformat=${Version}", "--show", "qemu-system"])
> + except subprocess.CalledProcessError:
> + return "unknown"

Not sure this is the right thing to do -- should we not parse the
-version of the qemu binary we actually use, which might not be the
packaged version?

> target_class = QEMUTarget
>

> === added file 'lava_dispatcher/tests/test_device_version.py'
> --- lava_dispatcher/tests/test_device_version.py 1970-01-01 00:00:00 +0000
> +++ lava_dispatcher/tests/test_device_version.py 2012-10-18 17:49:26 +0000
> @@ -0,0 +1,59 @@
> +# Copyright (C) 2012 Linaro Limited
> +#
> +# Author: Antonio Terceiro <email address hidden>
> +#
> +# This file is part of LAVA Dispatcher.
> +#
> +# LAVA Dispatcher is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# LAVA Dispatcher is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, see <http://www.gnu.org/licenses>.
> +
> +from unittest import TestCase
> +import re
> +from lava_dispatcher.tests.helper import LavaDispatcherTestCase, create_device_config
> +
> +from lava_dispatcher.device.target import Target
> +from lava_dispatcher.device.qemu import QEMUTarget
> +from lava_dispatcher.device.fastmodel import FastModelTarget
> +
> +def _create_fastmodel():
> + config = create_device_config('fastmodel01', { 'device_type': 'fastmodel', 'simulator_binary': '/path/to/fastmodel', 'license_server': 'foo.local' })
> + target = FastModelTarget(None, config)
> + return target

Could you call this _create_fastmodel_target() do you think?

> +class TestDeviceVersion(LavaDispatcherTestCase):
> +
> + def test_base(self):
> + target = Target(None, None)
> + assert(type(target.get_device_version()) is str)

It would be more Pyt...

[Antonio Terceiro (terceiro)]

Michael Hudson-Doyle escreveu:
> > === modified file 'lava_dispatcher/device/qemu.py' ---
> > lava_dispatcher/device/qemu.py 2012-10-17 16:52:05 +0000 +++
> > lava_dispatcher/device/qemu.py 2012-10-18 17:49:26 +0000 @@ -20,6
> > +20,7 @@
> >
> > import contextlib
> > import logging
> > +import subprocess
> >
> > from lava_dispatcher.device.target import (
> > Target
> > @@ -76,4 +77,10 @@
> > proc = logging_spawn(qemu_cmd, logfile=self.sio, timeout=1200)
> > return proc
> >
> > + def get_device_version(self):
> > + try:
> > + return subprocess.check_output(["dpkg-query", "--showformat=${Version}", "--show", "qemu-system"])
> > + except subprocess.CalledProcessError:
> > + return "unknown"
>
> Not sure this is the right thing to do -- should we not parse the
> -version of the qemu binary we actually use, which might not be the
> packaged version?

Yeah, you are right. I was kind of afraid of parsing the very
non-machine-friendly output of --version, but I guess that's the right
thing to do after all.

> > +def _create_fastmodel():
> > + config = create_device_config('fastmodel01', { 'device_type': 'fastmodel', 'simulator_binary': '/path/to/fastmodel', 'license_server': 'foo.local' })
> > + target = FastModelTarget(None, config)
> > + return target
>
> Could you call this _create_fastmodel_target() do you think?

Agreed. This name was from before Andy's reorganization, and after the
merge I ended up keeping it. But _create_fastmodel_target is indeed
better.

> > +class TestDeviceVersion(LavaDispatcherTestCase):
> > +
> > + def test_base(self):
> > + target = Target(None, None)
> > + assert(type(target.get_device_version()) is str)
>
> It would be more Pythonic to write this as:
>
> self.assertIsInstance(target.get_device_version(), str)

fixed - thanks.

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

[lp:~terceiro/lava-dispatcher/device-version]

400. By Antonio Terceiro on 2012-10-19

merge with latest code

401. By Antonio Terceiro on 2012-10-20

create more realistic target instances for testing

402. By Antonio Terceiro on 2012-10-20

Use actual qemu binary - we might not be using the packaged one

403. By Antonio Terceiro on 2012-10-20

more Pythonic assertion of instance type

404. By Antonio Terceiro on 2012-10-22

return last version present in the output of --version

[Michael Hudson-Doyle (mwhudson)]

Looks good now, thanks.

review: Approve

=== modified file 'lava_dispatcher/context.py'
--- lava_dispatcher/context.py	2012-09-30 17:01:44 +0000
+++ lava_dispatcher/context.py	2012-10-22 18:03:24 +0000
@@ -55,3 +55,6 @@
             self._host_result_dir = tempfile.mkdtemp()
             atexit.register(shutil.rmtree, self._host_result_dir)
         return self._host_result_dir
+
+    def get_device_version(self):
+        return self.client.target_device.get_device_version()

=== modified file 'lava_dispatcher/device/fastmodel.py'
--- lava_dispatcher/device/fastmodel.py	2012-10-17 16:52:05 +0000
+++ lava_dispatcher/device/fastmodel.py	2012-10-22 18:03:24 +0000
@@ -26,6 +26,8 @@
 import shutil
 import stat
 import threading
+import re
+import subprocess
 
 from lava_dispatcher.device.target import (
     Target
@@ -255,6 +257,20 @@
             return [create_attachment('rtsm.log', content)]
         return []
 
+    def get_device_version(self):
+        cmd = '%s --version' % self._sim_binary
+        try:
+            banner = subprocess.check_output(cmd, shell = True)
+            return self._parse_fastmodel_version(banner)
+        except subprocess.CalledProcessError:
+            return "unknown"
+
+    def _parse_fastmodel_version(self, banner):
+        match = re.search('Fast Models \[([0-9.]+)', banner)
+        if match:
+            return match.group(1)
+        else:
+            return "unknown"
 
 class _pexpect_drain(threading.Thread):
     ''' The simulator process can dump a lot of information to its console. If

=== modified file 'lava_dispatcher/device/qemu.py'
--- lava_dispatcher/device/qemu.py	2012-10-17 16:52:05 +0000
+++ lava_dispatcher/device/qemu.py	2012-10-22 18:03:24 +0000
@@ -20,6 +20,8 @@
 
 import contextlib
 import logging
+import subprocess
+import re
 
 from lava_dispatcher.device.target import (
     Target
@@ -76,4 +78,12 @@
         proc = logging_spawn(qemu_cmd, logfile=self.sio, timeout=1200)
         return proc
 
+    def get_device_version(self):
+        try:
+            output = subprocess.check_output([self.context.config.default_qemu_binary, '--version'])
+            matches = re.findall('[0-9]+\.[0-9a-z.+\-:~]+', output)
+            return matches[-1]
+        except subprocess.CalledProcessError:
+            return "unknown"
+
 target_class = QEMUTarget

=== modified file 'lava_dispatcher/device/target.py'
--- lava_dispatcher/device/target.py	2012-10-17 16:52:05 +0000
+++ lava_dispatcher/device/target.py	2012-10-22 18:03:24 +0000
@@ -70,9 +70,15 @@
         self.sio = SerialIO(sys.stdout)
 
         self.boot_options = []
-        self.scratch_dir = utils.mkdtemp(context.config.lava_image_tmpdir)
+        self._scratch_dir = None
         self.deployment_data = {}
 
+    @property
+    def scratch_dir(self):
+        if self._scratch_dir is None:
+            self._scratch_dir = utils.mkdtemp(context.config.lava_image_tmpdir)
+        return self._scratch_dir
+
     def power_on(self):
         """ responsible for powering on the target device and returning an
         instance of a pexpect session
@@ -148,6 +154,13 @@
     def get_test_data_attachments(self):
         return []
 
+    def get_device_version(self):
+        """ Returns the device version associated with the device, i.e. version
+        of emulation software, or version of master image. Must be overriden in
+        subclasses.
+        """
+        return 'unknown'
+
     def _customize_ubuntu(self, rootdir):
         self.deployment_data = Target.ubuntu_deployment_data
         with open('%s/root/.bashrc' % rootdir, 'a') as f:
@@ -169,7 +182,6 @@
                 # just no upstart or dash assumptions
                 self._customize_oe(mnt)
 
-
 class SerialIO(file):
     def __init__(self, logfile):
         self.serialio = StringIO()

=== modified file 'lava_dispatcher/job.py'
--- lava_dispatcher/job.py	2012-08-30 04:22:50 +0000
+++ lava_dispatcher/job.py	2012-10-22 18:03:24 +0000
@@ -142,6 +142,7 @@
 
         metadata = {
             'target.hostname': self.target,
+            'target.device_version': self.context.get_device_version(),
         }
 
         if 'device_type' in self.job_data:

=== modified file 'lava_dispatcher/tests/__init__.py'
--- lava_dispatcher/tests/__init__.py	2011-06-27 04:55:08 +0000
+++ lava_dispatcher/tests/__init__.py	2012-10-22 18:03:24 +0000
@@ -1,6 +1,9 @@
 import unittest
 
 def test_suite():
-    module_names = ['lava_dispatcher.tests.test_config',]
+    module_names = [
+        'lava_dispatcher.tests.test_config',
+        'lava_dispatcher.tests.test_device_version',
+    ]
     loader = unittest.TestLoader()
     return loader.loadTestsFromNames(module_names)

=== added file 'lava_dispatcher/tests/helper.py'
--- lava_dispatcher/tests/helper.py	1970-01-01 00:00:00 +0000
+++ lava_dispatcher/tests/helper.py	2012-10-22 18:03:24 +0000
@@ -0,0 +1,52 @@
+# Copyright (C) 2012 Linaro Limited
+#
+# Author: Antonio Terceiro <[e-mail]>
+#
+# This file is part of LAVA Dispatcher.
+#
+# LAVA Dispatcher is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# LAVA Dispatcher is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, see <http://www.gnu.org/licenses>.
+
+import os
+from lava_dispatcher.config import get_config, get_device_config
+
+__tmp_dir = os.getenv("TMPDIR") or '/tmp'
+__tmp_config_dir = os.path.join(__tmp_dir, 'lava-dispatcher-config')
+
+def create_config(name, data):
+    filename = os.path.join(__tmp_config_dir, name)
+    if not os.path.exists(os.path.dirname(filename)):
+        os.mkdir(os.path.dirname(filename))
+    with open(filename, 'w') as f:
+        for key in data.keys():
+            f.write("%s = %s\n" % (key, data[key]))
+
+def create_device_config(name, data):
+    create_config("devices/%s.conf" % name, data)
+    return get_device_config(name, __tmp_config_dir)
+
+def setup_config_dir():
+    os.mkdir(__tmp_config_dir)
+
+def cleanup_config_dir():
+    os.system('rm -rf %s' % __tmp_config_dir)
+
+from unittest import TestCase
+
+class LavaDispatcherTestCase(TestCase):
+
+    def setUp(self):
+        setup_config_dir()
+
+    def tearDown(self):
+        cleanup_config_dir()

=== modified file 'lava_dispatcher/tests/test_config.py'
--- lava_dispatcher/tests/test_config.py	2012-10-16 02:26:18 +0000
+++ lava_dispatcher/tests/test_config.py	2012-10-22 18:03:24 +0000
@@ -17,33 +17,23 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, see <http://www.gnu.org/licenses>.
 
-import os
 from unittest import TestCase
 
 from lava_dispatcher.config import get_config, get_device_config
 from lava_dispatcher.utils import string_to_list
 from lava_dispatcher.client.base import LavaClient
 
+from lava_dispatcher.tests.helper import *
+
 test_config_dir = os.path.join(os.path.dirname(__file__), 'test-config')
-print test_config_dir
-
-tmp_dir = os.getenv("TMPDIR") or '/tmp'
-tmp_config_dir = os.path.join(tmp_dir, 'lava-dispatcher-config')
-
-def create_config(name, data):
-    filename = os.path.join(tmp_config_dir, name)
-    os.mkdir(os.path.dirname(filename))
-    with open(filename, 'w') as f:
-        for key in data.keys():
-            f.write("%s = %s\n" % (key, data[key]))
 
 class TestConfigData(TestCase):
 
     def setUp(self):
-        os.mkdir(tmp_config_dir)
+        setup_config_dir()
 
     def tearDown(self):
-        os.system('rm -rf %s' % tmp_config_dir)
+        cleanup_config_dir()
 
     def test_beagle01_uboot_cmds(self):
         beagle01_config = get_device_config("beaglexm01", test_config_dir)

=== added file 'lava_dispatcher/tests/test_device_version.py'
--- lava_dispatcher/tests/test_device_version.py	1970-01-01 00:00:00 +0000
+++ lava_dispatcher/tests/test_device_version.py	2012-10-22 18:03:24 +0000
@@ -0,0 +1,69 @@
+# Copyright (C) 2012 Linaro Limited
+#
+# Author: Antonio Terceiro <[e-mail]>
+#
+# This file is part of LAVA Dispatcher.
+#
+# LAVA Dispatcher is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# LAVA Dispatcher is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, see <http://www.gnu.org/licenses>.
+
+from unittest import TestCase
+import re
+from lava_dispatcher.tests.helper import LavaDispatcherTestCase, create_device_config, create_config, __tmp_config_dir
+
+from lava_dispatcher.device.target import Target
+from lava_dispatcher.device.qemu import QEMUTarget
+from lava_dispatcher.device.fastmodel import FastModelTarget
+from lava_dispatcher.context import LavaContext
+from lava_dispatcher.config import get_config
+
+def _create_fastmodel_target():
+    config = create_device_config('fastmodel01', { 'device_type': 'fastmodel', 'simulator_binary': '/path/to/fastmodel', 'license_server': 'foo.local' })
+    target = FastModelTarget(None, config)
+    return target
+
+def _create_qemu_target():
+    create_config('lava-dispatcher.conf', {'default_qemu_binary': 'qemu-system-arm'})
+    device_config = create_device_config('qemu01', { 'device_type': 'qemu' })
+    dispatcher_config = get_config(__tmp_config_dir)
+
+    context = LavaContext('qemu01', dispatcher_config, None, None)
+    return QEMUTarget(context, device_config)
+
+class TestDeviceVersion(LavaDispatcherTestCase):
+
+    def test_base(self):
+        target = Target(None, None)
+        self.assertIsInstance(target.get_device_version(), str)
+
+    def test_qemu(self):
+        target = _create_qemu_target()
+        device_version = target.get_device_version()
+        assert(re.search('^[0-9.]+', device_version))
+
+    def test_fastmodel(self):
+        banner = "\n".join([
+            "Fast Models [7.1.36 (May 17 2012)]",
+            "Copyright 2000-2012 ARM Limited.",
+            "All Rights Reserved.",
+            "Top component name: RTSM_VE_Cortex_A15x1_A7x1"
+            ])
+        target = _create_fastmodel_target()
+        version = target._parse_fastmodel_version(banner)
+        self.assertEqual('7.1.36', version)
+
+    def test_fastmodel_wrong_format(self):
+        client = _create_fastmodel_target()
+        version = client._parse_fastmodel_version('random string')
+        self.assertEqual('unknown', version)
+