Merge lp:~doanac/ubuntu-ci-services-itself/bsbuilder-better-error-handling into lp:ubuntu-ci-services-itself

[Andy Doan]

bsbuilder: fix serval error cases

This branch helps prevent:

* returning success when an upload fails
* ensuring a package version is bumped if a prior version of the
  package has landed in the ppa before
* the package wasn't signed

This is another piece of the fginther bsb-fixes branch. This helps give better errors information back to the user.

I'd like to get tests written for the upload_ppa logic some day, but I think that's aiming a little too high for this week.

I've tested this with two tickets: 1 showed a failure because there was a newer package version in the PPA. The other was a proper package that built.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:355
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/367/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/367/rebuild

review: Approve (continuous-integration)

[Francis Ginther (fginther)]

This is a good separation of the package building (check and watch) updates. As Andy was able to refactor this, I'll approve.

review: Approve

=== modified file 'branch-source-builder/bsbuilder/run_worker.py'
--- branch-source-builder/bsbuilder/run_worker.py	2014-03-10 22:57:01 +0000
+++ branch-source-builder/bsbuilder/run_worker.py	2014-03-10 22:57:01 +0000
@@ -16,13 +16,16 @@
 
 import json
 import logging
+import os
 import time
+import yaml
 
-from bsbuilder import upload_package
+import upload_package
 import watch_ppa
 
 from ci_utils import (
     amqp_utils,
+    data_store,
     dump_stack,
 )
 
@@ -32,19 +35,64 @@
 TIME_BETWEEN_CHECKS = 60
 
 
+def get_auth_config():
+    DEFAULT_CONFIG_YAML = os.path.join(
+        os.path.dirname(__file__), '../../unit_config')
+    with open(DEFAULT_CONFIG_YAML) as f:
+        config = yaml.safe_load(f)
+    return config
+
+
+def create_result_file(datastore, content):
+    '''Creates a result file based on the final upload and build status.
+
+    The input content has the following format:
+        {
+            'message': 'The high level status of the upload',
+            'subticket_status': [{
+                'name': source_package_name,
+                'version': source_package_version,
+                'step': numeric_workflow_step,
+                'step_text': text_workflow_step,
+                'status': numeric_workflow_status,
+                'status_text': text_workflow_status,
+                'message': subticket_specific_message,
+            }]
+        }
+
+    The return is an artifact entry for the progress message.'''
+    package_build_result_filename = 'package_build.output.log'
+    body = '{}\n\n'.format(content['message'])
+    body += 'Uploaded source packages:\n'
+    for subticket_status in content['subticket_status']:
+        body += 'name: {}\n'.format(subticket_status['name'])
+        body += 'version: {}\n'.format(subticket_status['version'])
+        body += 'status: {}\n'.format(subticket_status['status_text'])
+        if subticket_status.get('message'):
+            body += 'message: {}\n'.format(subticket_status['message'])
+    location = datastore.put_file(
+        package_build_result_filename, str(body), 'text/plain')
+    return {
+        'name': package_build_result_filename,
+        'type': 'LOGS',
+        'reference': location,
+    }
+
+
 def on_message(msg):
     log.info('on_message: {}'.format(msg.body))
     params = json.loads(msg.body)
+    ticket_id = params['ticket_id']
     subtickets = params['subtickets']
     series = params['series']
     ppa = params['ppa']
     archive = params['archive']
