From 9199d97b26a1b3d6a9f10dcdf6ff1344a9dee9cb Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Sep 27 2017 10:49:26 +0000 Subject: Do not create multiple ErrataAdvisoryRPMsSignedEvents for single Errata advisory. --- diff --git a/freshmaker/consumer.py b/freshmaker/consumer.py index 5c73173..d52f8d4 100644 --- a/freshmaker/consumer.py +++ b/freshmaker/consumer.py @@ -83,8 +83,6 @@ class FreshmakerConsumer(fedmsg.consumers.FedmsgConsumer): super(FreshmakerConsumer, self).validate(message) def consume(self, message): - log.debug("Received %r" % message) - # Sometimes, the messages put into our queue are artificially put there # by other parts of our own codebase. If they are already abstracted # messages, then just use them as-is. If they are not already @@ -95,6 +93,10 @@ class FreshmakerConsumer(fedmsg.consumers.FedmsgConsumer): else: msg = self.get_abstracted_msg(message['body']) + if not msg: + log.debug("Received unparsed message: %r", message) + return + # Primary work is done here. try: self.process_event(msg) diff --git a/freshmaker/handlers/brew/sign_rpm.py b/freshmaker/handlers/brew/sign_rpm.py index 66f1fc6..82832f4 100644 --- a/freshmaker/handlers/brew/sign_rpm.py +++ b/freshmaker/handlers/brew/sign_rpm.py @@ -23,10 +23,12 @@ from freshmaker import conf from freshmaker import log +from freshmaker import db from freshmaker.events import BrewSignRPMEvent, ErrataAdvisoryRPMsSignedEvent from freshmaker.handlers import BaseHandler from freshmaker.errata import Errata from freshmaker.types import ArtifactType +from freshmaker.models import Event class BrewSignRPMHandler(BaseHandler): @@ -41,7 +43,29 @@ class BrewSignRPMHandler(BaseHandler): def can_handle(self, event): return isinstance(event, BrewSignRPMEvent) + def _filter_out_existing_advisories(self, advisories): + """ + Filter out all advisories which have been already handled by + Freshmaker. + + :param advisories: List of ErrataAdvisory instances. + :rtype: List of ErrataAdvisory + :return: List of ErrataAdvisory instances without already handled + advisories. + """ + ret = [] + for advisory in advisories: + if (db.session.query(Event).filter_by( + search_key=str(advisory.errata_id)).count() != 0): + log.info("Skipping advisory %s (%d), already handled by " + "Freshmaker", advisory.name, advisory.errata_id) + continue + ret.append(advisory) + return ret + def handle(self, event): + log.info("Finding out all advisories including %s", event.nvr) + # When get a signed RPM, first step is to find out advisories # containing that RPM and ensure all builds are signed. errata = Errata(conf.errata_tool_server_url) @@ -52,6 +76,10 @@ class BrewSignRPMHandler(BaseHandler): if self.allow_build( ArtifactType.IMAGE, advisory_name=advisory.name, advisory_security_impact=advisory.security_impact)] + + # Filter out advisories which are already in Freshmaker DB. + advisories = self._filter_out_existing_advisories(advisories) + if not advisories: log.info("No advisories found suitable for rebuilding Docker " "images") @@ -71,5 +99,10 @@ class BrewSignRPMHandler(BaseHandler): new_event = ErrataAdvisoryRPMsSignedEvent( event.msg_id + "." + str(advisory.name), advisory.name, advisory.errata_id, advisory.security_impact) + db_event = Event.create( + db.session, event.msg_id, new_event.search_key, + new_event.__class__, released=False) + db.session.add(db_event) new_events.append(new_event) + db.session.commit() return new_events diff --git a/freshmaker/parsers/brew/task_state_change.py b/freshmaker/parsers/brew/task_state_change.py index 05f63da..ebecc97 100644 --- a/freshmaker/parsers/brew/task_state_change.py +++ b/freshmaker/parsers/brew/task_state_change.py @@ -21,6 +21,7 @@ import re +from freshmaker import log from freshmaker.parsers import BaseParser from freshmaker.events import BrewContainerTaskStateChangeEvent @@ -55,3 +56,6 @@ class BrewTaskStateChangeParser(BaseParser): m = re.match(r".*/(?P[^#]*)", git_url) container = m.group('container') return BrewContainerTaskStateChangeEvent(msg_id, container, branch, target, task_id, old_state, new_state) + else: + log.debug("brew.task.closed or brew.task.failed of %s task_method " + "is not handled yet.", task_method) diff --git a/tests/test_brew_sign_rpm_handler.py b/tests/test_brew_sign_rpm_handler.py index 9815afa..0f6e29b 100644 --- a/tests/test_brew_sign_rpm_handler.py +++ b/tests/test_brew_sign_rpm_handler.py @@ -27,11 +27,23 @@ from mock import patch, MagicMock, PropertyMock from freshmaker.handlers.brew.sign_rpm import BrewSignRPMHandler from freshmaker.errata import ErrataAdvisory +from freshmaker import db class TestBrewSignHandler(unittest.TestCase): """Test BrewSignRPMHandler.handle""" + 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.errata.Errata.advisories_from_event') @patch('freshmaker.errata.Errata.builds_signed') @patch("freshmaker.config.Config.handler_build_whitelist", @@ -47,6 +59,7 @@ class TestBrewSignHandler(unittest.TestCase): builds_signed.return_value = True event = MagicMock() + event.msg_id = "msg_123" handler = BrewSignRPMHandler() ret = handler.handle(event) @@ -157,3 +170,30 @@ class TestBrewSignHandler(unittest.TestCase): handler.handle(event) builds_signed.assert_not_called() + + @patch('freshmaker.errata.Errata.advisories_from_event') + @patch('freshmaker.errata.Errata.builds_signed') + @patch("freshmaker.config.Config.handler_build_whitelist", + new_callable=PropertyMock, return_value={ + "BrewSignRPMHandler": {"image": [{"advisory_name": "RHSA-.*"}]}}) + def test_do_not_create_already_handled_event( + self, handler_build_whitelist, builds_signed, + advisories_from_event): + """ + Tests that BrewSignRPMHandler don't return Event which already exists + in Freshmaker DB. + """ + builds_signed.return_value = True + advisories_from_event.return_value = [ + ErrataAdvisory(123, "RHSA-2017", "REL_PREP")] + + event = MagicMock() + event.msg_id = "msg_123" + handler = BrewSignRPMHandler() + handler.handle(event) + + builds_signed.assert_called_once() + builds_signed.reset_mock() + + handler.handle(event) + builds_signed.assert_not_called()