From 2a0408c4463d40481c0cf285835aaafb79e10e6e Mon Sep 17 00:00:00 2001 From: Qixiang Wan Date: Nov 21 2017 08:55:55 +0000 Subject: Track event state for the manual rebuild case When rebuild is triggered by manual, we should log the event and the generated event event there is no action taken upon the events, so it can be tracked. --- diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index 6d499aa..53ceac6 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -30,7 +30,7 @@ from freshmaker import conf, log, db, models from freshmaker.kojiservice import koji_service, parse_NVR from freshmaker.mbs import MBS from freshmaker.models import ArtifactBuildState -from freshmaker.types import ArtifactType +from freshmaker.types import ArtifactType, EventState from freshmaker.models import ArtifactBuild, Event from freshmaker.utils import krb_context, get_rebuilt_nvr from freshmaker.errors import UnprocessableEntity, ProgrammingError @@ -65,9 +65,9 @@ def fail_event_on_handler_exception(func): db_event = db.session.query(Event).filter_by( id=db_event_id).first() if db_event: - db_event.builds_transition( - ArtifactBuildState.FAILED.value, "Handling of " - "event failed with traceback: %s" % (str(e))) + msg = "Handling of event failed with traceback: %s" % (str(e)) + db_event.transition(EventState.FAILED, msg) + db_event.builds_transition(ArtifactBuildState.FAILED.value, msg) db.session.commit() raise return decorator @@ -121,6 +121,10 @@ class BaseHandler(object): return self._db_event_id @property + def current_db_event(self): + return db.session.query(Event).filter_by(id=self.current_db_event_id).first() + + @property def current_db_artifact_build_id(self): return self._db_artifact_build_id diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index 3f02075..85286e1 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -26,7 +26,6 @@ import json import koji from freshmaker import conf, db, log -from freshmaker import messaging from freshmaker.events import ErrataAdvisoryRPMsSignedEvent from freshmaker.events import ODCSComposeStateChangeEvent from freshmaker.handlers import BaseHandler, fail_event_on_handler_exception @@ -67,22 +66,26 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): self.event = event - # Check if we are allowed to build this advisory. - if not event.manual and not self.allow_build( - ArtifactType.IMAGE, advisory_name=event.errata_name, - advisory_security_impact=event.security_impact): - msg = 'Errata advisory {0} not allowed to trigger ' \ - 'rebuilds.'.format(event.errata_id) - log.info(msg) - return [] + # Generate the Database representation of `event`, it can be + # triggered by user, we want to track what happened - # Generate the Database representation of `event`. db_event = Event.get_or_create( db.session, event.msg_id, event.search_key, event.__class__, released=False, manual=event.manual) db.session.commit() self.set_context(db_event) + # Check if we are allowed to build this advisory. + if not event.manual and not self.allow_build( + ArtifactType.IMAGE, advisory_name=event.errata_name, + advisory_security_impact=event.security_impact): + msg = ("Errata advisory {0} is not allowed by internal policy " + "to trigger rebuilds.".format(event.errata_id)) + db_event.transition(EventState.SKIPPED, msg) + db.session.commit() + log.info(msg) + return [] + # Get and record all images to rebuild based on the current # ErrataAdvisoryRPMsSignedEvent event. builds = {} @@ -93,6 +96,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): msg = 'No container images to rebuild for advisory %r' % event.errata_name log.info(msg) db_event.transition(EventState.SKIPPED, msg) + db.session.commit() return [] # Generate the ODCS compose with RPMs from the current advisory. diff --git a/freshmaker/handlers/internal/manual_rebuild.py b/freshmaker/handlers/internal/manual_rebuild.py index 5b0dd58..5d66f61 100644 --- a/freshmaker/handlers/internal/manual_rebuild.py +++ b/freshmaker/handlers/internal/manual_rebuild.py @@ -27,6 +27,7 @@ from freshmaker.handlers import ContainerBuildHandler from freshmaker.events import ( FreshmakerManualRebuildEvent, ErrataAdvisoryRPMsSignedEvent) from freshmaker.errata import Errata +from freshmaker.types import EventState __all__ = ('FreshmakerManualRebuildHandler',) @@ -52,15 +53,20 @@ class FreshmakerManualRebuildHandler(ContainerBuildHandler): event_type_id=EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent], search_key=str(errata_id)).first() if db_event: - log.info("Ignoring Errata advisory %d - it already exists in " - "Freshmaker db.", errata_id) + msg = ("Ignoring Errata advisory %d - it already exists in " + "Freshmaker db." % errata_id) + self.current_db_event.transition(EventState.SKIPPED, msg) + db.session.commit() + log.info(msg) return [] # Get additional info from Errata to fill in the needed data. errata = Errata() advisories = errata.advisories_from_event(event) if not advisories: - log.error("Unknown Errata advisory %d" % errata_id) + msg = "Unknown Errata advisory %d" % errata_id + self.current_db_event.transition(EventState.FAILED, msg) + db.session.commit() return [] log.info("Generating ErrataAdvisoryRPMsSignedEvent for Errata " @@ -70,9 +76,18 @@ class FreshmakerManualRebuildHandler(ContainerBuildHandler): event.msg_id + "." + str(advisory.name), advisory.name, advisory.errata_id, advisory.security_impact) new_event.manual = True + msg = ("Generated ErrataAdvisoryRPMsSignedEvent (%s) for errata: %s" + % (event.msg_id, errata_id)) + self.current_db_event.transition(EventState.COMPLETE, msg) + db.session.commit() return [new_event] def handle(self, event): + # for every manual triggered event, we log it in db + db_event = Event.get_or_create_from_event(db.session, event) + db.session.commit() + self.set_context(db_event) + extra_events = [] if event.errata_id: diff --git a/freshmaker/models.py b/freshmaker/models.py index f3ff99e..ff0b61f 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -24,7 +24,6 @@ """ SQLAlchemy Database models for the Flask app """ -import flask import json from datetime import datetime @@ -32,7 +31,7 @@ from sqlalchemy.orm import (validates, relationship) from flask_login import UserMixin -from freshmaker import app, db, log +from freshmaker import db, log from freshmaker import messaging from freshmaker.utils import get_url_for from freshmaker.types import ArtifactType, ArtifactBuildState, EventState @@ -40,7 +39,9 @@ from freshmaker.events import ( MBSModuleStateChangeEvent, GitModuleMetadataChangeEvent, GitRPMSpecChangeEvent, TestingEvent, GitDockerfileChangeEvent, BodhiUpdateCompleteStableEvent, KojiTaskStateChangeEvent, BrewSignRPMEvent, - ErrataAdvisoryRPMsSignedEvent) + ErrataAdvisoryRPMsSignedEvent, BrewContainerTaskStateChangeEvent, + ErrataAdvisoryStateChangedEvent, FreshmakerManualRebuildEvent, + ODCSComposeStateChangeEvent) EVENT_TYPES = { MBSModuleStateChangeEvent: 0, @@ -52,6 +53,10 @@ EVENT_TYPES = { KojiTaskStateChangeEvent: 6, BrewSignRPMEvent: 7, ErrataAdvisoryRPMsSignedEvent: 8, + BrewContainerTaskStateChangeEvent: 9, + ErrataAdvisoryStateChangedEvent: 10, + FreshmakerManualRebuildEvent: 11, + ODCSComposeStateChangeEvent: 12, } INVERSE_EVENT_TYPES = {v: k for k, v in EVENT_TYPES.items()} @@ -191,6 +196,12 @@ class Event(FreshmakerBase): released=released, manual=manual) @classmethod + def get_or_create_from_event(cls, session, event, released=True): + return cls.get_or_create(session, event.msg_id, + event.search_key, event.__class__, + released=released, manual=event.manual) + + @classmethod def get_unreleased(cls, session): return session.query(cls).filter_by(released=False).all() @@ -227,7 +238,7 @@ class Event(FreshmakerBase): for build in self.builds: build.transition(state, reason) - def transition(self, state, state_reason): + def transition(self, state, state_reason=None): """ Sets the state and state_reason of this Event. @@ -247,8 +258,10 @@ class Event(FreshmakerBase): return self.state = state - self.state_reason = state_reason + if state_reason is not None: + self.state_reason = state_reason + db.session.commit() messaging.publish('event.state.changed', self.json()) def __repr__(self): diff --git a/tests/test_errata_advisory_rpms_signed_handler.py b/tests/test_errata_advisory_rpms_signed_handler.py index 5ca4bc8..449c031 100644 --- a/tests/test_errata_advisory_rpms_signed_handler.py +++ b/tests/test_errata_advisory_rpms_signed_handler.py @@ -52,5 +52,5 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): handler.handle(event) db_event = Event.get(db.session, message_id='123') - self.assertEqual(db_event.state, EventState.HANDLED_NOOP.value) + self.assertEqual(db_event.state, EventState.SKIPPED.value) self.assertEqual(db_event.state_reason, "No container images to rebuild for advisory 'RHBA-2017'") diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index 2eb48d6..0c57aa3 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -114,7 +114,6 @@ class TestAllowBuild(unittest.TestCase): handler.handle(event) record_images.assert_not_called() - self.assertEqual(handler.current_db_event_id, None) @patch("freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler." "_find_images_to_rebuild", return_value=[]) diff --git a/tests/test_freshmaker_manual_rebuild_handler.py b/tests/test_freshmaker_manual_rebuild_handler.py index 8570c4d..5c7f3b0 100644 --- a/tests/test_freshmaker_manual_rebuild_handler.py +++ b/tests/test_freshmaker_manual_rebuild_handler.py @@ -32,6 +32,7 @@ from freshmaker.events import ( from freshmaker.errata import ErrataAdvisory from freshmaker import db from freshmaker.models import Event +from freshmaker.types import EventState class TestFreshmakerManualRebuildHandler(unittest.TestCase): @@ -61,6 +62,11 @@ class TestFreshmakerManualRebuildHandler(unittest.TestCase): self.assertEqual(ret[0].security_impact, "Critical") self.assertEqual(ret[0].errata_name, "RHSA-2017") + db_event = Event.query.filter_by(message_id=ev.msg_id).first() + self.assertEqual(db_event.state, EventState.COMPLETE.value) + self.assertEqual(db_event.state_reason, + 'Generated ErrataAdvisoryRPMsSignedEvent (msg123) for errata: 123') + @patch('freshmaker.errata.Errata.advisories_from_event') def test_rebuild_if_not_exists_already_exists( self, advisories_from_event): @@ -77,6 +83,11 @@ class TestFreshmakerManualRebuildHandler(unittest.TestCase): self.assertEqual(len(ret), 0) + db_event = Event.query.filter_by(message_id=ev.msg_id).first() + self.assertEqual(db_event.state, EventState.SKIPPED.value) + self.assertEqual(db_event.state_reason, + 'Ignoring Errata advisory 123 - it already exists in Freshmaker db.') + @patch('freshmaker.errata.Errata.advisories_from_event') def test_rebuild_if_not_exists_unknown_errata_id( self, advisories_from_event): @@ -87,3 +98,7 @@ class TestFreshmakerManualRebuildHandler(unittest.TestCase): ret = handler.handle(ev) self.assertEqual(len(ret), 0) + + db_event = Event.query.filter_by(message_id=ev.msg_id).first() + self.assertEqual(db_event.state, EventState.FAILED.value) + self.assertEqual(db_event.state_reason, "Unknown Errata advisory 123")