-    log.info('The PPA is: {}'.format(ppa))
     trigger = params['progress_trigger']
 
     # Setup the output data to send back to the caller
     # TODO: artifacts will be populated with artifacts from this build.
     out_data = {
+        'message': 'Build in progress.',
         'ppa': ppa,
         'archive': archive,
         'artifacts': [],
@@ -52,31 +100,63 @@
     amqp_utils.progress_update(trigger, params)
 
     try:
+        datastore = data_store.create_for_ticket(ticket_id, get_auth_config())
+
         upload_list = upload_package.upload_source_packages(ppa, subtickets)
         log.info('upload_list: {}'.format(upload_list))
-        start_time = time.time()
-        while True:
-            (ret, status) = watch_ppa.watch_ppa(start_time, series, ppa,
-                                                archive, None,
-                                                upload_list)
-            progress = {}
-            for key in status:
-                progress[key] = str(status[key])
-            log.info('progress: {}'.format(progress))
-            out_data['status'] = progress
-            amqp_utils.progress_update(trigger, out_data)
-            if ret == -1:
-                log.info('Going to sleep for {}'.format(TIME_BETWEEN_CHECKS))
-                time.sleep(TIME_BETWEEN_CHECKS)
+
+        # Check that an upload isn't going to fail due to version
+        (failed, subticket_status) = watch_ppa.check_ppa(
+            series, ppa, archive, None, upload_list)
+        if failed:
+            # This is the end of the line, the upload is blocked.
+            out_data['subticket_status'] = subticket_status
+            message = 'Source package upload failed.'
+            log.error(message)
+            out_data['message'] = message
+            out_data['artifacts'].append(
+                create_result_file(datastore, out_data))
+            amqp_utils.progress_failed(trigger, out_data)
+        else:
+            # Set ret to 1, the upload is failed until it succeeds
+            ret = 1
+            start_time = time.time()
+            while True:
+                (ret, subticket_status) = watch_ppa.watch_ppa(
+                    start_time, series, ppa, archive, None, upload_list)
+                log.info('subticket_status: {}'.format(subticket_status))
+                out_data['subticket_status'] = subticket_status
+                if ret == -1:
+                    # -1 indicates that the task is not finished
+                    # Send regular progress back to the lander
+                    amqp_utils.progress_update(trigger, out_data)
+                    log.info('Going to sleep for '
+                             '{}'.format(TIME_BETWEEN_CHECKS))
+                    time.sleep(TIME_BETWEEN_CHECKS)
+                else:
+                    log.info('All done')
+                    break
+
+            if ret == 1:
+                # '1' indicates a failure occurred during the upload/build
+                message = 'A package failed to upload or build.'
+                log.error(message)
+                out_data['message'] = message
+                out_data['artifacts'].append(
+                    create_result_file(datastore, out_data))
+                amqp_utils.progress_failed(trigger, out_data)
             else:
-                log.info('All done')
-                break
-
-        amqp_utils.progress_completed(trigger, out_data)
+                # '0' indicates the build was successful
+                message = 'The package build was successful.'
+                log.error(message)
+                out_data['message'] = message
+                out_data['artifacts'].append(
+                    create_result_file(datastore, out_data))
+                amqp_utils.progress_completed(trigger, out_data)
     except (KeyboardInterrupt, Exception) as e:
-        error_msg = 'Exception: {}'.format(e)
-        log.error(error_msg)
-        out_data['error_message'] = error_msg
+        message = 'Exception: {}'.format(e)
+        log.error(message)
+        out_data['message'] = message
         amqp_utils.progress_failed(trigger, out_data)
         if isinstance(e, KeyboardInterrupt):
             raise  # re-raise so amqp_utils.process_queue can exit

=== modified file 'branch-source-builder/cupstream2distro/packageinppamanager.py'
--- branch-source-builder/cupstream2distro/packageinppamanager.py	2014-02-19 16:34:46 +0000
+++ branch-source-builder/cupstream2distro/packageinppamanager.py	2014-03-10 22:57:01 +0000
@@ -49,12 +49,6 @@
             rev = _get_current_rev_from_config(substract[0])
             branch = _get_parent_branch(substract[0])
             result.add((substract[0], version, rev, branch))
-    foo = set()
-    foo.add(('autopilot', '1.3.1+13.10.20131003.1-0ubuntu2~fginther.1',
-            '100', 'lp:autopilot'))
-    foo.add(('autopilot', '1.3.1+13.10.20131003.1-0ubuntu2~fginther.2',
-            '105', 'lp:autopilot'))
-    return foo
     return result
 
 
@@ -74,7 +68,9 @@
     return result
 
 
-def update_all_packages_status(packages_not_in_ppa, packages_building, packages_failed, particular_arch=None):
+def update_all_packages_status(packages_not_in_ppa, packages_building,
+                               packages_failed, packages_complete,
+                               particular_arch=None):
     '''Update all packages status, checking in the ppa'''
 
     for current_package in (packages_not_in_ppa.union(packages_building)):
@@ -93,6 +89,7 @@
             elif package_status == PackageInPPA.PUBLISHED:
                 _ensure_removed_from_set(packages_building, current_package)  # in case we missed the "build" step
                 _ensure_removed_from_set(packages_not_in_ppa, current_package)  # in case we missed the "wait" step
+                packages_complete.add(current_package)
 
 
 def _get_current_packaging_version_from_config(source_package_name):

=== modified file 'branch-source-builder/setup.py'
--- branch-source-builder/setup.py	2014-03-10 22:57:01 +0000
+++ branch-source-builder/setup.py	2014-03-10 22:57:01 +0000
@@ -24,10 +24,11 @@
 
 requires = [
     'WebTest==2.0.10',
+    'amqplib==1.0.0',
     'chardet>=2.0.1',
     'dput>=1.6',
-    'amqplib==1.0.0',
     'launchpadlib==1.10.2',
+    'lazr.enum>=1.1.2',
     'mock==1.0.1',
     'restish==0.12.1',
 ]

=== modified file 'branch-source-builder/watch_ppa.py'
--- branch-source-builder/watch_ppa.py	2014-02-28 09:25:47 +0000
+++ branch-source-builder/watch_ppa.py	2014-03-10 22:57:01 +0000
@@ -22,7 +22,9 @@
 import json
 import logging
 import os
+import subprocess
 import sys
+import textwrap
 import time
 
 from cupstream2distro import (launchpadmanager, packageinppamanager)
@@ -33,6 +35,27 @@
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '../ci-utils'))
 from ci_utils import dump_stack
