From a07b44411c9a2df78dc247f728410908d82948de Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Tue, 6 Jun 2017 14:49:49 +0200 Subject: [PATCH] Set the state_reason when format_mmd raises in frontend and do not overwrite the state_reason in backend. --- .../scheduler/handlers/modules.py | 21 ++++----- module_build_service/utils.py | 5 ++- tests/test_views/test_views.py | 44 +++++++++++++++++-- 3 files changed, 52 insertions(+), 18 deletions(-) diff --git a/module_build_service/scheduler/handlers/modules.py b/module_build_service/scheduler/handlers/modules.py index a48df6b9..d277fc5f 100644 --- a/module_build_service/scheduler/handlers/modules.py +++ b/module_build_service/scheduler/handlers/modules.py @@ -79,14 +79,9 @@ def failed(config, session, msg): and c.state != koji.BUILD_STATES["FAILED"]) ] - groups = module_build_service.builder.GenericBuilder.default_buildroot_groups( - session, build) - if build.koji_tag: - builder = module_build_service.builder.GenericBuilder.create( - build.owner, build, config.system, config, tag_name=build.koji_tag, - components=[c.package for c in build.component_builds]) - builder.buildroot_connect(groups) + builder = module_build_service.builder.GenericBuilder.create_from_module( + session, build, config) for component in unbuilt_components: if component.task_id: @@ -95,11 +90,13 @@ def failed(config, session, msg): component.state_reason = build.state_reason session.add(component) else: - reason = "Missing koji tag. Assuming previously failed module lookup in PDC." - log.error(reason) - build.transition(config, state="failed", state_reason=reason) - session.commit() - return + # Do not overwrite state_reason set by Frontend if any. + if not build.state_reason: + reason = "Missing koji tag. Assuming previously failed module lookup in PDC." + log.error(reason) + build.transition(config, state="failed", state_reason=reason) + session.commit() + return build.transition(config, state="failed") session.commit() diff --git a/module_build_service/utils.py b/module_build_service/utils.py index d956d8cd..d574d318 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -612,8 +612,9 @@ def record_component_builds(mmd, module, initial_batch=1, # are branches with commit hashes try: format_mmd(mmd, module.scmurl) - except Exception: - module.transition(conf, models.BUILD_STATES["failed"]) + except Exception as e: + module.transition(conf, models.BUILD_STATES["failed"], + "Failed to validate modulemd file: %s" % str(e)) db.session.add(module) db.session.commit() raise diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 7c664657..0450be7a 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -28,7 +28,7 @@ import vcr import modulemd as _modulemd import module_build_service.scm -from mock import patch, Mock, PropertyMock +from mock import patch, Mock, PropertyMock, MagicMock from shutil import copyfile from os import path, mkdir from os.path import dirname @@ -37,7 +37,8 @@ import hashlib from tests import app, init_data from module_build_service.errors import UnprocessableEntity from module_build_service.models import ComponentBuild, ModuleBuild -from module_build_service import conf +from module_build_service import conf, db +import module_build_service.scheduler.handlers.modules user = ('Homer J. Simpson', set(['packager'])) @@ -48,7 +49,8 @@ cassette_dir = base_dir + '/vcr-request-data/' class MockedSCM(object): - def __init__(self, mocked_scm, name, mmd_filenames, commit=None, checkout_raise=False): + def __init__(self, mocked_scm, name, mmd_filenames, commit=None, checkout_raise=False, + get_latest_raise=False): """ Adds default testing checkout, get_latest and name methods to mocked_scm SCM class. @@ -76,7 +78,11 @@ class MockedSCM(object): self.mocked_scm.return_value.name = self.name self.mocked_scm.return_value.commit = self.commit - self.mocked_scm.return_value.get_latest = self.get_latest + if get_latest_raise: + self.mocked_scm.return_value.get_latest.side_effect = \ + RuntimeError("Failed to get_latest commit") + else: + self.mocked_scm.return_value.get_latest = self.get_latest self.mocked_scm.return_value.repository_root = "git://pkgs.stg.fedoraproject.org/modules/" self.mocked_scm.return_value.branch = 'master' @@ -796,6 +802,36 @@ class TestViews(unittest.TestCase): self.assertEquals(data['status'], 422) self.assertEquals(data['error'], 'Unprocessable Entity') + @patch('module_build_service.auth.get_user', return_value=user) + @patch('module_build_service.scm.SCM') + @patch('module_build_service.models.ModuleBuild.from_module_event') + def test_submit_build_get_latest_raises( + self, from_module_event, mocked_scm, mocked_get_user): + MockedSCM(mocked_scm, 'testmodule', 'testmodule.yaml', + '7035bd33614972ac66559ac1fdd019ff6027ad22', get_latest_raise=True) + + with app.app_context(): + rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + 'testmodule.git?#7035bd33614972ac66559ac1fdd019ff6027ad22'})) + data = json.loads(rv.data) + self.assertEquals(data['status'], 422) + self.assertEquals(data['error'], 'Unprocessable Entity') + + # Get the last module and check it has the state_reason set. + build = ModuleBuild.query.order_by(ModuleBuild.id).all()[-1] + db.session.add(build) + from_module_event.return_value = build + self.assertIn("Failed to validate modulemd file", build.state_reason) + + # Check that after the failed message is handled, the state_reason + # remains the same. + module_build_service.scheduler.handlers.modules.failed( + conf, db.session, MagicMock()) + db.session.expunge(build) + build = ModuleBuild.query.order_by(ModuleBuild.id).all()[-1] + self.assertIn("Failed to validate modulemd file", build.state_reason) + @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') @patch("module_build_service.config.Config.allow_custom_scmurls", new_callable=PropertyMock)