From 16938c9d7cc43ced9a408fd3b47d3353fa49ce71 Mon Sep 17 00:00:00 2001 From: Jan Kaluža Date: May 03 2019 12:09:31 +0000 Subject: Merge #380 `Add ArtifactBuild.rebuild_reason column.` --- 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 c92b6e4..7328ac7 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/')