Long story short: bodhi has been having intermittent network issues for a while now and those network issues tend to kill taskotron tasks. We implemented some retries in order to mitigate the effects of those network issues but I botched one of the last builds and it didn't actually get deployed before freeze.
The new build has been running in stg and dev for almost 2 weeks now without issue. I'd like to update the production taskotron clients with this latest build since we've seen 30+ bodhi related failures already today.
I've attached the diff at the end of this email, there are several unrelated fixes here but I don't want to make a new build with the cherry picked fix extracted because the fix was committed a while ago and freeze ends tomorrow. If it causes problems, we can downgrade to the existing build or fix it when freeze ends tomorrow.
Tim
diff --git a/libtaskotron/__init__.py b/libtaskotron/__init__.py index 66923e1..2eed326 100644 --- a/libtaskotron/__init__.py +++ b/libtaskotron/__init__.py @@ -4,4 +4,4 @@ # See the LICENSE file for more details on Licensing
from __future__ import absolute_import -__version__ = '0.3.7' +__version__ = '0.3.10' diff --git a/libtaskotron/arch_utils.py b/libtaskotron/arch_utils.py new file mode 100644 index 0000000..a62171d --- /dev/null +++ b/libtaskotron/arch_utils.py @@ -0,0 +1,41 @@ +# -*- coding: utf-8 -*- +# Copyright 2009-2014, Red Hat, Inc. +# License: GPL-2.0+ http://spdx.org/licenses/GPL-2.0+ +# See the LICENSE file for more details on Licensing + +import os + +class Arches(): + ''' + Helper class for working with supported arches inside taskotron + ''' + + #: all supported architectures + all = ['i386', 'i486', 'i586', 'i686', + 'x86_64', + 'armhfp', 'arm7hl', + 'noarch', 'src'] + + #: base architectures + base = ['i386', 'x86_64', 'armhfp'] + + #: meta architectures + meta = ['noarch', 'src'] + +def basearch(arch=None): + ''' + This returns the 'base' architecture identifier for a specified architecture + (e.g. ``i386`` for ``i[3-6]86``), to be used by YUM etc. + + :param str arch: an architecture to be converted to a basearch. If ``None``, + then the arch of the current machine is used. + :return: basearch, or ``arch`` if no basearch was found for it + :rtype: str + ''' + if arch is None: + arch = os.uname()[4] + if arch in ['i386', 'i486', 'i586', 'i686']: + arch = 'i386' + if arch in ['armhfp', 'arm7hl']: + arch = 'armhfp' + return arch \ No newline at end of file diff --git a/libtaskotron/bodhi_utils.py b/libtaskotron/bodhi_utils.py index 71396d9..403d94f 100644 --- a/libtaskotron/bodhi_utils.py +++ b/libtaskotron/bodhi_utils.py @@ -27,10 +27,10 @@ class BodhiUtils(object): default BodhiClient instance is used. '''
- taskotron_config = config.get_config() - username = taskotron_config.fas_username - password = taskotron_config.fas_password - bodhi_url = taskotron_config.bodhi_server + self.taskotron_config = config.get_config() + username = self.taskotron_config.fas_username + password = self.taskotron_config.fas_password + bodhi_url = self.taskotron_config.bodhi_server
if not client: log.debug('creating bodhi client to %s' % bodhi_url) @@ -42,7 +42,8 @@ class BodhiUtils(object):
def query_update(self, package): - '''Get the last Bodhi update matching criteria. + '''Get the last Bodhi update matching criteria. Retry + ``bodhi_request_max_retries``-times when server returns HTTP 5xx response.
:param str package: package NVR or package name or update title or update ID @@ -55,11 +56,23 @@ class BodhiUtils(object): :rtype: :class:`bunch.Bunch` ''' log.debug('Searching Bodhi updates for: %s', package) - res = self.client.query(package=package, limit=1) - if res['updates']: - return res['updates'][0] - else: - return None + max_retries = self.taskotron_config.bodhi_request_max_retries + for retry_num in xrange(1, max_retries + 1): + try: + res = self.client.query(package=package, limit=1) + if res['updates']: + return res['updates'][0] + else: + return None + except fedora.client.ServerError as e: + if retry_num >= max_retries: + log.critical('Maximum Bodhi server retries exceeded') + raise + elif e.code < 500 or e.code > 599: + raise + else: + log.warning('Got HTTP %d response from bodhi, retrying \ +(%d retries remaining)...', e.code, max_retries - retry_num)
def build2update(self, builds, strict=False): @@ -144,8 +157,7 @@ class BodhiUtils(object): if build in build2update: log.warn("The build %s is a part of two different updates: '%s'" " and '%s'. That's probably a bug in Bodhi." % (build, - failures[build]['title'], build2update[build]['title']) - ) - del build2update.pop[build] + failures[build]['title'], build2update[build]['title'])) + del build2update[build]
return (build2update, failures) diff --git a/libtaskotron/config_defaults.py b/libtaskotron/config_defaults.py index fc05abd..fb77a7c 100644 --- a/libtaskotron/config_defaults.py +++ b/libtaskotron/config_defaults.py @@ -46,13 +46,13 @@ class Config(object): koji_url = 'http://koji.fedoraproject.org/kojihub' #: pkg_url = 'http://kojipkgs.fedoraproject.org/packages' #: bodhi_server = 'https://admin.fedoraproject.org/updates/' #: - resultsdb_server = \ - 'http://resultsdb.qa.fedoraproject.org/resultsdb/api/v1.0/' #: - taskotron_master = 'http://taskotron.qa.fedoraproject.org/taskmaster/' #: + resultsdb_server = 'http://127.0.0.1/resultsdb/api/v1.0/' #: + taskotron_master = 'http://127.0.0.1/taskmaster/' #: buildbot_task_step = 'runtask' #:
bodhi_posting_comments_span = 4320 #: # 3 days (3*24*60 = 4320) + bodhi_request_max_retries = 3 #:
tmpdir = '/var/tmp/taskotron' #: logdir = '/var/log/taskotron' #: diff --git a/libtaskotron/directives/bodhi_comment_directive.py b/libtaskotron/directives/bodhi_comment_directive.py index 7a187b9..c37eeec 100644 --- a/libtaskotron/directives/bodhi_comment_directive.py +++ b/libtaskotron/directives/bodhi_comment_directive.py @@ -79,7 +79,7 @@ from libtaskotron import bodhi_utils from libtaskotron import buildbot_utils
from libtaskotron.exceptions import TaskotronDirectiveError, TaskotronValueError -from libtaskotron.rpm_utils import basearch +from libtaskotron.arch_utils import basearch
import datetime import re @@ -109,13 +109,11 @@ class BodhiCommentDirective(BaseDirective): def process(self, input_data, env_data): output_data = []
- Config = config.get_config() if not (Config.reporting_enabled and Config.report_to_bodhi): log.info("Reporting to Bodhi is disabled, not doing anything.") return
- if not ('doreport' in input_data and 'results' in input_data): detected_args = ', '.join(input_data.keys()) raise TaskotronDirectiveError("The bodhi_comment directive "\ @@ -148,6 +146,8 @@ class BodhiCommentDirective(BaseDirective): Config.taskotron_master, Config.buildbot_task_step)
+ log.info('Posting %s results to Bodhi...', len(check_details)) + for detail in check_details: outcome = _post_testresult(self.bodhi_api, detail.item, env_data['checkname'], @@ -410,7 +410,7 @@ def _is_comment_needed(old_result, comment_time, result, time_span = None):
def _post_testresult(bodhi_api, update, testname, result, url, - arch = 'noarch', karma = 0, doreport='onchange'): + arch = 'noarch', karma = 0, doreport='onchange'): '''Post comment and karma to bodhi
:param str update: the **title** of the update comment on @@ -448,7 +448,8 @@ def _post_testresult(bodhi_api, update, testname, result, url, comment_needed = _is_comment_needed(old_result, comment_time, result)
if not comment_needed and doreport != 'all': - log.info('Current test result is already present in bodhi.') + log.debug('Up-to-date test result is already present in Bodhi: %s %s', + update, result) return True
bodhi_update = bodhi_api.query_update(update) @@ -473,7 +474,8 @@ def _post_testresult(bodhi_api, update, testname, result, url, update, comment, karma, send_email) return False
- log.info('The test result was sent to bodhi successfully.') + log.debug('Test result successfully submitted to Bodhi: %s %s', + update, result)
except Exception as e: log.exception(e) diff --git a/libtaskotron/directives/bodhi_directive.py b/libtaskotron/directives/bodhi_directive.py index 2783698..fb7951a 100644 --- a/libtaskotron/directives/bodhi_directive.py +++ b/libtaskotron/directives/bodhi_directive.py @@ -67,7 +67,7 @@ rpmlint:: import libtaskotron.bodhi_utils as bodhi from libtaskotron.koji_utils import KojiClient from libtaskotron.directives import BaseDirective - +from libtaskotron.arch_utils import Arches from libtaskotron.logger import log from libtaskotron.exceptions import TaskotronDirectiveError
@@ -126,7 +126,7 @@ class BodhiDirective(BaseDirective): workdir = env_data['workdir'] update = input_data['bodhi_id'] if 'all' in input_data['arch']: - arches = ['i386', 'x86_64', 'noarch', 'armhfp'] + arches = Arches.base + ['noarch'] else: arches = input_data['arch'] if 'noarch' not in arches and arches != ['src']: diff --git a/libtaskotron/directives/koji_directive.py b/libtaskotron/directives/koji_directive.py index 463c0fa..aaf8ed2 100644 --- a/libtaskotron/directives/koji_directive.py +++ b/libtaskotron/directives/koji_directive.py @@ -15,10 +15,10 @@ description: | parameters: action: required: true - description: choose whether to download a single build or all builds - belonging to a Koji tag + description: choose whether to download a single build (``download`` value) + or all builds belonging to a Koji tag (``download_tag`` value) type: str - choices: [tag, build] + choices: [download, download_tag] arch: required: true description: | diff --git a/libtaskotron/directives/resultsdb_directive.py b/libtaskotron/directives/resultsdb_directive.py index 5d22d89..8496e1d 100644 --- a/libtaskotron/directives/resultsdb_directive.py +++ b/libtaskotron/directives/resultsdb_directive.py @@ -228,7 +228,9 @@ def report_summary(tap, checkname): # keep just the first and the last line of output lines = ('\n'.join(detail.output)).splitlines() if len(lines) > 3: - detail.output = [lines[0], '<stripped out>', lines[-1]] + detail.output = [lines[0], + '<... %s lines stripped out ...>' % (len(lines) - 2), + lines[-1]] out = check.export_TAP(detail, checkname=checkname) # strip "TAP version 13\n1..1" (first two lines) out = '\n'.join(out.splitlines()[2:]) diff --git a/libtaskotron/directives/yumrepoinfo_directive.py b/libtaskotron/directives/yumrepoinfo_directive.py index 2d17cb0..ccace1e 100644 --- a/libtaskotron/directives/yumrepoinfo_directive.py +++ b/libtaskotron/directives/yumrepoinfo_directive.py @@ -67,6 +67,7 @@ from libtaskotron.directives import BaseDirective from libtaskotron import yumrepoinfo from libtaskotron.exceptions import TaskotronDirectiveError from libtaskotron.logger import log +from libtaskotron.arch_utils import Arches
directive_class = 'YumrepoinfoDirective'
@@ -84,15 +85,14 @@ class YumrepoinfoDirective(BaseDirective):
arches = input_data['arch']
- processed_arches = [arch for arch in arches - if arch not in ['noarch', 'all', 'src']] + processed_arches = [arch for arch in arches if arch in Arches.base] if len(processed_arches) == 0: raise TaskotronDirectiveError("No valid yumrepo arches supplied to " "yumrepoinfo directive. Recieved %r" % arches)
if len(processed_arches) > 1: - raise TaskotronDirectiveError("Yumrepoinfo requires a single arch " - "but multiple arches were submitted: %r" % arches) + raise TaskotronDirectiveError("Yumrepoinfo requires a single base " + "arch but multiple arches were submitted: %r" % arches)
arch = processed_arches[0]
diff --git a/libtaskotron/koji_utils.py b/libtaskotron/koji_utils.py index 58026b2..8b60188 100644 --- a/libtaskotron/koji_utils.py +++ b/libtaskotron/koji_utils.py @@ -153,7 +153,8 @@ class KojiClient(object): req_arches = None if 'all' in arches else arches rpms = self.session.listRPMs(buildID=info['id'], arches=req_arches) if not debuginfo: - rpms = [r for r in rpms if not r['name'].endswith('-debuginfo')] + rpms = [r for r in rpms if (not r['name'].endswith('-debuginfo')) + and ('-debuginfo-' not in r['name'])] if not src: rpms = [r for r in rpms if not r['arch'] == 'src']
@@ -204,7 +205,7 @@ class KojiClient(object): :param str tag: Koji tag to be queried for available builds, e.g. ``f20-updates-pending`` ''' - log.debug('Querying Koji for tag: %s' % tag) + log.info('Querying Koji for tag: %s' % tag) tag_data = self.session.listTagged(tag)
nvrs = ["%(nvr)s" % x for x in tag_data] diff --git a/libtaskotron/rpm_utils.py b/libtaskotron/rpm_utils.py index f3083f1..d0674b9 100644 --- a/libtaskotron/rpm_utils.py +++ b/libtaskotron/rpm_utils.py @@ -12,23 +12,6 @@ import hawkey from libtaskotron import exceptions as exc
-def basearch(arch=None): - ''' - This returns the 'base' architecture identifier for a specified architecture - (e.g. ``i386`` for ``i[3-6]86``), to be used by YUM etc. - - :param str arch: an architecture to be converted to a basearch. If ``None``, - then the arch of the current machine is used. - :return: basearch, or ``arch`` if no basearch was found for it - :rtype: str - ''' - if arch == None: - arch = os.uname()[4] - if arch in ('i486', 'i586', 'i686'): - arch = 'i386' - return arch - - def rpmformat(rpmstr, fmt='nvr', end_arch=False): ''' Parse and convert an RPM package version string into a different format. diff --git a/libtaskotron/runner.py b/libtaskotron/runner.py index d83f2c7..9a43d77 100644 --- a/libtaskotron/runner.py +++ b/libtaskotron/runner.py @@ -12,6 +12,7 @@ import imp import copy import collections
+import libtaskotron from libtaskotron import taskformula from libtaskotron import logger from libtaskotron import python_utils @@ -222,6 +223,8 @@ def main(): # https://phab.qadevel.cloud.fedoraproject.org/T273 logger.init_prior_config()
+ log.debug('Using libtaskotron %s', libtaskotron.__version__) + # parse cmdline parser = get_argparser() args = parser.parse_args() diff --git a/libtaskotron/yumrepoinfo.py b/libtaskotron/yumrepoinfo.py index d67805f..006f37a 100644 --- a/libtaskotron/yumrepoinfo.py +++ b/libtaskotron/yumrepoinfo.py @@ -12,6 +12,7 @@ import os from . import config from .logger import log from . import rpm_utils +from . import arch_utils from . import exceptions as exc
@@ -31,7 +32,7 @@ def get_yumrepoinfo(arch=None): :raise TaskotronConfigError: if file config parsing and handling failed ''' # converts to basearch, returns local arch for None - arch = rpm_utils.basearch(arch) + arch = arch_utils.basearch(arch)
if not arch in _yumrepoinfo: _yumrepoinfo[arch] = YumRepoInfo(arch) @@ -62,7 +63,7 @@ class YumRepoInfo(object): # can provide its own with `filelist` - in that case, load it. return
- self.arch = rpm_utils.basearch(arch) + self.arch = arch_utils.basearch(arch) self.filelist = (filelist if filelist is not None else [os.path.join(confdir, 'yumrepoinfo.conf') for confdir in config.CONF_DIRS])
On Tue, Nov 04, 2014 at 10:03:10AM -0700, Tim Flink wrote:
Long story short: bodhi has been having intermittent network issues for a while now and those network issues tend to kill taskotron tasks. We implemented some retries in order to mitigate the effects of those network issues but I botched one of the last builds and it didn't actually get deployed before freeze.
The new build has been running in stg and dev for almost 2 weeks now without issue. I'd like to update the production taskotron clients with this latest build since we've seen 30+ bodhi related failures already today.
I've attached the diff at the end of this email, there are several unrelated fixes here but I don't want to make a new build with the cherry picked fix extracted because the fix was committed a while ago and freeze ends tomorrow. If it causes problems, we can downgrade to the existing build or fix it when freeze ends tomorrow.
Tim
diff --git a/libtaskotron/__init__.py b/libtaskotron/__init__.py index 66923e1..2eed326 100644 --- a/libtaskotron/__init__.py +++ b/libtaskotron/__init__.py @@ -4,4 +4,4 @@ # See the LICENSE file for more details on Licensing
from __future__ import absolute_import -__version__ = '0.3.7' +__version__ = '0.3.10' diff --git a/libtaskotron/arch_utils.py b/libtaskotron/arch_utils.py new file mode 100644 index 0000000..a62171d --- /dev/null +++ b/libtaskotron/arch_utils.py @@ -0,0 +1,41 @@ +# -*- coding: utf-8 -*- +# Copyright 2009-2014, Red Hat, Inc. +# License: GPL-2.0+ http://spdx.org/licenses/GPL-2.0+ +# See the LICENSE file for more details on Licensing
+import os
+class Arches():
- '''
- Helper class for working with supported arches inside taskotron
- '''
- #: all supported architectures
- all = ['i386', 'i486', 'i586', 'i686',
'x86_64',
'armhfp', 'arm7hl',
'noarch', 'src']
- #: base architectures
- base = ['i386', 'x86_64', 'armhfp']
- #: meta architectures
- meta = ['noarch', 'src']
+def basearch(arch=None):
- '''
- This returns the 'base' architecture identifier for a specified architecture
- (e.g. ``i386`` for ``i[3-6]86``), to be used by YUM etc.
- :param str arch: an architecture to be converted to a basearch. If ``None``,
then the arch of the current machine is used.
- :return: basearch, or ``arch`` if no basearch was found for it
- :rtype: str
- '''
- if arch is None:
arch = os.uname()[4]
- if arch in ['i386', 'i486', 'i586', 'i686']:
arch = 'i386'
- if arch in ['armhfp', 'arm7hl']:
arch = 'armhfp'
- return arch
Not at all related to the freeze-break, but have you consider putting the info used by basearch/Arches() into the config file? It would allow adding a new arch w/o making a new release.
return (build2update, failures)
diff --git a/libtaskotron/config_defaults.py b/libtaskotron/config_defaults.py index fc05abd..fb77a7c 100644 --- a/libtaskotron/config_defaults.py +++ b/libtaskotron/config_defaults.py @@ -46,13 +46,13 @@ class Config(object): koji_url = 'http://koji.fedoraproject.org/kojihub' #: pkg_url = 'http://kojipkgs.fedoraproject.org/packages' #: bodhi_server = 'https://admin.fedoraproject.org/updates/' #:
- resultsdb_server = \
'http://resultsdb.qa.fedoraproject.org/resultsdb/api/v1.0/' #:
- taskotron_master = 'http://taskotron.qa.fedoraproject.org/taskmaster/' #:
- resultsdb_server = 'http://127.0.0.1/resultsdb/api/v1.0/' #:
- taskotron_master = 'http://127.0.0.1/taskmaster/' #: buildbot_task_step = 'runtask' #:
Was this meant?
Assuming the answer to the question just above is: yes, then I am +1 as well :)
Pierre
On Tue, 4 Nov 2014 18:34:14 +0100 Pierre-Yves Chibon pingou@pingoured.fr wrote:
<snip>
+import os
+class Arches():
- '''
- Helper class for working with supported arches inside taskotron
- '''
- #: all supported architectures
- all = ['i386', 'i486', 'i586', 'i686',
'x86_64',
'armhfp', 'arm7hl',
'noarch', 'src']
- #: base architectures
- base = ['i386', 'x86_64', 'armhfp']
- #: meta architectures
- meta = ['noarch', 'src']
+def basearch(arch=None):
- '''
- This returns the 'base' architecture identifier for a
specified architecture
- (e.g. ``i386`` for ``i[3-6]86``), to be used by YUM etc.
- :param str arch: an architecture to be converted to a
basearch. If ``None``,
then the arch of the current machine is used.
- :return: basearch, or ``arch`` if no basearch was found for it
- :rtype: str
- '''
- if arch is None:
arch = os.uname()[4]
- if arch in ['i386', 'i486', 'i586', 'i686']:
arch = 'i386'
- if arch in ['armhfp', 'arm7hl']:
arch = 'armhfp'
- return arch
Not at all related to the freeze-break, but have you consider putting the info used by basearch/Arches() into the config file? It would allow adding a new arch w/o making a new release.
This is mostly a stopgap - we have other arch handling issues that we want to fix but haven't gotten to it yet.
https://phab.qadevel.cloud.fedoraproject.org/T227
return (build2update, failures)
diff --git a/libtaskotron/config_defaults.py b/libtaskotron/config_defaults.py index fc05abd..fb77a7c 100644 --- a/libtaskotron/config_defaults.py +++ b/libtaskotron/config_defaults.py @@ -46,13 +46,13 @@ class Config(object): koji_url = 'http://koji.fedoraproject.org/kojihub' #: pkg_url = 'http://kojipkgs.fedoraproject.org/packages' #: bodhi_server = 'https://admin.fedoraproject.org/updates/' #:
- resultsdb_server = \
'http://resultsdb.qa.fedoraproject.org/resultsdb/api/v1.0/' #:
- taskotron_master =
'http://taskotron.qa.fedoraproject.org/taskmaster/' #:
- resultsdb_server =
'http://127.0.0.1/resultsdb/api/v1.0/' #:
- taskotron_master =
'http://127.0.0.1/taskmaster/' #: buildbot_task_step = 'runtask' #:
Was this meant?
Yes, it was. We intend to have folks install libtaskotron in order to run stuff locally, so pointing them at the production instances by default isn't a great idea. Also, those values aren't aren't even valid internal or external hostnames so it's not as big of a change as it looks :)
https://phab.qadevel.cloud.fedoraproject.org/T344
Assuming the answer to the question just above is: yes, then I am +1 as well :)
Thanks
Tim
infrastructure@lists.fedoraproject.org