Merge lp:~qzhang/lava-scheduler/retrieve-job-fail-reason into lp:lava-scheduler

[Spring Zhang]

Based on the job failure reason summary, I made the commit to find out why health job failure from job logfile. If the failure is caused by 'LAVA', I prefer to keep health job status as it is. Other reasons will trigger the board goes offline.

Versus retrieving information from result bundle, I think the log information make sense, I'd like to keep the task getting information from result bundle in the future if necessary.

One question is about job.log_file, is it a string which could be use .find() method as:
if self.logfile.find(item[0]) != -1

[Zygmunt Krynicki (zyga)]

flake8 (pip install flake8) check your code please

9 +class JobFailReason:

Inherit object please

+ self.type = ["LAVA", "Master Image", "HW"]

Ugly way to specify an enum, I'd prefer
FAULT_LAVA, FAULT_MASTER, FAULT_HW = range(3)
And put it in the class scope

13 + # struct: [ "match string": ["error type", "detail reason"]]
16 + self.match_matrix = [

Array-of-arrays, ugh, use namedtuple to make that readable:

from collections import namedtuple
M = namedtuple("M", "match error detail")

M("tar: You must specify one of the", FAULT_LAVA, "deployment failed due to untar issue"),

... and so on

34 + def get_health_check_reason(self):
35 + return self.health_check_reason

Useless, attribute was public anyway, remove it

37 + def get_error_types(self):
38 + return self.type

This should be a class level property, JobFailReason.FAULT_xxx

+ detail_msg = "\r" + r[1] + " -- " + r[2]

\r ? really?

review: Needs Fixing

[lp:~qzhang/lava-scheduler/retrieve-job-fail-reason]

148. By Spring Zhang on 2012-03-08

use flake8 check to align PEP8

149. By Spring Zhang on 2012-03-08

Modify due to comments.

1. use namedtuple
2. use class property

[Spring Zhang (qzhang)]

Modify due to comments. And add a warning status of health check.

review: Needs Resubmitting

[Paul Larson (pwlars)]

I don't think we want to do this. One of the uses of health checks is to catch problems with lava after performing an update. The idea is that we would set all health statuses to unknown when doing the update, and force a health check before they process any new jobs. If there's a bug we introduced that drastically interferes with processing jobs then it can catch it, and prevent any more jobs from running.

review: Disapprove

[lp:~qzhang/lava-scheduler/retrieve-job-fail-reason]

150. By Spring Zhang on 2012-03-09

Revert to put board offline onece check job failed

151. By Spring Zhang on 2012-03-09

merge from upstream and revert

[Spring Zhang (qzhang)]

Now it is back to put board offline once there is a job failure, only add some possible reasons from job log file.

review: Needs Resubmitting

[Zygmunt Krynicki (zyga)]

72 + JobFailReason.M("tar: You must specify one of the",

You can just say M as it is already in scope.

144 + if not 'server' in params or \
145 + not isinstance(params['server'], unicode):

The general consensus around using trailing \ is not to do it. Please use parentheses in this case.

>>> if (something \n rest of if):

+ JobFailReason.FAULT_LAVA, "deployment failed due to untar issue"),

You can say self.FAULT_LAVA, this is shorter and is usually preferred.

99 + errors = ""
100 + for r in self._health_check_reason:
101 + errors += "\n" + r.match_text + " -- " + r.reason
102 + return errors

Don't join strings like this in python. It is quadratic in complexity.
Instead use str.join(), it is linear in complexity and you get the number of glue elements right (you otherwise get the unneeded leading newline). Compare:

return "\n".join(
           ["{0.match_text} -- {0.reason}".format(r)
           for f in self._health_check_reason])

review: Needs Fixing

[lp:~qzhang/lava-scheduler/retrieve-job-fail-reason]

152. By Spring Zhang on 2012-03-21

merge from mainline

153. By Spring Zhang on 2012-03-21

modify due to comments

154. By Spring Zhang on 2012-03-21

Add more error message text

155. By Spring Zhang on 2012-03-22

1. modify nameturple name to a make-sense one
2. use FieldFile to read logfile

[Spring Zhang (qzhang)]

1. Fix due to comments.
2. Have a test.

review: Needs Resubmitting

[Michael Hudson-Doyle (mwhudson)]

Firstly I should apologise; I think I said that I'd look at this weeks ago and haven't.

I definitely like the idea of attempting, even quite crudely, to automatically fish problems out of the log.

I don't think having a static list in the source is a very good way of doing this though -- perhaps we could have a simple table containing regular expressions and explanations that you could edit through the admin interface?

Also, I think if we're going to put more information in the transition table (which sounds fine), we should display it better. We have markdown support in lava-server already so it should be possible to render it as markdown if we really want to... (I also think we should be able to edit the most recent offline reason, but that surely is a separate bug).

[Spring Zhang (qzhang)]

yes, I also think it's a lot hard coded, a user-defined table is better.

Unmerged revisions

155. By Spring Zhang on 2012-03-22

1. modify nameturple name to a make-sense one
2. use FieldFile to read logfile

154. By Spring Zhang on 2012-03-21

Add more error message text

153. By Spring Zhang on 2012-03-21

modify due to comments

152. By Spring Zhang on 2012-03-21

merge from mainline

151. By Spring Zhang on 2012-03-09

merge from upstream and revert

150. By Spring Zhang on 2012-03-09

Revert to put board offline onece check job failed

149. By Spring Zhang on 2012-03-08

Modify due to comments.

1. use namedtuple
2. use class property

148. By Spring Zhang on 2012-03-08

use flake8 check to align PEP8

147. By Spring Zhang on 2012-03-07

Looking for health job failure reason in log file

146. By Spring Zhang on 2012-03-07

Set dispatcher error in log file as offline reason

A concern is if the logfile can be reached in job complete jobCompleted_impl, for there is also a getLogFileForJobOnBoard() in this file.

=== modified file 'lava_scheduler_app/logfile_helper.py'
--- lava_scheduler_app/logfile_helper.py	2012-03-19 02:00:36 +0000
+++ lava_scheduler_app/logfile_helper.py	2012-03-22 10:46:19 +0000
@@ -1,10 +1,11 @@
 import re
+from collections import namedtuple
 
 def getDispatcherErrors(logfile):
     errors = ""
     for line in logfile:
         if line.find("CriticalError:") != -1 or \
-           line.find("Lava failed on test:") != -1 :
+           line.find("Lava failed on test:") != -1:
             errors += line
 
     return errors
@@ -35,16 +36,19 @@
             logs.append((match.group(2), line, ""))
     return logs
 
-class Sections:
+
+class Sections(object):
     def __init__(self):
         self.sections = []
         self.cur_section_type = None
         self.cur_section = []
+
     def push(self, type, line):
         if type != self.cur_section_type:
             self.close()
             self.cur_section_type = type
         self.cur_section.append(line)
+
     def close(self):
         if self.cur_section_type is not None:
             self.sections.append(
@@ -54,6 +58,7 @@
         self.cur_section_type = None
         self.cur_section = []
 
+
 def formatLogFile(logfile):
     if not logfile:
         return [('log', 1, "Log file is missing")]
@@ -74,10 +79,65 @@
             continue
         elif line.find("<LAVA_DISPATCHER>") != -1 or \
                  line.find("lava_dispatcher") != -1 or \
-                 line.find("CriticalError:") != -1 :
+                 line.find("CriticalError:") != -1:
             sections.push('log', line)
         else:
             sections.push('console', line)
     sections.close()
 
     return sections.sections
+
+
+class JobFailReason(object):
+    # Error types
+    FAULT_LAVA, FAULT_MASTERIMAGE, FAULT_HW = range(3)
+    # match item struct
+    MATCH = namedtuple("MATCH", ['match_text', 'error_type', 'reason'])
+
+    def __init__(self, logfile):
+        self.logfile = logfile
+        self._health_check_reason = []
+        self._error_types = []
+        # TODO: load a user-defined text file to match matrix
+        # some hard coded
+        self.match_matrix = [
+            self.MATCH("tar: You must specify one of the",
+                self.FAULT_LAVA, "deployment failed due to untar issue"),
+            self.MATCH("tar: It is possible that the compressed file(s) have become corrupted.",
+                self.FAULT_LAVA, "deployment failed due to untar issue"),
+            self.MATCH("mount: invalid offset 'None' specified",
+                self.FAULT_LAVA, "deployment failed due to mount issue"),
+            self.MATCH("(initramfs)",
+                self.FAULT_MASTERIMAGE, "Master image corrupt"),
+            self.MATCH("---[ end trace",
+                self.FAULT_MASTERIMAGE, "kernel dump"),
+            self.MATCH("mmcblk0: error -110 transferring data",
+                self.FAULT_MASTERIMAGE, "SD card corrupted"),
+            self.MATCH("mmc0: error -110 whilst initialising SD card",
+                self.FAULT_MASTERIMAGE, "SD card corrupted"),
+            self.MATCH("<<<Not Connected>>>",
+                self.FAULT_HW, "conmux console connection corrupted"),
+                ]
+
+    def look_for_health_check_reason(self):
+        errors = []
+
+        self.logfile.open()
+        logfile_text = self.logfile.read()
+        self.logfile.close()
+        for item in self.match_matrix:
+            if logfile_text.find(item.match_text) != -1:
+                errors.append(item)
+                if not item.error_type in self._error_types:
+                    self._error_types.append(item.error_type)
+
+        self._health_check_reason = errors
+
+    @property
+    def health_check_reason(self):
+        return "\n".join(["{0.match_text} -- {0.reason}".format(r)
+            for r in self._health_check_reason])
+
+    @property
+    def error_types(self):
+        return self._error_types

=== modified file 'lava_scheduler_app/models.py'
--- lava_scheduler_app/models.py	2012-03-15 20:44:39 +0000
+++ lava_scheduler_app/models.py	2012-03-22 10:46:19 +0000
@@ -159,7 +159,6 @@
     #    return device_type.device_set.all()
 
 
-
 class TestJob(RestrictedResource):
     """
     A test job is a test process that will be run on a Device.

=== modified file 'lava_scheduler_daemon/dbjobsource.py'
--- lava_scheduler_daemon/dbjobsource.py	2012-03-09 01:27:51 +0000
+++ lava_scheduler_daemon/dbjobsource.py	2012-03-22 10:46:19 +0000
@@ -17,6 +17,7 @@
 from zope.interface import implements
 
 from lava_scheduler_app.models import Device, DeviceStateTransition, TestJob
+from lava_scheduler_app.logfile_helper import getDispatcherErrors, JobFailReason
 from lava_scheduler_daemon.jobsource import IJobSource
 
 
@@ -238,8 +239,13 @@
         if job.health_check is True:
             device.last_health_report_job = job
             if job.status == TestJob.INCOMPLETE:
+                offline_reason = "Health Check Job Failed: "
+                offline_reason += getDispatcherErrors(job.log_file)
+                reasons = JobFailReason(job.log_file)
+                reasons.look_for_health_check_reason()
+                offline_reason += reasons.health_check_reason
                 device.health_status = Device.HEALTH_FAIL
-                device.put_into_maintenance_mode(None, "Health Check Job Failed")
+                device.put_into_maintenance_mode(None, offline_reason)
             elif job.status == TestJob.COMPLETE:
                 device.health_status = Device.HEALTH_PASS