Merge lp:~adeuring/ci-director/bug-1365808 into lp:ci-director

[Abel Deuring]

A fix for bug 1365808: Exception when job number dosen't exit

This branch implements the suggestion from comment 4 of the bug.

[Curtis Hovey (sinzui)]

Thank you.

review: Approve (code)

=== modified file 'cidirector/storage.py'
--- cidirector/storage.py	2014-09-30 09:02:07 +0000
+++ cidirector/storage.py	2014-10-01 14:36:51 +0000
@@ -4,7 +4,9 @@
 from datetime import datetime
 import errno
 import fcntl
+from jenkins import JenkinsException
 import json
+import logging
 import os
 import subprocess
 
@@ -64,6 +66,7 @@
             }
         self._data = data
         self._fileobj = fileobj
+        self.logger = logging.getLogger('cidirector')
 
     @classmethod
     def from_file(cls, fileobj):
@@ -165,7 +168,7 @@
         # Handle legacy entries-- new entries will always have this set by
         # start_revision()
         build_data = version_data.setdefault('build', {'status': BUILDING})
-        return StateFileJob(BUILD_REVISION, build_data)
+        return StateFileJob(BUILD_REVISION, build_data, logger=self.logger)
 
     def publication_job(self):
         """Return the publication StateFileJob for the current version.
@@ -179,7 +182,8 @@
             if job.test_id == PUBLISH_REVISION:
                 return job
         return StateFileJob(
-            PUBLISH_REVISION, version_data.setdefault('publication', {}))
+            PUBLISH_REVISION, version_data.setdefault('publication', {}),
+            logger=self.logger)
 
     def get_build_windows_installer_job(self):
         """Return the StateFileJob for building the Windows installer."""
@@ -210,11 +214,11 @@
             return None
         build_data = version_data.setdefault(
             job_key, {'status': PENDING, 'failures': 0})
-        return StateFileJob(name, build_data, failure_threshold)
+        return StateFileJob(
+            name, build_data, failure_threshold, logger=self.logger)
 
-    @staticmethod
-    def _make_test_state_file_job(test_id, test_dict):
-        return StateFileJob(test_id, test_dict, 5)
+    def _make_test_state_file_job(self, test_id, test_dict):
+        return StateFileJob(test_id, test_dict, 5, logger=self.logger)
 
     def get_grouped_job(self, job_id,
                         group_name=None, failure_threshold=None, tags=None):
@@ -231,7 +235,8 @@
             'failures': 0,
         }
         job_dict = group.setdefault(job_id, default)
-        return StateFileJob(job_id, job_dict, failure_threshold)
+        return StateFileJob(
+            job_id, job_dict, failure_threshold, logger=self.logger)
 
     def iter_grouped_jobs(self):
         """Iterate through all group StateFileJobs for the current version.
@@ -245,12 +250,13 @@
             if job_id == 'build' or not isinstance(job_dict, dict):
                 continue
             if 'failures' in job_dict:
-                yield StateFileJob(job_id, job_dict, None)
+                yield StateFileJob(job_id, job_dict, None, logger=self.logger)
             else:
                 group = job_dict
                 for job_id, job_dict in group.items():
                     if isinstance(job_dict, dict) and 'failures' in job_dict:
-                        yield StateFileJob(job_id, job_dict, None)
+                        yield StateFileJob(
+                            job_id, job_dict, None, logger=self.logger)
 
     def get_test(self, test_id):
         """Retrieve the StateFileJob for a given test."""
@@ -279,7 +285,7 @@
         unit_tests = self._get_job_section(revision_build, 'unit-tests')
         return StateFileJob(test_id, unit_tests.setdefault(test_id, {
             'status': PENDING, 'failures': 0
-        }), failure_threshold=4)
+        }), failure_threshold=4, logger=self.logger)
 
     def iter_unit_test_jobs(self, revision_build):
         """Iterate through the unit test StateFileJobs for a revision build.
@@ -302,7 +308,7 @@
                                                  'functional-tests')
         return StateFileJob(job_id, functional_tests.setdefault(job_id, {
             'status': PENDING, 'failures': 0
-        }), failure_threshold=3)
+        }), failure_threshold=3, logger=self.logger)
 
     def iter_functional_test_jobs(self, revision_build):
         """Iterate the functional test StateFileJobs for a revision build.
