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

[Antonio Terceiro]

this branch adds support for obtaining the version (hwpack/rootfs) for master images.

[Andy Doan (doanac)]

On 12/06/2012 08:21 AM, Antonio Terceiro wrote:
> === modified file 'lava_dispatcher/job.py'
> --- lava_dispatcher/job.py 2012-11-24 20:58:07 +0000
> +++ lava_dispatcher/job.py 2012-12-06 14:20:27 +0000
> @@ -148,7 +148,6 @@
>
> metadata = {
> 'target.hostname': self.target,
> - 'target.device_version': self.context.get_device_version(),
> }
>
> if 'device_type' in self.job_data:
> @@ -232,6 +231,7 @@
> self.context.test_data.job_status = 'fail'
> raise
> finally:
> + self.context.test_data.add_metadata({ 'target.device_version': self.context.get_device_version() or 'error'})

Is the get_device_version sufficiently exception-safe? ie - the 3 lines
of code after this are important or the lava-results won't get submitted
properly. I'm not the best at proper error handling, maybe Michael can
speculate. Also - this wraps way past column 80 and is going to make
pylint/pflakes/etc complain.

> if submit_results:
> params = submit_results.get('parameters', {})
> action = lava_commands[submit_results['command']](

[Michael Hudson-Doyle (mwhudson)]

Andy Doan <email address hidden> writes:

> On 12/06/2012 08:21 AM, Antonio Terceiro wrote:
>> === modified file 'lava_dispatcher/job.py'
>> --- lava_dispatcher/job.py 2012-11-24 20:58:07 +0000
>> +++ lava_dispatcher/job.py 2012-12-06 14:20:27 +0000
>> @@ -148,7 +148,6 @@
>>
>> metadata = {
>> 'target.hostname': self.target,
>> - 'target.device_version': self.context.get_device_version(),
>> }
>>
>> if 'device_type' in self.job_data:
>> @@ -232,6 +231,7 @@
>> self.context.test_data.job_status = 'fail'
>> raise
>> finally:
>> + self.context.test_data.add_metadata({ 'target.device_version': self.context.get_device_version() or 'error'})
>
> Is the get_device_version sufficiently exception-safe? ie - the 3 lines
> of code after this are important or the lava-results won't get submitted
> properly. I'm not the best at proper error handling, maybe Michael can
> speculate.

It looks ok to me, but the part that concerns me is the point at which
it is called -- get_device_version will only work usefully if the board
is in the master image, but I don't think its guaranteed to be at this
point. I would rather have the target squirrel the device version away
the first time the device boots into the master image, if that's
possible.

CHeers,
mwh

> Also - this wraps way past column 80 and is going to make
> pylint/pflakes/etc complain.
>
>> if submit_results:
>> params = submit_results.get('parameters', {})
>> action = lava_commands[submit_results['command']](
>
>
> --
> https://code.launchpad.net/~terceiro/lava-dispatcher/master-device-version/+merge/138463
> You are subscribed to branch lp:lava-dispatcher.

[Andy Doan (doanac)]

On 12/06/2012 04:58 PM, Michael Hudson-Doyle wrote:
> It looks ok to me, but the part that concerns me is the point at which
> it is called -- get_device_version will only work usefully if the board
> is in the master image, but I don't think its guaranteed to be at this
> point. I would rather have the target squirrel the device version away
> the first time the device boots into the master image, if that's
> possible.

Read again - I thought the same thing the first time I read the code. It
actually sets this the first time it boots to master and is safe to call
from any context afterwards.

[Michael Hudson-Doyle (mwhudson)]

Andy Doan <email address hidden> writes:

> On 12/06/2012 04:58 PM, Michael Hudson-Doyle wrote:
>> It looks ok to me, but the part that concerns me is the point at which
>> it is called -- get_device_version will only work usefully if the board
>> is in the master image, but I don't think its guaranteed to be at this
>> point. I would rather have the target squirrel the device version away
>> the first time the device boots into the master image, if that's
>> possible.
>
> Read again - I thought the same thing the first time I read the code. It
> actually sets this the first time it boots to master and is safe to call
> from any context afterwards.

Ah quite right. +1 to merge.

Cheers,
mwh

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

481. By Antonio Terceiro on 2012-12-07

reformat long line

=== modified file 'lava_dispatcher/device/master.py'
--- lava_dispatcher/device/master.py	2012-12-05 19:10:40 +0000
+++ lava_dispatcher/device/master.py	2012-12-07 20:01:22 +0000
@@ -81,6 +81,7 @@
             }
 
         self.master_ip = None
+        self.device_version = None
 
         if config.pre_connect_command:
             logging_system(config.pre_connect_command)
@@ -88,6 +89,9 @@
         self.proc = self._connect_carefully(config.connection_command)
         atexit.register(self._close_logging_spawn)
 
+    def get_device_version(self):
+        return self.device_version
+
     def power_on(self):
         self._boot_linaro_image()
         return self.proc
@@ -400,6 +404,7 @@
             runner = MasterCommandRunner(self)
             try:
                 self.master_ip = runner.get_master_ip()
+                self.device_version = runner.get_device_version()
             except NetworkError as e:
                 msg = "Failed to get network up: " % e
                 logging.warning(msg)
@@ -518,6 +523,27 @@
         logging.debug("Master image IP is %s" % ip)
         return ip
 
+    def get_device_version(self):
+        pattern = 'device_version=(\d+-\d+/\d+-\d+)'
+        self.run("echo \"device_version="
+                 "$(lava-master-image-info --master-image-hwpack "
+                 "| sed 's/[^0-9-]//g; s/^-\+//')"
+                 "/"
+                 "$(lava-master-image-info --master-image-rootfs "
+                 "| sed 's/[^0-9-]//g; s/^-\+//')"
+                 "\"",
+                 [pattern, pexpect.EOF, pexpect.TIMEOUT],
+                 timeout = 5)
+
+        device_version = None
+        if self.match_id == 0:
+            device_version = self.match.group(1)
+            logging.debug('Master image version (hwpack/rootfs) is %s' % device_version)
+        else:
+            logging.warning('Could not determine image version!')
+
+        return device_version
+
     def has_partition_with_label(self, label):
         if not label:
             return False
 
=== modified file 'lava_dispatcher/job.py'
--- lava_dispatcher/job.py	2012-11-24 20:58:07 +0000
+++ lava_dispatcher/job.py	2012-12-07 20:01:22 +0000
@@ -148,7 +148,6 @@
 
         metadata = {
             'target.hostname': self.target,
-            'target.device_version': self.context.get_device_version(),
         }
 
         if 'device_type' in self.job_data:
@@ -232,6 +231,10 @@
             self.context.test_data.job_status = 'fail'
             raise
         finally:
+            device_version = self.context.get_device_version() or 'error'
+            self.context.test_data.add_metadata({
+                'target.device_version': device_version
+            })
             if submit_results:
                 params = submit_results.get('parameters', {})
                 action = lava_commands[submit_results['command']](