From cc885ea12626403ba57680455336c4aa722b8a1c Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jan 03 2018 05:41:10 +0000 Subject: [PATCH 1/7] Rewrite finding dependent events Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index 799bb4b..53b04bc 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -161,37 +161,14 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): return new_compose - def _prepare_yum_repos_for_rebuilds(self, db_event, event, builds): + def _prepare_yum_repos_for_rebuilds(self, db_event): repo_urls = [] repo_urls.append(self._prepare_yum_repo(db_event)) # noqa - # Find out extra events we want to include. These are advisories - # which are not released yet and touches some Docker images which - # are shared with the initial list of docker images we are going to - # rebuild. - # If we For example have NSS Errata advisory and httpd advisory, we - # need to rebuild some Docker images with both NSS and httpd - # advisories. - # We also want to search for extra events recursively, because there - # might for example be zlib advisory, and we want to include this zlib - # advisory when rebuilding NSS when rebuilding httpd... :) - prev_builds_count = 0 - seen_extra_events = [] - - # We stop when we did not find more docker images to rebuild and - # therefore cannot find more extra events. - while prev_builds_count != len(builds): - prev_builds_count = len(builds) - extra_events = self._find_events_to_include(db_event, builds) - self.log_info("Extra events: %r", extra_events) - for ev in extra_events: - if ev in seen_extra_events: - continue - seen_extra_events.append(ev) - db_event.add_event_dependency(db.session, ev) - for batches in self._find_images_to_rebuild(ev.search_key): - builds = self._record_batches(batches, event, builds) - repo_urls.append(self._prepare_yum_repo(ev)) + repo_urls += [ + self._prepare_yum_repo(dep_event) + for dep_event in db_event.find_dependent_events() + ] db.session.commit() # Remove duplicates from repo_urls. @@ -411,30 +388,6 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): batch += 1 - def _find_events_to_include(self, db_event, builds): - """ - Find out all unreleased events which built some image which is also - planned to be built as part of current image rebuild. - - :param db_event Event: Database representation of - ErrataAdvisoryRPMsSignedEvent. - :param builds dict: list of docker images to build as returned by - _find_images_to_rebuild(...). - """ - events_to_include = [] - for ev in Event.get_unreleased(db.session): - for build in ev.builds: - # Skip non IMAGE builds - if (build.type != ArtifactType.IMAGE.value or - ev.message_id == db_event.message_id): - continue - - if build.name in builds: - events_to_include.append(ev) - break - - return events_to_include - def _record_batches(self, batches, event, builds=None): """ Records the images from batches to database. diff --git a/freshmaker/models.py b/freshmaker/models.py index f88cf00..72830c4 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -310,6 +310,40 @@ class Event(FreshmakerBase): "builds": [b.json() for b in self.builds], } + def find_dependent_events(self): + """ + Find other unreleased Events which built the same builds (or just some + of them) as this Event and adds them as a dependency for this event. + + Dependent events of may also rebuild some same images that current event + will build. So, for building images found from current event, we also + need those YUM repositories used to build images in dependent events. + """ + builds_nvrs = [build.name for build in self.builds] + + states = [EventState.INITIALIZED.value, + EventState.BUILDING.value, + EventState.COMPLETE.value] + + query = db.session.query(ArtifactBuild.event_id) + dep_event_ids = query.join(ArtifactBuild.event).filter( + ArtifactBuild.name.in_(builds_nvrs), + ArtifactBuild.event_id != self.id, + ArtifactBuild.type == ArtifactType.IMAGE.value, + Event.manual_triggered == false(), + Event.released == false(), + Event.state.in_(states), + ).distinct() + + dep_events = [] + query = db.session.query(Event) + for row in dep_event_ids: + dep_event = query.filter_by(id=row[0]).first() + self.add_event_dependency(db.session, dep_event) + dep_events.append(dep_event) + db.session.commit() + return dep_events + class EventDependency(FreshmakerBase): __tablename__ = "event_dependencies" diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index 78c0cd2..0fa60e5 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -24,7 +24,7 @@ import unittest import json -from mock import patch, MagicMock, PropertyMock, Mock, call +from mock import patch, PropertyMock, Mock, call from freshmaker.handlers.errata import ErrataAdvisoryRPMsSignedHandler from freshmaker.handlers.errata import ErrataAdvisoryStateChangedHandler @@ -730,53 +730,6 @@ class TestFindImagesToRebuild(unittest.TestCase): self.assertEqual([], ret) -class TestFindEventsToInclude(unittest.TestCase): - """Test ErrataAdvisoryRPMsSignedHandler._find_events_to_include""" - - def setUp(self): - db.session.remove() - db.drop_all() - db.create_all() - db.session.commit() - - self.db_event = Event.get_or_create( - db.session, "msg1", "current_event", ErrataAdvisoryRPMsSignedEvent, - released=False) - ArtifactBuild.create(db.session, self.db_event, "foo", "image", 0) - - # Only this event should be reused, because it is unreleased and - # contains the foo build. - ev = Event.get_or_create( - db.session, "msg2", "old_event_foo", ErrataAdvisoryRPMsSignedEvent, - released=False) - ev.state = EventState.COMPLETE - ArtifactBuild.create(db.session, ev, "foo", "image", 0) - - ev = Event.get_or_create( - db.session, "msg3", "old_event_foo_released", - ErrataAdvisoryRPMsSignedEvent, released=True) - ArtifactBuild.create(db.session, ev, "foo", "image", 0) - - ev = Event.get_or_create( - db.session, "msg4", "old_event_bar", ErrataAdvisoryRPMsSignedEvent, - released=False) - ArtifactBuild.create(db.session, ev, "bar", "image", 0) - db.session.commit() - - def tearDown(self): - db.session.remove() - db.drop_all() - db.session.commit() - - def test_find_events_to_include(self): - builds = {"foo": MagicMock()} - handler = ErrataAdvisoryRPMsSignedHandler() - events = handler._find_events_to_include(self.db_event, builds) - - self.assertEqual(len(events), 1) - self.assertEqual(events[0].search_key, "old_event_foo") - - class TestErrataAdvisoryStateChangedHandler(unittest.TestCase): def setUp(self): diff --git a/tests/test_models.py b/tests/test_models.py index 9a96785..5ad49d9 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -23,8 +23,10 @@ import unittest from freshmaker import db, events -from freshmaker.models import Event, ArtifactBuild, EventState +from freshmaker.models import ArtifactBuild, ArtifactType +from freshmaker.models import Event, EventState, EVENT_TYPES, EventDependency from freshmaker.types import ArtifactBuildState +from freshmaker.events import ErrataAdvisoryRPMsSignedEvent class TestModels(unittest.TestCase): @@ -177,3 +179,101 @@ class TestModels(unittest.TestCase): event = Event.create(db.session, "test_msg_id1", "test", 1024) self.assertEqual( str(event), "") + + +class TestFindDependentEvents(unittest.TestCase): + """Test Event.find_dependent_events""" + + def setUp(self): + db.session.remove() + db.drop_all() + db.create_all() + db.session.commit() + + self.event_1 = Event.create( + db.session, 'msg-1', 'search-key-1', + EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent], + state=EventState.INITIALIZED, + released=False) + ArtifactBuild.create( + db.session, self.event_1, 'build-1', ArtifactType.IMAGE) + ArtifactBuild.create( + db.session, self.event_1, 'build-2', ArtifactType.IMAGE) + ArtifactBuild.create( + db.session, self.event_1, 'build-3', ArtifactType.IMAGE) + ArtifactBuild.create( + db.session, self.event_1, 'build-4', ArtifactType.IMAGE) + + self.event_2 = Event.create( + db.session, 'msg-2', 'search-key-2', + EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent], + state=EventState.BUILDING, + released=False) + ArtifactBuild.create( + db.session, self.event_2, 'build-2', ArtifactType.IMAGE) + ArtifactBuild.create( + db.session, self.event_2, 'build-5', ArtifactType.IMAGE) + ArtifactBuild.create( + db.session, self.event_2, 'build-6', ArtifactType.IMAGE) + + self.event_3 = Event.create( + db.session, 'msg-3', 'search-key-3', + EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent], + state=EventState.COMPLETE, + released=False) + ArtifactBuild.create( + db.session, self.event_3, 'build-2', ArtifactType.IMAGE) + ArtifactBuild.create( + db.session, self.event_3, 'build-4', ArtifactType.IMAGE) + ArtifactBuild.create( + db.session, self.event_3, 'build-7', ArtifactType.IMAGE) + ArtifactBuild.create( + db.session, self.event_3, 'build-8', ArtifactType.IMAGE) + + # Some noises + + # Failed events should not be included + self.event_4 = Event.create( + db.session, 'msg-4', 'search-key-4', + EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent], + state=EventState.FAILED, + released=False) + ArtifactBuild.create( + db.session, self.event_4, 'build-3', ArtifactType.IMAGE) + + # Manual triggered rebuild should not be included as well + self.event_5 = Event.create( + db.session, 'msg-5', 'search-key-5', + EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent], + state=EventState.BUILDING, + released=False, manual=True) + ArtifactBuild.create( + db.session, self.event_5, 'build-4', ArtifactType.IMAGE) + + # Released event should not be included also + self.event_6 = Event.create( + db.session, 'msg-6', 'search-key-6', + EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent], + state=EventState.COMPLETE, + released=True) + ArtifactBuild.create( + db.session, self.event_5, 'build-4', ArtifactType.IMAGE) + + db.session.commit() + + def tearDown(self): + db.session.remove() + db.drop_all() + db.session.commit() + + def test_find_dependent_events(self): + dep_events = self.event_1.find_dependent_events() + self.assertEqual([self.event_2.id, self.event_3.id], + sorted([event.id for event in dep_events])) + + dep_rels = db.session.query(EventDependency).all() + dep_rels = [(rel.event_id, rel.event_dependency_id) for rel in dep_rels] + + self.assertEqual(2, len(dep_rels)) + self.assertIn((self.event_1.id, self.event_2.id), dep_rels) + self.assertIn((self.event_1.id, self.event_3.id), dep_rels) From f308e0673951987a7c68baac16c6b089bc516ed1 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jan 03 2018 05:41:10 +0000 Subject: [PATCH 2/7] Add many-2-many relationship between ArtifactBuild and Compose This is used to refactor storing ODCS compose IDs for artifact builds that will be rebuilt. For a particular event to rebuild series of images, this new database schema change will be able to describe like this, an event will rebuild several images, and each build has ODCS composes containing updated RPMs. We can get all composes IDs through this many-2-many relationship instead of gathering them from Event.compose_id and ArtifactBuild.build_args. Meanwhile, it will be easy and flexible for adding more composes IDs to a build without changing schema. Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/migrations/versions/6004dadc9ac4_add_compose_model_and_build_m2m_.py b/freshmaker/migrations/versions/6004dadc9ac4_add_compose_model_and_build_m2m_.py new file mode 100644 index 0000000..bd76e00 --- /dev/null +++ b/freshmaker/migrations/versions/6004dadc9ac4_add_compose_model_and_build_m2m_.py @@ -0,0 +1,38 @@ +"""Add Compose model and build m2m relationship with ArtifactBuild + +Revision ID: 6004dadc9ac4 +Revises: 90f8444d5ab7 +Create Date: 2017-12-21 10:18:32.008115 + +""" + +# revision identifiers, used by Alembic. +revision = '6004dadc9ac4' +down_revision = '90f8444d5ab7' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('composes', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('odcs_compose_id', sa.Integer(), nullable=False), + sa.PrimaryKeyConstraint('id') + ) + op.create_table('artifact_build_composes', + sa.Column('build_id', sa.Integer(), nullable=False), + sa.Column('compose_id', sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(['build_id'], ['artifact_builds.id'], ), + sa.ForeignKeyConstraint(['compose_id'], ['composes.id'], ), + sa.PrimaryKeyConstraint('build_id', 'compose_id') + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table('artifact_build_composes') + op.drop_table('composes') + # ### end Alembic commands ### diff --git a/freshmaker/models.py b/freshmaker/models.py index 72830c4..f830647 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -382,6 +382,8 @@ class ArtifactBuild(FreshmakerBase): # Build args in json format. build_args = db.Column(db.String, 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, @@ -506,3 +508,29 @@ class ArtifactBuild(FreshmakerBase): else: break return dep_on + + +class Compose(FreshmakerBase): + __tablename__ = 'composes' + + id = db.Column(db.Integer, primary_key=True) + odcs_compose_id = db.Column(db.Integer, nullable=False) + + builds = db.relationship('ArtifactBuildCompose', back_populates='compose') + + +class ArtifactBuildCompose(FreshmakerBase): + __tablename__ = 'artifact_build_composes' + + build_id = db.Column( + db.Integer, + db.ForeignKey('artifact_builds.id'), + primary_key=True) + + compose_id = db.Column( + db.Integer, + db.ForeignKey('composes.id'), + primary_key=True) + + build = db.relationship('ArtifactBuild', back_populates='composes') + compose = db.relationship('Compose', back_populates='builds') diff --git a/tests/test_models.py b/tests/test_models.py index 5ad49d9..241cc9d 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -25,6 +25,7 @@ import unittest 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.events import ErrataAdvisoryRPMsSignedEvent @@ -277,3 +278,83 @@ class TestFindDependentEvents(unittest.TestCase): self.assertEqual(2, len(dep_rels)) self.assertIn((self.event_1.id, self.event_2.id), dep_rels) self.assertIn((self.event_1.id, self.event_3.id), dep_rels) + + +class TestArtifactBuildComposesRel(unittest.TestCase): + """Test m2m relationship between ArtifactBuild and Compose""" + + def setUp(self): + db.session.remove() + db.drop_all() + db.create_all() + db.session.commit() + + self.compose_1 = Compose(odcs_compose_id=1) + self.compose_2 = Compose(odcs_compose_id=2) + self.compose_3 = Compose(odcs_compose_id=3) + self.compose_4 = Compose(odcs_compose_id=4) + db.session.add(self.compose_1) + db.session.add(self.compose_2) + db.session.add(self.compose_3) + db.session.add(self.compose_4) + + self.event = Event.create( + db.session, 'msg-1', 'search-key-1', + EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent], + state=EventState.INITIALIZED, + released=False) + self.build_1 = ArtifactBuild.create( + db.session, self.event, 'build-1', ArtifactType.IMAGE) + self.build_2 = ArtifactBuild.create( + db.session, self.event, 'build-2', ArtifactType.IMAGE) + self.build_3 = ArtifactBuild.create( + db.session, self.event, 'build-3', ArtifactType.IMAGE) + + db.session.commit() + + rels = ( + (self.build_1.id, self.compose_1.id), + (self.build_1.id, self.compose_2.id), + (self.build_1.id, self.compose_3.id), + (self.build_2.id, self.compose_2.id), + (self.build_2.id, self.compose_4.id), + ) + + for build_id, compose_id in rels: + db.session.add( + ArtifactBuildCompose( + build_id=build_id, compose_id=compose_id)) + + db.session.commit() + + def tearDown(self): + db.session.remove() + db.drop_all() + db.session.commit() + + def test_build_composes(self): + self.assertEqual(3, len(self.build_1.composes)) + self.assertEqual( + [self.compose_1.id, self.compose_2.id, self.compose_3.id], + sorted([rel.compose.id for rel in self.build_1.composes])) + + self.assertEqual(2, len(self.build_2.composes)) + self.assertEqual( + [self.compose_2.id, self.compose_4.id], + sorted([rel.compose.id for rel in self.build_2.composes])) + + self.assertEqual([], self.build_3.composes) + + def test_compose_builds(self): + expected_rels = ( + (self.compose_1, 1, [self.build_1.id]), + (self.compose_2, 2, [self.build_1.id, self.build_2.id]), + (self.compose_3, 1, [self.build_1.id]), + (self.compose_4, 1, [self.build_2.id]), + ) + + for compose, builds_count, builds in expected_rels: + self.assertEqual(builds_count, len(compose.builds)) + self.assertEqual( + builds, + sorted([rel.build.id for rel in compose.builds])) From 8a5f85876805e8ace82b6a1d901e1523ae218b93 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jan 03 2018 05:51:00 +0000 Subject: [PATCH 3/7] Refactor to store compose id to Compose model Instead of storing compose ID to Event.compose_id and ArtifactBuild.build_args['odcs_pulp_compose_id'], this patch stores compose IDs to Compose model and build the relationship to each ArtifactBuild. This is flexible for adding more composes, for example, a base image requires a compose containing boot.iso but other images don't. Major changes * ErrataAdvisoryRPMsSignedHandler._prepare_yum_repo is refactored and now it returns the requested new compose instead of storing into database directly. * ErrataAdvisoryRPMsSignedHandler._record_batches is updated to store pulp compose id to Compose instead of ArtifactBuild.build_args['odcs_pulp_compose_id']. * ContainerBuildHandler.get_repo_urls now simply gathers an ArtifactBuild's repo URLs by querying database through ArtifactBuildCompose relationship. * ContainerBuildHandler._build_first_batch is removed. * In ErrataAdvisoryRPMsSignedHandler.handle, call start_to_build_images to build first batch directly. It's simple enough, so no need of extra call to _build_first_batch. * ErrataAdvisoryRPMsSignedHandler._prepare_yum_repos_for_rebuilds is updated to store composes into Compose, which are requested for current event and dependent events. Original code was to store compose id into dependent event's compose_id, that was actually a bug. Now, it is fixed. * ComposeStateChangeHandler.handle is rewritten. Start to rebuild a image, which is in first batch, only when all composes finish. In handle method, just call start_to_build_images with builds that can be rebuilt. * Some helper methods are added to models. * Add and update tests as well. Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index 948ffed..763ad74 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -32,7 +32,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, EventState +from freshmaker.types import EventState from freshmaker.models import ArtifactBuild, Event from freshmaker.utils import krb_context, get_rebuilt_nvr from freshmaker.errors import UnprocessableEntity, ProgrammingError @@ -406,33 +406,18 @@ class ContainerBuildHandler(BaseHandler): with krb_context(): return odcs.get_compose(compose_id) - def get_repo_urls(self, db_event, build): + def get_repo_urls(self, build): """ Returns list of URLs to ODCS repositories which should be used to rebuild the container image for this event. - """ - rebuild_event = Event.get(db.session, db_event.message_id) - - # Get compose ids of ODCS composes of all event dependencies. - compose_ids = [rebuild_event.compose_id] - for event in rebuild_event.event_dependencies: - compose_ids.append(event.compose_id) - - # Use compose ids to get the repofile URLs. - repo_urls = [] - for compose_id in compose_ids: - if not compose_id: - continue - compose = self.odcs_get_compose(compose_id) - repo_urls.append(compose["result_repofile"]) - # Add PULP compose repo url. - if build.build_args and "odcs_pulp_compose_id" in build.build_args: - args = json.loads(build.build_args) - compose = self.odcs_get_compose(args["odcs_pulp_compose_id"]) - repo_urls.append(compose["result_repofile"]) - - return repo_urls + :param build: from this build to gather repo URLs. + :type build: ArtifactBuild + :return: list of repository URLs. + :rtype: list + """ + return [self.odcs_get_compose(rel.compose.id)['result_repofile'] + for rel in build.composes] def start_to_build_images(self, builds): """Start to build images @@ -444,7 +429,7 @@ class ContainerBuildHandler(BaseHandler): def build_image(build): self.set_context(build) - repo_urls = self.get_repo_urls(build.event, build) + repo_urls = self.get_repo_urls(build) build.build_id = self.build_image_artifact_build(build, repo_urls) if build.build_id: build.transition( @@ -458,15 +443,3 @@ class ContainerBuildHandler(BaseHandler): db.session.commit() list(six.moves.map(build_image, builds)) - - def _build_first_batch(self, db_event): - """ - Rebuilds all the parents images - images in the first batch which don't - depend on other images. - """ - - builds = db.session.query(ArtifactBuild).filter_by( - type=ArtifactType.IMAGE.value, event_id=db_event.id, - dep_on=None).all() - self.start_to_build_images(builds) - self.set_context(db_event) diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index 53b04bc..d5d7ef5 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.lightblue import LightBlue from freshmaker.pulp import Pulp from freshmaker.errata import Errata from freshmaker.types import ArtifactType, ArtifactBuildState, EventState -from freshmaker.models import Event +from freshmaker.models import Event, Compose from freshmaker.consumer import work_queue_put from freshmaker.utils import krb_context, retry, get_rebuilt_nvr @@ -107,8 +107,7 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): # available from official YUM repositories. # # Generate the ODCS compose with RPMs from the current advisory. - repo_urls = self._prepare_yum_repos_for_rebuilds( - db_event, event, builds) + repo_urls = self._prepare_yum_repos_for_rebuilds(db_event) self.log_info( "Following repositories will be used for the rebuild:") for url in repo_urls: @@ -121,7 +120,8 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): # As mentioned above, no need to wait for the event of new compose # is generated in ODCS, so we can start to rebuild the first batch # from here immediately. - self._build_first_batch(db_event) + self.start_to_build_images( + db_event.get_image_builds_in_first_batch()) if event.manual: msg = 'Base images are scheduled to be rebuilt due to manual rebuild.' @@ -163,25 +163,42 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): def _prepare_yum_repos_for_rebuilds(self, db_event): repo_urls = [] - repo_urls.append(self._prepare_yum_repo(db_event)) # noqa + db_composes = [] - repo_urls += [ - self._prepare_yum_repo(dep_event) - for dep_event in db_event.find_dependent_events() - ] + compose = self._prepare_yum_repo(db_event) + db_composes.append(Compose(odcs_compose_id=compose['id'])) + db.session.add(db_composes[-1]) + repo_urls.append(compose['result_repofile']) + for dep_event in db_event.find_dependent_events(): + compose = self._prepare_yum_repo(dep_event) + db_composes.append(Compose(odcs_compose_id=compose['id'])) + db.session.add(db_composes[-1]) + repo_urls.append(compose['result_repofile']) + + # commit all new composes + db.session.commit() + + for build in db_event.builds: + build.add_composes(db.session, db_composes) db.session.commit() + # Remove duplicates from repo_urls. return list(set(repo_urls)) def _prepare_yum_repo(self, db_event): """ - Prepare a yum repo for rebuild + Request a compose from ODCS for builds included in Errata advisory Run a compose in ODCS to contain required RPMs for rebuilding images later. - """ + :param Event db_event: current event being handled that contains errata + advisory to get builds containing updated RPMs. + :return: a mapping returned from ODCS that represents the request + compose. + :rtype: dict + """ errata_id = int(db_event.search_key) packages = [] @@ -223,14 +240,7 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): new_compose = self._fake_odcs_new_compose( compose_source, 'tag', packages=packages) - compose_id = new_compose['id'] - yum_repourl = new_compose['result_repofile'] - - rebuild_event = Event.get(db.session, db_event.message_id) - rebuild_event.compose_id = compose_id - db.session.commit() - - return yum_repourl + return new_compose def _prepare_pulp_repo(self, db_event, content_sets): """ @@ -453,17 +463,23 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): build.transition(state, state_reason) - compose = self._prepare_pulp_repo(build.event, image["content_sets"]) - build_args = {} build_args["repository"] = image["repository"] build_args["commit"] = image["commit"] build_args["parent"] = parent_nvr build_args["target"] = image["target"] build_args["branch"] = image["git_branch"] - build_args["odcs_pulp_compose_id"] = compose["id"] build.build_args = json.dumps(build_args) + + db.session.commit() + + # Store odcs pulp compose to build + compose = self._prepare_pulp_repo( + build.event, image["content_sets"]) + db_compose = Compose(odcs_compose_id=compose['id']) + db.session.add(db_compose) db.session.commit() + build.add_composes(db.session, [db_compose]) builds[nvr] = build diff --git a/freshmaker/handlers/odcs/compose_state_change.py b/freshmaker/handlers/odcs/compose_state_change.py index 4d3f35a..d7cbbdd 100644 --- a/freshmaker/handlers/odcs/compose_state_change.py +++ b/freshmaker/handlers/odcs/compose_state_change.py @@ -21,8 +21,10 @@ # # Written by Chenxiong Qi +import six + from freshmaker import db -from freshmaker.models import Event +from freshmaker.models import ArtifactBuild, ArtifactBuildState, Compose from freshmaker.handlers import ( ContainerBuildHandler, fail_event_on_handler_exception) from freshmaker.events import ODCSComposeStateChangeEvent @@ -42,8 +44,11 @@ class ComposeStateChangeHandler(ContainerBuildHandler): @fail_event_on_handler_exception def handle(self, event): - errata_signed_events = db.session.query(Event).filter( - Event.compose_id == event.compose['id']).all() - for db_event in errata_signed_events: - self.set_context(db_event) - self._build_first_batch(db_event) + query = db.session.query(ArtifactBuild).join('composes') + first_batch_builds = query.filter( + ArtifactBuild.dep_on == None, # noqa + ArtifactBuild.state == ArtifactBuildState.PLANNED.value, + Compose.odcs_compose_id == event.compose['id']) + builds_ready_to_rebuild = six.moves.filter( + lambda build: build.composes_ready, first_batch_builds) + self.start_to_build_images(builds_ready_to_rebuild) diff --git a/freshmaker/models.py b/freshmaker/models.py index f830647..b727633 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -32,9 +32,10 @@ from sqlalchemy.sql.expression import false from flask_login import UserMixin -from freshmaker import db, log +from freshmaker import conf, db, log from freshmaker import messaging -from freshmaker.utils import get_url_for +from freshmaker.odcsclient import ODCS, AuthMech +from freshmaker.utils import get_url_for, krb_context from freshmaker.types import ArtifactType, ArtifactBuildState, EventState from freshmaker.events import ( MBSModuleStateChangeEvent, GitModuleMetadataChangeEvent, @@ -226,6 +227,13 @@ class Event(FreshmakerBase): return session.query(cls).filter(cls.released == false(), cls.state.in_(states)).all() + def get_image_builds_in_first_batch(self, session): + return session.query(ArtifactBuild).filter_by( + dep_on=None, + type=ArtifactType.IMAGE.value, + event_id=self.id, + ).all() + @property def event_type(self): return INVERSE_EVENT_TYPES[self.event_type_id] @@ -509,6 +517,17 @@ class ArtifactBuild(FreshmakerBase): break return dep_on + def add_composes(self, session, composes): + """Add an ODCS compose to this build""" + for compose in composes: + session.add(ArtifactBuildCompose( + build_id=self.id, compose_id=compose.id)) + + @property + def composes_ready(self): + """Check if composes this build has have been done in ODCS""" + return all((rel.compose.finished for rel in self.composes)) + class Compose(FreshmakerBase): __tablename__ = 'composes' @@ -518,6 +537,15 @@ class Compose(FreshmakerBase): builds = db.relationship('ArtifactBuildCompose', back_populates='compose') + @property + def finished(self): + odcs = ODCS(conf.odcs_server_url, + auth_mech=AuthMech.Kerberos, + verify_ssl=conf.odcs_verify_ssl) + with krb_context(): + return 'done' == odcs.get_compose( + self.odcs_compose_id)['state_name'] + class ArtifactBuildCompose(FreshmakerBase): __tablename__ = 'artifact_build_composes' diff --git a/tests/test_errata_advisory_rpms_signed_handler.py b/tests/test_errata_advisory_rpms_signed_handler.py index 13f49fd..b13dae8 100644 --- a/tests/test_errata_advisory_rpms_signed_handler.py +++ b/tests/test_errata_advisory_rpms_signed_handler.py @@ -176,9 +176,9 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): @patch('freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' '_prepare_yum_repos_for_rebuilds') @patch('freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' - '_build_first_batch') + 'start_to_build_images') def test_rebuild_if_errata_state_is_prior_to_SHIPPED_LIVE( - self, build_first_batch, prepare_yum_repos_for_rebuilds, + self, start_to_build_images, prepare_yum_repos_for_rebuilds, allow_build): event = ErrataAdvisoryRPMsSignedEvent( 'msg-id-123', 'RHSA-2017', 123, '', 'REL_PREP') @@ -186,7 +186,7 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): handler.handle(event) prepare_yum_repos_for_rebuilds.assert_called_once() - build_first_batch.assert_not_called() + start_to_build_images.assert_not_called() db_event = Event.get(db.session, event.msg_id) self.assertEqual(EventState.BUILDING.value, db_event.state) @@ -196,17 +196,19 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): @patch('freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' '_prepare_yum_repos_for_rebuilds') @patch('freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' - '_build_first_batch') + 'start_to_build_images') + @patch('freshmaker.models.Event.get_image_builds_in_first_batch') def test_rebuild_if_errata_state_is_SHIPPED_LIVE( - self, build_first_batch, prepare_yum_repos_for_rebuilds, - allow_build): + self, get_image_builds_in_first_batch, start_to_build_images, + prepare_yum_repos_for_rebuilds, allow_build): event = ErrataAdvisoryRPMsSignedEvent( 'msg-id-123', 'RHSA-2017', 123, '', 'SHIPPED_LIVE') handler = ErrataAdvisoryRPMsSignedHandler() handler.handle(event) prepare_yum_repos_for_rebuilds.assert_not_called() - build_first_batch.assert_called_once() + get_image_builds_in_first_batch.assert_called_once() + start_to_build_images.assert_called_once() db_event = Event.get(db.session, event.msg_id) self.assertEqual(EventState.BUILDING.value, db_event.state) diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index 0fa60e5..4b8e067 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -631,10 +631,10 @@ class TestPrepareYumRepo(unittest.TestCase): errata.return_value.get_builds.return_value = set(["httpd-2.4.15-1.f27"]) handler = ErrataAdvisoryRPMsSignedHandler() - repo_url = handler._prepare_yum_repo(self.ev) + compose = handler._prepare_yum_repo(self.ev) db.session.refresh(self.ev) - self.assertEqual(3, self.ev.compose_id) + self.assertEqual(3, compose['id']) _get_compose_source.assert_called_once_with("httpd-2.4.15-1.f27") _get_packages_for_compose.assert_called_once_with("httpd-2.4.15-1.f27") @@ -647,7 +647,7 @@ class TestPrepareYumRepo(unittest.TestCase): # We should get the right repo URL eventually self.assertEqual( "http://localhost/composes/latest-odcs-3-1/compose/Temporary/odcs-3.repo", - repo_url) + compose['result_repofile']) @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS') @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.' @@ -965,9 +965,6 @@ class TestRecordBatchesImages(unittest.TestCase): self.assertNotEqual(None, parent_image) self.assertEqual(ArtifactBuildState.PLANNED.value, parent_image.state) - build_args = json.loads(parent_image.build_args) - self.assertEqual(1, build_args['odcs_pulp_compose_id']) - # Check child image child_image = query.filter( ArtifactBuild.original_nvr == 'rh-dotnetcore10-docker-1.0-16' @@ -976,12 +973,70 @@ class TestRecordBatchesImages(unittest.TestCase): self.assertEqual(parent_image, child_image.dep_on) self.assertEqual(ArtifactBuildState.PLANNED.value, child_image.state) - build_args = json.loads(child_image.build_args) - self.assertEqual(2, build_args['odcs_pulp_compose_id']) + def test_pulp_compose_is_stored_for_each_build(self): + batches = [ + [{ + "brew": { + "completion_date": "20170420T17:05:37.000-0400", + "build": "rhel-server-docker-7.3-82", + "package": "rhel-server-docker" + }, + "parent": None, + "content_sets": ["content-set-1"], + "repository": "repo-1", + "commit": "123456789", + "target": "target-candidate", + "git_branch": "rhel-7", + "error": None + }], + [{ + "brew": { + "build": "rh-dotnetcore10-docker-1.0-16", + "package": "rh-dotnetcore10-docker", + "completion_date": "20170511T10:06:09.000-0400" + }, + "parent": { + "brew": { + "completion_date": "20170420T17:05:37.000-0400", + "build": "rhel-server-docker-7.3-82", + "package": "rhel-server-docker" + }, + "parent": None, + "content_sets": ["content-set-1"], + "repository": "repo-1", + "commit": "123456789", + "target": "target-candidate", + "git_branch": "rhel-7", + "error": None + }, + "content_sets": ["content-set-1"], + "repository": "repo-1", + "commit": "987654321", + "target": "target-candidate", + "git_branch": "rhel-7", + "error": None + }] + ] + + handler = ErrataAdvisoryRPMsSignedHandler() + handler._record_batches(batches, self.mock_event) + + query = db.session.query(ArtifactBuild) + parent_build = query.filter( + ArtifactBuild.original_nvr == 'rhel-server-docker-7.3-82' + ).first() + self.assertEqual(1, len(parent_build.composes)) + self.assertEqual(1, parent_build.composes[0].compose.id) + + child_build = query.filter( + ArtifactBuild.original_nvr == 'rh-dotnetcore10-docker-1.0-16' + ).first() + self.assertEqual(1, len(child_build.composes)) + self.assertEqual(2, child_build.composes[0].compose.id) self.mock_prepare_pulp_repo.assert_has_calls([ - call(child_image.event, ["content-set-1"]), - call(child_image.event, ["content-set-1"]) + call(child_build.event, ["content-set-1"]), + call(child_build.event, ["content-set-1"]) ]) def test_mark_failed_state_if_image_has_error(self): @@ -1070,3 +1125,80 @@ class TestRecordBatchesImages(unittest.TestCase): ArtifactBuild.original_nvr == 'rh-dotnetcore10-docker-1.0-16' ).first() self.assertEqual(ArtifactBuildState.FAILED.value, build.state) + + +class TestPrepareYumReposForRebuilds(unittest.TestCase): + """Test ErrataAdvisoryRPMsSignedHandler._prepare_yum_repos_for_rebuilds""" + + def setUp(self): + db.session.remove() + db.drop_all() + db.create_all() + db.session.commit() + + self.prepare_yum_repo_patcher = patch( + 'freshmaker.handlers.errata.errata_advisory_rpms_signed.' + 'ErrataAdvisoryRPMsSignedHandler._prepare_yum_repo', + side_effect=[ + {'id': 1, 'result_repofile': 'http://localhost/repo/1'}, + {'id': 2, 'result_repofile': 'http://localhost/repo/2'}, + {'id': 3, 'result_repofile': 'http://localhost/repo/3'}, + {'id': 4, 'result_repofile': 'http://localhost/repo/4'}, + ]) + self.mock_prepare_yum_repo = self.prepare_yum_repo_patcher.start() + + self.find_dependent_event_patcher = patch( + 'freshmaker.models.Event.find_dependent_events') + self.mock_find_dependent_event = \ + self.find_dependent_event_patcher.start() + + self.db_event = Event.create( + db.session, 'msg-1', 'search-key-1', + EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent], + state=EventState.INITIALIZED, + released=False) + self.build_1 = ArtifactBuild.create( + db.session, self.db_event, 'build-1', ArtifactType.IMAGE) + self.build_2 = ArtifactBuild.create( + db.session, self.db_event, 'build-2', ArtifactType.IMAGE) + + db.session.commit() + + def tearDown(self): + self.find_dependent_event_patcher.stop() + self.prepare_yum_repo_patcher.stop() + + db.session.remove() + db.drop_all() + db.session.commit() + + def test_prepare_without_dependent_events(self): + self.mock_find_dependent_event.return_value = [] + + handler = ErrataAdvisoryRPMsSignedHandler() + urls = handler._prepare_yum_repos_for_rebuilds(self.db_event) + + self.assertEqual(1, self.build_1.composes[0].compose.id) + self.assertEqual(1, self.build_2.composes[0].compose.id) + self.assertEqual(['http://localhost/repo/1'], urls) + + def test_prepare_with_dependent_events(self): + self.mock_find_dependent_event.return_value = [ + Mock(), Mock(), Mock() + ] + + handler = ErrataAdvisoryRPMsSignedHandler() + urls = handler._prepare_yum_repos_for_rebuilds(self.db_event) + + odcs_compose_ids = [rel.compose.id for rel in self.build_1.composes] + self.assertEqual([1, 2, 3, 4], sorted(odcs_compose_ids)) + + odcs_compose_ids = [rel.compose.id for rel in self.build_2.composes] + self.assertEqual([1, 2, 3, 4], sorted(odcs_compose_ids)) + + self.assertEqual([ + 'http://localhost/repo/1', + 'http://localhost/repo/2', + 'http://localhost/repo/3', + 'http://localhost/repo/4', + ], sorted(urls)) diff --git a/tests/test_handler.py b/tests/test_handler.py index bd67492..30bc5f7 100644 --- a/tests/test_handler.py +++ b/tests/test_handler.py @@ -22,8 +22,6 @@ # # Written by Chenxiong Qi -import json - from mock import patch, PropertyMock from unittest import TestCase @@ -32,11 +30,12 @@ import freshmaker from freshmaker import db from freshmaker.events import ErrataAdvisoryRPMsSignedEvent from freshmaker.handlers import ContainerBuildHandler -from freshmaker.models import ArtifactBuild -from freshmaker.models import ArtifactBuildState -from freshmaker.models import Event +from freshmaker.models import ( + ArtifactBuild, ArtifactBuildState, ArtifactBuildCompose, + Compose, Event, EVENT_TYPES +) from freshmaker.errors import UnprocessableEntity, ProgrammingError -from freshmaker.types import ArtifactType +from freshmaker.types import ArtifactType, EventState class MyHandler(ContainerBuildHandler): @@ -148,25 +147,65 @@ class TestGetRepoURLs(TestCase): db.create_all() db.session.commit() - self.db_event = Event.get_or_create( - db.session, "msg1", "current_event", ErrataAdvisoryRPMsSignedEvent, + self.compose_1 = Compose(odcs_compose_id=1) + self.compose_2 = Compose(odcs_compose_id=2) + self.compose_3 = Compose(odcs_compose_id=3) + self.compose_4 = Compose(odcs_compose_id=4) + db.session.add(self.compose_1) + db.session.add(self.compose_2) + db.session.add(self.compose_3) + db.session.add(self.compose_4) + + self.event = Event.create( + db.session, 'msg-1', 'search-key-1', + EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent], + state=EventState.BUILDING, released=False) - self.build = ArtifactBuild.create( - db.session, self.db_event, "parent1-1-4", "image", - state=ArtifactBuildState.PLANNED, original_nvr="parent1-1-4") + self.build_1 = ArtifactBuild.create( + db.session, self.event, 'build-1', ArtifactType.IMAGE, + state=ArtifactBuildState.PLANNED) + self.build_2 = ArtifactBuild.create( + db.session, self.event, 'build-2', ArtifactType.IMAGE, + state=ArtifactBuildState.PLANNED) + db.session.commit() - def mocked_odcs_get_compose(compose_id): - return { - "id": compose_id, - "result_repofile": "http://localhost/%d.repo" % compose_id, - } + rels = ( + (self.build_1.id, self.compose_1.id), + (self.build_1.id, self.compose_2.id), + (self.build_1.id, self.compose_3.id), + (self.build_1.id, self.compose_4.id), + ) + + for build_id, compose_id in rels: + db.session.add( + ArtifactBuildCompose( + build_id=build_id, compose_id=compose_id)) + + db.session.commit() self.patch_odcs_get_compose = patch( - "freshmaker.handlers.ContainerBuildHandler.odcs_get_compose") + "freshmaker.handlers.ContainerBuildHandler.odcs_get_compose", + side_effect=[ + { + "id": self.compose_1.id, + "result_repofile": "http://localhost/1.repo", + }, + { + "id": self.compose_2.id, + "result_repofile": "http://localhost/2.repo", + }, + { + "id": self.compose_3.id, + "result_repofile": "http://localhost/3.repo", + }, + { + "id": self.compose_4.id, + "result_repofile": "http://localhost/4.repo", + }, + ]) self.odcs_get_compose = self.patch_odcs_get_compose.start() - self.odcs_get_compose.side_effect = mocked_odcs_get_compose def tearDown(self): db.session.remove() @@ -176,41 +215,20 @@ class TestGetRepoURLs(TestCase): def test_get_repo_urls_no_composes(self): handler = MyHandler() - repos = handler.get_repo_urls(self.db_event, self.build) + repos = handler.get_repo_urls(self.build_2) self.assertEqual(repos, []) - def test_get_repo_urls_only_main_compose(self): - self.db_event.compose_id = 1 - db.session.commit() - - handler = MyHandler() - repos = handler.get_repo_urls(self.db_event, self.build) - self.assertEqual(repos, ["http://localhost/1.repo"]) - - def test_get_repo_urls_only_pulp_compose(self): - build_args = json.dumps({ - "odcs_pulp_compose_id": 15, - }) - self.build.build_args = build_args - db.session.commit() - - handler = MyHandler() - repos = handler.get_repo_urls(self.db_event, self.build) - self.assertEqual(repos, ["http://localhost/15.repo"]) - def test_get_repo_urls_both_pulp_and_main_compose(self): - build_args = json.dumps({ - "odcs_pulp_compose_id": 15, - }) - self.db_event.compose_id = 1 - self.build.build_args = build_args - db.session.commit() - handler = MyHandler() - repos = handler.get_repo_urls(self.db_event, self.build) + repos = handler.get_repo_urls(self.build_1) self.assertEqual( - repos, - ["http://localhost/1.repo", "http://localhost/15.repo"]) + [ + 'http://localhost/1.repo', + 'http://localhost/2.repo', + 'http://localhost/3.repo', + 'http://localhost/4.repo', + ], + sorted(repos)) class TestAllowBuildBasedOnWhitelist(TestCase): @@ -343,165 +361,3 @@ class TestAllowBuildBasedOnWhitelist(TestCase): advisory_name='RHSA-2017', advisory_state='REL_PREP') self.assertTrue(allowed) - - -class AnyStringWith(str): - def __eq__(self, other): - return self in other - - -class TestBuildFirstBatch(TestCase): - """Test ErrataAdvisoryRPMsSignedHandler._build_first_batch""" - - def setUp(self): - db.session.remove() - db.drop_all() - db.create_all() - db.session.commit() - - build_args = json.dumps({ - "parent": "nvr", - "repository": "repo", - "target": "target", - "commit": "hash", - "branch": "mybranch", - "yum_repourl": "http://localhost/composes/latest-odcs-3-1/compose/" - "Temporary/odcs-3.repo", - "odcs_pulp_compose_id": 15, - }) - - self.db_event = Event.get_or_create( - db.session, "msg1", "current_event", ErrataAdvisoryRPMsSignedEvent, - released=False) - self.db_event.compose_id = 3 - - p1 = ArtifactBuild.create(db.session, self.db_event, "parent1-1-4", - "image", - state=ArtifactBuildState.PLANNED, - original_nvr="parent1-1-4") - p1.build_args = build_args - self.p1 = p1 - - b = ArtifactBuild.create(db.session, self.db_event, - "parent1_child1", "image", - state=ArtifactBuildState.PLANNED, - dep_on=p1, - original_nvr="parent1_child1-1-4") - b.build_args = build_args - - # Not in PLANNED state. - b = ArtifactBuild.create(db.session, self.db_event, "parent3", "image", - state=ArtifactBuildState.BUILD, - original_nvr="parent3-1-4") - b.build_args = build_args - - # No build args - b = ArtifactBuild.create(db.session, self.db_event, "parent4", "image", - state=ArtifactBuildState.PLANNED, - original_nvr="parent4-1-4") - db.session.commit() - - # No parent - base image - b = ArtifactBuild.create(db.session, self.db_event, "parent5", "image", - state=ArtifactBuildState.PLANNED, - original_nvr="parent5-1-4") - b.build_args = build_args - b.build_args = b.build_args.replace("nvr", "") - - def tearDown(self): - db.session.remove() - db.drop_all() - db.session.commit() - - @patch('freshmaker.handlers.ODCS') - @patch('koji.ClientSession') - @patch('freshmaker.utils.krbContext') - def test_build_first_batch(self, krb, ClientSession, ODCS): - """ - Tests that only PLANNED images without a parent are submitted to - build system. - """ - - def _fake_get_compose(compose_id): - return { - "id": compose_id, - "result_repo": "http://localhost/composes/latest-odcs-%d-1/compose/Temporary" % compose_id, - "result_repofile": "http://localhost/composes/latest-odcs-%d-1/compose/Temporary/odcs-%s.repo" % (compose_id, compose_id), - "source": "f26", - "source_type": 1, - "state": 2, - "state_name": "done", - } - - ODCS.return_value.get_compose = _fake_get_compose - - mock_session = ClientSession.return_value - mock_session.buildContainer.return_value = 123 - - handler = MyHandler() - handler._build_first_batch(self.db_event) - - mock_session.buildContainer.assert_called_once_with( - 'git://pkgs.fedoraproject.org/repo#hash', - 'target', - {'scratch': True, 'isolated': True, 'koji_parent_build': u'nvr', - 'git_branch': 'mybranch', 'release': AnyStringWith('4.'), - 'yum_repourls': [ - 'http://localhost/composes/latest-odcs-3-1/compose/Temporary/odcs-3.repo', - 'http://localhost/composes/latest-odcs-15-1/compose/Temporary/odcs-15.repo']}) - - db.session.refresh(self.db_event) - for build in self.db_event.builds: - if build.name == "parent1-1-4": - self.assertEqual(build.build_id, 123) - elif build.name == "parent3": - self.assertEqual(build.state, ArtifactBuildState.FAILED.value) - self.assertEqual(build.state_reason, "Container image build " - "is not in PLANNED state.") - elif build.name == "parent4": - self.assertEqual(build.state, ArtifactBuildState.FAILED.value) - self.assertEqual(build.state_reason, "Container image does " - "not have 'build_args' filled in.") - elif build.name == "parent5": - self.assertEqual(build.state, ArtifactBuildState.FAILED.value) - self.assertEqual(build.state_reason, "Rebuild of container " - "base image is not supported yet.") - else: - self.assertEqual(build.build_id, None) - self.assertEqual(build.state, ArtifactBuildState.PLANNED.value) - - @patch('freshmaker.handlers.ODCS') - @patch('koji.ClientSession') - @patch('freshmaker.utils.krbContext') - def test_build_first_batch_exception(self, krb, ClientSession, ODCS): - """ - Tests that only PLANNED images without a parent are submitted to - build system. - """ - - def _fake_get_compose(compose_id): - return { - "id": compose_id, - "result_repo": "http://localhost/composes/latest-odcs-%d-1/compose/Temporary" % compose_id, - "result_repofile": "http://localhost/composes/latest-odcs-%d-1/compose/Temporary/odcs-%s.repo" % (compose_id, compose_id), - "source": "f26", - "source_type": 1, - "state": 2, - "state_name": "done", - } - - ODCS.return_value.get_compose = _fake_get_compose - - def mock_buildContainer(*args, **kwargs): - raise ValueError("Expected exception") - - mock_session = ClientSession.return_value - mock_session.buildContainer.side_effect = mock_buildContainer - - handler = MyHandler() - self.assertRaises(ValueError, handler._build_first_batch, self.db_event) - - db.session.refresh(self.p1) - self.assertEqual(self.p1.state, ArtifactBuildState.FAILED.value) - self.assertTrue(self.p1.state_reason.startswith( - "Handling of build failed with traceback")) diff --git a/tests/test_odcs_compose_state_change.py b/tests/test_odcs_compose_state_change.py index 63dde15..59c9430 100644 --- a/tests/test_odcs_compose_state_change.py +++ b/tests/test_odcs_compose_state_change.py @@ -21,15 +21,18 @@ # # Written by Chenxiong Qi +import six import unittest -from mock import call, patch +from mock import patch, PropertyMock from freshmaker import db -from freshmaker.models import Event -from freshmaker.models import EVENT_TYPES +from freshmaker.models import ( + Event, EventState, EVENT_TYPES, + ArtifactBuild, ArtifactType, ArtifactBuildState, ArtifactBuildCompose, + Compose +) from freshmaker.events import ErrataAdvisoryRPMsSignedEvent -from freshmaker.events import KojiTaskStateChangeEvent from freshmaker.handlers.odcs import ComposeStateChangeHandler from freshmaker.events import ODCSComposeStateChangeEvent @@ -43,18 +46,56 @@ class TestComposeStateChangeHandler(unittest.TestCase): db.create_all() db.session.commit() - self.adv_signed_event1 = Event.get_or_create( - db.session, 'msg-id-1', 'msg-id-1', - EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent]) - self.adv_signed_event2 = Event.get_or_create( - db.session, 'msg-id-2', 'msg-id-2', - EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent]) - self.unrelated_event = Event.get_or_create( - db.session, 'msg-id-3', 'msg-id-3', - EVENT_TYPES[KojiTaskStateChangeEvent]) - - self.adv_signed_event1.compose_id = 1 - self.adv_signed_event2.compose_id = 1 + # Test data + # (Inner build depends on outer build) + # Event (ErrataAdvisoryRPMsSignedEvent): + # build 1: [compose 1, pulp compose 1] + # build 2: [compose 1, pulp compose 2] + # build 3: [compose 1, pulp compose 3] + # build 4: [compose 1, pulp compose 4] + # build 5: [compose 1, pulp compose 5] + # build 6 (not planned): [compose 1, pulp compose 6] + + self.db_event = Event.create( + db.session, 'msg-1', 'search-key-1', + EVENT_TYPES[ErrataAdvisoryRPMsSignedEvent], + state=EventState.INITIALIZED, + released=False) + + self.build_1 = ArtifactBuild.create( + db.session, self.db_event, 'build-1', ArtifactType.IMAGE, + state=ArtifactBuildState.PLANNED) + self.build_2 = ArtifactBuild.create( + db.session, self.db_event, 'build-2', ArtifactType.IMAGE, + dep_on=self.build_1, + state=ArtifactBuildState.PLANNED) + + self.build_3 = ArtifactBuild.create( + db.session, self.db_event, 'build-3', ArtifactType.IMAGE, + state=ArtifactBuildState.PLANNED) + self.build_4 = ArtifactBuild.create( + db.session, self.db_event, 'build-4', ArtifactType.IMAGE, + dep_on=self.build_3, + state=ArtifactBuildState.PLANNED) + self.build_5 = ArtifactBuild.create( + db.session, self.db_event, 'build-5', ArtifactType.IMAGE, + dep_on=self.build_3, + state=ArtifactBuildState.PLANNED) + + self.build_6 = ArtifactBuild.create( + db.session, self.db_event, 'build-6', ArtifactType.IMAGE, + state=ArtifactBuildState.BUILD) + + self.compose_1 = Compose(odcs_compose_id=1) + db.session.add(self.compose_1) + db.session.commit() + + builds = [self.build_1, self.build_2, self.build_3, + self.build_4, self.build_5, self.build_6] + composes = [self.compose_1] * 6 + for build, compose in six.moves.zip(builds, composes): + db.session.add(ArtifactBuildCompose( + build_id=build.id, compose_id=compose.id)) db.session.commit() def tearDown(self): @@ -70,20 +111,19 @@ class TestComposeStateChangeHandler(unittest.TestCase): can_handle = handler.can_handle(event) self.assertFalse(can_handle) - @patch('freshmaker.handlers.ContainerBuildHandler._build_first_batch') - @patch('freshmaker.handlers.ContainerBuildHandler.set_context') - def test_start_to_build(self, set_context, build_first_batch): + @patch('freshmaker.models.ArtifactBuild.composes_ready', + new_callable=PropertyMock) + @patch('freshmaker.handlers.ContainerBuildHandler.start_to_build_images') + def test_start_to_build(self, start_to_build_images, composes_ready): + composes_ready.return_value = True + event = ODCSComposeStateChangeEvent( - 'msg-id', {'id': 1, 'state': 'done'} + 'msg-id', {'id': self.compose_1.id, 'state': 'done'} ) + handler = ComposeStateChangeHandler() handler.handle(event) - build_first_batch.assert_has_calls([ - call(self.adv_signed_event1), - call(self.adv_signed_event2), - ]) - - set_context.assert_has_calls([ - call(self.adv_signed_event1), - call(self.adv_signed_event2) - ]) + + args, kwargs = start_to_build_images.call_args + passed_builds = sorted(args[0], key=lambda build: build.id) + self.assertEqual([self.build_1, self.build_3], passed_builds) From 314854c8dd5f8938483e267becdec4d05b853f05 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jan 03 2018 05:51:00 +0000 Subject: [PATCH 4/7] Remove Event.compose_id Event.compose_id is not used anymore. This patch also contains data migration fro legacy compose IDs from Event.compose_id to Compose model. Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/migrations/versions/b17231ee8220_remove_event_compose_id.py b/freshmaker/migrations/versions/b17231ee8220_remove_event_compose_id.py new file mode 100644 index 0000000..ce8bfa8 --- /dev/null +++ b/freshmaker/migrations/versions/b17231ee8220_remove_event_compose_id.py @@ -0,0 +1,102 @@ +"""Remove Event.compose_id + +Revision ID: b17231ee8220 +Revises: 6004dadc9ac4 +Create Date: 2017-12-25 10:51:05.484216 + +""" + +# revision identifiers, used by Alembic. +revision = 'b17231ee8220' +down_revision = '6004dadc9ac4' + +import logging +logger = logging.getLogger(__name__) + +from itertools import count +from six import next +import sqlalchemy as sa +from alembic import op +from sqlalchemy import text + +from freshmaker import db +from freshmaker.models import ArtifactBuild, ArtifactBuildCompose +from freshmaker.models import Event, Compose + + +def upgrade(): + # First, we must migrate data from Event.compose_id to Compose model + session = db.session + connection = op.get_bind() + + for row in connection.execute('SELECT id, compose_id FROM events'): + event_id, odcs_compose_id = row + + logger.info('Create Compose with odcs_compose_id %s from Event %s', + odcs_compose_id, event_id) + connection.execute( + text( + 'INSERT INTO composes (odcs_compose_id) VALUES (:compose_id)' + ).bindparams(compose_id=odcs_compose_id), + autocommit=True + ) + + result = connection.execute( + text( + 'SELECT id FROM composes WHERE odcs_compose_id = :compose_id' + ).bindparams(compose_id=odcs_compose_id) + ) + new_compose_id = result.fetchall()[0][0] + + build_ids = connection.execute( + text( + 'SELECT DISTINCT id FROM artifact_builds ' + 'WHERE event_id = :event_id' + ).bindparams(event_id=event_id) + ) + + with connection.begin(): + for row in build_ids: + build_id, = row + connection.execute( + text( + 'INSERT INTO artifact_build_composes (build_id, compose_id) ' + 'VALUES (:build_id, :compose_id)' + ).bindparams(build_id=build_id, + compose_id=new_compose_id) + ) + + # Now, migration is doable + with op.batch_alter_table("events") as batch_op: + batch_op.drop_column('compose_id') + + +def downgrade(): + # First, we have to restore Event.compose_id + with op.batch_alter_table("events") as batch_op: + batch_op.add_column(sa.Column('compose_id', sa.Integer(), nullable=True)) + + # It is time to restore data from Compose model back to Event.compose_id + session = db.session + connection = op.get_bind() + + with connection.begin(): + for compose in session.query(Compose).all(): + if len(compose.builds) == 0: + raise ValueError( + 'Compose {} is not associated with any ArtifactBuild. ' + 'This must be a problem in production. Or may not be a ' + 'problem due to dirty data in development environment. ' + 'Please confirm and handle by yourself.'.format(compose.id)) + event = compose.builds[0].build.event + logger.info( + 'Restore odcs compose id %s from Compose %s back to Event %s', + compose.odcs_compose_id, compose.id, event.id) + connection.execute( + 'UPDATE events SET compose_id = {} WHERE id = {}'.format( + compose.odcs_compose_id, event.id)) + + logger.info('Clear data from ArtifactBuildCompose') + connection.execute('DELETE FROM artifact_build_composes') + logger.info('Clear data from Compose') + connection.execute('DELETE FROM composes') \ No newline at end of file diff --git a/freshmaker/models.py b/freshmaker/models.py index b727633..e775793 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -146,11 +146,6 @@ class Event(FreshmakerBase): # List of builds associated with this Event. builds = relationship("ArtifactBuild", back_populates="event") - compose_id = db.Column( - db.Integer, - default=None, - doc='Used to include new version packages to rebuild docker images') - manual_triggered = db.Column( db.Boolean, default=False, From ed59c2da2c9b762cf9594171a4383ac24ba3db8a Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jan 03 2018 05:51:00 +0000 Subject: [PATCH 5/7] Request boot.iso compose for base image rebuild Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index d5d7ef5..9e75519 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -24,6 +24,10 @@ import json import koji +import requests + +from six.moves import cStringIO +from six.moves import configparser from freshmaker import conf, db, log from freshmaker.events import ErrataAdvisoryRPMsSignedEvent @@ -132,7 +136,8 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): return [] - def _fake_odcs_new_compose(self, compose_source, tag, packages=None): + def _fake_odcs_new_compose( + self, compose_source, tag, packages=None, results=[]): """ Fake KojiSession.buildContainer method used dry run mode. @@ -143,7 +148,7 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): :return: Fake odcs.new_compose dict. """ self.log_info("DRY RUN: Calling fake odcs.new_compose with args: %r", - (compose_source, tag, packages)) + (compose_source, tag, packages, results)) # Generate the new_compose dict. ErrataAdvisoryRPMsSignedHandler._FAKE_COMPOSE_ID += 1 @@ -152,6 +157,8 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): new_compose['result_repofile'] = "http://localhost/%d.repo" % ( new_compose['id']) new_compose['state'] = COMPOSE_STATES['done'] + if results: + new_compose['results'] = ['boot.iso'] # Generate and inject the ODCSComposeStateChangeEvent event. event = ODCSComposeStateChangeEvent( @@ -295,6 +302,61 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): return new_compose + def _get_base_image_build_target(self, image): + dockerfile = image.dockerfile + image_build_conf_url = dockerfile['content_url'].replace( + dockerfile['filename'], 'image-build.conf') + response = requests.get(image_build_conf_url) + try: + response.raise_for_status() + except requests.exceptions.HTTPError as e: + log.error( + 'Cannot get image-build.conf from %s.', image_build_conf_url) + log.exception('Server response: %s', e) + return None + config_buf = cStringIO(response.content) + config = configparser.RawConfigParser() + try: + config.readfp(config_buf) + except configparser.MissingSectionHeaderError: + return None + finally: + config_buf.close() + try: + return config.get('image-build', 'target') + except (configparser.NoOptionError, configparser.NoSectionError): + log.exception('image-build.conf does not have option target.') + return None + + def _get_base_image_build_tag(self, build_target): + with koji_service(conf.koji_profile, log) as session: + target_info = session.get_build_target(build_target) + if target_info is None: + return target_info + else: + return target_info['build_tag_name'] + + def _request_boot_iso_compose(self, image): + """Request boot.iso compose for base image""" + target = self._get_base_image_build_target(image) + if not target: + return None + build_tag = self._get_base_image_build_tag(target) + if not build_tag: + return None + + odcs = ODCS(conf.odcs_server_url, + auth_mech=AuthMech.Kerberos, + verify_ssl=conf.odcs_verify_ssl) + if conf.dry_run: + new_compose = self._fake_odcs_new_compose( + build_tag, 'tag', results=['boot.iso']) + else: + with krb_context(): + new_compose = odcs.new_compose( + build_tag, 'tag', results=['boot.iso']) + return new_compose + def _get_packages_for_compose(self, nvr): """Get RPMs of current build NVR @@ -481,6 +543,23 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): db.session.commit() build.add_composes(db.session, [db_compose]) + if image.is_base_image: + compose = self._request_boot_iso_compose(image) + if compose is None: + log.error( + 'Failed to request boot.iso compose for base ' + 'image %s.', nvr) + build.transition( + ArtifactBuildState.FAILED.value, + 'Cannot rebuild this base image because failed to ' + 'requeset boot.iso compose.') + # FIXME: mark all builds associated with build.event FAILED? + else: + db_compose = Compose(odcs_compose_id=compose['id']) + db.session.add(db_compose) + db.session.commit() + build.add_composes(db.session, [db_compose]) + builds[nvr] = build return builds diff --git a/freshmaker/kojiservice.py b/freshmaker/kojiservice.py index a02889c..540e9bd 100644 --- a/freshmaker/kojiservice.py +++ b/freshmaker/kojiservice.py @@ -164,6 +164,9 @@ class KojiService(object): def get_task_request(self, task_id): return self.session.getTaskRequest(task_id) + def get_build_target(self, target_name): + return self.session.getBuildTarget(target_name) + @contextlib.contextmanager def koji_service(profile=None, logger=None, login=True): diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index 5ea677c..4e8ab8c 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -118,6 +118,21 @@ class ContainerImage(dict): def __hash__(self): return hash((self['brew']['build'])) + @property + def is_base_image(self): + return (self['parent'] is None and + len(self['parsed_data']['layers']) == 2) + + @property + def dockerfile(self): + dockerfile = [file for file in self['parsed_data']['files'] + if file['filename'] == 'Dockerfile'] + if not dockerfile: + log.warning('Image %s does not contain a Dockerfile.', + self['brew']['build']) + return None + return dockerfile[0] + def _get_default_additional_data(self): return {"repository": None, "commit": None, "target": None, "git_branch": None, "error": None} diff --git a/tests/test_errata_advisory_rpms_signed_handler.py b/tests/test_errata_advisory_rpms_signed_handler.py index b13dae8..31e3ce1 100644 --- a/tests/test_errata_advisory_rpms_signed_handler.py +++ b/tests/test_errata_advisory_rpms_signed_handler.py @@ -20,6 +20,7 @@ # SOFTWARE. import unittest +import requests from mock import patch @@ -28,6 +29,7 @@ import freshmaker from freshmaker import db from freshmaker.events import ErrataAdvisoryRPMsSignedEvent from freshmaker.handlers.errata import ErrataAdvisoryRPMsSignedHandler +from freshmaker.lightblue import ContainerImage from freshmaker.models import Event from freshmaker.types import EventState @@ -60,6 +62,13 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): '_find_images_to_rebuild') self.mock_find_images_to_rebuild = self.find_images_patcher.start() + self.request_boot_iso_compose_patcher = patch( + 'freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' + '_request_boot_iso_compose', + side_effect=[{'id': 1}, {'id': 2}]) + self.mock_request_boot_iso_compose = \ + self.request_boot_iso_compose_patcher.start() + # Fake images found to rebuild has these relationships # # Batch 1 | Batch 2 | Batch 3 @@ -67,7 +76,7 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): # image_b | image_d (child of image_a) | # | image_e (child of image_b) | # - self.image_a = { + self.image_a = ContainerImage({ 'repository': 'repo_1', 'commit': '1234567', 'target': 'docker-container-candidate', @@ -77,8 +86,14 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): 'build': 'image-a-1.0-2', }, 'parent': None, - } - self.image_b = { + 'parsed_data': { + 'layers': [ + 'sha512:7890', + 'sha512:5678', + ] + }, + }) + self.image_b = ContainerImage({ 'repository': 'repo_2', 'commit': '23e9f22', 'target': 'docker-container-candidate', @@ -88,8 +103,14 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): 'build': 'image-b-1.0-1' }, 'parent': None, - } - self.image_c = { + 'parsed_data': { + 'layers': [ + 'sha512:1234', + 'sha512:4567', + ] + }, + }) + self.image_c = ContainerImage({ 'repository': 'repo_2', 'commit': '2345678', 'target': 'docker-container-candidate', @@ -99,8 +120,15 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): 'build': 'image-c-0.2-9', }, 'parent': self.image_a, - } - self.image_d = { + 'parsed_data': { + 'layers': [ + 'sha512:4ef3', + 'sha512:7890', + 'sha512:5678', + ] + }, + }) + self.image_d = ContainerImage({ 'repository': 'repo_2', 'commit': '5678901', 'target': 'docker-container-candidate', @@ -110,8 +138,15 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): 'build': 'image-d-2.14-1', }, 'parent': self.image_a, - } - self.image_e = { + 'parsed_data': { + 'layers': [ + 'sha512:f109', + 'sha512:7890', + 'sha512:5678', + ] + }, + }) + self.image_e = ContainerImage({ 'repository': 'repo_2', 'commit': '7890123', 'target': 'docker-container-candidate', @@ -121,8 +156,15 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): 'build': 'image-e-1.0-1', }, 'parent': self.image_b, - } - self.image_f = { + 'parsed_data': { + 'layers': [ + 'sha512:5aae', + 'sha512:1234', + 'sha512:4567', + ] + }, + }) + self.image_f = ContainerImage({ 'repository': 'repo_2', 'commit': '3829384', 'target': 'docker-container-candidate', @@ -132,7 +174,14 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): 'build': 'image-f-0.2-1', }, 'parent': self.image_b, - } + 'parsed_data': { + 'layers': [ + 'sha512:8b9e', + 'sha512:1234', + 'sha512:4567', + ] + }, + }) # For simplicify, mocking _find_images_to_rebuild to just return one # batch, which contains images found for rebuild from parent to # childrens. @@ -145,6 +194,7 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): ]) def tearDown(self): + self.request_boot_iso_compose_patcher.stop() self.find_images_patcher.stop() self.prepare_pulp_repo_patcher.stop() self.messaging_publish_patcher.stop() @@ -212,3 +262,205 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): db_event = Event.get(db.session, event.msg_id) self.assertEqual(EventState.BUILDING.value, db_event.state) + + +class TestGetBaseImageBuildTarget(unittest.TestCase): + """Test ErrataAdvisoryRPMsSignedHandler._get_base_image_build_target""" + + def setUp(self): + self.image = ContainerImage({ + 'repository': 'repo_1', + 'commit': '1234567', + 'target': 'docker-container-candidate', + 'git_branch': 'rhel-7.4', + 'content_sets': ['image_a_content_set_1', 'image_a_content_set_2'], + 'brew': { + 'build': 'image-a-1.0-2', + }, + 'parent': None, + 'parsed_data': { + 'layers': [ + 'sha512:7890', + 'sha512:5678', + ], + 'files': [ + { + 'filename': 'Dockerfile', + 'content_url': 'http://pkgs.localhost/cgit/rpms/' + 'image-a/plain/Dockerfile?id=fa521323', + 'key': 'buildfile' + } + ] + }, + }) + self.handler = ErrataAdvisoryRPMsSignedHandler() + + @patch('requests.get') + def test_get_target_from_image_build_conf(self, get): + get.return_value.content = '''\ +[image-build] +name = image-a +arches = x86_64 +format = docker +disk_size = 10 +ksurl = git://git.localhost/spin-kickstarts.git?rhel7#HEAD +kickstart = rhel-7.4-server-docker.ks +version = 7.4 +target = guest-rhel-7.4-docker +distro = RHEL-7.4 +ksversion = RHEL7''' + + result = self.handler._get_base_image_build_target(self.image) + self.assertEqual('guest-rhel-7.4-docker', result) + + @patch('requests.get') + def test_image_build_conf_is_unavailable_in_distgit(self, get): + get.return_value.raise_for_status.side_effect = \ + requests.exceptions.HTTPError('error') + + result = self.handler._get_base_image_build_target(self.image) + self.assertIsNone(result) + + @patch('requests.get') + def test_image_build_conf_is_empty(self, get): + get.return_value.content = '' + + result = self.handler._get_base_image_build_target(self.image) + self.assertIsNone(result) + + @patch('requests.get') + def test_image_build_conf_is_not_INI(self, get): + get.return_value.content = 'abc' + + result = self.handler._get_base_image_build_target(self.image) + self.assertIsNone(result) + + +class TestGetBaseImageBuildTag(unittest.TestCase): + """Test ErrataAdvisoryRPMsSignedHandler._get_base_image_build_tag""" + + def setUp(self): + self.image = ContainerImage({ + 'repository': 'repo_1', + 'commit': '1234567', + 'target': 'docker-container-candidate', + 'git_branch': 'rhel-7.4', + 'content_sets': ['image_a_content_set_1', 'image_a_content_set_2'], + 'brew': { + 'build': 'image-a-1.0-2', + }, + 'parent': None, + 'parsed_data': { + 'layers': [ + 'sha512:7890', + 'sha512:5678', + ], + 'files': [ + { + 'filename': 'Dockerfile', + 'content_url': 'http://pkgs.localhost/cgit/rpms/' + 'image-a/plain/Dockerfile?id=fa521323', + 'key': 'buildfile' + } + ] + }, + }) + self.handler = ErrataAdvisoryRPMsSignedHandler() + + @patch('freshmaker.kojiservice.KojiService') + def test_get_build_tag_name(self, KojiService): + koji_service = KojiService.return_value + koji_service.get_build_target.return_value = { + 'build_tag': 10052, + 'build_tag_name': 'guest-rhel-7.4-docker-build', + 'dest_tag': 10051, + 'dest_tag_name': 'guest-rhel-7.4-candidate', + 'id': 3205, + 'name': 'guest-rhel-7.4-docker' + } + + result = self.handler._get_base_image_build_tag( + 'guest-rhel-7.4-docker') + self.assertEqual('guest-rhel-7.4-docker-build', result) + + @patch('freshmaker.kojiservice.KojiService') + def test_no_target_is_returned(self, KojiService): + koji_service = KojiService.return_value + koji_service.get_build_target.return_value = None + + result = self.handler._get_base_image_build_tag( + 'guest-rhel-7.4-docker') + self.assertIsNone(result) + + +class TestRequestBootISOCompose(unittest.TestCase): + """Test ErrataAdvisoryRPMsSignedHandler._request_boot_iso_compose""" + + def setUp(self): + self.image = ContainerImage({ + 'repository': 'repo_1', + 'commit': '1234567', + 'target': 'docker-container-candidate', + 'git_branch': 'rhel-7.4', + 'content_sets': ['image_a_content_set_1', 'image_a_content_set_2'], + 'brew': { + 'build': 'image-a-1.0-2', + }, + 'parent': None, + 'parsed_data': { + 'layers': [ + 'sha512:7890', + 'sha512:5678', + ], + 'files': [ + { + 'filename': 'Dockerfile', + 'content_url': 'http://pkgs.localhost/cgit/rpms/' + 'image-a/plain/Dockerfile?id=fa521323', + 'key': 'buildfile' + } + ] + }, + }) + self.handler = ErrataAdvisoryRPMsSignedHandler() + + @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.krb_context') + @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS') + @patch('freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' + '_get_base_image_build_target') + @patch('freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' + '_get_base_image_build_tag') + def test_get_boot_iso_compose( + self, get_base_image_build_tag, get_base_image_build_target, + ODCS, krb_context): + odcs = ODCS.return_value + odcs.new_compose.return_value = {'id': 1} + + get_base_image_build_target.return_value = 'build-target' + get_base_image_build_tag.return_value = 'build-tag' + + result = self.handler._request_boot_iso_compose(self.image) + + self.assertEqual(odcs.new_compose.return_value, result) + odcs.new_compose.assert_called_once_with( + 'build-tag', 'tag', results=['boot.iso']) + + @patch('freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' + '_get_base_image_build_target') + def test_cannot_get_image_build_target(self, get_base_image_build_target): + get_base_image_build_target.return_value = None + + result = self.handler._request_boot_iso_compose(self.image) + self.assertIsNone(result) + + @patch('freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' + '_get_base_image_build_target') + @patch('freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' + '_get_base_image_build_tag') + def test_cannot_get_build_tag_from_target( + self, get_base_image_build_tag, get_base_image_build_target): + get_base_image_build_target.return_value = 'build-target' + get_base_image_build_tag.return_value = None + + result = self.handler._request_boot_iso_compose(self.image) + self.assertIsNone(result) diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index 4b8e067..82fa0e3 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -26,13 +26,13 @@ import json from mock import patch, PropertyMock, Mock, call -from freshmaker.handlers.errata import ErrataAdvisoryRPMsSignedHandler -from freshmaker.handlers.errata import ErrataAdvisoryStateChangedHandler +from freshmaker import conf, db, events +from freshmaker.errata import ErrataAdvisory from freshmaker.events import ErrataAdvisoryRPMsSignedEvent from freshmaker.events import ErrataAdvisoryStateChangedEvent -from freshmaker.errata import ErrataAdvisory - -from freshmaker import conf, db, events +from freshmaker.handlers.errata import ErrataAdvisoryRPMsSignedHandler +from freshmaker.handlers.errata import ErrataAdvisoryStateChangedHandler +from freshmaker.lightblue import ContainerImage from freshmaker.models import Event, ArtifactBuild, EVENT_TYPES from freshmaker.types import ArtifactBuildState, ArtifactType, EventState @@ -271,10 +271,23 @@ class TestBatches(unittest.TestCase): def _mock_build(self, build, parent=None, error=None): if parent: parent = {"brew": {"build": parent + "-1-1.25"}} - return {'brew': {'build': build + "-1-1.25"}, - 'repository': build + '_repo', 'commit': build + '_123', - 'parent': parent, "target": "t1", 'git_branch': 'mybranch', - "error": error, "content_sets": ["first-content-set"]} + return ContainerImage({ + 'brew': {'build': build + "-1-1.25"}, + 'repository': build + '_repo', + 'parsed_data': { + 'layers': [ + 'sha512:1234', + 'sha512:4567', + 'sha512:7890', + ], + }, + 'commit': build + '_123', + 'parent': parent, + "target": "t1", + 'git_branch': 'mybranch', + "error": error, + "content_sets": ["first-content-set"] + }) @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS.new_compose') @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS.get_compose') @@ -901,7 +914,15 @@ class TestRecordBatchesImages(unittest.TestCase): side_effect=[{'id': 1}, {'id': 2}]) self.mock_prepare_pulp_repo = self.prepare_pulp_repo_patcher.start() + self.request_boot_iso_compose_patcher = patch( + 'freshmaker.handlers.errata.' + 'ErrataAdvisoryRPMsSignedHandler._request_boot_iso_compose', + side_effect=[{'id': 100}, {'id': 200}]) + self.mock_request_boot_iso_compose = \ + self.request_boot_iso_compose_patcher.start() + def tearDown(self): + self.request_boot_iso_compose_patcher.stop() self.prepare_pulp_repo_patcher.stop() self.event_types_patcher.stop() @@ -911,12 +932,18 @@ class TestRecordBatchesImages(unittest.TestCase): def test_record_batches(self): batches = [ - [{ + [ContainerImage({ "brew": { "completion_date": "20170420T17:05:37.000-0400", "build": "rhel-server-docker-7.3-82", "package": "rhel-server-docker" }, + 'parsed_data': { + 'layers': [ + 'sha512:12345678980', + 'sha512:10987654321' + ] + }, "parent": None, "content_sets": ["content-set-1"], "repository": "repo-1", @@ -924,19 +951,32 @@ class TestRecordBatchesImages(unittest.TestCase): "target": "target-candidate", "git_branch": "rhel-7", "error": None - }], - [{ + })], + [ContainerImage({ "brew": { "build": "rh-dotnetcore10-docker-1.0-16", "package": "rh-dotnetcore10-docker", "completion_date": "20170511T10:06:09.000-0400" }, - "parent": { + 'parsed_data': { + 'layers': [ + 'sha512:2345af2e293', + 'sha512:12345678980', + 'sha512:10987654321' + ] + }, + "parent": ContainerImage({ "brew": { "completion_date": "20170420T17:05:37.000-0400", "build": "rhel-server-docker-7.3-82", "package": "rhel-server-docker" }, + 'parsed_data': { + 'layers': [ + 'sha512:12345678980', + 'sha512:10987654321' + ] + }, "parent": None, "content_sets": ["content-set-1"], "repository": "repo-1", @@ -944,14 +984,14 @@ class TestRecordBatchesImages(unittest.TestCase): "target": "target-candidate", "git_branch": "rhel-7", "error": None - }, + }), "content_sets": ["content-set-1"], "repository": "repo-1", "commit": "987654321", "target": "target-candidate", "git_branch": "rhel-7", "error": None - }] + })] ] handler = ErrataAdvisoryRPMsSignedHandler() @@ -975,12 +1015,18 @@ class TestRecordBatchesImages(unittest.TestCase): def test_pulp_compose_is_stored_for_each_build(self): batches = [ - [{ + [ContainerImage({ "brew": { "completion_date": "20170420T17:05:37.000-0400", "build": "rhel-server-docker-7.3-82", "package": "rhel-server-docker" }, + 'parsed_data': { + 'layers': [ + 'sha512:12345678980', + 'sha512:10987654321' + ] + }, "parent": None, "content_sets": ["content-set-1"], "repository": "repo-1", @@ -988,19 +1034,32 @@ class TestRecordBatchesImages(unittest.TestCase): "target": "target-candidate", "git_branch": "rhel-7", "error": None - }], - [{ + })], + [ContainerImage({ "brew": { "build": "rh-dotnetcore10-docker-1.0-16", "package": "rh-dotnetcore10-docker", "completion_date": "20170511T10:06:09.000-0400" }, - "parent": { + 'parsed_data': { + 'layers': [ + 'sha512:2345af2e293', + 'sha512:12345678980', + 'sha512:10987654321' + ] + }, + "parent": ContainerImage({ "brew": { "completion_date": "20170420T17:05:37.000-0400", "build": "rhel-server-docker-7.3-82", "package": "rhel-server-docker" }, + 'parsed_data': { + 'layers': [ + 'sha512:12345678980', + 'sha512:10987654321' + ] + }, "parent": None, "content_sets": ["content-set-1"], "repository": "repo-1", @@ -1008,14 +1067,14 @@ class TestRecordBatchesImages(unittest.TestCase): "target": "target-candidate", "git_branch": "rhel-7", "error": None - }, + }), "content_sets": ["content-set-1"], "repository": "repo-1", "commit": "987654321", "target": "target-candidate", "git_branch": "rhel-7", "error": None - }] + })] ] handler = ErrataAdvisoryRPMsSignedHandler() @@ -1025,28 +1084,40 @@ class TestRecordBatchesImages(unittest.TestCase): parent_build = query.filter( ArtifactBuild.original_nvr == 'rhel-server-docker-7.3-82' ).first() - self.assertEqual(1, len(parent_build.composes)) - self.assertEqual(1, parent_build.composes[0].compose.id) + self.assertEqual(2, len(parent_build.composes)) + compose_ids = sorted([rel.compose.odcs_compose_id + for rel in parent_build.composes]) + # Ensure both pulp compose id and boot.iso compose id are stored + self.assertEqual([1, 100], compose_ids) child_build = query.filter( ArtifactBuild.original_nvr == 'rh-dotnetcore10-docker-1.0-16' ).first() self.assertEqual(1, len(child_build.composes)) - self.assertEqual(2, child_build.composes[0].compose.id) + self.assertEqual(2, child_build.composes[0].compose.odcs_compose_id) self.mock_prepare_pulp_repo.assert_has_calls([ call(child_build.event, ["content-set-1"]), call(child_build.event, ["content-set-1"]) ]) + self.mock_request_boot_iso_compose.assert_called_once_with( + batches[0][0]) + def test_mark_failed_state_if_image_has_error(self): batches = [ - [{ + [ContainerImage({ "brew": { "completion_date": "20170420T17:05:37.000-0400", "build": "rhel-server-docker-7.3-82", "package": "rhel-server-docker" }, + 'parsed_data': { + 'layers': [ + 'sha512:12345678980', + 'sha512:10987654321' + ] + }, "parent": None, "content_sets": ["content-set-1"], "repository": "repo-1", @@ -1054,7 +1125,7 @@ class TestRecordBatchesImages(unittest.TestCase): "target": "target-candidate", "git_branch": "rhel-7", "error": "Some error occurs while getting this image." - }] + })] ] handler = ErrataAdvisoryRPMsSignedHandler() @@ -1069,12 +1140,18 @@ class TestRecordBatchesImages(unittest.TestCase): def test_mark_state_failed_if_depended_image_is_failed(self): batches = [ - [{ + [ContainerImage({ "brew": { "completion_date": "20170420T17:05:37.000-0400", "build": "rhel-server-docker-7.3-82", "package": "rhel-server-docker" }, + 'parsed_data': { + 'layers': [ + 'sha512:12345678980', + 'sha512:10987654321' + ] + }, "parent": None, "content_sets": ["content-set-1"], "repository": "repo-1", @@ -1082,19 +1159,32 @@ class TestRecordBatchesImages(unittest.TestCase): "target": "target-candidate", "git_branch": "rhel-7", "error": "Some error occured." - }], - [{ + })], + [ContainerImage({ "brew": { "build": "rh-dotnetcore10-docker-1.0-16", "package": "rh-dotnetcore10-docker", "completion_date": "20170511T10:06:09.000-0400" }, - "parent": { + 'parsed_data': { + 'layers': [ + 'sha512:378a8ef2730', + 'sha512:12345678980', + 'sha512:10987654321' + ] + }, + "parent": ContainerImage({ "brew": { "completion_date": "20170420T17:05:37.000-0400", "build": "rhel-server-docker-7.3-82", "package": "rhel-server-docker" }, + 'parsed_data': { + 'layers': [ + 'sha512:12345678980', + 'sha512:10987654321' + ] + }, "parent": None, "content_sets": ["content-set-1"], "repository": "repo-1", @@ -1102,14 +1192,14 @@ class TestRecordBatchesImages(unittest.TestCase): "target": "target-candidate", "git_branch": "rhel-7", "error": None - }, + }), "content_sets": ["content-set-1"], "repository": "repo-1", "commit": "987654321", "target": "target-candidate", "git_branch": "rhel-7", "error": "Some error occured too." - }] + })] ] handler = ErrataAdvisoryRPMsSignedHandler() @@ -1126,6 +1216,37 @@ class TestRecordBatchesImages(unittest.TestCase): ).first() self.assertEqual(ArtifactBuildState.FAILED.value, build.state) + def test_mark_base_image_failed_if_fail_to_request_boot_iso_compose(self): + batches = [ + [ContainerImage({ + "brew": { + "completion_date": "20170420T17:05:37.000-0400", + "build": "rhel-server-docker-7.3-82", + "package": "rhel-server-docker" + }, + 'parsed_data': { + 'layers': [ + 'sha512:12345678980', + 'sha512:10987654321' + ] + }, + "parent": None, + "content_sets": ["content-set-1"], + "repository": "repo-1", + "commit": "123456789", + "target": "target-candidate", + "git_branch": "rhel-7", + "error": "Some error occured." + })], + ] + + handler = ErrataAdvisoryRPMsSignedHandler() + handler._record_batches(batches, self.mock_event) + + build = db.session.query(ArtifactBuild).filter_by( + original_nvr='rhel-server-docker-7.3-82').first() + self.assertEqual(ArtifactBuildState.FAILED.value, build.state) + class TestPrepareYumReposForRebuilds(unittest.TestCase): """Test ErrataAdvisoryRPMsSignedHandler._prepare_yum_repos_for_rebuilds""" From 61a3cb832d5e1436c9a479dae4cf5ef2f1479d47 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jan 03 2018 05:51:00 +0000 Subject: [PATCH 6/7] Add unique index on Compose.odcs_compose_id Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/migrations/versions/e06434b3ef5e_add_unique_constraint_to_compose_odcs_.py b/freshmaker/migrations/versions/e06434b3ef5e_add_unique_constraint_to_compose_odcs_.py new file mode 100644 index 0000000..73e7bc6 --- /dev/null +++ b/freshmaker/migrations/versions/e06434b3ef5e_add_unique_constraint_to_compose_odcs_.py @@ -0,0 +1,26 @@ +"""Add unique constraint to Compose.odcs_compose_id + +Revision ID: e06434b3ef5e +Revises: b17231ee8220 +Create Date: 2017-12-27 14:53:42.321947 + +""" + +# revision identifiers, used by Alembic. +revision = 'e06434b3ef5e' +down_revision = 'b17231ee8220' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_index('idx_odcs_compose_id', 'composes', ['odcs_compose_id'], unique=True) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_index('idx_odcs_compose_id', table_name='composes') + # ### end Alembic commands ### diff --git a/freshmaker/models.py b/freshmaker/models.py index e775793..1bf7ac9 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -28,6 +28,7 @@ import json from datetime import datetime from sqlalchemy.orm import (validates, relationship) +from sqlalchemy.schema import Index from sqlalchemy.sql.expression import false from flask_login import UserMixin @@ -542,6 +543,9 @@ class Compose(FreshmakerBase): self.odcs_compose_id)['state_name'] +Index('idx_odcs_compose_id', Compose.odcs_compose_id, unique=True) + + class ArtifactBuildCompose(FreshmakerBase): __tablename__ = 'artifact_build_composes' diff --git a/tests/test_errata_advisory_rpms_signed_handler.py b/tests/test_errata_advisory_rpms_signed_handler.py index 31e3ce1..b5583ea 100644 --- a/tests/test_errata_advisory_rpms_signed_handler.py +++ b/tests/test_errata_advisory_rpms_signed_handler.py @@ -62,10 +62,13 @@ class TestErrataAdvisoryRPMsSignedHandler(unittest.TestCase): '_find_images_to_rebuild') self.mock_find_images_to_rebuild = self.find_images_patcher.start() + # boot.iso composes IDs should be different from pulp composes IDs as + # when each time to request a compose from ODCS, new compose ID will + # be returned along with new comopse. self.request_boot_iso_compose_patcher = patch( 'freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler.' '_request_boot_iso_compose', - side_effect=[{'id': 1}, {'id': 2}]) + side_effect=[{'id': 100}, {'id': 101}]) self.mock_request_boot_iso_compose = \ self.request_boot_iso_compose_patcher.start() diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index 82fa0e3..23059d6 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -296,11 +296,15 @@ class TestBatches(unittest.TestCase): """ Tests that batches are properly recorded in DB. """ - - compose = {'id': 2, 'result_repofile': 'http://localhost/2.repo', - 'state_name': 'done'} - new_compose.return_value = compose - get_compose.return_value = compose + # There are 8 mock builds below and each of them requires one pulp + # compose. + composes = [{ + 'id': compose_id, + 'result_repofile': 'http://localhost/{}.repo'.format(compose_id), + 'state_name': 'done' + } for compose_id in range(1, 9)] + new_compose.side_effect = composes + get_compose.side_effect = composes # Creates following tree: # shared_parent From d448735b51bcb3900dac0e3dda88267bb7535871 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jan 03 2018 07:44:47 +0000 Subject: [PATCH 7/7] Disable to request boot.iso compose When merging code to request boot.iso compose, we realized that boot.iso compose support is not deployed in ODCS server. So, this commit is for disabling the request at this moment for a while. Revert when the support is availabe in ODCS server. Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index 9e75519..1b0f81f 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -543,22 +543,24 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): db.session.commit() build.add_composes(db.session, [db_compose]) - if image.is_base_image: - compose = self._request_boot_iso_compose(image) - if compose is None: - log.error( - 'Failed to request boot.iso compose for base ' - 'image %s.', nvr) - build.transition( - ArtifactBuildState.FAILED.value, - 'Cannot rebuild this base image because failed to ' - 'requeset boot.iso compose.') - # FIXME: mark all builds associated with build.event FAILED? - else: - db_compose = Compose(odcs_compose_id=compose['id']) - db.session.add(db_compose) - db.session.commit() - build.add_composes(db.session, [db_compose]) + # TODO: uncomment following code after boot.iso compose is + # deployed in ODCS server. +# if image.is_base_image: +# compose = self._request_boot_iso_compose(image) +# if compose is None: +# log.error( +# 'Failed to request boot.iso compose for base ' +# 'image %s.', nvr) +# build.transition( +# ArtifactBuildState.FAILED.value, +# 'Cannot rebuild this base image because failed to ' +# 'requeset boot.iso compose.') +# # FIXME: mark all builds associated with build.event FAILED? +# else: +# db_compose = Compose(odcs_compose_id=compose['id']) +# db.session.add(db_compose) +# db.session.commit() +# build.add_composes(db.session, [db_compose]) builds[nvr] = build diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index 23059d6..d32ea61 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -1017,6 +1017,7 @@ class TestRecordBatchesImages(unittest.TestCase): self.assertEqual(parent_image, child_image.dep_on) self.assertEqual(ArtifactBuildState.PLANNED.value, child_image.state) + @unittest.skip('Enable again when enable to request boot.iso compose') def test_pulp_compose_is_stored_for_each_build(self): batches = [ [ContainerImage({