From 8a5f85876805e8ace82b6a1d901e1523ae218b93 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Jan 03 2018 05:51:00 +0000 Subject: 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)