From 4f6d6836456b303dd79e088597c19464ebe08096 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Wed, 23 Nov 2016 06:37:33 +0100 Subject: [PATCH] Fix #173 - Handle includedmodules by building their components recursively. --- module_build_service/utils.py | 117 ++++++++++++++++---------- tests/test_views/fakemodule.yaml | 1 + tests/test_views/includedmodules.yaml | 38 +++++++++ tests/test_views/test_views.py | 64 +++++++++++++- 4 files changed, 171 insertions(+), 49 deletions(-) create mode 100644 tests/test_views/includedmodules.yaml diff --git a/module_build_service/utils.py b/module_build_service/utils.py index 6ec72ec4..0f90a83f 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -208,7 +208,7 @@ def filter_module_builds(flask_request): return query.paginate(page, per_page, False) -def submit_module_build(username, url): +def _fetch_mmd(url): # Import it here, because SCM uses utils methods # and fails to import them because of dep-chain. import module_build_service.scm @@ -240,50 +240,12 @@ def submit_module_build(username, url): except Exception as e: log.error('Invalid modulemd: %s' % str(e)) raise UnprocessableEntity('Invalid modulemd: %s' % str(e)) + return mmd, scm, yaml - # If undefined, set the name field to VCS repo name. - if not mmd.name and scm: - mmd.name = scm.name - - # If undefined, set the stream field to the VCS branch name. - if not mmd.stream and scm: - mmd.stream = scm.branch - - # If undefined, set the version field to int represenation of VCS commit. - if not mmd.version and scm: - mmd.version = int(scm.version) - - module = models.ModuleBuild.query.filter_by(name=mmd.name, - stream=mmd.stream, - version=mmd.version).first() - if module: - log.debug('Checking whether module build already exist.') - # TODO: make this configurable, we might want to allow - # resubmitting any stuck build on DEV no matter the state - if module.state not in (models.BUILD_STATES['failed'],): - log.error('Module (state=%s) already exists. ' - 'Only new or failed builds are allowed.' - % module.state) - raise Conflict('Module (state=%s) already exists. ' - 'Only new or failed builds are allowed.' - % module.state) - log.debug('Resuming existing module build %r' % module) - module.username = username - module.transition(conf, models.BUILD_STATES["init"]) - log.info("Resumed existing module build in previous state %s" - % module.state) - else: - log.debug('Creating new module build') - module = models.ModuleBuild.create( - db.session, - conf, - name=mmd.name, - stream=mmd.stream, - version=mmd.version, - modulemd=yaml, - scmurl=url, - username=username - ) +def record_component_builds(mmd, module, initial_batch = 1): + # Import it here, because SCM uses utils methods + # and fails to import them because of dep-chain. + import module_build_service.scm # List of (pkg_name, git_url) tuples to be used to check # the availability of git URLs paralelly later. @@ -343,9 +305,19 @@ def submit_module_build(username, url): # We do not start with batch = 0 here, because the first batch is # reserved for module-build-macros. First real components must be # planned for batch 2 and following. - batch = 1 + batch = initial_batch for pkg in components: + # If the pkg is another module, we fetch its modulemd file + # and record its components recursively with the initial_batch + # set to our current batch, so the components of this module + # are built in the right global order. + if isinstance(pkg, modulemd.ModuleComponentModule): + full_url = pkg.repository + "?#" + pkg.ref + mmd = _fetch_mmd(full_url)[0] + batch = record_component_builds(mmd, module, batch) + continue + if previous_buildorder != pkg.buildorder: previous_buildorder = pkg.buildorder batch += 1 @@ -369,6 +341,61 @@ def submit_module_build(username, url): ) db.session.add(build) + return batch + +def submit_module_build(username, url): + # Import it here, because SCM uses utils methods + # and fails to import them because of dep-chain. + import module_build_service.scm + + mmd, scm, yaml = _fetch_mmd(url) + + # If undefined, set the name field to VCS repo name. + if not mmd.name and scm: + mmd.name = scm.name + + # If undefined, set the stream field to the VCS branch name. + if not mmd.stream and scm: + mmd.stream = scm.branch + + # If undefined, set the version field to int represenation of VCS commit. + if not mmd.version and scm: + mmd.version = int(scm.version) + + module = models.ModuleBuild.query.filter_by(name=mmd.name, + stream=mmd.stream, + version=mmd.version).first() + if module: + log.debug('Checking whether module build already exist.') + # TODO: make this configurable, we might want to allow + # resubmitting any stuck build on DEV no matter the state + if module.state not in (models.BUILD_STATES['failed'],): + log.error('Module (state=%s) already exists. ' + 'Only new or failed builds are allowed.' + % module.state) + raise Conflict('Module (state=%s) already exists. ' + 'Only new or failed builds are allowed.' + % module.state) + log.debug('Resuming existing module build %r' % module) + module.username = username + module.transition(conf, models.BUILD_STATES["init"]) + log.info("Resumed existing module build in previous state %s" + % module.state) + else: + log.debug('Creating new module build') + module = models.ModuleBuild.create( + db.session, + conf, + name=mmd.name, + stream=mmd.stream, + version=mmd.version, + modulemd=yaml, + scmurl=url, + username=username + ) + + record_component_builds(mmd, module) + module.modulemd = mmd.dumps() module.transition(conf, models.BUILD_STATES["wait"]) db.session.add(module) diff --git a/tests/test_views/fakemodule.yaml b/tests/test_views/fakemodule.yaml index e3f8ca60..d85c8c72 100644 --- a/tests/test_views/fakemodule.yaml +++ b/tests/test_views/fakemodule.yaml @@ -30,3 +30,4 @@ data: bash: rationale: It's here to test the whole thing! ref: 70fa7516b83768595a4f3280ae890a7ac957e0c7 + buildorder: 20 diff --git a/tests/test_views/includedmodules.yaml b/tests/test_views/includedmodules.yaml new file mode 100644 index 00000000..8446bec1 --- /dev/null +++ b/tests/test_views/includedmodules.yaml @@ -0,0 +1,38 @@ +document: modulemd +version: 1 +data: + name: fakemodule + stream: "4.3.44" + version: 5 + summary: A fake module containing the bash shell + description: > + A fake module used for testing + license: + module: + - MIT + content: [] + xmd: ~ + dependencies: + buildrequires: + base_runtime: 1.0-0 + references: + community: https://fedoraproject.org/wiki/Modularity + tracker: https://taiga.fedorainfracloud.org/project/modularity + profiles: + default: + rpms: + - bash + api: + rpms: + - bash + components: + rpms: + file: + rationale: It's here to test the whole thing! + ref: 70fa7516b83768595a4f3280ae890a7ac957e0c7 + buildorder: 10 + modules: + fakemodule: + rationale: foobar + repository: git://foo.bar/fakemodule.git + ref: master diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index f3ddaac1..631e8d88 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -28,23 +28,42 @@ from shutil import copyfile from os import path, mkdir from tests import app, init_data +from module_build_service.models import ComponentBuild class MockedSCM(object): - def __init__(self, mocked_scm, name, mmd_filename): + def __init__(self, mocked_scm, name, mmd_filenames): + """ + Adds default testing checkout, get_latest and name methods + to mocked_scm SCM class. + + :param mmd_filenames: List of ModuleMetadata yaml files which + will be checkouted by the SCM class in the same order as they + are stored in the list. + """ self.mocked_scm = mocked_scm self.name = name - self.mmd_filename = mmd_filename + if not isinstance(mmd_filenames, list): + mmd_filenames = [mmd_filenames] + self.mmd_filenames = mmd_filenames + self.checkout_id = 0 self.mocked_scm.return_value.checkout = self.checkout self.mocked_scm.return_value.name = self.name self.mocked_scm.return_value.get_latest = self.get_latest def checkout(self, temp_dir): + try: + mmd_filename = self.mmd_filenames[self.checkout_id] + except: + mmd_filename = self.mmd_filenames[0] + scm_dir = path.join(temp_dir, self.name) mkdir(scm_dir) base_dir = path.abspath(path.dirname(__file__)) - copyfile(path.join(base_dir, self.mmd_filename), - path.join(scm_dir, self.mmd_filename)) + copyfile(path.join(base_dir, mmd_filename), + path.join(scm_dir, self.name + ".yaml")) + + self.checkout_id += 1 return scm_dir @@ -346,3 +365,40 @@ class TestViews(unittest.TestCase): self.assertEquals(data['message'][:15], "Cannot checkout") self.assertEquals(data['error'], "Unprocessable Entity") + @patch('module_build_service.auth.get_username', return_value='Homer J. Simpson') + @patch('module_build_service.auth.assert_is_packager') + @patch('module_build_service.scm.SCM') + def test_submit_build_includedmodule(self, mocked_scm, mocked_assert_is_packager, + mocked_get_username): + mocked_scm_obj = MockedSCM(mocked_scm, "includedmodules", + ["includedmodules.yaml", "fakemodule.yaml"]) + + rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( + {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + 'testmodule.git?#68932c90de214d9d13feefbd35246a81b6cb8d49'})) + data = json.loads(rv.data) + + self.assertEquals(data['component_builds'], [61, 62]) + self.assertEquals(data['name'], 'fakemodule') + self.assertEquals(data['scmurl'], + ('git://pkgs.stg.fedoraproject.org/modules/testmodule' + '.git?#68932c90de214d9d13feefbd35246a81b6cb8d49')) + self.assertEquals(data['version'], '5') + self.assertTrue(data['time_submitted'] is not None) + self.assertTrue(data['time_modified'] is not None) + self.assertEquals(data['version'], '5') + self.assertEquals(data['time_completed'], None) + self.assertEquals(data['stream'], '4.3.44') + self.assertEquals(data['owner'], 'Homer J. Simpson') + self.assertEquals(data['id'], 31) + self.assertEquals(data['state_name'], 'wait') + self.assertEquals(data['state_url'], '/module-build-service/1/module-builds/31') + + batches = {} + for build in ComponentBuild.query.filter_by(module_id=31).all(): + batches[build.package] = build.batch + + self.assertEquals(batches["bash"], 2) + self.assertEquals(batches["file"], 3) + +