diff --git a/module_build_service/builder/KojiModuleBuilder.py b/module_build_service/builder/KojiModuleBuilder.py index 81f6ae25..17ac614b 100644 --- a/module_build_service/builder/KojiModuleBuilder.py +++ b/module_build_service/builder/KojiModuleBuilder.py @@ -368,22 +368,32 @@ chmod 644 %buildroot/%_sysconfdir/rpm/macros.zz-modules self.koji_session.groupPackageListAdd(build_tag, group, name) self.koji_session.multiCall(strict=True) - def tag_artifacts(self, artifacts): - dest_tag = self._get_tag(self.module_tag)['id'] - - tagged_nvrs = self._get_tagged_nvrs(self.module_tag['name']) + def tag_artifacts(self, artifacts, dest_tag=True): + """ Tag the provided artifacts to the module tag + :param artifacts: a list of NVRs to tag + :kwarg dest_tag: a boolean determining if the destination or build tag should be used + :return: None + """ + if dest_tag: + tag = self._get_tag(self.module_tag)['id'] + tagged_nvrs = self._get_tagged_nvrs(self.module_tag['name']) + else: + tag = self._get_tag(self.module_build_tag)['id'] + tagged_nvrs = self._get_tagged_nvrs(self.module_build_tag['name']) self.koji_session.multicall = True for nvr in artifacts: if nvr in tagged_nvrs: continue - log.info("%r tagging %r into %r" % (self, nvr, dest_tag)) - self.koji_session.tagBuild(dest_tag, nvr) + log.info("%r tagging %r into %r" % (self, nvr, tag)) + self.koji_session.tagBuild(tag, nvr) self.koji_session.multiCall(strict=True) def untag_artifacts(self, artifacts): - """ + """ Untag the provided artifacts from the module destination and build tag + :param artifacts: a list of NVRs to untag + :return: None """ dest_tag = self._get_tag(self.module_tag)['id'] build_tag = self._get_tag(self.module_build_tag)['id'] @@ -421,63 +431,80 @@ chmod 644 %buildroot/%_sysconfdir/rpm/macros.zz-modules return get_result() - def _get_build_by_artifact(self, artifact_name): + def recover_orphaned_artifact(self, component_build): """ - :param artifact_name: e.g. bash - - Searches for a complete build of artifact belonging to this module. - The returned build can be even untagged. - - Returns koji_session.getBuild response or None. - + Searches for a complete build of an artifact belonging to the module and sets the + component_build in the MBS database to the found build. This usually returns nothing since + these builds should *not* exist. + :param artifact_name: a ComponentBuild object + :return: a list of msgs that MBS needs to process """ - # yaml file can hold only one reference to a package name, so - # I expect that we can have only one build of package within single module - # Rules for searching: - # * latest: True so I can return only single task_id. - # * we do want only build explicitly tagged in the module tag (inherit: False) + opts = {'latest': True, 'package': component_build.package, 'inherit': False} + build_tagged = self.koji_session.listTagged(self.module_build_tag['name'], **opts) + dest_tagged = None + # Only check the destination tag if the component is not a build_time_only component + if not component_build.build_time_only: + dest_tagged = self.koji_session.listTagged(self.module_tag['name'], **opts) + for rv in [build_tagged, dest_tagged]: + if rv and len(rv) != 1: + raise ValueError("Expected exactly one item in list. Got %s" % rv) - opts = {'latest': True, 'package': artifact_name, 'inherit': False} + build = None + if build_tagged: + build = build_tagged[0] + elif dest_tagged: + build = dest_tagged[0] - if artifact_name == "module-build-macros": - tag = self.module_build_tag['name'] + if not build: + # If the build cannot be found in the tags, it may be untagged as a result + # of some earlier inconsistent situation. Let's find the task_info + # based on the list of untagged builds + release = module_build_service.utils.get_rpm_release_from_mmd(self.mmd) + untagged = self.koji_session.untaggedBuilds(name=component_build.package) + for untagged_build in untagged: + if untagged_build["release"].endswith(release): + nvr = "{name}-{version}-{release}".format(**untagged_build) + build = self.koji_session.getBuild(nvr) + break + further_work = [] + # If the build doesn't exist, then return + if not build: + return further_work + + # Start setting up MBS' database to use the existing build + log.info('Skipping build of "{0}" since it already exists.'.format(build['nvr'])) + # Set it to COMPLETE so it doesn't count towards the concurrent component threshold + component_build.state = koji.BUILD_STATES['COMPLETE'] + component_build.nvr = build['nvr'] + component_build.task_id = build['task_id'] + component_build.state_reason = 'Found existing build' + nvr_dict = kobo.rpmlib.parse_nvr(component_build.nvr) + # Trigger a completed build message + further_work.append(module_build_service.messaging.KojiBuildChange( + 'recover_orphaned_artifact: fake message', build['build_id'], + build['task_id'], koji.BUILD_STATES['COMPLETE'], component_build.package, + nvr_dict['version'], nvr_dict['release'], component_build.module_build.id)) + + component_tagged_in = [] + if build_tagged: + component_tagged_in.append(self.module_build_tag['name']) else: - tag = self.module_tag['name'] - - tagged = self.koji_session.listTagged(tag, **opts) - - if tagged: - assert len(tagged) == 1, "Expected exactly one item in list. Got %s" % tagged - return tagged[0] - - # If the build cannot be found in tag, it may be untagged as a result - # of some earlier inconsistent situation. Let's find the task_info - # based on the list of untagged builds - release = module_build_service.utils.get_rpm_release_from_mmd(self.mmd) - opts = {'name': artifact_name} - untagged = self.koji_session.untaggedBuilds(**opts) - for build in untagged: - if build["release"].endswith(release): - # Tag it. - nvr = "{name}-{version}-{release}".format(**build) - self.tag_artifacts([nvr]) - - # Now, make the same query we made earlier to return a dict - # with the same schema. - tagged = self.koji_session.listTagged(tag, package=artifact_name) - if not tagged: - # Should be impossible. - raise ValueError("Just tagged %s but didn't find it" % nvr) - return tagged[0] - - return None + # Tag it in the build tag if it's not there + self.tag_artifacts([component_build.nvr], dest_tag=False) + if dest_tagged: + component_tagged_in.append(self.module_tag['name']) + for tag in component_tagged_in: + log.info('The build being skipped isn\'t tagged in the "{0}" tag. Will send a ' + 'message to the tag handler'.format(tag)) + further_work.append(module_build_service.messaging.KojiTagChange( + 'recover_orphaned_artifact: fake message', tag, component_build.package)) + return further_work def build(self, artifact_name, source): """ - :param source : scmurl to spec repository - : param artifact_name: name of artifact (which we couldn't get - from spec due involved macros) - :return 4-tuple of the form (koji build task id, state, reason, nvr) + :param artifact_name: a string of the name of the artifact + :param source: a string of the scmurl to the spec repository + :return: 4-tuple of the form (koji build task id, state, reason, nvr) """ # TODO: If we are sure that this method is thread-safe, we can just @@ -503,14 +530,6 @@ chmod 644 %buildroot/%_sysconfdir/rpm/macros.zz-modules if not self.__prep: raise RuntimeError("Buildroot is not prep-ed") - # Skip existing builds - task_info = self._get_build_by_artifact(artifact_name) - if task_info: - log.info("skipping build of %s. Build already exists (task_id=%s), via %s" % ( - source, task_info['task_id'], self)) - return (task_info['task_id'], koji.BUILD_STATES['COMPLETE'], - 'Build already exists.', task_info['nvr']) - self._koji_whitelist_packages([artifact_name]) if '://' not in source: # treat source as an srpm and upload it diff --git a/module_build_service/builder/base.py b/module_build_service/builder/base.py index ed04967f..053e0ce5 100644 --- a/module_build_service/builder/base.py +++ b/module_build_service/builder/base.py @@ -336,6 +336,17 @@ class GenericBuilder(six.with_metaclass(ABCMeta)): """ raise NotImplementedError() + @classmethod + def recover_orphaned_artifact(cls, component_build): + """ + Searches for a complete build of an artifact belonging to the module and sets the + component_build in the MBS database to the found build. This usually returns nothing since + these builds should *not* exist. + :param artifact_name: a ComponentBuild object + :return: a list of msgs that MBS needs to process + """ + return [] + @classmethod def get_average_build_time(self, component): """ diff --git a/module_build_service/scheduler/handlers/modules.py b/module_build_service/scheduler/handlers/modules.py index 9c448522..1f914d12 100644 --- a/module_build_service/scheduler/handlers/modules.py +++ b/module_build_service/scheduler/handlers/modules.py @@ -309,30 +309,37 @@ def wait(config, session, msg): session.commit() artifact_name = "module-build-macros" - task_id, state, reason, nvr = builder.build(artifact_name=artifact_name, source=srpm) component_build = models.ComponentBuild.from_component_name( session, artifact_name, build.id) - if component_build: - component_build.task_id = task_id - component_build.state = state - component_build.state_reason = reason - component_build.nvr = nvr - else: + further_work = [] + if not component_build: component_build = models.ComponentBuild( module_id=build.id, package=artifact_name, format="rpms", scmurl=srpm, - task_id=task_id, - state=state, - state_reason=reason, - nvr=nvr, batch=1, build_time_only=True ) session.add(component_build) + # Commit and refresh so that the SQLAlchemy relationships are available + session.commit() + session.refresh(component_build) + msgs = builder.recover_orphaned_artifact(component_build) + if msgs: + further_work += msgs + # There was no existing artifact found, so lets submit the build instead + else: + task_id, state, reason, nvr = builder.build(artifact_name=artifact_name, source=srpm) + component_build.task_id = task_id + component_build.state = state + component_build.reason = reason + component_build.nvr = nvr + elif component_build.state != koji.BUILD_STATES['COMPLETE']: + task_id, state, reason, nvr = builder.build(artifact_name=artifact_name, source=srpm) + session.add(component_build) build.transition(config, state="build") session.add(build) session.commit() @@ -345,5 +352,6 @@ def wait(config, session, msg): build.new_repo_task_id = task_id session.commit() else: - return [module_build_service.messaging.KojiRepoChange( - 'fake msg', builder.module_build_tag['name'])] + further_work.append(module_build_service.messaging.KojiRepoChange( + 'fake msg', builder.module_build_tag['name'])) + return further_work diff --git a/module_build_service/utils.py b/module_build_service/utils.py index e7264bce..76be3c15 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -169,7 +169,19 @@ def continue_batch_build(config, module, session, builder, components=None): unbuilt_components.sort(key=lambda c: builder.get_average_build_time(c), reverse=True) log.info('Done sorting the unbuilt components by their average build time') + # Check for builds that exist in the build system but MBS doesn't know about + for component in unbuilt_components: + # Only evaluate new components + if component.state is not None: + continue + msgs = builder.recover_orphaned_artifact(component) + further_work += msgs + for c in unbuilt_components: + # If a previous build of the component was found, then the state will be marked as + # COMPLETE so we should skip this + if c.state == koji.BUILD_STATES['COMPLETE']: + continue # Check the concurrent build threshold. if at_concurrent_component_threshold(config, session): log.info('Concurrent build threshold met') @@ -195,17 +207,6 @@ def continue_batch_build(config, module, session, builder, components=None): for future in futures: future.result() - # If all components in this batch are already done, it can mean that they - # have been built in the past and have been skipped in this module build. - # We therefore have to generate fake KojiRepoChange message, because the - # repo has been also done in the past and build system will not send us - # any message now. - if (all(c.state in [koji.BUILD_STATES['COMPLETE'], - koji.BUILD_STATES['FAILED']] or c.reused_component_id - for c in unbuilt_components) and builder.module_build_tag): - further_work += [module_build_service.messaging.KojiRepoChange( - 'start_build_batch: fake msg', builder.module_build_tag['name'])] - session.commit() return further_work @@ -219,7 +220,6 @@ def start_next_batch_build(config, module, session, builder, components=None): :return: a list of BaseMessage instances to be handled by the MBSConsumer. """ - import koji # Placed here to avoid py2/py3 conflicts... # Check the status of the module build and current batch so we can @@ -1157,7 +1157,8 @@ def attempt_to_reuse_all_components(builder, session, module): # Tag them builder.buildroot_add_artifacts(components_to_tag, install=False) - builder.tag_artifacts(components_to_tag) + builder.tag_artifacts(components_to_tag, dest_tag=False) + builder.tag_artifacts(components_to_tag, dest_tag=True) return True diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index 47f1c3d3..9b90f58a 100644 --- a/tests/test_build/test_build.py +++ b/tests/test_build/test_build.py @@ -28,6 +28,7 @@ from os import path, mkdir from os.path import dirname from shutil import copyfile from datetime import datetime, timedelta +from random import randint from nose.tools import timed @@ -38,6 +39,7 @@ from module_build_service.errors import Forbidden from module_build_service import db, models, conf, build_logs from mock import patch, PropertyMock, Mock +import kobo from tests import app, test_reuse_component_init_data, clean_database import json @@ -87,14 +89,6 @@ class FakeSCM(object): return path.join(self.sourcedir, self.name + ".yaml") -def _on_build_cb(cls, artifact_name, source): - # Tag the build in the -build tag - cls._send_tag(artifact_name) - if not artifact_name.startswith("module-build-macros"): - # Tag the build in the final tag - cls._send_tag(artifact_name, build=False) - - class FakeModuleBuilder(GenericBuilder): """ Fake module builder which succeeds for every build. @@ -105,10 +99,11 @@ class FakeModuleBuilder(GenericBuilder): _build_id = 1 BUILD_STATE = "COMPLETE" + # Simulates a situation when a component is already built in Koji INSTANT_COMPLETE = False DEFAULT_GROUPS = None - on_build_cb = _on_build_cb + on_build_cb = None on_cancel_cb = None on_buildroot_add_artifacts_cb = None on_tag_artifacts_cb = None @@ -118,13 +113,12 @@ class FakeModuleBuilder(GenericBuilder): self.module_str = module self.tag_name = tag_name self.config = config - self.on_build_cb = _on_build_cb @classmethod def reset(cls): FakeModuleBuilder.BUILD_STATE = "COMPLETE" FakeModuleBuilder.INSTANT_COMPLETE = False - FakeModuleBuilder.on_build_cb = _on_build_cb + FakeModuleBuilder.on_build_cb = None FakeModuleBuilder.on_cancel_cb = None FakeModuleBuilder.on_buildroot_add_artifacts_cb = None FakeModuleBuilder.on_tag_artifacts_cb = None @@ -165,21 +159,24 @@ class FakeModuleBuilder(GenericBuilder): def buildroot_add_repos(self, dependencies): pass - def tag_artifacts(self, artifacts): + def tag_artifacts(self, artifacts, dest_tag=True): if FakeModuleBuilder.on_tag_artifacts_cb: - FakeModuleBuilder.on_tag_artifacts_cb(self, artifacts) + FakeModuleBuilder.on_tag_artifacts_cb(self, artifacts, dest_tag=dest_tag) + for nvr in artifacts: # tag_artifacts received a list of NVRs, but the tag message expects the # component name artifact = models.ComponentBuild.query.filter_by(nvr=nvr).one().package - self._send_tag(artifact) - if not artifact.startswith('module-build-macros'): - self._send_tag(artifact, build=False) + self._send_tag(artifact, dest_tag=dest_tag) @property def koji_session(self): session = Mock() - session.newRepo.return_value = 123 + + def _newRepo(tag): + session.newRepo = self._send_repo_done() + return 123 + session.newRepo = _newRepo return session @property @@ -193,11 +190,11 @@ class FakeModuleBuilder(GenericBuilder): ) module_build_service.scheduler.consumer.work_queue_put(msg) - def _send_tag(self, artifact, build=True): - if build: - tag = self.tag_name + "-build" - else: + def _send_tag(self, artifact, dest_tag=True): + if dest_tag: tag = self.tag_name + else: + tag = self.tag_name + "-build" msg = module_build_service.messaging.KojiTagChange( msg_id='a faked internal message', tag=tag, @@ -221,7 +218,6 @@ class FakeModuleBuilder(GenericBuilder): def build(self, artifact_name, source): print("Starting building artifact %s: %s" % (artifact_name, source)) - FakeModuleBuilder._build_id += 1 if FakeModuleBuilder.on_build_cb: @@ -232,13 +228,12 @@ class FakeModuleBuilder(GenericBuilder): koji.BUILD_STATES[FakeModuleBuilder.BUILD_STATE], source, FakeModuleBuilder._build_id) - if FakeModuleBuilder.INSTANT_COMPLETE: - state = koji.BUILD_STATES['COMPLETE'] - else: - state = koji.BUILD_STATES['BUILDING'] + if FakeModuleBuilder.BUILD_STATE == 'COMPLETE': + # Tag the build in the -build tag + self._send_tag(artifact_name, dest_tag=False) reason = "Submitted %s to Koji" % (artifact_name) - return FakeModuleBuilder._build_id, state, reason, None + return FakeModuleBuilder._build_id, koji.BUILD_STATES['BUILDING'], reason, None @staticmethod def get_disttag_srpm(disttag, module_build): @@ -252,6 +247,28 @@ class FakeModuleBuilder(GenericBuilder): def list_tasks_for_components(self, component_builds=None, state='active'): pass + def recover_orphaned_artifact(self, component_build): + msgs = [] + if self.INSTANT_COMPLETE: + disttag = module_build_service.utils.get_rpm_release_from_mmd( + component_build.module_build.mmd()) + # We don't know the version or release, so just use a random one here + nvr = '{0}-1.0-1.{1}'.format(component_build.package, disttag) + component_build.nvr = nvr + component_build.task_id = self._build_id + 51234 + component_build.state = koji.BUILD_STATES['BUILDING'] + nvr_dict = kobo.rpmlib.parse_nvr(component_build.nvr) + # Send a message stating the build is complete + msgs.append(module_build_service.messaging.KojiBuildChange( + 'recover_orphaned_artifact: fake message', randint(1, 9999999), + component_build.task_id, koji.BUILD_STATES['COMPLETE'], component_build.package, + nvr_dict['version'], nvr_dict['release'], component_build.module_build.id)) + # Send a message stating that the build was tagged in the build tag + msgs.append(module_build_service.messaging.KojiTagChange( + 'recover_orphaned_artifact: fake message', + component_build.module_build.koji_tag + '-build', component_build.package)) + return msgs + def cleanup_moksha(): # Necessary to restart the twisted reactor for the next test. @@ -322,7 +339,7 @@ class TestBuild(unittest.TestCase): tag_groups.append(set([u'perl-Tangerine?#f24-1-1', u'perl-List-Compare?#f25-1-1'])) tag_groups.append(set([u'tangerine?#f23-1-1'])) - def on_tag_artifacts_cb(cls, artifacts): + def on_tag_artifacts_cb(cls, artifacts, dest_tag=True): self.assertEqual(tag_groups.pop(0), set(artifacts)) FakeModuleBuilder.on_tag_artifacts_cb = on_tag_artifacts_cb @@ -517,8 +534,6 @@ class TestBuild(unittest.TestCase): data = json.loads(rv.data) module_build_id = data['id'] - - FakeModuleBuilder.BUILD_STATE = "BUILDING" FakeModuleBuilder.INSTANT_COMPLETE = True msgs = [] @@ -629,7 +644,7 @@ class TestBuild(unittest.TestCase): num_builds = [k for k, g in itertools.groupby(TestBuild._global_var)] self.assertEqual(num_builds.count(1), 2) - @timed(30) + @timed(60) @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') @patch("module_build_service.config.Config.num_concurrent_builds", @@ -656,14 +671,12 @@ class TestBuild(unittest.TestCase): FakeModuleBuilder.BUILD_STATE = "FAILED" else: FakeModuleBuilder.BUILD_STATE = "COMPLETE" - # Tag the build in the -build tag - cls._send_tag(artifact_name) FakeModuleBuilder.on_build_cb = on_build_cb # Check that no components are tagged when single component fails # in batch. - def on_tag_artifacts_cb(cls, artifacts): + def on_tag_artifacts_cb(cls, artifacts, dest_tag=True): raise ValueError("No component should be tagged.") FakeModuleBuilder.on_tag_artifacts_cb = on_tag_artifacts_cb @@ -763,8 +776,9 @@ class TestBuild(unittest.TestCase): 'perl-List-Compare-0.53-5.module_testmodule_master_20170109091357', 'tangerine-0.22-3.module_testmodule_master_20170109091357'])) - def on_tag_artifacts_cb(cls, artifacts): - self.assertEqual(tag_groups.pop(0), set(artifacts)) + def on_tag_artifacts_cb(cls, artifacts, dest_tag=True): + if dest_tag is True: + self.assertEqual(tag_groups.pop(0), set(artifacts)) FakeModuleBuilder.on_tag_artifacts_cb = on_tag_artifacts_cb buildtag_groups = [] @@ -822,8 +836,9 @@ class TestBuild(unittest.TestCase): 'perl-List-Compare-0.53-5.module_testmodule_master_20170109091357', 'tangerine-0.22-3.module_testmodule_master_20170109091357'])) - def on_tag_artifacts_cb(cls, artifacts): - self.assertEqual(tag_groups.pop(0), set(artifacts)) + def on_tag_artifacts_cb(cls, artifacts, dest_tag=True): + if dest_tag is True: + self.assertEqual(tag_groups.pop(0), set(artifacts)) FakeModuleBuilder.on_tag_artifacts_cb = on_tag_artifacts_cb buildtag_groups = [] @@ -871,7 +886,7 @@ class TestBuild(unittest.TestCase): build_one.modulemd = f.read() build_one.koji_tag = 'module-testmodule-master-1' build_one.scmurl = 'git://pkgs.stg.fedoraproject.org/modules/testmodule.git?#7fea453' - build_one.batch = models.BUILD_STATES['failed'] + build_one.batch = 2 build_one.owner = 'Homer J. Simpson' build_one.time_submitted = submitted_time build_one.time_modified = now @@ -915,11 +930,24 @@ class TestBuild(unittest.TestCase): component_three.scmurl = 'git://pkgs.stg.fedoraproject.org/rpms/tangerine.git?#f24' component_three.batch = 3 component_three.module_id = 1 + # module-build-macros + component_four = models.ComponentBuild() + component_four.package = 'module-build-macros' + component_four.format = 'rpms' + component_four.state = koji.BUILD_STATES['COMPLETE'] + component_four.scmurl = ( + '/tmp/module_build_service-build-macrosqr4AWH/SRPMS/module-build-macros-0.1-1.' + 'module_testmodule_master_20170109091357.src.rpm') + component_four.batch = 1 + component_four.module_id = 1 + component_four.tagged = True + component_four.build_time_only = True db.session.add(build_one) db.session.add(component_one) db.session.add(component_two) db.session.add(component_three) + db.session.add(component_four) db.session.commit() db.session.expire_all() @@ -931,10 +959,6 @@ class TestBuild(unittest.TestCase): data = json.loads(rv.data) module_build_id = data['id'] - - FakeModuleBuilder.BUILD_STATE = 'BUILDING' - FakeModuleBuilder.INSTANT_COMPLETE = True - module_build = models.ModuleBuild.query.filter_by(id=module_build_id).one() components = models.ComponentBuild.query.filter_by( module_id=module_build_id, batch=2).order_by(models.ComponentBuild.id).all() @@ -991,9 +1015,6 @@ class TestBuild(unittest.TestCase): {'branch': 'master', 'scmurl': ('git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#7fea453')})) - FakeModuleBuilder.BUILD_STATE = 'BUILDING' - FakeModuleBuilder.INSTANT_COMPLETE = True - module_build = models.ModuleBuild.query.filter_by(id=module_build_id).one() components = models.ComponentBuild.query.filter_by( module_id=module_build_id, batch=2).order_by(models.ComponentBuild.id).all() diff --git a/tests/test_builder/test_koji.py b/tests/test_builder/test_koji.py index 50a01107..35aad747 100644 --- a/tests/test_builder/test_koji.py +++ b/tests/test_builder/test_koji.py @@ -72,30 +72,8 @@ class TestKojiBuilder(unittest.TestCase): self.assertEquals(repo, "https://kojipkgs.stg.fedoraproject.org/repos" "/module-base-runtime-0.25-9/latest/x86_64") - def test_get_build_by_artifact_when_tagged(self): - """ Test the _get_build_by_artifact happy path. """ - builder = FakeKojiModuleBuilder(owner=self.module.owner, - module=self.module, - config=conf, - tag_name='module-foo', - components=[]) - - builder.module_tag = {"name": "module-foo", "id": 1} - builder.module_build_tag = {"name": "module-foo-build", "id": 2} - - # Set listTagged to return test data - tagged = [{"nvr": "foo-1.0-1.module_e0095747"}] - builder.koji_session.listTagged.return_value = tagged - - actual = builder._get_build_by_artifact('foo') - expected = {'nvr': 'foo-1.0-1.module_e0095747'} - self.assertEquals(actual, expected) - self.assertEquals(builder.koji_session.tagBuild.call_count, 0) - - def test_get_build_by_artifact_when_untagged(self): - """ Test the _get_build_by_artifact un-happy path. - - If there's an untagged build that matches, tag it and return it. + def test_recover_orphaned_artifact_when_tagged(self): + """ Test recover_orphaned_artifact when the artifact is found and tagged in both tags """ builder = FakeKojiModuleBuilder(owner=self.module.owner, module=self.module, @@ -106,22 +84,50 @@ class TestKojiBuilder(unittest.TestCase): builder.module_tag = {"name": "module-foo", "id": 1} builder.module_build_tag = {"name": "module-foo-build", "id": 2} - tagged_build = { - "id": 9000, - "name": "foo", - "version": "1.0", - "release": "1.module_e0095747", - "nvr": "foo-1.0-1.module_e0095747", - } # Set listTagged to return test data - builder.koji_session.listTagged.side_effect = [ - # Return nothing the first time - [], - # Return nothing the second time - [], - # But something the third time. - [tagged_build], - ] + build_tagged = [{"nvr": "foo-1.0-1.module+e0095747", "task_id": 12345, 'build_id': 91}] + dest_tagged = [{"nvr": "foo-1.0-1.module+e0095747", "task_id": 12345, 'build_id': 91}] + builder.koji_session.listTagged.side_effect = [build_tagged, dest_tagged] + module_build = module_build_service.models.ModuleBuild.query.get(30) + component_build = module_build.component_builds[0] + component_build.task_id = None + component_build.state = None + component_build.nvr = None + + actual = builder.recover_orphaned_artifact(component_build) + self.assertEqual(len(actual), 3) + self.assertEquals(type(actual[0]), module_build_service.messaging.KojiBuildChange) + self.assertEquals(actual[0].build_id, 91) + self.assertEquals(actual[0].task_id, 12345) + self.assertEquals(actual[0].build_new_state, koji.BUILD_STATES['COMPLETE']) + self.assertEquals(actual[0].build_name, 'rubygem-rails') + self.assertEquals(actual[0].build_version, '1.0') + self.assertEquals(actual[0].build_release, '1.module+e0095747') + self.assertEquals(actual[0].module_build_id, 30) + self.assertEquals(type(actual[1]), module_build_service.messaging.KojiTagChange) + self.assertEquals(actual[1].tag, 'module-foo-build') + self.assertEquals(actual[1].artifact, 'rubygem-rails') + self.assertEquals(type(actual[2]), module_build_service.messaging.KojiTagChange) + self.assertEquals(actual[2].tag, 'module-foo') + self.assertEquals(actual[2].artifact, 'rubygem-rails') + self.assertEqual(component_build.state, koji.BUILD_STATES['COMPLETE']) + self.assertEqual(component_build.task_id, 12345) + self.assertEqual(component_build.state_reason, 'Found existing build') + self.assertEquals(builder.koji_session.tagBuild.call_count, 0) + + def test_recover_orphaned_artifact_when_untagged(self): + """ Tests recover_orphaned_artifact when the build is found but untagged + """ + builder = FakeKojiModuleBuilder(owner=self.module.owner, + module=self.module, + config=conf, + tag_name='module-foo', + components=[]) + + builder.module_tag = {"name": "module-foo", "id": 1} + builder.module_build_tag = {"name": "module-foo-build", "id": 2} + # Set listTagged to return test data + builder.koji_session.listTagged.side_effect = [[], [], []] untagged = [{ "id": 9000, "name": "foo", @@ -129,18 +135,35 @@ class TestKojiBuilder(unittest.TestCase): "release": "1.module+e0095747", }] builder.koji_session.untaggedBuilds.return_value = untagged + build_info = { + 'nvr': 'foo-1.0-1.module+e0095747', + 'task_id': 12345, + 'build_id': 91 + } + builder.koji_session.getBuild.return_value = build_info + module_build = module_build_service.models.ModuleBuild.query.get(30) + component_build = module_build.component_builds[0] + component_build.task_id = None + component_build.nvr = None + component_build.state = None - actual = builder._get_build_by_artifact('foo') - expected = tagged_build - self.assertEquals(actual, expected) - builder.koji_session.tagBuild.assert_called_once_with( - 1, 'foo-1.0-1.module+e0095747') + actual = builder.recover_orphaned_artifact(component_build) + self.assertEqual(len(actual), 1) + self.assertEquals(type(actual[0]), module_build_service.messaging.KojiBuildChange) + self.assertEquals(actual[0].build_id, 91) + self.assertEquals(actual[0].task_id, 12345) + self.assertEquals(actual[0].build_new_state, koji.BUILD_STATES['COMPLETE']) + self.assertEquals(actual[0].build_name, 'rubygem-rails') + self.assertEquals(actual[0].build_version, '1.0') + self.assertEquals(actual[0].build_release, '1.module+e0095747') + self.assertEquals(actual[0].module_build_id, 30) + self.assertEqual(component_build.state, koji.BUILD_STATES['COMPLETE']) + self.assertEqual(component_build.task_id, 12345) + self.assertEqual(component_build.state_reason, 'Found existing build') + builder.koji_session.tagBuild.assert_called_once_with(2, 'foo-1.0-1.module+e0095747') - def test_get_build_by_artifact_when_nothing_exists(self): - """ Test the _get_build_by_artifact super un-happy path. - - If there's an untagged build but it doesn't match our module... we'd - better not touch it or try to tag it. Confirm! + def test_recover_orphaned_artifact_when_nothing_exists(self): + """ Test recover_orphaned_artifact when the build is not found """ builder = FakeKojiModuleBuilder(owner=self.module.owner, module=self.module, @@ -154,16 +177,20 @@ class TestKojiBuilder(unittest.TestCase): # Set listTagged to return nothing... tagged = [] builder.koji_session.listTagged.return_value = tagged - # See how this is nope and not module_e0095747? untagged = [{ "nvr": "foo-1.0-1.nope", "release": "nope", }] builder.koji_session.untaggedBuilds.return_value = untagged + module_build = module_build_service.models.ModuleBuild.query.get(30) + component_build = module_build.component_builds[0] + component_build.task_id = None + component_build.nvr = None + component_build.state = None - actual = builder._get_build_by_artifact('foo') - expected = None - self.assertEquals(actual, expected) + actual = builder.recover_orphaned_artifact(component_build) + self.assertEqual(actual, []) + # Make sure nothing erroneous gets tag self.assertEquals(builder.koji_session.tagBuild.call_count, 0) @patch('koji.util') diff --git a/tests/test_scheduler/test_repo_done.py b/tests/test_scheduler/test_repo_done.py index 271e51f8..17e2c842 100644 --- a/tests/test_scheduler/test_repo_done.py +++ b/tests/test_scheduler/test_repo_done.py @@ -58,6 +58,8 @@ class TestRepoDone(unittest.TestCase): module_build_service.scheduler.handlers.repos.done( config=conf, session=db.session, msg=msg) + @mock.patch('module_build_service.builder.KojiModuleBuilder.' + 'KojiModuleBuilder.recover_orphaned_artifact', return_value=[]) @mock.patch('module_build_service.builder.KojiModuleBuilder.' 'KojiModuleBuilder.get_average_build_time', return_value=0.0) @@ -72,7 +74,8 @@ class TestRepoDone(unittest.TestCase): 'KojiModuleBuilder.build') @mock.patch('module_build_service.builder.KojiModuleBuilder.' 'KojiModuleBuilder.buildroot_connect') - def test_a_single_match(self, connect, build_fn, get_session, ready, list_tasks_fn, mock_gabt): + def test_a_single_match(self, connect, build_fn, get_session, ready, list_tasks_fn, mock_gabt, + mock_uea): """ Test that when a repo msg hits us and we have a single match. """ get_session.return_value = mock.Mock(), 'development' @@ -87,6 +90,8 @@ class TestRepoDone(unittest.TestCase): source=('git://pkgs.domain.local/rpms/communicator' '?#da95886c8a443b36a9ce31abda1f9bed22f2f9c2')) + @mock.patch('module_build_service.builder.KojiModuleBuilder.' + 'KojiModuleBuilder.recover_orphaned_artifact', return_value=[]) @mock.patch('module_build_service.builder.KojiModuleBuilder.' 'KojiModuleBuilder.get_average_build_time', return_value=0.0) @@ -102,7 +107,7 @@ class TestRepoDone(unittest.TestCase): @mock.patch('module_build_service.builder.KojiModuleBuilder.' 'KojiModuleBuilder.buildroot_connect') def test_a_single_match_build_fail(self, connect, build_fn, config, ready, list_tasks_fn, - mock_gabt): + mock_gabt, mock_uea): """ Test that when a KojiModuleBuilder.build fails, the build is marked as failed with proper state_reason. """ diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 47d61439..483804cf 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -745,6 +745,9 @@ class DummyModuleBuilder(GenericBuilder): def tag_artifacts(self, artifacts): pass + def recover_orphaned_artifact(self, component_build): + return [] + @property def module_build_tag(self): return {"name": self.tag_name + "-build"} @@ -842,6 +845,7 @@ class TestBatches(unittest.TestCase): plc_component.ref = '5ceea46add2366d8b8c5a623a2fb563b625b9abd' builder = mock.MagicMock() + builder.recover_orphaned_artifact.return_value = [] further_work = module_build_service.utils.start_next_batch_build( conf, module_build, db.session, builder) @@ -878,6 +882,7 @@ class TestBatches(unittest.TestCase): module_build.batch = 1 builder = mock.MagicMock() + builder.recover_orphaned_artifact.return_value = [] further_work = module_build_service.utils.start_next_batch_build( conf, module_build, db.session, builder) @@ -908,6 +913,7 @@ class TestBatches(unittest.TestCase): plc_component.ref = '5ceea46add2366d8b8c5a623a2fb563b625b9abd' builder = mock.MagicMock() + builder.recover_orphaned_artifact.return_value = [] further_work = module_build_service.utils.start_next_batch_build( conf, module_build, db.session, builder) @@ -966,6 +972,7 @@ class TestBatches(unittest.TestCase): plc_component.ref = '5ceea46add2366d8b8c5a623a2fb563b625b9abd' builder = mock.MagicMock() + builder.recover_orphaned_artifact.return_value = [] # The call order of get_average_build_time should be by the component's ID. Having this # side_effect tells continue_batch_build to build the second component in the build batch # first and the first component in the build batch second.