+from ci_utils.ticket_states import (
+    SubTicketWorkflowStep,
+    SubTicketWorkflowStepStatus
+)
+
+EXISTING_SOURCE_MESSAGE = textwrap.dedent('''\
+        ERROR: A source package with an equal or higher version
+        of {} exists in {}.
+        The upload of {} with version {} is failed.
+        Source package uploads must have a higher version.''')
+FAILED_SOURCE_MESSAGE = textwrap.dedent('''\
+        The source package build has failed in {}.
+        Please review the build logs for the failed package(s).''')
+MISSING_SOURCE_MESSAGE = textwrap.dedent('''\
+        The source package for {} with version {}
+        failed to upload to {}.
+        Please check that the source package is signed with a signature of
+        a launchpad user with permission to upload to the PPA.
+        Also check for a rejection message sent to the package signer.''')
+PASSED_SOURCE_MESSAGE = textwrap.dedent('''\
+        The source packages have been built in {}.''')
 
 
 def parse_arguments():
@@ -72,6 +95,117 @@
     return launchpadmanager.get_ppa(ppa)
 
 
+def create_package_status(package, step, status, message=None):
+    return {
+        'name': package.source_name,
+        'version': package.version,
+        'step': step.value,
+        'step_text': step.title,
+        'status': status.value,
+        'status_text': status.title,
+        'message': message,
+    }
+
+
+def collect_subticket_status(ppa, packages_not_in_ppa, packages_building,
+                             packages_failed, packages_complete):
+    '''Collects the sets of package build status into a subticket_status.'''
+    subticket_status = []
+    for package in packages_not_in_ppa:
+        subticket_status.append(create_package_status(
+            package, SubTicketWorkflowStep.QUEUED,
+            SubTicketWorkflowStepStatus.PKG_BUILDING_WAITING))
+
+    for package in packages_building:
+        subticket_status.append(create_package_status(
+            package, SubTicketWorkflowStep.PKG_BUILDING,
+            SubTicketWorkflowStepStatus.PKG_BUILDING_INPROGRESS))
+
+    for package in packages_failed:
+        subticket_status.append(create_package_status(
+            package, SubTicketWorkflowStep.COMPLETED,
+            SubTicketWorkflowStepStatus.PKG_BUILDING_FAILED,
+            FAILED_SOURCE_MESSAGE.format(ppa.web_link)))
+
+    for package in packages_complete:
+        subticket_status.append(create_package_status(
+            package, SubTicketWorkflowStep.COMPLETED,
+            SubTicketWorkflowStepStatus.PKG_BUILDING_COMPLETED,
+            PASSED_SOURCE_MESSAGE.format(ppa.web_link)))
+
+    return subticket_status
+
+
+def get_versions_for_source_package(series, ppa, source_name):
+    try:
+        source = ppa.getPublishedSources(exact_match=True,
+                                         source_name=source_name,
+                                         distro_series=series)[0]
+        return source.source_package_version
+    except (KeyError, IndexError):
+        return None
+
+
+def check_ppa(series, ppa, dest_ppa, arch, upload_list):
+    # Prepare launchpad connection:
+    lp_series = launchpadmanager.get_series(series)
+    monitored_ppa = launchpadmanager.get_ppa(ppa)
+    if dest_ppa:
+        dest_archive = get_ppa(dest_ppa)
+    else:
+        dest_archive = launchpadmanager.get_ubuntu_archive()
+    logging.info('Series: {}'.format(lp_series))
+    logging.info('Monitoring PPA: {}'.format(monitored_ppa))
+    logging.info('Destination Archive: {}'.format(dest_archive))
+
+    failed = False
+    check_status = []
+    for source_package in upload_list:
+        source = source_package['name']
+        version = source_package['version']
+        logging.info('Inspecting upload: {} - {}'.format(source, version))
+        message_list = []
+        subticket_status = {
+            'name': source,
+            'id': source_package['id'],
+            'version': version,
+        }
+        wf_step = SubTicketWorkflowStep.NEW
+        wf_status = SubTicketWorkflowStepStatus.NEW
+        check_status.append(subticket_status)
+        for ppa in [monitored_ppa, dest_archive]:
+            last_version = get_versions_for_source_package(lp_series, ppa,
+                                                           source)
+            logging.info('Last source: {}'.format(last_version))
+
+            if last_version:
+                try:
+                    subprocess.check_call(['dpkg', '--compare-versions',
+                                           last_version, 'lt', version])
+                except subprocess.CalledProcessError:
+                    # The version in the PPA is equal or higher then the
+                    # source package, the build will fail
+                    wf_step = SubTicketWorkflowStep.COMPLETED
+                    wf_status = SubTicketWorkflowStepStatus.PKG_BUILDING_FAILED
+                    message_list.append(EXISTING_SOURCE_MESSAGE.format(
+                        last_version, ppa.web_link, source, version))
+                    failed = True
+        subticket_status['step'] = wf_step.value
+        subticket_status['step_text'] = wf_step.title
+        subticket_status['status'] = wf_status.value
+        subticket_status['status_text'] = wf_status.title
+        subticket_status['message'] = '\n'.join(message_list)
+
+    return (failed, check_status)
+
+
+def find_subticket(upload_list, name, version):
+    for subticket in upload_list:
+        if subticket['name'] == name and subticket['version'] == version:
+            return subticket
+    return None
+
+
 def watch_ppa(time_start, series, ppa, dest_ppa, arch, upload_list):
     # Prepare launchpad connection:
     lp_series = launchpadmanager.get_series(series)
