From 8ca8a153fbfe2b0ebb61827f8df76e537a2e09f4 Mon Sep 17 00:00:00 2001 From: Qixiang Wan Date: Nov 21 2017 08:55:49 +0000 Subject: [PATCH 1/2] Add state and state_reason for Event model States for Event: 1. Event is created with default state EventState.INITIALIZED 2. Event state is updated to EventState.BUILDING when some artifacts are found and under building. 3. Event state is updated to EventState.COMPLETE when the event is handled successfully. 4. Event state is updated to EventState.FAILED when there is error happens while handling the event. 5. Event state is updated to EventState.SKIPPED when no action to be taken upon the event, e.g. no ArtifactBuild to be built according to the blacklist/whitelist policy. FIXES: #137 --- diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index 3e0a847..3f02075 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -34,7 +34,7 @@ from freshmaker.kojiservice import koji_service from freshmaker.lightblue import LightBlue from freshmaker.pulp import Pulp from freshmaker.errata import Errata -from freshmaker.types import ArtifactType, ArtifactBuildState +from freshmaker.types import ArtifactType, ArtifactBuildState, EventState from freshmaker.models import Event from freshmaker.consumer import work_queue_put from freshmaker.utils import krb_context, retry, get_rebuilt_nvr @@ -90,8 +90,9 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): builds = self._record_batches(batches, event, builds) if not builds: - log.info('No container images to rebuild for advisory %r', - event.errata_name) + msg = 'No container images to rebuild for advisory %r' % event.errata_name + log.info(msg) + db_event.transition(EventState.SKIPPED, msg) return [] # Generate the ODCS compose with RPMs from the current advisory. @@ -136,9 +137,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): for url in repo_urls: log.info(" - %s", url) - # TODO: Once https://pagure.io/freshmaker/issue/137 is fixed, this - # should be moved to models.Event.transition(). - messaging.publish('event.state.changed', db_event.json()) + db_event.transition(EventState.COMPLETE, "All docker images have been rebuilt.") return [] diff --git a/freshmaker/migrations/versions/90f8444d5ab7_add_event_state_and_timestamp.py b/freshmaker/migrations/versions/90f8444d5ab7_add_event_state_and_timestamp.py new file mode 100644 index 0000000..4ed5c29 --- /dev/null +++ b/freshmaker/migrations/versions/90f8444d5ab7_add_event_state_and_timestamp.py @@ -0,0 +1,33 @@ +"""Add event state and timestamp + +Revision ID: 90f8444d5ab7 +Revises: 2f5a2f4385a0 +Create Date: 2017-11-20 23:16:44.079911 + +""" + +# revision identifiers, used by Alembic. +revision = '90f8444d5ab7' +down_revision = '2f5a2f4385a0' + +from alembic import op +import sqlalchemy as sa + +from freshmaker.types import EventState + + +def upgrade(): + with op.batch_alter_table('events', schema=None) as batch_op: + batch_op.add_column(sa.Column('state', sa.Integer(), server_default=str(EventState.INITIALIZED.value), nullable=False)) + batch_op.add_column(sa.Column('state_reason', sa.String(), nullable=True)) + batch_op.add_column(sa.Column('time_created', sa.DateTime(), nullable=True)) + + # update state to 'COMPLETE' for historical events + op.execute("UPDATE events SET state = %s" % EventState.COMPLETE.value) + + +def downgrade(): + with op.batch_alter_table('events', schema=None) as batch_op: + batch_op.drop_column('state') + batch_op.drop_column('state_reason') + batch_op.drop_column('time_created') diff --git a/freshmaker/models.py b/freshmaker/models.py index fb62fd3..f3ff99e 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -35,7 +35,7 @@ from flask_login import UserMixin from freshmaker import app, db, log from freshmaker import messaging from freshmaker.utils import get_url_for -from freshmaker.types import ArtifactType, ArtifactBuildState +from freshmaker.types import ArtifactType, ArtifactBuildState, EventState from freshmaker.events import ( MBSModuleStateChangeEvent, GitModuleMetadataChangeEvent, GitRPMSpecChangeEvent, TestingEvent, GitDockerfileChangeEvent, @@ -133,7 +133,9 @@ class Event(FreshmakerBase): # This is currently only used for internal Docker images rebuilds, but in # the future might be used even for modules or Fedora Docker images. released = db.Column(db.Boolean, default=True) - + state = db.Column(db.Integer, nullable=False) + state_reason = db.Column(db.String, nullable=True) + time_created = db.Column(db.DateTime, nullable=True) # List of builds associated with this Event. builds = relationship("ArtifactBuild", back_populates="event") @@ -148,20 +150,33 @@ class Event(FreshmakerBase): doc='Whether this event is triggered manually') @classmethod - def create(cls, session, message_id, search_key, event_type, - released=True, manual=False): + def create(cls, session, message_id, search_key, event_type, released=True, + state=None, manual=False): if event_type in EVENT_TYPES: event_type = EVENT_TYPES[event_type] + now = datetime.utcnow() event = cls( message_id=message_id, search_key=search_key, event_type_id=event_type, released=released, + state=state or EventState.INITIALIZED.value, + time_created=now, manual_triggered=manual, ) session.add(event) return event + @validates('state') + def validate_state(self, key, field): + if field in [s.value for s in list(EventState)]: + return field + if field in [s.name.lower() for s in list(EventState)]: + return EventState[field.upper()].value + if isinstance(field, EventState): + return field.value + raise ValueError("%s: %s, not in %r" % (key, field, list(EventState))) + @classmethod def get(cls, session, message_id): return session.query(cls).filter_by(message_id=message_id).first() @@ -206,14 +221,34 @@ class Event(FreshmakerBase): def builds_transition(self, state, reason): """ - Calls transition(state, reason) for all builds associated whit this + Calls transition(state, reason) for all builds associated with this event. """ for build in self.builds: build.transition(state, reason) - # TODO: Once https://pagure.io/freshmaker/issue/137 is fixed, this - # should be moved to models.Event.transition(). + def transition(self, state, state_reason): + """ + Sets the state and state_reason of this Event. + + :param state: EventState value + :param state_reason: Reason why this state has been set. + """ + + # Log the state and state_reason + if state == EventState.FAILED.value: + log_fnc = log.error + else: + log_fnc = log.info + log_fnc("Event %r moved to state %s, %r" % ( + self, EventState(state).name, state_reason)) + + if self.state == state: + return + + self.state = state + self.state_reason = state_reason + messaging.publish('event.state.changed', self.json()) def __repr__(self): @@ -227,6 +262,9 @@ class Event(FreshmakerBase): "message_id": self.message_id, "search_key": self.search_key, "event_type_id": self.event_type_id, + "state": self.state, + "state_name": EventState(self.state).name, + "state_reason": self.state_reason, "url": event_url, "builds": [b.json() for b in self.builds], } diff --git a/freshmaker/types.py b/freshmaker/types.py index 93d240b..ab1a2aa 100644 --- a/freshmaker/types.py +++ b/freshmaker/types.py @@ -34,3 +34,15 @@ class ArtifactBuildState(Enum): FAILED = 2 CANCELED = 3 PLANNED = 4 + + +class EventState(Enum): + INITIALIZED = 0 + # some artifacts has been found and under building + BUILDING = 1 + # event is handled successfully + COMPLETE = 2 + # error happens while handling the event + FAILED = 3 + # no action to take upon the event + SKIPPED = 4 diff --git a/tests/test_errata_advisory_rpms_signed_handler.py b/tests/test_errata_advisory_rpms_signed_handler.py new file mode 100644 index 0000000..5ca4bc8 --- /dev/null +++ b/tests/test_errata_advisory_rpms_signed_handler.py @@ -0,0 +1,56 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2017 Red Hat, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +import unittest + +from mock import patch + +from freshmaker.handlers.errata import ErrataAdvisoryRPMsSignedHandler +from freshmaker.events import ErrataAdvisoryRPMsSignedEvent + +from freshmaker import db +from freshmaker.models import Event +from freshmaker.types import EventState + + +class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): + + def setUp(self): + db.session.remove() + db.drop_all() + db.create_all() + db.session.commit() + + def tearDown(self): + db.session.remove() + db.drop_all() + db.session.commit() + + @patch("freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler._find_images_to_rebuild") + def test_event_state_updated_when_no_images_to_rebuild(self, mock_find_images): + mock_find_images.return_value = [] + event = ErrataAdvisoryRPMsSignedEvent("123", "RHBA-2017", 123, "") + handler = ErrataAdvisoryRPMsSignedHandler() + 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_reason, "No container images to rebuild for advisory 'RHBA-2017'") From 2a0408c4463d40481c0cf285835aaafb79e10e6e Mon Sep 17 00:00:00 2001 From: Qixiang Wan Date: Nov 21 2017 08:55:55 +0000 Subject: [PATCH 2/2] 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")