From 830b1dd111eb522a7432f53948da4b30a53a5c9c Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Mar 16 2017 10:20:51 +0000 Subject: [PATCH 1/4] Add tests for 'continue' part of start_next_batch and component resubmit --- diff --git a/module_build_service/__init__.py b/module_build_service/__init__.py index 3651668..60ef8a0 100644 --- a/module_build_service/__init__.py +++ b/module_build_service/__init__.py @@ -59,21 +59,6 @@ conf = init_config(app) db = SQLAlchemy(app) - -def get_url_for(*args, **kwargs): - """ - flask.url_for wrapper which creates the app_context on-the-fly. - """ - if has_app_context(): - return url_for(*args, **kwargs) - - # Localhost is right URL only when the scheduler runs on the same - # system as the web views. - app.config['SERVER_NAME'] = 'localhost' - with app.app_context(): - return url_for(*args, **kwargs) - - @app.errorhandler(ValidationError) def validationerror_error(e): """Flask error handler for ValidationError exceptions""" @@ -118,4 +103,20 @@ def notfound_error(e): init_logging(conf) log = getLogger(__name__) +def get_url_for(*args, **kwargs): + """ + flask.url_for wrapper which creates the app_context on-the-fly. + """ + if has_app_context(): + return url_for(*args, **kwargs) + + # Localhost is right URL only when the scheduler runs on the same + # system as the web views. + app.config['SERVER_NAME'] = 'localhost' + with app.app_context(): + log.debug("WARNING: get_url_for() has been called without the Flask " + "app_context. That can lead to SQLAlchemy errors caused by " + "multiple session being used in the same time.") + return url_for(*args, **kwargs) + from module_build_service import views diff --git a/module_build_service/utils.py b/module_build_service/utils.py index a7ed437..58bd7fa 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -524,6 +524,8 @@ def format_mmd(mmd, scmurl): raise UnprocessableEntity(err_msg) def record_component_builds(scm, mmd, module, initial_batch = 1): + import koji # Placed here to avoid py2/py3 conflicts... + # Format the modulemd by putting in defaults and replacing streams that # are branches with commit hashes try: @@ -574,10 +576,10 @@ def record_component_builds(scm, mmd, module, initial_batch = 1): existing_build = models.ComponentBuild.query.filter_by( module_id=module.id, package=pkg.name).first() - if existing_build and \ - existing_build.state != models.BUILD_STATES['done']: - existing_build.state = models.BUILD_STATES['init'] - db.session.add(existing_build) + if existing_build: + if existing_build.state != koji.BUILD_STATES['COMPLETE']: + existing_build.state = None + db.session.add(existing_build) else: # XXX: what about components that were present in previous # builds but are gone now (component reduction)? diff --git a/tests/__init__.py b/tests/__init__.py index be4a91d..6fadfad 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -33,6 +33,7 @@ from module_build_service.utils import get_scm_url_re import module_build_service.pdc app = module_build_service.app +app.config['SERVER_NAME'] = 'localhost' conf = init_config(app) def init_data(): diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index e9d8fae..f3a1469 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -19,7 +19,8 @@ # SOFTWARE. import unittest -from os import path +from os import path, mkdir +from shutil import copyfile import vcr import modulemd from mock import patch @@ -33,14 +34,46 @@ from mock import PropertyMock import koji import module_build_service.scheduler.handlers.components from module_build_service.builder import GenericBuilder, KojiModuleBuilder +from tests import app BASE_DIR = path.abspath(path.dirname(__file__)) CASSETTES_DIR = path.join( path.abspath(path.dirname(__file__)), '..', 'vcr-request-data') +class MockedSCM(object): + def __init__(self, mocked_scm, name, mmd_filename, commit=None): + self.mocked_scm = mocked_scm + self.name = name + self.commit = commit + self.mmd_filename = mmd_filename + + self.mocked_scm.return_value.checkout = self.checkout + self.mocked_scm.return_value.name = self.name + self.mocked_scm.return_value.branch = 'master' + self.mocked_scm.return_value.get_latest = self.get_latest + self.mocked_scm.return_value.commit = self.commit + self.mocked_scm.return_value.repository_root = "git://pkgs.stg.fedoraproject.org/modules/" + + def checkout(self, temp_dir): + scm_dir = path.join(temp_dir, self.name) + mkdir(scm_dir) + base_dir = path.abspath(path.dirname(__file__)) + copyfile(path.join(base_dir, '..', 'staged_data', self.mmd_filename), + path.join(scm_dir, self.name + ".yaml")) + + return scm_dir + + def get_latest(self, branch = 'master'): + return branch class TestUtils(unittest.TestCase): + def setUp(self): + pass + + def tearDown(self): + init_data() + @vcr.use_cassette( path.join(CASSETTES_DIR, 'tests.test_utils.TestUtils.test_format_mmd')) @patch('module_build_service.scm.SCM') @@ -284,6 +317,40 @@ class TestUtils(unittest.TestCase): self.assertTrue(str(cm.exception).endswith(' No value provided.')) + @patch('module_build_service.scm.SCM') + def test_resubmit(self, mocked_scm): + """ + Tests that the module resubmit reintializes the module state and + component states properly. + """ + MockedSCM(mocked_scm, 'testmodule', 'testmodule-bootstrap.yaml', + '620ec77321b2ea7b0d67d82992dda3e1d67055b4') + with app.app_context(): + test_resuse_component_init_data() + # Mark the module build as failed, so we can resubmit it. + module_build = models.ModuleBuild.query.filter_by(id=2).one() + module_build.batch = 2 + module_build.state = models.BUILD_STATES['failed'] + module_build.version = 1 + + # Mark the components as COMPLETE/FAILED/CANCELED + components = module_build.component_builds + complete_component = components[0] + complete_component.state = koji.BUILD_STATES['COMPLETE'] + failed_component = components[1] + failed_component.state = koji.BUILD_STATES['FAILED'] + canceled_component = components[2] + canceled_component.state = koji.BUILD_STATES['CANCELED'] + db.session.commit() + + module_build_service.utils.submit_module_build_from_scm( + "Tom Brady", 'git://pkgs.stg.fedoraproject.org/modules/testmodule.git?#8fea453', + 'master') + + self.assertEqual(module_build.state, models.BUILD_STATES['wait']) + self.assertEqual(complete_component.state, koji.BUILD_STATES['COMPLETE']) + self.assertEqual(failed_component.state, None) + self.assertEqual(canceled_component.state, None) class DummyModuleBuilder(GenericBuilder): """ @@ -360,6 +427,15 @@ class TestBatches(unittest.TestCase): GenericBuilder.register_backend_class(KojiModuleBuilder) def test_start_next_batch_build_reuse(self, default_buildroot_groups): + """ + Tests that start_next_batch_build: + 1) Increments module.batch. + 2) Can reuse all components in batch + 3) Returns proper further_work messages for reused components. + 4) Returns the fake Repo change message + 5) Handling the further_work messages lead to proper tagging of + reused components. + """ module_build = models.ModuleBuild.query.filter_by(id=2).one() module_build.batch = 1 @@ -393,3 +469,31 @@ class TestBatches(unittest.TestCase): # Check that packages have been tagged just once. self.assertEqual(len(DummyModuleBuilder.TAGGED_COMPONENTS), 2) + + def test_start_next_batch_continue(self, default_buildroot_groups): + """ + Tests that start_next_batch_build does not start new batch when + there are unbuilt components in the current one. + """ + module_build = models.ModuleBuild.query.filter_by(id=2).one() + module_build.batch = 2 + + # Mark the component as BUILDING. + building_component = module_build.current_batch()[0] + building_component.state = koji.BUILD_STATES['BUILDING'] + db.session.commit() + + builder = mock.MagicMock() + further_work = module_build_service.utils.start_next_batch_build( + conf, module_build, db.session, builder) + + # Batch number should not increase. + self.assertEqual(module_build.batch, 2) + + # Single component should be reused this time, second message is fake + # KojiRepoChange. + self.assertEqual(len(further_work), 2) + self.assertEqual(further_work[0].build_name, "perl-List-Compare") + + + From 2ade8ccf79c1655ad9b2bbe6ea8764fb203f12fb Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Mar 16 2017 12:19:09 +0000 Subject: [PATCH 2/4] Add missing testmodule-bootstrap.yaml --- diff --git a/tests/staged_data/testmodule-bootstrap.yaml b/tests/staged_data/testmodule-bootstrap.yaml new file mode 100644 index 0000000..b30c8bc --- /dev/null +++ b/tests/staged_data/testmodule-bootstrap.yaml @@ -0,0 +1,36 @@ +document: modulemd +version: 1 +data: + summary: A test module in all its beauty + description: This module demonstrates how to write simple modulemd files And can be used for testing the build and release pipeline. + license: + module: [ MIT ] + dependencies: + buildrequires: + bootstrap: master + requires: + bootstrap: master + references: + community: https://fedoraproject.org/wiki/Modularity + documentation: https://fedoraproject.org/wiki/Fedora_Packaging_Guidelines_for_Modules + tracker: https://taiga.fedorainfracloud.org/project/modularity + profiles: + default: + rpms: + - tangerine + api: + rpms: + - perl-Tangerine + - tangerine + components: + rpms: + perl-List-Compare: + rationale: A dependency of tangerine. + ref: f25 + perl-Tangerine: + rationale: Provides API for this module and is a dependency of tangerine. + ref: f25 + tangerine: + rationale: Provides API for this module. + buildorder: 10 + ref: f25 From a72a97754a5150b287d7681c6ad58002d84ca8d4 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Mar 17 2017 07:58:58 +0000 Subject: [PATCH 3/4] Fix typo in test_reuse_component_init_data name, set SERVER_NAME config variable in config.py, return commit in MockedSCM.get_latest --- diff --git a/conf/config.py b/conf/config.py index b25932c..e1522f8 100644 --- a/conf/config.py +++ b/conf/config.py @@ -155,6 +155,7 @@ class TestConfiguration(BaseConfiguration): KOJI_CONFIG = './conf/koji.conf' KOJI_PROFILE = 'staging' + SERVER_NAME = 'localhost' class ProdConfiguration(BaseConfiguration): diff --git a/tests/__init__.py b/tests/__init__.py index 6fadfad..e667092 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -33,7 +33,6 @@ from module_build_service.utils import get_scm_url_re import module_build_service.pdc app = module_build_service.app -app.config['SERVER_NAME'] = 'localhost' conf = init_config(app) def init_data(): @@ -243,7 +242,7 @@ def scheduler_init_data(): session.commit() -def test_resuse_component_init_data(): +def test_reuse_component_init_data(): db.session.remove() db.drop_all() db.create_all() diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index f3a1469..93c8808 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -28,7 +28,7 @@ import module_build_service.utils import module_build_service.scm from module_build_service import models, conf from module_build_service.errors import ProgrammingError, ValidationError -from tests import test_resuse_component_init_data, init_data, db +from tests import test_reuse_component_init_data, init_data, db import mock from mock import PropertyMock import koji @@ -64,7 +64,7 @@ class MockedSCM(object): return scm_dir def get_latest(self, branch = 'master'): - return branch + return self.commit if self.commit else branch class TestUtils(unittest.TestCase): @@ -116,14 +116,14 @@ class TestUtils(unittest.TestCase): self.assertEqual(mmd.xmd, xmd) def test_get_reusable_component_same(self): - test_resuse_component_init_data() + test_reuse_component_init_data() new_module = models.ModuleBuild.query.filter_by(id=2).one() rv = module_build_service.utils.get_reusable_component( db.session, new_module, 'tangerine') self.assertEqual(rv.package, 'tangerine') def test_get_reusable_component_different_perl_tangerine(self): - test_resuse_component_init_data() + test_reuse_component_init_data() second_module_build = models.ModuleBuild.query.filter_by(id=2).one() mmd = second_module_build.mmd() mmd.components.rpms['perl-Tangerine'].ref = \ @@ -153,7 +153,7 @@ class TestUtils(unittest.TestCase): self.assertEqual(tangerine_rv, None) def test_get_reusable_component_different_buildrequires_hash(self): - test_resuse_component_init_data() + test_reuse_component_init_data() second_module_build = models.ModuleBuild.query.filter_by(id=2).one() mmd = second_module_build.mmd() mmd.xmd['mbs']['buildrequires']['base-runtime']['ref'] = \ @@ -177,7 +177,7 @@ class TestUtils(unittest.TestCase): self.assertEqual(tangerine_rv, None) def test_get_reusable_component_different_buildrequires(self): - test_resuse_component_init_data() + test_reuse_component_init_data() second_module_build = models.ModuleBuild.query.filter_by(id=2).one() mmd = second_module_build.mmd() mmd.buildrequires = {'some_module': 'master'} @@ -326,7 +326,7 @@ class TestUtils(unittest.TestCase): MockedSCM(mocked_scm, 'testmodule', 'testmodule-bootstrap.yaml', '620ec77321b2ea7b0d67d82992dda3e1d67055b4') with app.app_context(): - test_resuse_component_init_data() + test_reuse_component_init_data() # Mark the module build as failed, so we can resubmit it. module_build = models.ModuleBuild.query.filter_by(id=2).one() module_build.batch = 2 @@ -418,7 +418,7 @@ class DummyModuleBuilder(GenericBuilder): class TestBatches(unittest.TestCase): def setUp(self): - test_resuse_component_init_data() + test_reuse_component_init_data() GenericBuilder.register_backend_class(DummyModuleBuilder) def tearDown(self): From 0fb8bd221f782a3ddca78aff900dcf9102fee1e1 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Mar 17 2017 11:11:08 +0000 Subject: [PATCH 4/4] Reset batch counter and state_reason when resubmiting module --- diff --git a/module_build_service/utils.py b/module_build_service/utils.py index 58bd7fa..aae2b7f 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -627,7 +627,9 @@ def submit_module_build(username, url, mmd, scm, yaml, optional_params=None): raise Conflict(err_msg) log.debug('Resuming existing module build %r' % module) module.username = username - module.transition(conf, models.BUILD_STATES["init"]) + module.transition(conf, models.BUILD_STATES["init"], + "Resubmitted by %s" % username) + module.batch = 0 log.info("Resumed existing module build in previous state %s" % module.state) else: diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 93c8808..3337d6e 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -331,6 +331,7 @@ class TestUtils(unittest.TestCase): module_build = models.ModuleBuild.query.filter_by(id=2).one() module_build.batch = 2 module_build.state = models.BUILD_STATES['failed'] + module_build.state_reason = "Cancelled" module_build.version = 1 # Mark the components as COMPLETE/FAILED/CANCELED @@ -348,6 +349,8 @@ class TestUtils(unittest.TestCase): 'master') self.assertEqual(module_build.state, models.BUILD_STATES['wait']) + self.assertEqual(module_build.batch, 0) + self.assertEqual(module_build.state_reason, "Resubmitted by Tom Brady") self.assertEqual(complete_component.state, koji.BUILD_STATES['COMPLETE']) self.assertEqual(failed_component.state, None) self.assertEqual(canceled_component.state, None)