@@ -82,7 +216,6 @@
         dest_archive = launchpadmanager.get_ubuntu_archive()
     logging.info('Series: {}'.format(lp_series))
     logging.info('Monitoring PPA: {}'.format(monitored_ppa))
-
     logging.info('Destination Archive: {}'.format(dest_archive))
 
     # Get archs available and archs to ignore
@@ -102,6 +235,7 @@
     packages_not_in_ppa = set()
     packages_building = set()
     packages_failed = set()
+    packages_complete = set()
     for source_package in upload_list:
         source = source_package['name']
         version = source_package['version']
@@ -119,8 +253,6 @@
     # to eventually appear in the ppa.
     logging.info('Packages not in PPA: {}'.format(
         list_packages_info_in_str(packages_not_in_ppa)))
-    logging.info('Packages building: {}'.format(packages_building))
-    logging.info('Packages failed: {}'.format(packages_failed))
 
     # Check the status regularly on all packages
     # TODO The following is the original check loop. This can be extracted
@@ -129,11 +261,24 @@
         list_packages_info_in_str(
             packages_not_in_ppa.union(packages_building))))
     packageinppamanager.update_all_packages_status(
-        packages_not_in_ppa, packages_building, packages_failed, arch)
-
-    status = {'pending': packages_not_in_ppa,
-              'building': packages_building,
-              'failed': packages_failed}
+        packages_not_in_ppa, packages_building, packages_failed,
+        packages_complete, arch)
+
+    status = collect_subticket_status(monitored_ppa, packages_not_in_ppa,
+                                      packages_building, packages_failed,
+                                      packages_complete)
+    for package in status:
+        subticket = find_subticket(upload_list, package['name'],
+                                   package['version'])
+        if not subticket:
+            # No match, this should not happen
+            return (1, status)
+
+        # Populate the remaining fields necessary to update ticket status
+        package['id'] = subticket['id']
+        package['resource'] = subticket['resource']
+
+    logging.info("Status: {}".format(status))
     # if we have no package building or failing and have wait for
     # long enough to have some package appearing in the ppa, exit
     if (packages_not_in_ppa and not packages_building and
@@ -143,6 +288,16 @@
         logging.info(
             "Some source packages were never published in the ppa: "
             "{}".format(list_packages_info_in_str(packages_not_in_ppa)))
+        for package in status:
+            # If any packages are WAITING, set them to FAILED
+            if package['status'] == SubTicketWorkflowStepStatus.WAITING.value:
+                package['step'] = SubTicketWorkflowStep.COMPLETED.value,
+                package['status'] = \
+                    SubTicketWorkflowStepStatus.PKG_BUILDING_COMPLETED.value
+            # Add a message indicating the cause of the failure
+            message_list.append(MISSING_SOURCE_MESSAGE.format(
+                package['name'], package['version'], monitored_ppa.web_link))
+        status['message'] = '\n'.join(message_list)
         return (1, status)
 
     # break out of status check loop if all packages have arrived in

=== modified file 'juju-deployer/branch-source-builder.yaml.tmpl'
--- juju-deployer/branch-source-builder.yaml.tmpl	2014-03-10 01:35:59 +0000
+++ juju-deployer/branch-source-builder.yaml.tmpl	2014-03-10 22:57:01 +0000
@@ -29,7 +29,7 @@
                 branch: ${CI_BRANCH}
                 tarball: ${CI_PAYLOAD_URL}
                 unit-config: include-base64://configs/unit_config.yaml
-                packages: "dput python-dput python-swiftclient"
+                packages: "dput python-dput python-swiftclient lazr.enum"
                 install_sources: |
                     - ${CI_PPA}
                 install_keys: |