From 50e9e653000617a075947ea9f8373e2326fa7596 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Oct 11 2017 11:27:14 +0000 Subject: Fix the problem causing endless loop in _log_images_to_rebuild caused by not using new build.original_nvr instead of build.name. Improve the method to detect endless loop and exit with an error in this cse. --- diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index 7999471..4c8449c 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -124,7 +124,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): repo_urls = list(set(repo_urls)) # Log what we are going to rebuild - self._log_images_to_rebuild(builds) + self._check_images_to_rebuild(db_event, builds) log.info("Following repositories will be used for the rebuild:") for url in repo_urls: log.info(" - %s", url) @@ -303,17 +303,20 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): if latest_build and latest_build[0]['nvr'] == nvr: return tag - def _log_images_to_rebuild(self, builds): + def _check_images_to_rebuild(self, db_event, builds): """ - Logs the information about images to rebuilt using log.info(...). + Checks the images to rebuild and logs them using log.info(...). + :param Event db_event: Database Event associated with images. :param builds dict: list of docker images to build as returned by _find_and_record_images_to_rebuild(...). """ log.info('Found docker images to rebuild in following order:') batch = 0 printed = [] - while len(printed) != len(builds.values()): + while (len(printed) != len(builds.values()) or + len(printed) != len(db_event.builds)): log.info(' Batch %d:', batch) + old_printed_count = len(printed) for build in builds.values(): # Print build only if: # a) It depends on other build, but this dependency has not @@ -322,15 +325,30 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): # batch 0 - this handles the base images # In call cases, print only builds which have not been printed # so far. - if (build.name not in printed and - ((build.dep_on and build.dep_on.name in printed) or + if (build.original_nvr not in printed and + ((build.dep_on and build.dep_on.original_nvr in printed) or (not build.dep_on and batch == 0))): args = json.loads(build.build_args) based_on = "based on %s" % args["parent"] \ if args["parent"] else "base image" log.info(' - %s#%s (%s)' % (args["repository"], args["commit"], based_on)) - printed.append(build.name) + printed.append(build.original_nvr) + + # Nothing has been printed, that means the dependencies between + # images are not OK and we would loop forever. Instead of that, + # print error and stop the rebuild. + if old_printed_count == len(printed): + db_event.builds_transition( + ArtifactBuildState.FAILED.value, + "No image to be built in batch %d." % (batch)) + log.error("Dumping the builds:") + for build in builds.values(): + log.error(" %r", build.original_nvr) + log.error("Printed ones:") + for p in printed: + log.error(" %r", p) + break batch += 1 diff --git a/setup.py b/setup.py index 4cd89e3..1a1cf6b 100644 --- a/setup.py +++ b/setup.py @@ -8,7 +8,7 @@ with open('test-requirements.txt') as f: setup(name='freshmaker', description='Continuous Compose Service', - version='0.0.4', + version='0.0.5', classifiers=[ "Programming Language :: Python", "Topic :: Software Development :: Build Tools" diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index 8e3e82a..2450ffd 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -336,6 +336,90 @@ class TestBatches(unittest.TestCase): build.dep_on.rebuilt_nvr if build.dep_on else None) +class TestCheckImagesToRebuild(unittest.TestCase): + """Test handling of batches""" + + 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.ev = Event.create(db.session, 'msg-id', '123', 100) + self.b1 = ArtifactBuild.create( + db.session, self.ev, "parent", "image", + state=ArtifactBuildState.PLANNED.value, + original_nvr="parent-1-25") + self.b1.build_args = build_args + self.b2 = ArtifactBuild.create( + db.session, self.ev, "child", "image", + state=ArtifactBuildState.PLANNED.value, + dep_on=self.b1, + original_nvr="child-1-25") + self.b2.build_args = build_args + db.session.commit() + + def tearDown(self): + db.session.remove() + db.drop_all() + db.session.commit() + + def test_check_images_to_rebuild(self): + builds = { + "parent-1-25": self.b1, + "child-1-25": self.b2 + } + + handler = ErrataAdvisoryRPMsSignedHandler() + handler._check_images_to_rebuild(self.ev, builds) + + # Check that the images have proper data in proper db columns. + e = db.session.query(Event).filter(Event.id == 1).one() + for build in e.builds: + self.assertEqual(build.state, ArtifactBuildState.PLANNED.value) + + def test_check_images_to_rebuild_missing_dep(self): + # Do not include child nvr here to test that _check_images_to_rebuild + # sets the state of event to failed. + builds = { + "parent-1-25": self.b1 + } + + handler = ErrataAdvisoryRPMsSignedHandler() + handler._check_images_to_rebuild(self.ev, builds) + + # Check that the images have proper data in proper db columns. + e = db.session.query(Event).filter(Event.id == 1).one() + for build in e.builds: + self.assertEqual(build.state, ArtifactBuildState.FAILED.value) + + def test_check_images_to_rebuild_extra_build(self): + builds = { + "parent-1-25": self.b1, + "child-1-25": self.b2, + "something-1-25": self.b1, + } + + handler = ErrataAdvisoryRPMsSignedHandler() + handler._check_images_to_rebuild(self.ev, builds) + + # Check that the images have proper data in proper db columns. + e = db.session.query(Event).filter(Event.id == 1).one() + for build in e.builds: + self.assertEqual(build.state, ArtifactBuildState.FAILED.value) + + class TestGetPackagesForCompose(unittest.TestCase): """Test ErrataAdvisoryRPMsSignedHandler._get_packages_for_compose"""