diff --git a/bodhi-server/bodhi/server/models.py b/bodhi-server/bodhi/server/models.py index a38839e6a3..21dd56cb37 100644 --- a/bodhi-server/bodhi/server/models.py +++ b/bodhi-server/bodhi/server/models.py @@ -3798,7 +3798,14 @@ def check_requirements(self, session, settings): def check_karma_thresholds(self, db, agent): """ - Check if we have reached either karma threshold, and adjust state as necessary. + Check if we have reached some karma thresholds, and adjust state as necessary. + + If we receive negative karma, disable autopush (both time and karma). If we + reach the karma autopush threshold (and the release is composed by Bodhi), set + the request to stable. If we reach the auto-unpush threshold, obsolete the + update. This method does **NOT** handle commenting and setting date_approved + when the update reaches the manual push karma threshold. That is done by the + approve_testing.py script. This method will call :meth:`set_request` if necessary. If the update is locked, it will ignore karma thresholds and raise an Exception. @@ -3831,11 +3838,14 @@ def check_karma_thresholds(self, db, agent): self.comment(db, text, author='bodhi') elif self.stable_karma and self.karma >= self.stable_karma \ and self.release.composed_by_bodhi and self.autokarma: + # Updates for releases not "composed by Bodhi" (Rawhide, + # ELN...) are pushed stable only by approve_testing.py if config.get('test_gating.required') and not self.test_gating_passed: log.info("%s reached stable karma threshold, but does not meet gating " "requirements", self.alias) return - self.date_approved = datetime.utcnow() + if not self.date_approved: + self.date_approved = datetime.utcnow() log.info("Automatically marking %s as stable", self.alias) self.set_request(db, UpdateRequest.stable, agent) notifications.publish(update_schemas.UpdateKarmaThresholdV1.from_dict( diff --git a/bodhi-server/bodhi/server/tasks/approve_testing.py b/bodhi-server/bodhi/server/tasks/approve_testing.py index edfc8bc432..22f4456d6d 100644 --- a/bodhi-server/bodhi/server/tasks/approve_testing.py +++ b/bodhi-server/bodhi/server/tasks/approve_testing.py @@ -15,11 +15,30 @@ # You should have received a copy of the GNU General Public License # along with this program; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -"""Comment on updates after they reach the mandatory amount of time in the testing repository.""" -import logging +""" +Make appropriate changes to updates that reach certain karma or time-in-stable thresholds. + +This script is intended to run as a regular job (cronjob or the like). It finds all updates in +UpdateStatus.testing with no request, then does one of three things. If the update does not meet +testing requirements (i.e. it doesn't have enough karma or time in testing to even be manually +pushed, or it fails gating checks), we do nothing. Otherwise, we set the date_approved +for the update if it's not already set, then choose one of the two other options. + +If the update meets testing requirements, has autopush after a certain time in testing (autotime) +enabled, and has reached that threshold, we push it. Note this is the **ONLY** way updates +for releases "not composed by Bodhi" (Rawhide, ELN, Branched for the first few weeks) are ever +pushed stable. The other way updates can be autopushed stable - Update.check_karma_thresholds(), +called by Update.comment() - opts out of handling "not composed by Bodhi" release updates. -from sqlalchemy import func +If the update meets testing requirements but does not have autotime enabled or has +not reached that threshold, we check to see if the update already has a comment saying it is +now "approved" for push to stable. If not, we post that comment, and publish the +UpdateRequirementsMetStableV1 message. +""" + +import datetime +import logging from bodhi.messages.schemas import update as update_schemas from bodhi.server import Session, notifications, buildsys @@ -32,18 +51,13 @@ def main(): - """ - Comment on updates that are eligible to be pushed to stable. - - Queries for updates in the testing state that have a NULL request, and run approve_update on - them. - """ + """Query for updates in the testing state that have a NULL request, and run process_update.""" db_factory = transactional_session_maker() try: with db_factory() as db: testing = db.query(Update).filter_by(status=UpdateStatus.testing, request=None) for update in testing: - approve_update(update, db) + process_update(update, db) db.commit() except Exception: log.exception("There was an error approving testing updates.") @@ -51,117 +65,149 @@ def main(): db_factory._end_session() -def approve_update(update: Update, db: Session): - """Add a comment to an update if it is ready for stable. +def autopush_update(update: Update, db: Session): + """ + Push an update that has autopush for time enabled and has reached the required threshold. + + For releases composed by Bodhi, we set the request and leave the compose process to do the + status change. For releases not composed by Bodhi, we do the status change here. + """ + if not update.has_stable_comment: + notifications.publish(update_schemas.UpdateRequirementsMetStableV1.from_dict( + dict(update=update))) + log.info(f"Automatically marking {update.alias} as stable") + # For releases composed by Bodhi, just set the request, and leave + # the rest to the composer + if update.release.composed_by_bodhi: + update.set_request(db=db, action=UpdateRequest.stable, username="bodhi") + return + # For releases not composed by Bodhi, do all the work here + # Both side-tag and non-side-tag updates + conflicting_builds = update.find_conflicting_builds() + if conflicting_builds: + builds_str = str.join(", ", conflicting_builds) + update.comment( + db, + "This update cannot be pushed to stable. " + f"These builds {builds_str} have a more recent " + f"build in koji's {update.release.stable_tag} tag.", + author="bodhi") + update.request = None + if update.from_tag is not None: + update.status = UpdateStatus.pending + update.remove_tag( + update.release.get_pending_testing_side_tag(update.from_tag)) + else: + update.status = UpdateStatus.obsolete + update.remove_tag(update.release.pending_testing_tag) + update.remove_tag(update.release.candidate_tag) + db.commit() + log.info(f"{update.alias} has conflicting builds - bailing") + return + update.add_tag(update.release.stable_tag) + update.status = UpdateStatus.stable + update.request = None + update.pushed = True + update.date_stable = datetime.datetime.utcnow() + update.comment(db, "This update has been submitted for stable by bodhi", + author=u'bodhi') + update.modify_bugs() + db.commit() + if update.from_tag: + # Merging the side tag should happen here + pending_signing_tag = update.release.get_pending_signing_side_tag( + update.from_tag) + testing_tag = update.release.get_pending_testing_side_tag(update.from_tag) + update.remove_tag(pending_signing_tag) + update.remove_tag(testing_tag) + update.remove_tag(update.from_tag) + # Delete side-tag and its children after Update has enter stable + # We can't fully rely on Koji's auto-purge-when-empty because + # there may be older nvrs tagged in the side-tag + koji = buildsys.get_session() + koji.multicall = True + koji.deleteTag(pending_signing_tag) + koji.deleteTag(testing_tag) + koji.deleteTag(update.from_tag) + koji.multiCall() + else: + # Non side-tag updates + update.remove_tag(update.release.pending_testing_tag) + update.remove_tag(update.release.pending_stable_tag) + update.remove_tag(update.release.pending_signing_tag) + update.remove_tag(update.release.testing_tag) + update.remove_tag(update.release.candidate_tag) + + +def approved_comment_message(update: Update, db: Session): + """Post "approved" comment and publish UpdatesRequirementsMetStable message.""" + # If this update was already commented, skip it + if update.has_stable_comment: + log.info(f"{update.alias} has already the comment that it can be pushed to stable - " + "bailing") + return + # post the comment + update.comment( + db, + str(config.get('testing_approval_msg')), + author='bodhi', + # Only send email notification about the update reaching + # testing approval on releases composed by bodhi + email_notification=update.release.composed_by_bodhi + ) + # publish the message + notifications.publish(update_schemas.UpdateRequirementsMetStableV1.from_dict( + dict(update=update))) + + +def process_update(update: Update, db: Session): + """ + Check requirements, update date_approved, then call appropriate handler function. + + In all cases, this will set date_approved if the update "meets testing requirements" - + which means it has reached either the minimum karma or time threshold to be pushed + stable, and its gating status is passed - and that date has not been set before. + + After that, if the update has automatic push for time enabled and has reached the required + time threshold, this will call autopush_update to handle pushing it. It is intentional + that we do not called approved_comment_message() in this case - there is no point alerting + the maintainer that the update can be pushed manually if we are already going to push it + automatically. See issue #3846. + + Otherwise - if the update is approved but is not being autopushed for time - this will + call approved_comment_message() to post the approval comment and publish the + UpdateRequirementsMetStable message, if it has not been done before. - Check that the update is eligible to be pushed to stable but hasn't had comments from Bodhi to - this effect. Add a comment stating that the update may now be pushed to stable. + It has not yet proven necessary to check the karma autopush threshold here. For releases that + are "composed by Bodhi", Update.check_karma_thresholds() pushes updates as soon as they + reach the karma autopush threshold. For releases that are not "composed by Bodhi", on update + creation, autotime is forced to True and the time threshold is forced to 0, thus updates for + these releases are *always* eligible for autotime push here, as soon as they pass gating, + there is no case in which autokarma would be relevant. Args: update: an update in testing that may be ready for stable. + db: a database session. """ - if not update.release.mandatory_days_in_testing and not update.autotime: - # If this release does not have any testing requirements and is not autotime, - # skip it - log.info(f"{update.alias} doesn't have mandatory days in testing - bailing") - return - # If updates have reached the testing threshold, say something! Keep in mind - # that we don't care about karma here, because autokarma updates get their request set - # to stable by the Update.comment() workflow when they hit the required threshold. Thus, - # this function only needs to consider the time requirements because these updates have - # not reached the karma threshold. + # meets_testing_requirements will be True if all non-karma / non-time + # requirements are met, and the update has reached the minimum karma + # threshold or wait period for a manual push to be allowed, so this + # means "update is eligible to be manually pushed stable" if not update.meets_testing_requirements: log.info(f"{update.alias} has not met testing requirements - bailing") return log.info(f'{update.alias} now meets testing requirements') - # If the update is going to be pushed automatically to stable, do not - # double comment that the maintainer can push it manually (#3846) - if not update.autotime or update.days_in_testing < update.stable_days: - # If this update was already commented, skip it - if update.has_stable_comment: - log.info(f"{update.alias} has already the comment that it can be pushed to stable - " - "bailing") - return - # Only send email notification about the update reaching - # testing approval on releases composed by bodhi - update.comment( - db, - str(config.get('testing_approval_msg')), - author='bodhi', - email_notification=update.release.composed_by_bodhi - ) - notifications.publish(update_schemas.UpdateRequirementsMetStableV1.from_dict( - dict(update=update))) + # always set date_approved, if it has never been set before: this + # date indicates "first date update became eligible for manual push" + if not update.date_approved: + update.date_approved = datetime.datetime.utcnow() if update.autotime and update.days_in_testing >= update.stable_days: - if not update.has_stable_comment: - notifications.publish(update_schemas.UpdateRequirementsMetStableV1.from_dict( - dict(update=update))) - log.info(f"Automatically marking {update.alias} as stable") - # For now only rawhide update can be created using side tag - # Do not add the release.pending_stable_tag if the update - # was created from a side tag. - if update.release.composed_by_bodhi: - if not update.date_approved: - update.date_approved = func.current_timestamp() - update.set_request(db=db, action=UpdateRequest.stable, username="bodhi") - # For updates that are not included in composes run by bodhi itself, - # mark them as stable - else: - # Single and Multi build update - conflicting_builds = update.find_conflicting_builds() - if conflicting_builds: - builds_str = str.join(", ", conflicting_builds) - update.comment( - db, - "This update cannot be pushed to stable. " - f"These builds {builds_str} have a more recent " - f"build in koji's {update.release.stable_tag} tag.", - author="bodhi") - update.request = None - if update.from_tag is not None: - update.status = UpdateStatus.pending - update.remove_tag( - update.release.get_pending_testing_side_tag(update.from_tag)) - else: - update.status = UpdateStatus.obsolete - update.remove_tag(update.release.pending_testing_tag) - update.remove_tag(update.release.candidate_tag) - db.commit() - log.info(f"{update.alias} has conflicting builds - bailing") - return - update.add_tag(update.release.stable_tag) - update.status = UpdateStatus.stable - update.request = None - update.pushed = True - update.date_stable = update.date_approved = func.current_timestamp() - update.comment(db, "This update has been submitted for stable by bodhi", - author=u'bodhi') - update.modify_bugs() - db.commit() - # Multi build update - if update.from_tag: - # Merging the side tag should happen here - pending_signing_tag = update.release.get_pending_signing_side_tag( - update.from_tag) - testing_tag = update.release.get_pending_testing_side_tag(update.from_tag) - update.remove_tag(pending_signing_tag) - update.remove_tag(testing_tag) - update.remove_tag(update.from_tag) - # Delete side-tag and its children after Update has enter stable - # We can't fully rely on Koji's auto-purge-when-empty because - # there may be older nvrs tagged in the side-tag - koji = buildsys.get_session() - koji.multicall = True - koji.deleteTag(pending_signing_tag) - koji.deleteTag(testing_tag) - koji.deleteTag(update.from_tag) - koji.multiCall() - else: - # Single build update - update.remove_tag(update.release.pending_testing_tag) - update.remove_tag(update.release.pending_stable_tag) - update.remove_tag(update.release.pending_signing_tag) - update.remove_tag(update.release.testing_tag) - update.remove_tag(update.release.candidate_tag) + # if update *additionally* meets the time-based autopush threshold, + # push it + autopush_update(update, db) + else: + # otherwise, post the comment and publish the message announcing + # it is eligible for manual push, if this has not been done + approved_comment_message(update, db) log.info(f'{update.alias} processed by approve_testing') diff --git a/bodhi-server/tests/tasks/test_approve_testing.py b/bodhi-server/tests/tasks/test_approve_testing.py index 1399839cec..dd4fb2be6a 100644 --- a/bodhi-server/tests/tasks/test_approve_testing.py +++ b/bodhi-server/tests/tasks/test_approve_testing.py @@ -86,7 +86,6 @@ def _assert_not_pushed(self): """Run all checks to ensure we did not push the update.""" assert self.update.status == models.UpdateStatus.testing assert self.update.request is None - assert self.update.date_approved is None assert self.update.date_pushed is None assert not self.update.pushed @@ -105,24 +104,6 @@ def _assert_commented(self, times): ] assert len(approvalcomments) == times - # for robustness, mock this as True so we *would* do something without the early return - @patch('bodhi.server.models.Update.meets_testing_requirements', True) - @patch.dict(config, [('fedora.mandatory_days_in_testing', 0)]) - def test_no_mandatory_days_or_autotime(self): - """Ensure that if the update has no mandatory days in testing - and autotime is False, we do nothing. - """ - self.update.autotime = False - # this should publish no messages - with fml_testing.mock_sends(): - approve_testing_main() - # we should not have pushed - self._assert_not_pushed() - # we should not have posted approval comment - self._assert_commented(0) - # we should not have set date_approved - assert self.update.date_approved is None - @patch('bodhi.server.models.Update.meets_testing_requirements', False) def test_update_not_approved(self): """ @@ -166,9 +147,8 @@ def test_update_approved_only(self, autotime_enabled): self._assert_not_pushed() # we should have posted approval comment once self._assert_commented(1) - # we should not have set date_approved (currently we only do that - # when pushing, not approving) - assert self.update.date_approved is None + # we should have set date_approved + assert self.update.date_approved is not None # re-run, this time no additional message should be published with fml_testing.mock_sends(): approve_testing_main() @@ -284,12 +264,15 @@ def test_update_conflicting_build_not_pushed(self, build_creation_time, # message publishing happens before the conflicting build check, so # even when there's a conflicting build, we publish this message - # (and set date_approved) with fml_testing.mock_sends(update_schemas.UpdateRequirementsMetStableV1): approve_testing_main() assert self.update.status == update_status - assert self.update.date_approved is None + # date_approved is also set before the conflicting build check, so + # we do set it in this case. this isn't really wrong, because that + # date really indicates the first date an update met all the requirements + # to be manually submitted stable, which is still the case here + assert self.update.date_approved is not None bodhi = self.db.query(models.User).filter_by(name='bodhi').one() cmnts = self.db.query(models.Comment).filter_by(update_id=self.update.id, user_id=bodhi.id)