@@ -387,7 +393,7 @@
 class StateFileJob:
     """Representation of a Job in the StateFile."""
 
-    def __init__(self, test_id, data, failure_threshold=1):
+    def __init__(self, test_id, data, failure_threshold=1, logger=None):
         """Constructor.
 
         :param test_id: The ID of the job.
@@ -412,6 +418,7 @@
         base_data.update(data)
         data.update(base_data)
         self._data = data
+        self.logger = logger
 
     def __repr__(self):
         return 'StateFileJob<%r, %r>' % (self.test_id, self.get_status())
@@ -475,7 +482,19 @@
             'deploy-via-windows' : WIN_CLIENT_DEPLOY,
             }
         test_id = specific_to_resourceful.get(self.test_id, self.test_id)
-        info = jenkins.get_build_info(test_id, last_build)
+        try:
+            info = jenkins.get_build_info(test_id, last_build)
+        except JenkinsException as err:
+            if str(err).endswith('number[%s] does not exist' % last_build):
+                # A build can really disappear, see bug 1365808.
+                self._data['status'] = None
+                self._data['last_build'] = None
+                if self.logger is not None:
+                    self.logger.warn(
+                        'Build %s of job %s is missing' % (last_build, test_id))
+                return
+            else:
+                raise
         result = info.get('result')
         if result is None:
             return

=== modified file 'cidirector/tests/test_cidirector.py'
--- cidirector/tests/test_cidirector.py	2014-09-29 13:02:53 +0000
+++ cidirector/tests/test_cidirector.py	2014-10-01 14:36:51 +0000
@@ -190,7 +190,11 @@
 
     @log_calls
     def get_build_info(self, test_id, build_id):
-        return self.build_info[test_id][build_id]
+        if build_id in self.build_info[test_id]:
+            return self.build_info[test_id][build_id]
+        else:
+            raise JenkinsException(
+                'job[%s] number[%s] does not exist'% (test_id, build_id))
 
     def set_build_result(self, job_id, build_number, result, artifacts=[]):
         info = self.build_info.setdefault(job_id, {build_number: {}})

=== modified file 'cidirector/tests/test_storage.py'
--- cidirector/tests/test_storage.py	2014-09-29 13:02:53 +0000
+++ cidirector/tests/test_storage.py	2014-10-01 14:36:51 +0000
@@ -1,6 +1,7 @@
 __metaclass__ = type
 
 from datetime import datetime
+import logging
 import os.path
 from StringIO import StringIO
 import subprocess
@@ -233,6 +234,27 @@
                 artifacts, current_version, 'fakettp://fake.fake/job/foo/1234',
                 build_number)
 
+    def test_update_from_jenkins_missing_build(self):
+        build_number = 1234
+        foo_job = 'foo'
+        foo_data = {
+            'status': BUILDING, 'failures': 0, 'failure-threshold': 3,
+            'build_number': build_number, 'last_build': build_number - 1,
+            'building': True,
+        }
+        jobs = {foo_job: foo_data}
+        current_version = 4567
+        jenkins = FakeJenkins(jobs, current_version)
+        logger = logging.getLogger()
+        handler = logging.handlers.MemoryHandler(10)
+        logger.addHandler(handler)
+        sfj = StateFileJob(foo_job, foo_data, None, logger)
+        sfj.update_from_jenkins(jenkins, current_version)
+        self.assertIs(None, sfj._data['status'])
+        self.assertIs(None, sfj._data['last_build'])
+        self.assertEqual(
+            'Build 1233 of job foo is missing', handler.buffer[0].msg)
+
     def test_archive_results(self):
         sfj = StateFileJob('foo', {})
         artifacts = [