PR#1127: Allow resubmiting the very same module build in case it results in new MSE build. - fm-orchestrator

fm-orchestrator

#1127 Allow resubmiting the very same module build in case it results in new MSE build.

Merged 5 years ago by mprahl. Opened 5 years ago by jkaluza.

jkaluza/fm-orchestrator factory-3674 into master

Allow resubmiting the very same module build in case it results in new MSE build.

Jan Kaluza • 5 years ago

26d707c

module_build_service/utils/submit.py

file modified

+20 -6

		`@@ -486,22 +486,27 @@`
		`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 @@`
		`(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`

tests/test_build/test_build.py

file modified

+2 -2

		`@@ -1180,8 +1180,8 @@`
		`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`

tests/test_utils/test_utils.py

file modified

+26 -1

		`@@ -30,7 +30,7 @@`
		`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 @@`
		`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):`
		`"""`

jkaluza commented 5 years ago

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.