From 66725af7227a75954614d6449f8e32023e68e317 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Apr 23 2019 08:21:00 +0000 Subject: Add ArtifactBuild.rebuild_reason column. This new column shows the reason why the artifact is rebuilt. This is useful to decide if the artifact is directly affected by some change which triggered the rebuild, or if it is pulled-in just as a dependency to rebuild other artifact. --- diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index 7ea08e3..19437d5 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -258,7 +258,8 @@ class BaseHandler(object): def record_build(self, event, name, artifact_type, build_id=None, dep_on=None, state=None, - original_nvr=None, rebuilt_nvr=None): + original_nvr=None, rebuilt_nvr=None, + rebuild_reason=0): """ Record build in db. @@ -273,6 +274,8 @@ class BaseHandler(object): ``ArtifactBuildState.BUILD``. :param original_nvr: The original NVR of artifact. :param rebuilt_nvr: The NVR of newly rebuilt artifact. + :param rebuild_reason: The reason why this artifact is included in + this event. :return: recorded build. :rtype: ArtifactBuild. """ @@ -285,7 +288,8 @@ class BaseHandler(object): build = models.ArtifactBuild.create(db.session, ev, name, artifact_type.name.lower(), build_id, dep_on, state, - original_nvr, rebuilt_nvr) + original_nvr, rebuilt_nvr, + rebuild_reason) db.session.commit() return build diff --git a/freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py b/freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py index 9de2e69..0a0dbe2 100644 --- a/freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py +++ b/freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py @@ -32,7 +32,8 @@ from freshmaker.handlers import ContainerBuildHandler, fail_event_on_handler_exc from freshmaker.lightblue import LightBlue from freshmaker.pulp import Pulp from freshmaker.errata import Errata -from freshmaker.types import ArtifactType, ArtifactBuildState, EventState +from freshmaker.types import ( + ArtifactType, ArtifactBuildState, EventState, RebuildReason) from freshmaker.models import Event, Compose from freshmaker.utils import get_rebuilt_nvr @@ -243,12 +244,22 @@ class RebuildImagesOnRPMAdvisoryChange(ContainerBuildHandler): rebuilt_nvr = get_rebuilt_nvr(ArtifactType.IMAGE.value, nvr) image_name = koji.parse_NVR(image["brew"]["build"])["name"] + # Only released images are considered as directly affected for + # rebuild. If some image is not in the latest released version and + # it is included in a rebuild, it must be just a dependency of + # other image. + if "latest_released" in image: + rebuild_reason = RebuildReason.DIRECTLY_AFFECTED.value + else: + rebuild_reason = RebuildReason.DEPENDENCY.value + build = self.record_build( event, image_name, ArtifactType.IMAGE, dep_on=dep_on, state=ArtifactBuildState.PLANNED.value, original_nvr=nvr, - rebuilt_nvr=rebuilt_nvr) + rebuilt_nvr=rebuilt_nvr, + rebuild_reason=rebuild_reason) # Set context to particular build so logging shows this build # in case of error. diff --git a/freshmaker/migrations/versions/fbc2eac9bfa5_.py b/freshmaker/migrations/versions/fbc2eac9bfa5_.py new file mode 100644 index 0000000..df09ac3 --- /dev/null +++ b/freshmaker/migrations/versions/fbc2eac9bfa5_.py @@ -0,0 +1,22 @@ +"""Add rebuild_reason column. + +Revision ID: fbc2eac9bfa5 +Revises: 5bdd5566615a +Create Date: 2019-04-23 10:20:00.766108 + +""" + +# revision identifiers, used by Alembic. +revision = 'fbc2eac9bfa5' +down_revision = '5bdd5566615a' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('artifact_builds', sa.Column('rebuild_reason', sa.Integer(), nullable=True)) + + +def downgrade(): + op.drop_column('artifact_builds', 'rebuild_reason') diff --git a/freshmaker/models.py b/freshmaker/models.py index c709b2c..8bb3c47 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -37,7 +37,8 @@ from flask_login import UserMixin from freshmaker import db, log from freshmaker import messaging from freshmaker.utils import get_url_for, krb_context -from freshmaker.types import ArtifactType, ArtifactBuildState, EventState +from freshmaker.types import (ArtifactType, ArtifactBuildState, EventState, + RebuildReason) from freshmaker.events import ( MBSModuleStateChangeEvent, GitModuleMetadataChangeEvent, GitRPMSpecChangeEvent, TestingEvent, GitDockerfileChangeEvent, @@ -473,12 +474,17 @@ class ArtifactBuild(FreshmakerBase): # Build args in json format. build_args = db.Column(db.String, nullable=True) + # The reason why this artifact is rebuilt. Set according to + # `freshmaker.types.RebuildReason`. + rebuild_reason = db.Column(db.Integer, nullable=True) + composes = db.relationship('ArtifactBuildCompose', back_populates='build') @classmethod def create(cls, session, event, name, type, build_id=None, dep_on=None, state=None, - original_nvr=None, rebuilt_nvr=None): + original_nvr=None, rebuilt_nvr=None, + rebuild_reason=0): now = datetime.utcnow() build = cls( @@ -490,7 +496,8 @@ class ArtifactBuild(FreshmakerBase): state=state or ArtifactBuildState.BUILD.value, build_id=build_id, time_submitted=now, - dep_on=dep_on + dep_on=dep_on, + rebuild_reason=rebuild_reason ) session.add(build) return build @@ -610,6 +617,7 @@ class ArtifactBuild(FreshmakerBase): "url": build_url, "build_args": build_args, "odcs_composes": [rel.compose.odcs_compose_id for rel in self.composes], + "rebuild_reason": RebuildReason(self.rebuild_reason or 0).name.lower() } def get_root_dep_on(self): diff --git a/freshmaker/types.py b/freshmaker/types.py index bcb5fbd..a304c2b 100644 --- a/freshmaker/types.py +++ b/freshmaker/types.py @@ -90,3 +90,14 @@ class EventState(Enum): SKIPPED = 4 # handling of the event has been canceled (also canceling builds etc.) CANCELED = 5 + + +class RebuildReason(Enum): + # Rebuild reason is unknown - mainly to have some value for old + # ArtifactBuilds. + UNKNOWN = 0 + # The artifact is directly rebuilt, because it is directly affected by some + # CVE, RPM update, ... + DIRECTLY_AFFECTED = 1 + # The artifact is rebuilt, because it is dependency of other artifact. + DEPENDENCY = 2 diff --git a/tests/handlers/internal/test_update_db_on_advisory_change.py b/tests/handlers/internal/test_update_db_on_advisory_change.py index 9b4b682..edaf54f 100644 --- a/tests/handlers/internal/test_update_db_on_advisory_change.py +++ b/tests/handlers/internal/test_update_db_on_advisory_change.py @@ -34,7 +34,8 @@ from freshmaker.handlers.koji import RebuildImagesOnRPMAdvisoryChange from freshmaker.handlers.internal import UpdateDBOnAdvisoryChange from freshmaker.lightblue import ContainerImage from freshmaker.models import Event, ArtifactBuild, EVENT_TYPES -from freshmaker.types import ArtifactBuildState, ArtifactType, EventState +from freshmaker.types import ( + ArtifactBuildState, ArtifactType, EventState, RebuildReason) from tests import helpers @@ -274,10 +275,11 @@ class TestBatches(helpers.ModelsTestCase): super(TestBatches, self).tearDown() self.patcher.unpatch_all() - def _mock_build(self, build, parent=None, error=None): + def _mock_build( + self, build, parent=None, error=None, **kwargs): if parent: parent = {"brew": {"build": parent + "-1-1.25"}} - return ContainerImage({ + d = { 'brew': {'build': build + "-1-1.25"}, 'repository': build + '_repo', 'parsed_data': { @@ -297,7 +299,9 @@ class TestBatches(helpers.ModelsTestCase): "arches": "x86_64", "odcs_compose_ids": [10, 11], "published": False, - }) + } + d.update(kwargs) + return ContainerImage(d) @patch('freshmaker.odcsclient.create_odcs_client') def test_batches_records(self, create_odcs_client): @@ -330,8 +334,8 @@ class TestBatches(helpers.ModelsTestCase): [self._mock_build("child1_parent2", "child1_parent3"), self._mock_build("child2_parent1", "child2_parent2")], [self._mock_build("child1_parent1", "child1_parent2", error="Fail"), - self._mock_build("child2", "child2_parent1")], - [self._mock_build("child1", "child1_parent1")]] + self._mock_build("child2", "child2_parent1", latest_released=True)], + [self._mock_build("child1", "child1_parent1", latest_released=True)]] # Flat list of images from batches with brew build id as a key. images = {} @@ -362,6 +366,11 @@ class TestBatches(helpers.ModelsTestCase): else: self.assertEqual(build.dep_on, None) + if build.name in ["child1", "child2"]: + self.assertEqual(build.rebuild_reason, RebuildReason.DIRECTLY_AFFECTED.value) + else: + self.assertEqual(build.rebuild_reason, RebuildReason.DEPENDENCY.value) + args = json.loads(build.build_args) self.assertEqual(args["repository"], build.name + "_repo") self.assertEqual(args["commit"], build.name + "_123") diff --git a/tests/test_models.py b/tests/test_models.py index c6d089b..de4a9d2 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -24,7 +24,7 @@ from freshmaker import db, events from freshmaker.models import ArtifactBuild, ArtifactType from freshmaker.models import Event, EventState, EVENT_TYPES, EventDependency from freshmaker.models import Compose, ArtifactBuildCompose -from freshmaker.types import ArtifactBuildState +from freshmaker.types import ArtifactBuildState, RebuildReason from freshmaker.events import ErrataAdvisoryRPMsSignedEvent from tests import helpers @@ -41,8 +41,10 @@ class TestModels(helpers.ModelsTestCase): def test_creating_event_and_builds(self): event = Event.create(db.session, "test_msg_id", "RHSA-2017-284", events.TestingEvent) - build = ArtifactBuild.create(db.session, event, "ed", "module", 1234) - ArtifactBuild.create(db.session, event, "mksh", "module", 1235, build) + build = ArtifactBuild.create(db.session, event, "ed", "module", 1234, + rebuild_reason=RebuildReason.DIRECTLY_AFFECTED.value) + ArtifactBuild.create(db.session, event, "mksh", "module", 1235, build, + rebuild_reason=RebuildReason.DEPENDENCY.value) db.session.commit() db.session.expire_all() @@ -57,12 +59,14 @@ class TestModels(helpers.ModelsTestCase): self.assertEqual(e.builds[0].state, 0) self.assertEqual(e.builds[0].build_id, 1234) self.assertEqual(e.builds[0].dep_on, None) + self.assertEqual(e.builds[0].rebuild_reason, RebuildReason.DIRECTLY_AFFECTED.value) self.assertEqual(e.builds[1].name, "mksh") self.assertEqual(e.builds[1].type, 2) self.assertEqual(e.builds[1].state, 0) self.assertEqual(e.builds[1].build_id, 1235) self.assertEqual(e.builds[1].dep_on.name, "ed") + self.assertEqual(e.builds[1].rebuild_reason, RebuildReason.DEPENDENCY.value) def test_get_root_dep_on(self): event = Event.create(db.session, "test_msg_id", "test", events.TestingEvent) diff --git a/tests/test_views.py b/tests/test_views.py index 3d2fdf6..c54fa6a 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -139,6 +139,7 @@ class TestViews(helpers.ModelsTestCase): self.assertEqual(data['event_id'], 1) self.assertEqual(data['build_id'], 1234) self.assertEqual(data['build_args'], {"key": "value"}) + self.assertEqual(data['rebuild_reason'], "unknown") def test_query_builds(self): resp = self.client.get('/api/1/builds/')