Merge lp:~abentley/ci-director/update-outcome-artifacts into lp:ci-director

[Aaron Bentley]

Upload artifacts from update-outcome, not ci-director.

This branch moves the artifact uploading from ci-director to update-outcome.

This makes sense, because the script is already producing output for s3 (although results.json is not being uploaded to s3 directly).

It means that artifacts will be uploaded for all completed ci-director builds, not just those for the current revision-build, supporting parallelism and re-testing.

[Curtis Hovey (sinzui)]

Thank you.

review: Approve (code)

=== modified file 'cidirector/storage.py'
--- cidirector/storage.py	2016-06-24 14:25:39 +0000
+++ cidirector/storage.py	2016-06-24 19:33:36 +0000
@@ -644,9 +644,6 @@
             return
         old_status = self.get_status()
         self.update_from_build_result(result)
-        if old_status == BUILDING and old_status != self.get_status():
-            self.archive_results(
-                jenkins, info, current_version, state_file.s3_storage)
 
     def update_from_build_result(self, result):
         """Update a provider test from its build result.

=== modified file 'cidirector/tests/test_storage.py'
--- cidirector/tests/test_storage.py	2016-06-24 14:07:03 +0000
+++ cidirector/tests/test_storage.py	2016-06-24 19:33:36 +0000
@@ -501,45 +501,6 @@
             sfj.update_from_jenkins(jenkins, 2, sf)
             self.assertEqual('succeeded', sfj._data['status'])
 
-    def test_update_from_jenkins_triggers_archiving_artifacts(self):
-        build_number = 1234
-        foo_job = 'foo'
-        foo_data = {
-            'status': PENDING, 'failures': 0, 'failure-threshold': 3,
-            'build_number': build_number, 'last_build': build_number,
-            'building': True,
-        }
-        jobs = {foo_job: foo_data}
-        current_version = 4567
-        jenkins = FakeJenkins(jobs, current_version)
-        sf = StateFile()
-        sf._data['versions'][4567] = {}
-        sfj = StateFileJob(foo_job, foo_data, None)
-        with patch.object(sfj, 'archive_results') as mocked_archive_results:
-            self.assertEqual(BUILDING, sfj.get_status())
-            sfj.update_from_jenkins(jenkins, current_version, sf)
-            self.assertEqual(BUILDING, sfj.get_status())
-            self.assertEqual(0, len(mocked_archive_results.mock_calls))
-
-            artifacts = [
-                {
-                    'fileName': 'some-product.tar.gz',
-                    'relativePath': 'build/some-product.tar.gz',
-                },
-            ]
-            jenkins.set_build_result(
-                foo_job, build_number, 'SUCCESS', artifacts)
-            sfj.update_from_jenkins(jenkins, current_version, sf)
-            self.assertEqual(SUCCEEDED, sfj.get_status())
-            mocked_archive_results.assert_called_once_with(jenkins, {
-                'artifacts': artifacts,
-                'building': True,
-                'number': build_number,
-                'result': SUCCESS,
-                'timestamp': 0,
-                'url': 'fakettp://fake.fake/job/foo/1234',
-                }, current_version, None)
-
     def test_update_from_jenkins_missing_build(self):
         build_number = 1234
         foo_job = 'foo'

=== modified file 'cidirector/tests/test_update_outcome.py'
--- cidirector/tests/test_update_outcome.py	2016-06-21 19:42:27 +0000
+++ cidirector/tests/test_update_outcome.py	2016-06-24 19:33:36 +0000
@@ -1,5 +1,6 @@
 from calendar import timegm
 from contextlib import contextmanager
+from collections import OrderedDict
 from copy import deepcopy
 from datetime import (
     date,
@@ -7,9 +8,16 @@
     timedelta,
     )
 import json
+import logging
 import os
 from unittest import TestCase
 
+from mock import (
+    call,
+    Mock,
+    patch,
+    )
+
 from cidirector.jobs import (
     BUILD_REVISION,
     FAILURE,
@@ -36,6 +44,7 @@
     OutcomeJobSource,
     OutcomeState,
     scan_new_builds,
+    upload_artifacts,
     )
 from cidirector.utility import temp_dir
 from cidirector.tests.test_cidirector import (
@@ -825,3 +834,63 @@
             state.write_mongo_data(32, rb_data, lbs_data, {})
             mongo_name = state.mongo_filename(32)
             self.assertFalse(os.path.lexists(mongo_name))
+
+
+class TestUploadArtifacts(TestCase):
+
+    @contextmanager
+    def cxt(self, aj_instance_mock):
+        with patch('cidirector.update_outcome.ArtifactJob',
+                   spec=['__call__']) as aj_mock:
+            aj_mock.return_value = aj_instance_mock
+            yield aj_mock
+
+    @staticmethod
+    def make_by_revision_build():
+        build_110 = {'result': 'bar', 'number': 110}
+        build_111 = {'result': 'qux', 'number': 111}
+        build_120 = {'result': 'baz', 'number': 120}
+        return OrderedDict([
+            ('7081', {'foo': {'builds': {120: build_120}}}),
+            ('7021', {'bar': {'builds': OrderedDict([
+                (110, build_110), (111, build_111)])}}),
+            ])
+
+    def test_upload_artifacts(self):
+        dummy_jenkins = object()
+        dummy_s3_storage = object()
+        by_revision = self.make_by_revision_build()
+        aj_instance_mock = Mock(spec=['archive_results'])
+        with self.cxt(aj_instance_mock) as aj_mock:
+            upload_artifacts(dummy_jenkins, by_revision, dummy_s3_storage)
+        self.assertEqual([
+            call('foo', logging.getLogger()),
+            call('bar', logging.getLogger()),
+            ], aj_mock.call_args_list)
+        self.assertEqual([
+            call(dummy_jenkins, by_revision['7081']['foo']['builds'][120],
+                 '7081', dummy_s3_storage),
+            call(dummy_jenkins, by_revision['7021']['bar']['builds'][110],
+                 '7021', dummy_s3_storage),
+            call(dummy_jenkins, by_revision['7021']['bar']['builds'][111],
+                 '7021', dummy_s3_storage),
+            ], aj_instance_mock.archive_results.mock_calls)
+
+    def test_upload_artifacts_null_result(self):
+        dummy_jenkins = object()
+        dummy_s3_storage = object()
+        by_revision = self.make_by_revision_build()
+        by_revision['7021']['bar']['builds'][111]['result'] = None
+        aj_instance_mock = Mock(spec=['archive_results'])
+        with self.cxt(aj_instance_mock) as aj_mock:
+            upload_artifacts(dummy_jenkins, by_revision, dummy_s3_storage)
+        self.assertEqual([
+            call('foo', logging.getLogger()),
+            call('bar', logging.getLogger()),
+            ], aj_mock.call_args_list)
+        self.assertEqual([
+            call(dummy_jenkins, by_revision['7081']['foo']['builds'][120],
+                 '7081', dummy_s3_storage),
+            call(dummy_jenkins, by_revision['7021']['bar']['builds'][110],
+                 '7021', dummy_s3_storage),
+            ], aj_instance_mock.archive_results.mock_calls)

=== modified file 'cidirector/update_outcome.py'
--- cidirector/update_outcome.py	2016-06-21 19:42:27 +0000
+++ cidirector/update_outcome.py	2016-06-24 19:33:36 +0000
@@ -28,6 +28,7 @@
     SUCCESS,
 )
 from storage import (
+    ArtifactJob,
     BUILDING,
     datetime_from_timestamp,
     FAILED,
@@ -41,6 +42,7 @@
     date_str,
     ensure_dir,
     locked_open_rw,
+    S3Storage,
     )
 
 
@@ -56,7 +58,7 @@
     parser.add_argument('-v', '--verbose', help='Verbose info',
                         action='store_true')
     parser.add_argument('--log-path', help='Where to write logs.',
-                        default='ci-director.log')
+                        default='update-outcome.log')
     parser.add_argument('--log-count', help='The number of backups to keep.',
                         default=2, type=int)
     return parser.parse_args()
@@ -642,6 +644,33 @@
         self.write_json(mongo_name, mongo_data, make_dir=True)
 
 
+def upload_artifacts(jenkins, by_revision_build, s3_storage):
+    """Upload the artifacts for newly-completed builds.
+
+    by_revision_build should not include previously-uploaded builds, to avoid
+    unnecessary re-uploads.
+
+    Builds that do not have a result are not uploaded.  They lack artifacts,
+    because that is a post-build action, and they have incomplete console
+    logs, which could confuse other tools.
+    """
+    logger = logging.getLogger()
+    for revision_build, jobs in by_revision_build.items():
+        for job, job_data in jobs.items():
+            artifact_job = ArtifactJob(job, logger)
+            for build_info in job_data['builds'].values():
+                if build_info['result'] is None:
+                    logger.info(
+                        'No result for {} #{}.  Skipping.'.format(
+                            job, build_info['number']))
+                    continue
+                logger.info(
+                    'Uploading artifacts for {} #{}'.format(
+                        job, build_info['number']))
+                artifact_job.archive_results(
+                    jenkins, build_info, revision_build, s3_storage)
+
+
 def main():
     """Entry point for update-outcome script."""
     args = parse_args()
@@ -649,11 +678,13 @@
     config = ConfigReader.read_config()
     setdefaulttimeout(30)
     jenkins = make_jenkins(config)
+    s3_storage = S3Storage.from_config(config)
     outcome_dir = os.path.join(os.environ['HOME'],
                                '.config/ci-director-outcome')
     with OutcomeState(outcome_dir) as state:
         by_revision_build, cloud_health, new_last_completed = scan_new_builds(
             state.data, jenkins)
+        upload_artifacts(jenkins, by_revision_build, s3_storage)
         cloud_health_builds = BuildFiles.for_cloud_health(state)
         list(cloud_health_builds.update_files(cloud_health))
         revision_build_files = BuildFiles.for_revision_builds(state)