From 26d707cb8e9507b69da80eb782fb827b956e7c73 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Mon, 28 Jan 2019 10:53:37 +0100 Subject: [PATCH] Allow resubmiting the very same module build in case it results in new MSE build. This is needed for Fedora branching, but it is generally useful. For example, there is a module buildrequiring "platform:[]". In the time this module has been built, only platform:f29 existed, so it has been built just against platform:f29. After a while, the platform:f30 is released and the maintainer needs to rebuild the module against platform:f30. Right now, he needs to create new commit in the module and submit the build, but this will result in useless rebuild of the module also against platform:f29. In this commit, MBS allows to resubmit the module build in a case there are new MSE builds to build. MBS will send all the module builds back to the user - so the existing builds will be already marked as "ready" and the newly submitted builds will have the "init" state in the REST API response. However, in case when there are no new MSE builds to build, MBS still sends back the Conflict error as it used to. This is done for backwards compatibility and also to not confuse the users in case when no new build has been submitted. --- module_build_service/utils/submit.py | 26 ++++++++++++++++++++------ tests/test_build/test_build.py | 4 ++-- tests/test_utils/test_utils.py | 27 ++++++++++++++++++++++++++- 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/module_build_service/utils/submit.py b/module_build_service/utils/submit.py index 90a2ac24..239d972b 100644 --- a/module_build_service/utils/submit.py +++ b/module_build_service/utils/submit.py @@ -486,22 +486,27 @@ def submit_module_build(username, url, mmd, optional_params=None): mmds = generate_expanded_mmds(db.session, mmd, raise_if_stream_ambigous, default_streams) modules = [] + # True if all module builds are skipped so MBS will actually not rebuild + # anything. To keep the backward compatibility, we need to raise an exception + # later in the end of this method. + all_modules_skipped = True + for mmd in mmds: # Prefix the version of the modulemd based on the base module it buildrequires version = get_prefixed_version(mmd) mmd.set_version(version) version_str = str(version) + nsvc = ":".join([mmd.get_name(), mmd.get_stream(), version_str, mmd.get_context()]) - log.debug('Checking whether module build already exists: %s.', - ":".join([mmd.get_name(), mmd.get_stream(), version_str, mmd.get_context()])) + log.debug('Checking whether module build already exists: %s.', nsvc) module = models.ModuleBuild.get_build_from_nsvc( db.session, mmd.get_name(), mmd.get_stream(), version_str, mmd.get_context()) if module: if module.state != models.BUILD_STATES['failed']: - err_msg = ('Module (state=%s) already exists. Only a new build or resubmission of ' - 'a failed build is allowed.' % module.state) - log.error(err_msg) - raise Conflict(err_msg) + log.info("Skipping rebuild of %s, only rebuild of modules in failed state " + "is allowed.", nsvc) + modules.append(module) + continue if optional_params: rebuild_strategy = optional_params.get('rebuild_strategy') if rebuild_strategy and module.rebuild_strategy != rebuild_strategy: @@ -540,11 +545,20 @@ def submit_module_build(username, url, mmd, optional_params=None): (module.ref_build_context, module.build_context, module.runtime_context, module.context) = module.contexts_from_mmd(module.modulemd) + all_modules_skipped = False db.session.add(module) db.session.commit() modules.append(module) log.info("%s submitted build of %s, stream=%s, version=%s, context=%s", username, mmd.get_name(), mmd.get_stream(), version_str, mmd.get_context()) + + if all_modules_skipped: + err_msg = ('Module (state=%s) already exists. Only a new build, resubmission of ' + 'a failed build or build against new buildrequirements is ' + 'allowed.' % module.state) + log.error(err_msg) + raise Conflict(err_msg) + return modules diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index 6a7ad5b3..c99adffc 100644 --- a/tests/test_build/test_build.py +++ b/tests/test_build/test_build.py @@ -1180,8 +1180,8 @@ class TestBuild: data = json.loads(rv2.data) expected = { 'error': 'Conflict', - 'message': ('Module (state=5) already exists. Only a new build or resubmission of a ' - 'failed build is allowed.'), + 'message': ('Module (state=5) already exists. Only a new build, resubmission of a ' + 'failed build or build against new buildrequirements is allowed.'), 'status': 409 } assert data == expected diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 47c9398e..f4cf34b8 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -30,7 +30,7 @@ from module_build_service import models, conf from module_build_service.errors import ProgrammingError, ValidationError, UnprocessableEntity from tests import ( reuse_component_init_data, db, reuse_shared_userspace_init_data, clean_database, init_data, - scheduler_init_data) + scheduler_init_data, make_module) import mock import koji import pytest @@ -667,6 +667,31 @@ class TestUtils: v = module_build_service.utils.submit.get_prefixed_version(mmd) assert v == 7000120180205135154 + @patch('module_build_service.utils.submit.generate_expanded_mmds') + def test_submit_build_new_mse_build(self, generate_expanded_mmds): + """ + Tests that finished build can be resubmitted in case the resubmitted + build adds new MSE build (it means there are new expanded + buildrequires). + """ + build = make_module("foo:stream:0:c1", {}, {}) + assert build.state == models.BUILD_STATES['ready'] + + mmd1 = build.mmd() + mmd2 = build.mmd() + mmd2.set_context("c2") + + generate_expanded_mmds.return_value = [mmd1, mmd2] + + builds = module_build_service.utils.submit_module_build("foo", "bar", mmd1) + ret = {b.mmd().get_context(): b.state for b in builds} + assert ret == { + "c1": models.BUILD_STATES['ready'], + "c2": models.BUILD_STATES['init']} + + assert builds[0].siblings == [builds[1].id] + assert builds[1].siblings == [builds[0].id] + class DummyModuleBuilder(GenericBuilder): """