diff --git a/check_maintenance_incidents.py b/check_maintenance_incidents.py index 21763000..9b0e6d3a 100755 --- a/check_maintenance_incidents.py +++ b/check_maintenance_incidents.py @@ -75,6 +75,10 @@ class MaintenanceChecker(ReviewBot.ReviewBot): for p in root.findall('./owner'): prj = p.get("project") pkg = p.get("package") + # packages dropped from Factory sometimes point to maintained distros + if prj.startswith('openSUSE:Leap') or prj.startswith('openSUSE:1'): + self.logger.debug("%s looks wrong as maintainer, skipped", prj) + continue if ((prj, pkg) in package_reviews): self.logger.debug("%s/%s already is a reviewer, not adding again" % (prj, pkg)) continue @@ -121,7 +125,8 @@ class MaintenanceChecker(ReviewBot.ReviewBot): else: origin = mapping[pkgname] self.logger.debug("{} comes from {}, submitted from {}".format(pkgname, origin, a.src_project)) - if origin.startswith('SUSE:SLE-12') and a.src_project.startswith('SUSE:SLE-12'): + if origin.startswith('SUSE:SLE-12') and a.src_project.startswith('SUSE:SLE-12') \ + or origin.startswith('openSUSE:Leap') and a.src_project.startswith('openSUSE:Leap'): self.logger.info("{} submitted from {}, no maintainer review needed".format(pkgname, a.src_project)) return diff --git a/check_source_in_factory.py b/check_source_in_factory.py index b7f1c45e..fba206c6 100755 --- a/check_source_in_factory.py +++ b/check_source_in_factory.py @@ -135,7 +135,7 @@ class FactorySourceChecker(ReviewBot.ReviewBot): try: requests = osc.core.get_request_list(self.apiurl, project, package, None, ['new', 'review'], 'submit') except (urllib2.HTTPError, urllib2.URLError): - self.logger.debug("none request") + self.logger.error("caught exception while checking %s/%s", project, package) return None for req in requests: @@ -144,26 +144,34 @@ class FactorySourceChecker(ReviewBot.ReviewBot): self.logger.debug("rq %s: %s/%s@%s"%(req.reqid, a.src_project, a.src_package, si.verifymd5)) if si.verifymd5 == rev: if req.state.name == 'new': - self.logger.debug("request ok") + self.logger.info("sr#%s ok", req.reqid) return True elif req.state.name == 'review': - self.logger.info("request still in review") + self.logger.debug("sr#%s still in review", req.reqid) if not req.reviews: - self.logger.error("request in state review but no reviews?") + self.logger.error("sr#%s in state review but no reviews?", req.reqid) return False for r in req.reviews: if r.by_project and r.state == 'new' and r.by_project.startswith('openSUSE:Factory:Staging:'): - self.logger.info("%s review by %s ok", r.state, r.by_project) + self.logger.info("sr#%s review by %s ok", req.reqid, r.by_project) continue if r.state != 'accepted': - self.logger.debug("review %s/%s/%s in state %s", r.by_user, r.by_group, r.by_package, r.state) + if r.by_user: + self.logger.info("sr#%s waiting for review by %s", req.reqid, r.by_user) + elif r.by_group: + self.logger.info("sr#%s waiting for review by %s", req.reqid, r.by_group) + elif r.by_project: + if r.by_package: + self.logger.info("sr#%s waiting for review by %s/%s", req.reqid, r.by_project, r.by_package) + else: + self.logger.info("sr#%s waiting for review by %s", req.reqid, r.by_project) return None return True else: - self.logger.error("request in state %s not expected"%req.state.name) + self.logger.error("sr#%s in state %s not expected", req.reqid, req.state.name) return None else: - self.logger.debug("submission has different sources") + self.logger.info("sr#%s has different sources", req.reqid) return False class CommandLineInterface(ReviewBot.CommandLineInterface): diff --git a/leaper.py b/leaper.py index f1d0510b..17794f77 100755 --- a/leaper.py +++ b/leaper.py @@ -21,7 +21,9 @@ # SOFTWARE. from pprint import pprint -import os, sys, re +import os +import sys +import re import logging from optparse import OptionParser import cmdln @@ -39,17 +41,47 @@ import ReviewBot from check_maintenance_incidents import MaintenanceChecker from check_source_in_factory import FactorySourceChecker +from osclib.comments import CommentAPI + +class LogToString(logging.Filter): + def __init__(self, obj, propname): + self.obj = obj + self.propname = propname + + def filter(self, record): + if record.levelno >= logging.INFO: + line = record.getMessage() + comment_log = getattr(self.obj, self.propname) + if comment_log is not None: + comment_log.append(line) + setattr(self.obj, self.propname, comment_log) + return True + class Leaper(ReviewBot.ReviewBot): def __init__(self, *args, **kwargs): ReviewBot.ReviewBot.__init__(self, *args, **kwargs) + + self.do_comments = True + self.commentapi = CommentAPI(self.apiurl) + self.maintbot = MaintenanceChecker(*args, **kwargs) # for FactorySourceChecker self.factory = FactorySourceChecker(*args, **kwargs) self.needs_reviewteam = False self.pending_factory_submission = False - self.source_in_factory = False + self.source_in_factory = None + self.needs_release_manager = False + self.release_manager_group = 'leap-reviewers' + self.must_approve_version_updates = False + self.must_approve_maintenance_updates = False + + self.comment_marker_re = re.compile(r'') + + self.comment_log = None + self.commentlogger = LogToString(self, 'comment_log') + self.logger.addFilter(self.commentlogger) def prepare_review(self): @@ -77,20 +109,39 @@ class Leaper(ReviewBot.ReviewBot): if package in self.lookup_422: origin = self.lookup_422[package] + is_fine_if_factory = False + not_in_factory_okish = False if origin: - self.logger.debug("origin {}".format(origin)) + self.logger.info("expected origin is '%s'", origin) if origin.startswith('Devel;'): - self.needs_reviewteam = True (dummy, origin, dummy) = origin.split(';') - if origin == src_project: - self.logger.debug("exact match") - return True + if origin != src_project: + self.logger.debug("not submitted from devel project") + return False + is_fine_if_factory = True + not_in_factory_okish = True + if self.must_approve_version_updates: + self.needs_release_manager = True + # fall through to check history and requests elif origin.startswith('openSUSE:Factory'): - return self._check_factory(target_package, src_srcinfo) + if self.must_approve_version_updates: + self.needs_release_manager = True + if origin == src_project: + self.source_in_factory = True + return True + is_fine_if_factory = True + # fall through to check history and requests + elif origin == 'FORK': + is_fine_if_factory = True + not_in_factory_okish = True + self.needs_release_manager = True + # fall through to check history and requests elif origin.startswith('openSUSE:Leap:42.1'): + if self.must_approve_maintenance_updates: + self.needs_release_manager = True # submitted from :Update if src_project.startswith(origin): - self.logger.debug("match 42.1") + self.logger.debug("submission from 42.1 ok") return True # submitted from elsewhere but is in :Update else: @@ -110,23 +161,25 @@ class Leaper(ReviewBot.ReviewBot): # Factory. So it's ok to keep upgrading it to Factory # TODO: whitelist packages where this is ok and block others? if oldorigin.startswith('openSUSE:Factory'): - if src_project == oldorigin: - self.logger.debug("Upgrade to Factory again. Submitted from Factory") - return True - good = self._check_factory(target_package, src_srcinfo) - if good or good == None: - self.logger.debug("Upgrade to Factory again. It's in Factory") - return good - # or maybe in SP2? + self.logger.info("Package was from Factory in 42.1") + # check if an attempt to switch to SLE package is made good = self.factory._check_project('SUSE:SLE-12-SP2:GA', target_package, src_srcinfo.verifymd5) if good: - self.logger.debug("hope it's ok to change to SP2") + self.logger.info("request sources come from SLE") + self.needs_release_manager = True return good - # else other project or FORK, fall through + # the release manager needs to review attempts to upgrade to Factory + is_fine_if_factory = True + self.needs_release_manager = True elif origin.startswith('SUSE:SLE-12'): + if self.must_approve_maintenance_updates: + self.needs_release_manager = True # submitted from :Update - if origin.endswith(':GA') \ + if origin == src_project: + self.logger.debug("submission origin ok") + return True + elif origin.endswith(':GA') \ and src_project == origin[:-2]+'Update': self.logger.debug("sle update submission") return True @@ -134,41 +187,56 @@ class Leaper(ReviewBot.ReviewBot): if origin.startswith('SUSE:SLE-12:'): if src_project.startswith('SUSE:SLE-12-SP1:') \ or src_project.startswith('SUSE:SLE-12-SP2:'): - self.logger.debug("higher service pack ok") + self.logger.info("submission from service pack ok") return True elif origin.startswith('SUSE:SLE-12-SP1:'): if src_project.startswith('SUSE:SLE-12-SP2:'): - self.logger.debug("higher service pack ok") - # else other project or FORK, fall through + self.logger.info("submission from service pack ok") + return True - # we came here because none of the above checks find it good, so - # let's see if the package is in Factory at least - is_in_factory = self._check_factory(target_package, src_srcinfo) - if is_in_factory: - self.source_in_factory = True - elif is_in_factory is None: - self.pending_factory_submission = True + self.needs_release_manager = True + good = self._check_project_and_request('openSUSE:Leap:42.2:SLE-workarounds', target_package, src_srcinfo) + if good or good == None: + self.logger.info("found sources in SLE-workarounds") + return good + # the release manager needs to review attempts to upgrade to Factory + is_fine_if_factory = True else: - if not src_project.startswith('SUSE:SLE-12'): - self.needs_reviewteam = True - + self.logger.error("unhandled origin %s", origin) + return False else: # no origin - # SLE and Factory are ok + # submission from SLE is ok + if src_project.startswith('SUSE:SLE-12'): + return True + + is_fine_if_factory = True + self.needs_release_manager = True + + # we came here because none of the above checks find it good, so + # let's see if the package is in Factory at least + is_in_factory = self._check_factory(target_package, src_srcinfo) + if is_in_factory: + self.source_in_factory = True + self.needs_reviewteam = False + elif is_in_factory is None: + self.pending_factory_submission = True + self.needs_reviewteam = False + else: if src_project.startswith('SUSE:SLE-12') \ - or src_project.startswith('openSUSE:Factory'): + or src_project.startswith('openSUSE:Leap:42.'): + self.needs_reviewteam = False + else: + self.needs_reviewteam = True + self.source_in_factory = False + + if is_fine_if_factory: + if self.source_in_factory: return True - # submitted from elsewhere, check it's in Factory - good = self._check_factory(target_package, src_srcinfo) - if good: - self.source_in_factory = True + elif self.pending_factory_submission: + return None + elif not_in_factory_okish: + self.needs_reviewteam = True return True - elif good == None: - self.pending_factory_submission = True - return good - # or maybe in SP2? - good = self.factory._check_project('SUSE:SLE-12-SP2:GA', target_package, src_srcinfo.verifymd5) - if good: - return good return False @@ -189,35 +257,73 @@ class Leaper(ReviewBot.ReviewBot): return good return False + def _check_project_and_request(self, project, target_package, src_srcinfo): + good = self.factory._check_project(project, target_package, src_srcinfo.verifymd5) + if good: + return good + good = self.factory._check_requests(project, target_package, src_srcinfo.verifymd5) + if good or good == None: + return good + return False + def check_one_request(self, req): self.review_messages = self.DEFAULT_REVIEW_MESSAGES.copy() self.needs_reviewteam = False + self.needs_release_manager = False self.pending_factory_submission = False - self.source_in_factory = False + self.source_in_factory = None + self.comment_log = [] if len(req.actions) != 1: msg = "only one action per request please" self.review_messages['declined'] = msg return False - # if the fallback reviewer created the request she probably - # knows what she does :-) - if self.fallback_user and req.get_creator() == self.fallback_user: - self.logger.debug("skip fallback review") - return True - - has_upstream_sources = ReviewBot.ReviewBot.check_one_request(self, req) + request_ok = ReviewBot.ReviewBot.check_one_request(self, req) has_correct_maintainer = self.maintbot.check_one_request(req) - # not reviewed yet? - if has_upstream_sources is None: - return None + self.logger.debug("review result: %s", request_ok) + self.logger.debug("has_correct_maintainer: %s", has_correct_maintainer) + if self.pending_factory_submission: + self.logger.info("submission is waiting for a Factory request to complete") + elif self.source_in_factory: + self.logger.info("the submitted sources are in or accepted for Factory") + elif self.source_in_factory == False: + self.logger.info("the submitted sources are NOT in Factory") - self.logger.debug("upstream sources: {}, maintainer ok: {}".format(has_upstream_sources, has_correct_maintainer)) + if request_ok == False: + self.logger.info("NOTE: if you think the automated review was wrong here, please talk to the release team before reopening the request") + elif self.needs_release_manager: + self.logger.info("request needs review by release management") + + if self.comment_log: + result = None + if request_ok is None: + state = 'seen' + elif request_ok: + state = 'done' + result = 'accepted' + else: + state = 'done' + result = 'declined' + self.add_comment(req, '\n\n'.join(self.comment_log), state) + self.comment_log = None + + if self.needs_release_manager: + add_review = True + for r in req.reviews: + if r.by_group == self.release_manager_group and (r.state == 'new' or r.state == 'accepted'): + add_review = False + self.logger.debug("%s already is a reviewer", self.release_manager_group) + break + if add_review: + if self.add_review(req, by_group = self.release_manager_group) != True: + self.review_messages['declined'] += '\nadding %s failed' % self.release_manager_group + return False if self.needs_reviewteam: add_review = True - self.logger.debug("%s needs review by opensuse-review-team"%req.reqid) + self.logger.info("%s needs review by opensuse-review-team"%req.reqid) for r in req.reviews: if r.by_group == 'opensuse-review-team': add_review = False @@ -228,32 +334,52 @@ class Leaper(ReviewBot.ReviewBot): self.review_messages['declined'] += '\nadding opensuse-review-team failed' return False - if has_upstream_sources != True or has_correct_maintainer != True: - if has_upstream_sources != True: - self.review_messages['declined'] += '\nOrigin project changed' - pkg = req.actions[0].tgt_package - if pkg in self.lookup_422: - self.review_messages['declined'] += '(was {})'.format(self.lookup_422[pkg]) - if self.source_in_factory: - self.review_messages['declined'] += '\nsource is in Factory' - if self.pending_factory_submission: - self.review_messages['declined'] += '\na submission to Factory is pending' - self.logger.debug("origin changed but waiting for Factory submission to complete") - # FXIME: we should add the human reviewer here - # and leave a comment - return None - # shouldn't happen actually - if has_correct_maintainer != True: - self.review_messages['declined'] += '\nMaintainer check failed' - return False - - return True + return request_ok def check_action__default(self, req, a): # decline all other requests for fallback reviewer self.logger.debug("auto decline request type %s"%a.type) return False + # TODO: make generic, move to Reviewbot. Used by multiple bots + def add_comment(self, req, msg, state, result=None): + if not self.do_comments: + return + + comment = "\n" % (state, ' result=%s' % result if result else '') + comment += "\n" + msg + + (comment_id, comment_state, comment_result, comment_text) = self.find_obs_request_comment(req, state) + + if comment_id is not None and state == comment_state: + # count number of lines as aproximation to avoid spamming requests + # for slight wording changes in the code + if len(comment_text.split('\n')) == len(comment.split('\n')): + self.logger.debug("not worth the update, previous comment %s is state %s", comment_id, comment_state) + return + + self.logger.debug("adding comment to %s, state %s result %s", req.reqid, state, result) + self.logger.debug("message: %s", msg) + if not self.dryrun: + if comment_id is not None: + self.commentapi.delete(comment_id) + self.commentapi.add_comment(request_id=req.reqid, comment=str(comment)) + + def find_obs_request_comment(self, req, state=None): + """Return previous comments (should be one).""" + if self.do_comments: + comments = self.commentapi.get_comments(request_id=req.reqid) + for c in comments.values(): + m = self.comment_marker_re.match(c['comment']) + if m and (state is None or state == m.group('state')): + return c['id'], m.group('state'), m.group('result'), c['comment'] + return None, None, None, None + + def check_action__default(self, req, a): + self.logger.info("unhandled request type %s"%a.type) + self.needs_release_manager = True + return True + class CommandLineInterface(ReviewBot.CommandLineInterface): def __init__(self, *args, **kwargs): @@ -262,6 +388,10 @@ class CommandLineInterface(ReviewBot.CommandLineInterface): def get_optparser(self): parser = ReviewBot.CommandLineInterface.get_optparser(self) + parser.add_option("--no-comment", dest='comment', action="store_false", default=True, help="don't actually post comments to obs") + parser.add_option("--manual-version-updates", action="store_true", help="release manager must approve version updates") + parser.add_option("--manual-maintenance-updates", action="store_true", help="release manager must approve maintenance updates") + return parser def setup_checker(self): @@ -281,6 +411,12 @@ class CommandLineInterface(ReviewBot.CommandLineInterface): group = group, \ logger = self.logger) + if self.options.manual_version_updates: + bot.must_approve_version_updates = True + if self.options.manual_maintenance_updates: + bot.must_approve_maintenance_updates = True + bot.do_comments = self.options.comment + return bot if __name__ == "__main__": diff --git a/osclib/stagingapi.py b/osclib/stagingapi.py index 7084e752..1708145b 100644 --- a/osclib/stagingapi.py +++ b/osclib/stagingapi.py @@ -25,6 +25,7 @@ import yaml from osc import conf from osc import oscerr +from osc.core import show_package_meta from osc.core import change_review_state from osc.core import delete_package from osc.core import get_group @@ -1282,7 +1283,24 @@ class StagingAPI(object): name = p.attrib['name'] self._package_metas[project][name] = p + + def _get_devel_project(self, project, package): + """ get devel project for a single project""" + if not self.item_exists(project, package): + return None + + m = show_package_meta(self.apiurl, project, package) + node = ET.fromstring(''.join(m)).find('devel') + if node is None: + return None + else: + return node.get('project') + def get_devel_project(self, project, package): + # if _package_metas is None we force individual queries + if self._package_metas is None: + return self._get_devel_project(project,package) + if not project in self._package_metas: self._fill_package_meta(project)