Skip to content

Commit

Permalink
Refactor approve_testing.py and set date_approved consistently
Browse files Browse the repository at this point in the history 
This was a logical consequence of the previous change to drop
a branch in check_karma_thresholds. That change affected when
date_approved got set, and investigating that, I realized it is
not set at all consistently. With this, we consistently set
date_approved to be the first time - now always as a
`datetime.datetime` instance - an update becomes eligible for
manual push. Previously it was sometimes set then, sometimes
set when an update was autopushed, and sometimes never set at
all.

We also drop an early bailout in approve_testing.py that is no
longer ever hit in the real world. It initially bailed any time
the release had no "mandatory days in testing", which would have
been true for Rawhide (and early Branched). But when autotime
was added, it was changed to bail only if the release had no
mandatory days in testing *and* the update did not have autotime
set. Updates for Rawhide and other releases like it (ones not
"composed by Bodhi") are always forced to have autotime enabled,
and all releases that *are* "composed by Bodhi" have mandatory
days in testing, so we just never really satisfy the conditions
any more. I verified this by asking nirik to check Bodhi's logs
for the message that should be logged when this bailout is hit;
there were no occurrences of it. Anyway, I don't think an early
bailout is ever really justifiable any more, because we have
gating checks for all branches of Fedora these days.

This refactors approve_testing a bit just to make the different
paths through it clearer, and separate different work into
different functions. We also add comments to make it clearer how
the different autopush paths actually *work*, which was not at
all clear to me before researching this commit.

Signed-off-by: Adam Williamson <[email protected]>
(cherry picked from commit bddee6a)
  • Loading branch information
@AdamWill @mergify
AdamWill authored and mergify[bot] committed Oct 5, 2024
1 parent 329f426 commit 7a415ad
Show file tree
Hide file tree
Showing 3 changed files with 177 additions and 138 deletions.
14 changes: 12 additions & 2 deletions bodhi-server/bodhi/server/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(
Expand Down
270 changes: 158 additions & 112 deletions bodhi-server/bodhi/server/tasks/approve_testing.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -32,136 +51,163 @@


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.")
finally:
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')
Loading

0 comments on commit 7a415ad

Please sign in to comment.