diff --git a/module_build_service/config.py b/module_build_service/config.py index 44455e4e..b51cb183 100644 --- a/module_build_service/config.py +++ b/module_build_service/config.py @@ -33,6 +33,11 @@ import sys from module_build_service import logger +# TODO: It'd be nice to reuse this from models.ModuleBuild.rebuild_strategies but models.py +# currently relies on this file, so we can't import it +SUPPORTED_STRATEGIES = ['changed-and-after', 'only-changed', 'all'] + + def init_config(app): """ Configure MBS and the Flask app """ @@ -397,6 +402,20 @@ class Config(object): 'desc': "The name of Koji tag which should be used as fallback " "when koji_cg_build_tag_template tag is not found in " "Koji."}, + 'rebuild_strategy': { + 'type': str, + 'default': 'changed-and-after', + 'desc': 'The module rebuild strategy to use by default.'}, + 'rebuild_strategy_allow_override': { + 'type': bool, + 'default': False, + 'desc': ('Allows a user to specify the rebuild strategy they want to use when ' + 'submitting a module build.')}, + 'rebuild_strategies_allowed': { + 'type': list, + 'default': SUPPORTED_STRATEGIES, + 'desc': ('The allowed module rebuild strategies. This is only used when ' + 'REBUILD_STRATEGY_ALLOW_OVERRIDE is True.')} } def __init__(self, conf_section_obj): @@ -564,3 +583,22 @@ class Config(object): raise ValueError('LDAP_URI is invalid. It must start with "ldap://" or "ldaps://"') self._ldap_uri = ldap_uri + + def _setifok_rebuild_strategy(self, strategy): + if strategy not in SUPPORTED_STRATEGIES: + raise ValueError('The strategy "{0}" is not supported. Chose from: {1}' + .format(strategy, ', '.join(SUPPORTED_STRATEGIES))) + self._rebuild_strategy = strategy + + def _setifok_rebuild_strategies_allowed(self, strategies): + if not isinstance(strategies, list): + raise ValueError('REBUILD_STRATEGIES_ALLOWED must be a list') + elif not strategies: + raise ValueError('REBUILD_STRATEGIES_ALLOWED must contain at least one rebuild ' + 'strategy') + for strategy in strategies: + if strategy not in SUPPORTED_STRATEGIES: + raise ValueError('REBUILD_STRATEGIES_ALLOWED must be one of: {0}' + .format(', '.join(SUPPORTED_STRATEGIES))) + + self._rebuild_strategies_allowed = strategies diff --git a/module_build_service/migrations/versions/524c3b1c683c_rebuild_strategy.py b/module_build_service/migrations/versions/524c3b1c683c_rebuild_strategy.py new file mode 100644 index 00000000..6e3e073e --- /dev/null +++ b/module_build_service/migrations/versions/524c3b1c683c_rebuild_strategy.py @@ -0,0 +1,27 @@ +"""Adds the rebuild strategy column + +Revision ID: 524c3b1c683c +Revises: edb537dd1e8c +Create Date: 2017-11-01 15:35:37.043545 + +""" + +# revision identifiers, used by Alembic. +revision = '524c3b1c683c' +down_revision = 'edb537dd1e8c' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('module_builds', sa.Column( + 'rebuild_strategy', sa.String(), server_default='changed-and-after', nullable=False)) + # Remove migration-only defaults + with op.batch_alter_table('module_builds') as b: + b.alter_column('rebuild_strategy', server_default=None) + + +def downgrade(): + with op.batch_alter_table('module_builds') as b: + b.drop_column('rebuild_strategy') diff --git a/module_build_service/models.py b/module_build_service/models.py index 5b560e31..79419056 100644 --- a/module_build_service/models.py +++ b/module_build_service/models.py @@ -158,12 +158,20 @@ class ModuleBuild(MBSBase): time_modified = db.Column(db.DateTime) time_completed = db.Column(db.DateTime) new_repo_task_id = db.Column(db.Integer) + rebuild_strategy = db.Column(db.String, nullable=False) # A monotonically increasing integer that represents which batch or # iteration this module is currently on for successive rebuilds of its # components. Think like 'mockchain --recurse' batch = db.Column(db.Integer, default=0) + rebuild_strategies = { + 'all': 'All components will be rebuilt', + 'changed-and-after': ('All components that have changed and those in subsequent batches ' + 'will be rebuilt'), + 'only-changed': 'All changed components will be rebuilt' + } + def current_batch(self, state=None): """ Returns all components of this module in the current batch. """ @@ -221,6 +229,14 @@ class ModuleBuild(MBSBase): return BUILD_STATES[field] raise ValueError("%s: %s, not in %r" % (key, field, BUILD_STATES)) + @validates('rebuild_strategy') + def validate_rebuild_stategy(self, key, rebuild_strategy): + if rebuild_strategy not in self.rebuild_strategies.keys(): + choices = ', '.join(self.rebuild_strategies.keys()) + raise ValueError('The rebuild_strategy of "{0}" is invalid. Chose from: {1}' + .format(rebuild_strategy, choices)) + return rebuild_strategy + @classmethod def from_module_event(cls, session, event): if type(event) == module_build_service.messaging.MBSModule: @@ -232,7 +248,7 @@ class ModuleBuild(MBSBase): @classmethod def create(cls, session, conf, name, stream, version, modulemd, scmurl, username, - copr_owner=None, copr_project=None): + copr_owner=None, copr_project=None, rebuild_strategy=None): now = datetime.utcnow() module = cls( name=name, @@ -245,7 +261,9 @@ class ModuleBuild(MBSBase): time_submitted=now, time_modified=now, copr_owner=copr_owner, - copr_project=copr_project + copr_project=copr_project, + # If the rebuild_strategy isn't specified, use the default + rebuild_strategy=rebuild_strategy or conf.rebuild_strategy ) session.add(module) session.commit() @@ -362,6 +380,7 @@ class ModuleBuild(MBSBase): 'version': self.version, 'owner': self.owner, 'name': self.name, + 'rebuild_strategy': self.rebuild_strategy, 'scmurl': self.scmurl, 'time_submitted': _utc_datetime_to_iso(self.time_submitted), 'time_modified': _utc_datetime_to_iso(self.time_modified), diff --git a/module_build_service/utils.py b/module_build_service/utils.py index ef4c8ecc..31c66695 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -327,13 +327,16 @@ def start_next_batch_build(config, module, session, builder, components=None): # Attempt to reuse any components possible in the batch before attempting to build any further_work = [] - # Try to figure out if it's even worth checking if the components can be - # reused by checking to see if the previous batch had all their builds - # reused except for when the previous batch was 1 because that always - # has the module-build-macros component built unbuilt_components_after_reuse = [] components_reused = False - if prev_batch == 1 or all_reused_in_prev_batch: + should_try_reuse = True + # If the rebuild strategy is "changed-and-after", try to figure out if it's worth checking if + # the components can be reused to save on resources + if module.rebuild_strategy == 'changed-and-after': + # Check to see if the previous batch had all their builds reused except for when the + # previous batch was 1 because that always has the module-build-macros component built + should_try_reuse = all_reused_in_prev_batch or prev_batch == 1 + if should_try_reuse: for c in unbuilt_components: previous_component_build = get_reusable_component( session, module, c.package) @@ -968,6 +971,11 @@ def submit_module_build(username, url, mmd, scm, yaml, optional_params=None): 'a failed build is allowed.' % module.state) log.error(err_msg) raise Conflict(err_msg) + if optional_params: + rebuild_strategy = optional_params.get('rebuild_strategy') + if rebuild_strategy and module.rebuild_strategy != rebuild_strategy: + raise ValidationError('You cannot change the module\'s "rebuild_strategy" when ' + 'resuming a module build') log.debug('Resuming existing module build %r' % module) module.username = username module.transition(conf, models.BUILD_STATES["init"], @@ -1152,6 +1160,11 @@ def get_reusable_component(session, module, component_name): if conf.system not in ['koji', 'test']: return None + # If the rebuild strategy is "all", that means that nothing can be reused + if module.rebuild_strategy == 'all': + log.info('Cannot re-use the component because the rebuild strategy is "all".') + return None + mmd = module.mmd() # Find the latest module that is in the done or ready state previous_module_build = session.query(models.ModuleBuild)\ @@ -1159,8 +1172,13 @@ def get_reusable_component(session, module, component_name): .filter_by(stream=mmd.stream)\ .filter(models.ModuleBuild.state.in_([3, 5]))\ .filter(models.ModuleBuild.scmurl.isnot(None))\ - .order_by(models.ModuleBuild.time_completed.desc())\ - .first() + .order_by(models.ModuleBuild.time_completed.desc()) + # If we are rebuilding with the "changed-and-after" option, then we can't reuse + # components from modules that were built more liberally + if module.rebuild_strategy == 'changed-and-after': + previous_module_build = previous_module_build.filter( + models.ModuleBuild.rebuild_strategy.in_(['all', 'changed-and-after'])) + previous_module_build = previous_module_build.first() # The component can't be reused if there isn't a previous build in the done # or ready state if not previous_module_build: @@ -1188,39 +1206,6 @@ def get_reusable_component(session, module, component_name): .format(previous_module_build.version, previous_module_build.name)) return None - # If the mmd.buildopts.macros.rpms changed, we cannot reuse - modulemd_macros = "" - old_modulemd_macros = "" - if mmd.buildopts and mmd.buildopts.rpms: - modulemd_macros = mmd.buildopts.rpms.macros - if old_mmd.buildopts and old_mmd.buildopts.rpms: - modulemd_macros = old_mmd.buildopts.rpms.macros - if modulemd_macros != old_modulemd_macros: - log.info('Cannot re-use. Old modulemd macros do not match the new.') - return None - - # If the module buildrequires are different, then we can't reuse the - # component - if mmd.buildrequires.keys() != old_mmd.buildrequires.keys(): - log.info('Cannot re-use. The set of module buildrequires changed') - return None - - # Make sure that the module buildrequires commit hashes are exactly the same - for br_module_name, br_module in \ - mmd.xmd['mbs']['buildrequires'].items(): - # Assumes that the streams have been replaced with commit hashes, so we - # can compare to see if they have changed. Since a build is unique to - # a commit hash, this is a safe test. - ref1 = br_module.get('ref') - ref2 = old_mmd.xmd['mbs']['buildrequires'][br_module_name].get('ref') - if not (ref1 and ref2) or ref1 != ref2: - log.info('Cannot re-use. The module buildrequires hashes changed') - return None - - # At this point we've determined that both module builds depend(ed) on the - # same exact module builds. Now it's time to determine if the batch of the - # components have changed - # # If the chosen component for some reason was not found in the database, # or the ref is missing, something has gone wrong and the component cannot # be reused @@ -1241,58 +1226,95 @@ def get_reusable_component(session, module, component_name): log.info('Cannot re-use. Previous component not found in the db.') return None - # Make sure the batch number for the component that is trying to be reused - # hasn't changed since the last build - if prev_module_build_component.batch != new_module_build_component.batch: - log.info('Cannot re-use. Batch numbers do not match.') - return None - # Make sure the ref for the component that is trying to be reused # hasn't changed since the last build if prev_module_build_component.ref != new_module_build_component.ref: log.info('Cannot re-use. Component commit hashes do not match.') return None - # Convert the component_builds to a list and sort them by batch - new_component_builds = list(module.component_builds) - new_component_builds.sort(key=lambda x: x.batch) - prev_component_builds = list(previous_module_build.component_builds) - prev_component_builds.sort(key=lambda x: x.batch) + # At this point we've determined that both module builds contain the component + # and the components share the same commit hash + if module.rebuild_strategy == 'changed-and-after': + # Make sure the batch number for the component that is trying to be reused + # hasn't changed since the last build + if prev_module_build_component.batch != new_module_build_component.batch: + log.info('Cannot re-use. Batch numbers do not match.') + return None - new_module_build_components = [] - previous_module_build_components = [] - # Create separate lists for the new and previous module build. These lists - # will have an entry for every build batch *before* the component's - # batch except for 1, which is reserved for the module-build-macros RPM. - # Each batch entry will contain a set of "(name, ref)" with the name and - # ref (commit) of the component. - for i in range(new_module_build_component.batch - 1): - # This is the first batch which we want to skip since it will always - # contain only the module-build-macros RPM and it gets built every time - if i == 0: - continue + # If the mmd.buildopts.macros.rpms changed, we cannot reuse + modulemd_macros = "" + old_modulemd_macros = "" + if mmd.buildopts and mmd.buildopts.rpms: + modulemd_macros = mmd.buildopts.rpms.macros + if old_mmd.buildopts and old_mmd.buildopts.rpms: + modulemd_macros = old_mmd.buildopts.rpms.macros + if modulemd_macros != old_modulemd_macros: + log.info('Cannot re-use. Old modulemd macros do not match the new.') + return None - new_module_build_components.append(set([ - (value.package, value.ref) for value in - new_component_builds if value.batch == i + 1 - ])) + # If the module buildrequires are different, then we can't reuse the + # component + if mmd.buildrequires.keys() != old_mmd.buildrequires.keys(): + log.info('Cannot re-use. The set of module buildrequires changed') + return None - previous_module_build_components.append(set([ - (value.package, value.ref) for value in - prev_component_builds if value.batch == i + 1 - ])) + # Make sure that the module buildrequires commit hashes are exactly the same + for br_module_name, br_module in \ + mmd.xmd['mbs']['buildrequires'].items(): + # Assumes that the streams have been replaced with commit hashes, so we + # can compare to see if they have changed. Since a build is unique to + # a commit hash, this is a safe test. + ref1 = br_module.get('ref') + ref2 = old_mmd.xmd['mbs']['buildrequires'][br_module_name].get('ref') + if not (ref1 and ref2) or ref1 != ref2: + log.info('Cannot re-use. The module buildrequires hashes changed') + return None - # If the previous batches have the same ordering and hashes, then the - # component can be reused - if previous_module_build_components == new_module_build_components: - reusable_component = models.ComponentBuild.query.filter_by( - package=component_name, module_id=previous_module_build.id).one() - log.debug('Found reusable component!') - return reusable_component + # At this point we've determined that both module builds contain the component + # with the same commit hash and they are in the same batch. We've also determined + # that both module builds depend(ed) on the same exact module builds. Now it's time + # to determine if the components before it have changed. + # + # Convert the component_builds to a list and sort them by batch + new_component_builds = list(module.component_builds) + new_component_builds.sort(key=lambda x: x.batch) + prev_component_builds = list(previous_module_build.component_builds) + prev_component_builds.sort(key=lambda x: x.batch) - log.info('Cannot re-use. Ordering or commit hashes of ' - 'previous batches differ.') - return None + new_module_build_components = [] + previous_module_build_components = [] + # Create separate lists for the new and previous module build. These lists + # will have an entry for every build batch *before* the component's + # batch except for 1, which is reserved for the module-build-macros RPM. + # Each batch entry will contain a set of "(name, ref)" with the name and + # ref (commit) of the component. + for i in range(new_module_build_component.batch - 1): + # This is the first batch which we want to skip since it will always + # contain only the module-build-macros RPM and it gets built every time + if i == 0: + continue + + new_module_build_components.append(set([ + (value.package, value.ref) for value in + new_component_builds if value.batch == i + 1 + ])) + + previous_module_build_components.append(set([ + (value.package, value.ref) for value in + prev_component_builds if value.batch == i + 1 + ])) + + # If the previous batches don't have the same ordering and hashes, then the + # component can't be reused + if previous_module_build_components != new_module_build_components: + log.info('Cannot re-use. Ordering or commit hashes of ' + 'previous batches differ.') + return None + + reusable_component = models.ComponentBuild.query.filter_by( + package=component_name, module_id=previous_module_build.id).one() + log.debug('Found reusable component!') + return reusable_component def validate_koji_tag(tag_arg_names, pre='', post='-', dict_key='name'): diff --git a/module_build_service/views.py b/module_build_service/views.py index 35f48f20..a99a0964 100644 --- a/module_build_service/views.py +++ b/module_build_service/views.py @@ -77,6 +77,12 @@ api_v1 = { 'options': { 'methods': ['GET'] } + }, + 'rebuild_strategies_list': { + 'url': '/module-build-service/1/rebuild-strategies/', + 'options': { + 'methods': ['GET'] + } } } @@ -203,6 +209,30 @@ class AboutAPI(MethodView): return jsonify(json), 200 +class RebuildStrategies(MethodView): + def get(self): + items = [] + # Sort the items list by name + for strategy in sorted(models.ModuleBuild.rebuild_strategies.keys()): + default = False + if strategy == conf.rebuild_strategy: + default = True + allowed = True + elif conf.rebuild_strategy_allow_override and \ + strategy in conf.rebuild_strategies_allowed: + allowed = True + else: + allowed = False + items.append({ + 'name': strategy, + 'description': models.ModuleBuild.rebuild_strategies[strategy], + 'allowed': allowed, + 'default': default + }) + + return jsonify({'items': items}), 200 + + class BaseHandler(object): def __init__(self, request): self.username, self.groups = module_build_service.auth.get_user(request) @@ -215,7 +245,7 @@ class BaseHandler(object): def validate_optional_params(self): forbidden_params = [k for k in self.data if k not in models.ModuleBuild.__table__.columns and - k not in ["branch"]] + k not in ["branch", "rebuild_strategy"]] if forbidden_params: raise ValidationError('The request contains unspecified parameters: {}' .format(", ".join(forbidden_params))) @@ -230,6 +260,17 @@ class BaseHandler(object): raise ValidationError(("The request contains 'owner' parameter," " however NO_AUTH is not allowed")) + if not conf.rebuild_strategy_allow_override and 'rebuild_strategy' in self.data: + raise ValidationError('The request contains the "rebuild_strategy" parameter but ' + 'overriding the default isn\'t allowed') + + if 'rebuild_strategy' in self.data: + if self.data['rebuild_strategy'] not in conf.rebuild_strategies_allowed: + raise ValidationError( + 'The rebuild method of "{0}" is not allowed. Chose from: {1}.' + .format(self.data['rebuild_strategy'], + ', '.join(conf.rebuild_strategies_allowed))) + class SCMHandler(BaseHandler): def __init__(self, request): @@ -298,6 +339,7 @@ def register_api_v1(): module_view = ModuleBuildAPI.as_view('module_builds') component_view = ComponentBuildAPI.as_view('component_builds') about_view = AboutAPI.as_view('about') + rebuild_strategies_view = RebuildStrategies.as_view('rebuild_strategies') for key, val in api_v1.items(): if key.startswith('component_build'): app.add_url_rule(val['url'], @@ -314,6 +356,11 @@ def register_api_v1(): endpoint=key, view_func=about_view, **val['options']) + elif key == 'rebuild_strategies_list': + app.add_url_rule(val['url'], + endpoint=key, + view_func=rebuild_strategies_view, + **val['options']) else: raise NotImplementedError("Unhandled api key.") diff --git a/tests/__init__.py b/tests/__init__.py index 54b71320..1577071f 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -47,11 +47,14 @@ def uncompress_vcrpy_cassette(): uncompress_vcrpy_cassette() -def init_data(): +def clean_database(): db.session.remove() db.drop_all() db.create_all() - db.session.commit() + + +def init_data(): + clean_database() for index in range(10): build_one = ModuleBuild() build_one.name = 'nginx' @@ -72,6 +75,7 @@ def init_data(): datetime(2016, 9, 3, 11, 25, 32) + timedelta(minutes=(index * 10)) build_one.time_completed = \ datetime(2016, 9, 3, 11, 25, 32) + timedelta(minutes=(index * 10)) + build_one.rebuild_strategy = 'changed-and-after' component_one_build_one = ComponentBuild() component_one_build_one.package = 'nginx' @@ -115,6 +119,7 @@ def init_data(): datetime(2016, 9, 3, 12, 27, 19) + timedelta(minutes=(index * 10)) build_two.time_completed = \ datetime(2016, 9, 3, 11, 27, 19) + timedelta(minutes=(index * 10)) + build_two.rebuild_strategy = 'changed-and-after' component_one_build_two = ComponentBuild() component_one_build_two.package = 'postgresql' @@ -157,6 +162,7 @@ def init_data(): build_three.time_modified = \ datetime(2016, 9, 3, 12, 28, 40) + timedelta(minutes=(index * 10)) build_three.time_completed = None + build_three.rebuild_strategy = 'changed-and-after' component_one_build_three = ComponentBuild() component_one_build_three.package = 'rubygem-rails' @@ -197,10 +203,7 @@ def init_data(): def scheduler_init_data(communicator_state=None): - db.session.remove() - db.drop_all() - db.create_all() - db.session.commit() + clean_database() current_dir = os.path.dirname(__file__) star_command_yml_path = os.path.join( @@ -222,6 +225,7 @@ def scheduler_init_data(communicator_state=None): build_one.owner = 'Buzz Lightyear' build_one.time_submitted = datetime(2016, 12, 9, 11, 23, 20) build_one.time_modified = datetime(2016, 12, 9, 11, 25, 32) + build_one.rebuild_strategy = 'changed-and-after' component_one_build_one = module_build_service.models.ComponentBuild() component_one_build_one.package = 'communicator' @@ -256,10 +260,7 @@ def scheduler_init_data(communicator_state=None): def test_reuse_component_init_data(): - db.session.remove() - db.drop_all() - db.create_all() - db.session.commit() + clean_database() current_dir = os.path.dirname(__file__) formatted_testmodule_yml_path = os.path.join( @@ -281,6 +282,7 @@ def test_reuse_component_init_data(): build_one.time_submitted = datetime(2017, 2, 15, 16, 8, 18) build_one.time_modified = datetime(2017, 2, 15, 16, 19, 35) build_one.time_completed = datetime(2017, 2, 15, 16, 19, 35) + build_one.rebuild_strategy = 'changed-and-after' component_one_build_one = module_build_service.models.ComponentBuild() component_one_build_one.package = 'perl-Tangerine' @@ -353,6 +355,7 @@ def test_reuse_component_init_data(): build_two.owner = 'Tom Brady' build_two.time_submitted = datetime(2017, 2, 19, 16, 8, 18) build_two.time_modified = datetime(2017, 2, 19, 16, 8, 18) + build_two.rebuild_strategy = 'changed-and-after' component_one_build_two = module_build_service.models.ComponentBuild() component_one_build_two.package = 'perl-Tangerine' @@ -412,10 +415,7 @@ def test_reuse_component_init_data(): def test_reuse_shared_userspace_init_data(): - db.session.remove() - db.drop_all() - db.create_all() - db.session.commit() + clean_database() with make_session(conf) as session: mmd = modulemd.ModuleMetadata() @@ -443,6 +443,7 @@ def test_reuse_shared_userspace_init_data(): build_one.time_submitted = datetime(2017, 2, 15, 16, 8, 18) build_one.time_modified = datetime(2017, 2, 15, 16, 19, 35) build_one.time_completed = datetime(2017, 2, 15, 16, 19, 35) + build_one.rebuild_strategy = 'changed-and-after' session.add(build_one) @@ -490,6 +491,7 @@ def test_reuse_shared_userspace_init_data(): build_one.time_submitted = datetime(2017, 2, 15, 16, 8, 18) build_one.time_modified = datetime(2017, 2, 15, 16, 19, 35) build_one.time_completed = datetime(2017, 2, 15, 16, 19, 35) + build_one.rebuild_strategy = 'changed-and-after' session.add(build_one) diff --git a/tests/test_models/__init__.py b/tests/test_models/__init__.py index 19df3e37..4585689a 100644 --- a/tests/test_models/__init__.py +++ b/tests/test_models/__init__.py @@ -26,7 +26,7 @@ import module_build_service import modulemd from datetime import datetime -from module_build_service import db +from tests import db, clean_database from module_build_service.config import init_config from module_build_service.models import ModuleBuild, BUILD_STATES @@ -53,13 +53,12 @@ def module_build_from_modulemd(yaml): build.time_submitted = datetime(2016, 9, 3, 12, 28, 33) build.time_modified = datetime(2016, 9, 3, 12, 28, 40) build.time_completed = None + build.rebuild_strategy = 'changed-and-after' return build def init_data(): - db.session.remove() - db.drop_all() - db.create_all() + clean_database() for filename in os.listdir(datadir): with open(datadir + filename, 'r') as f: yaml = f.read() diff --git a/tests/test_scheduler/test_module_init.py b/tests/test_scheduler/test_module_init.py index ec7bdb17..56d41675 100644 --- a/tests/test_scheduler/test_module_init.py +++ b/tests/test_scheduler/test_module_init.py @@ -25,7 +25,7 @@ import unittest from mock import patch, PropertyMock import vcr -from tests import conf, db +from tests import conf, db, clean_database from tests.test_views.test_views import FakeSCM import module_build_service.messaging import module_build_service.scheduler.handlers.modules @@ -46,10 +46,7 @@ class TestModuleInit(unittest.TestCase): with open(testmodule_yml_path, 'r') as f: yaml = f.read() scmurl = ('git://pkgs.domain.local/modules/testmodule?#da95886') - db.session.remove() - db.drop_all() - db.create_all() - db.session.commit() + clean_database() with make_session(conf) as session: ModuleBuild.create( session, conf, 'testmodule', '1', 3, yaml, scmurl, 'mprahl') diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 4af5a935..83f2d88e 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -813,6 +813,93 @@ class TestBatches(unittest.TestCase): self.assertIsNone(plc_component.reused_component_id) mock_sbc.assert_called_once() + @patch('module_build_service.utils.start_build_component') + @patch('module_build_service.config.Config.rebuild_strategy', + new_callable=mock.PropertyMock, return_value='all') + def test_start_next_batch_build_rebuild_strategy_all( + self, mock_rm, mock_sbc, default_buildroot_groups): + """ + Tests that start_next_batch_build can't reuse any components in the batch because the + rebuild method is set to "all". + """ + module_build = models.ModuleBuild.query.filter_by(id=2).one() + module_build.rebuild_strategy = 'all' + module_build.batch = 1 + + builder = mock.MagicMock() + further_work = module_build_service.utils.start_next_batch_build( + conf, module_build, db.session, builder) + + # Batch number should increase. + self.assertEqual(module_build.batch, 2) + # No component reuse messages should be returned + self.assertEqual(len(further_work), 0) + # Make sure that both components in the batch were submitted + self.assertEqual(len(mock_sbc.mock_calls), 2) + + @patch('module_build_service.utils.start_build_component') + @patch('module_build_service.config.Config.rebuild_strategy', + new_callable=mock.PropertyMock, return_value='only-changed') + def test_start_next_batch_build_rebuild_strategy_only_changed( + self, mock_rm, mock_sbc, default_buildroot_groups): + """ + Tests that start_next_batch_build reuses all unchanged components in the batch because the + rebuild method is set to "only-changed". This means that one component is reused in batch + 2, and even though the other component in batch 2 changed and was rebuilt, the component + in batch 3 can be reused. + """ + module_build = models.ModuleBuild.query.filter_by(id=2).one() + module_build.rebuild_strategy = 'only-changed' + module_build.batch = 1 + # perl-List-Compare changed + plc_component = models.ComponentBuild.query.filter_by( + module_id=2, package='perl-List-Compare').one() + plc_component.ref = '5ceea46add2366d8b8c5a623a2fb563b625b9abd' + + builder = mock.MagicMock() + further_work = module_build_service.utils.start_next_batch_build( + conf, module_build, db.session, builder) + + # Batch number should increase + self.assertEqual(module_build.batch, 2) + + # Make sure we only have one message returned for the one reused component + self.assertEqual(len(further_work), 1) + # The KojiBuildChange message in further_work should have build_new_state + # set to COMPLETE, but the current component build state in the DB should be set + # to BUILDING, so KojiBuildChange message handler handles the change + # properly. + self.assertEqual(further_work[0].build_new_state, koji.BUILD_STATES['COMPLETE']) + component_build = models.ComponentBuild.from_component_event(db.session, further_work[0]) + self.assertEqual(component_build.state, koji.BUILD_STATES['BUILDING']) + self.assertEqual(component_build.package, 'perl-Tangerine') + self.assertIsNotNone(component_build.reused_component_id) + # Make sure perl-List-Compare is set to the build state as well but not reused + self.assertEqual(plc_component.state, koji.BUILD_STATES['BUILDING']) + self.assertIsNone(plc_component.reused_component_id) + mock_sbc.assert_called_once() + mock_sbc.reset_mock() + + # Complete the build + plc_component.state = koji.BUILD_STATES['COMPLETE'] + pt_component = models.ComponentBuild.query.filter_by( + module_id=2, package='perl-Tangerine').one() + pt_component.state = koji.BUILD_STATES['COMPLETE'] + + # Start the next build batch + further_work = module_build_service.utils.start_next_batch_build( + conf, module_build, db.session, builder) + # Batch number should increase + self.assertEqual(module_build.batch, 3) + # Verify that tangerine was reused even though perl-Tangerine was rebuilt in the previous + # batch + self.assertEqual(further_work[0].build_new_state, koji.BUILD_STATES['COMPLETE']) + component_build = models.ComponentBuild.from_component_event(db.session, further_work[0]) + self.assertEqual(component_build.state, koji.BUILD_STATES['BUILDING']) + self.assertEqual(component_build.package, 'tangerine') + self.assertIsNotNone(component_build.reused_component_id) + mock_sbc.assert_not_called() + @patch('module_build_service.utils.start_build_component') def test_start_next_batch_build_smart_scheduling(self, mock_sbc, default_buildroot_groups): """ diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 3edc79f9..c2bb6435 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -153,6 +153,7 @@ class TestViews(unittest.TestCase): self.assertEquals(data['time_completed'], '2016-09-03T11:25:32Z') self.assertEquals(data['time_modified'], '2016-09-03T11:25:32Z') self.assertEquals(data['time_submitted'], '2016-09-03T11:23:20Z') + self.assertEqual(data['rebuild_strategy'], 'changed-and-after') def test_query_build_with_verbose_mode(self): rv = self.client.get('/module-build-service/1/module-builds/1?verbose=true') @@ -195,6 +196,7 @@ class TestViews(unittest.TestCase): self.assertEquals(data['time_modified'], u'2016-09-03T11:25:32Z') self.assertEquals(data['time_submitted'], u'2016-09-03T11:23:20Z') self.assertEquals(data['version'], '2') + self.assertEqual(data['rebuild_strategy'], 'changed-and-after') def test_pagination_metadata(self): rv = self.client.get('/module-build-service/1/module-builds/?per_page=8&page=2') @@ -231,6 +233,7 @@ class TestViews(unittest.TestCase): 'id': 30, 'koji_tag': None, 'name': 'testmodule', + 'rebuild_strategy': 'changed-and-after', 'owner': 'some_other_user', 'scmurl': ('git://pkgs.domain.local/modules/testmodule?' '#ca95886c7a443b36a9ce31abda1f9bef22f2f8c9'), @@ -259,11 +262,12 @@ class TestViews(unittest.TestCase): 'time_submitted': '2016-09-03T13:58:33Z', 'version': '6' }, - { + { 'id': 29, 'koji_tag': 'module-postgressql-1.2', 'name': 'postgressql', 'owner': 'some_user', + 'rebuild_strategy': 'changed-and-after', 'scmurl': ('git://pkgs.domain.local/modules/postgressql?' '#aa95886c7a443b36a9ce31abda1f9bef22f2f8c9'), 'state': 3, @@ -492,6 +496,7 @@ class TestViews(unittest.TestCase): self.assertEquals(data['stream'], 'master') self.assertEquals(data['owner'], 'Homer J. Simpson') self.assertEquals(data['id'], 31) + self.assertEquals(data['rebuild_strategy'], 'changed-and-after') self.assertEquals(data['state_name'], 'init') self.assertEquals(data['state_url'], '/module-build-service/1/module-builds/31') self.assertEquals(data['state_trace'], []) @@ -499,6 +504,65 @@ class TestViews(unittest.TestCase): mmd = _modulemd.ModuleMetadata() mmd.loads(data["modulemd"]) + @patch('module_build_service.auth.get_user', return_value=user) + @patch('module_build_service.scm.SCM') + @patch('module_build_service.config.Config.rebuild_strategy_allow_override', + new_callable=PropertyMock, return_value=True) + def test_submit_build_rebuild_strategy(self, mocked_rmao, mocked_scm, mocked_get_user): + FakeSCM(mocked_scm, 'testmodule', 'testmodule.yaml', + '620ec77321b2ea7b0d67d82992dda3e1d67055b4') + + rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( + {'branch': 'master', 'rebuild_strategy': 'only-changed', + 'scmurl': ('git://pkgs.stg.fedoraproject.org/modules/testmodule.git?' + '#68931c90de214d9d13feefbd35246a81b6cb8d49')})) + data = json.loads(rv.data) + self.assertEquals(data['rebuild_strategy'], 'only-changed') + + @patch('module_build_service.auth.get_user', return_value=user) + @patch('module_build_service.scm.SCM') + @patch('module_build_service.config.Config.rebuild_strategies_allowed', + new_callable=PropertyMock, return_value=['all']) + @patch('module_build_service.config.Config.rebuild_strategy_allow_override', + new_callable=PropertyMock, return_value=True) + def test_submit_build_rebuild_strategy_not_allowed(self, mock_rsao, mock_rsa, mocked_scm, + mocked_get_user): + FakeSCM(mocked_scm, 'testmodule', 'testmodule.yaml', + '620ec77321b2ea7b0d67d82992dda3e1d67055b4') + + rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( + {'branch': 'master', 'rebuild_strategy': 'only-changed', + 'scmurl': ('git://pkgs.stg.fedoraproject.org/modules/testmodule.git?' + '#68931c90de214d9d13feefbd35246a81b6cb8d49')})) + data = json.loads(rv.data) + self.assertEqual(rv.status_code, 400) + expected_error = { + 'error': 'Bad Request', + 'message': ('The rebuild method of "only-changed" is not allowed. Chose from: all.'), + 'status': 400 + } + self.assertEqual(data, expected_error) + + @patch('module_build_service.auth.get_user', return_value=user) + @patch('module_build_service.scm.SCM') + def test_submit_build_rebuild_strategy_override_not_allowed(self, mocked_scm, mocked_get_user): + FakeSCM(mocked_scm, 'testmodule', 'testmodule.yaml', + '620ec77321b2ea7b0d67d82992dda3e1d67055b4') + + rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( + {'branch': 'master', 'rebuild_strategy': 'only-changed', + 'scmurl': ('git://pkgs.stg.fedoraproject.org/modules/testmodule.git?' + '#68931c90de214d9d13feefbd35246a81b6cb8d49')})) + data = json.loads(rv.data) + self.assertEqual(rv.status_code, 400) + expected_error = { + 'error': 'Bad Request', + 'message': ('The request contains the "rebuild_strategy" parameter but overriding ' + 'the default isn\'t allowed'), + 'status': 400 + } + self.assertEqual(data, expected_error) + @patch('module_build_service.auth.get_user', return_value=user) @patch('module_build_service.scm.SCM') def test_submit_componentless_build(self, mocked_scm, mocked_get_user): @@ -523,6 +587,7 @@ class TestViews(unittest.TestCase): self.assertEquals(data['owner'], 'Homer J. Simpson') self.assertEquals(data['id'], 31) self.assertEquals(data['state_name'], 'init') + self.assertEquals(data['rebuild_strategy'], 'changed-and-after') def test_submit_build_auth_error(self): base_dir = path.abspath(path.dirname(__file__)) @@ -820,3 +885,95 @@ class TestViews(unittest.TestCase): data = json.loads(rv.data) self.assertEqual(rv.status_code, 200) self.assertEquals(data, {'auth_method': 'kerberos', 'version': version}) + + def test_rebuild_strategy_api(self): + rv = self.client.get('/module-build-service/1/rebuild-strategies/') + data = json.loads(rv.data) + self.assertEqual(rv.status_code, 200) + expected = { + 'items': [ + { + 'allowed': False, + 'default': False, + 'description': 'All components will be rebuilt', + 'name': 'all' + }, + { + 'allowed': True, + 'default': True, + 'description': ('All components that have changed and those in subsequent ' + 'batches will be rebuilt'), + 'name': 'changed-and-after' + }, + { + 'allowed': False, + 'default': False, + 'description': 'All changed components will be rebuilt', + 'name': 'only-changed' + } + ] + } + self.assertEquals(data, expected) + + def test_rebuild_strategy_api_only_changed_default(self): + with patch.object(mbs_config.Config, 'rebuild_strategy', new_callable=PropertyMock) as r_s: + r_s.return_value = 'only-changed' + rv = self.client.get('/module-build-service/1/rebuild-strategies/') + data = json.loads(rv.data) + self.assertEqual(rv.status_code, 200) + expected = { + 'items': [ + { + 'allowed': False, + 'default': False, + 'description': 'All components will be rebuilt', + 'name': 'all' + }, + { + 'allowed': False, + 'default': False, + 'description': ('All components that have changed and those in subsequent ' + 'batches will be rebuilt'), + 'name': 'changed-and-after' + }, + { + 'allowed': True, + 'default': True, + 'description': 'All changed components will be rebuilt', + 'name': 'only-changed' + } + ] + } + self.assertEquals(data, expected) + + def test_rebuild_strategy_api_override_allowed(self): + with patch.object(mbs_config.Config, 'rebuild_strategy_allow_override', + new_callable=PropertyMock) as rsao: + rsao.return_value = True + rv = self.client.get('/module-build-service/1/rebuild-strategies/') + data = json.loads(rv.data) + self.assertEqual(rv.status_code, 200) + expected = { + 'items': [ + { + 'allowed': True, + 'default': False, + 'description': 'All components will be rebuilt', + 'name': 'all' + }, + { + 'allowed': True, + 'default': True, + 'description': ('All components that have changed and those in subsequent ' + 'batches will be rebuilt'), + 'name': 'changed-and-after' + }, + { + 'allowed': True, + 'default': False, + 'description': 'All changed components will be rebuilt', + 'name': 'only-changed' + } + ] + } + self.assertEquals(data, expected)