Module scratch build fixups per PR review feedback:

- Keep scratch module builds in the 'done' state.
- Make koji tagging for scratch modules unique so the same
  commit can be resubmitted.
- Use alternate prefix for scratch module build components so they can
  be identified later.
- Prevent scratch build components from being reused.
- Assorted code and comment cleanup.

Signed-off-by: Merlin Mathesius <mmathesi@redhat.com>
This commit is contained in:
Merlin Mathesius
2019-03-01 10:22:37 -06:00
parent 6daa4d3776
commit 152419f376
13 changed files with 82 additions and 52 deletions

View File

@@ -1232,7 +1232,8 @@ chmod 644 %buildroot/etc/rpm/macros.zz-modules
tags = []
koji_tags = session.listTags(rpm_md["build_id"])
for t in koji_tags:
if not t["name"].endswith("-build") and t["name"].startswith(("module-", "scrmod-")):
if (not t["name"].endswith("-build") and
t["name"].startswith(tuple(conf.koji_tag_prefixes))):
tags.append(t["name"])
return tags

View File

@@ -318,7 +318,6 @@ class MockModuleBuilder(GenericBuilder):
# module build repository.
if tag.startswith(conf.mock_resultsdir):
repo_name = os.path.basename(tag)
# TODO scrmod: is a check also needed for scratch module tag prefix?
if repo_name.startswith("module-"):
repo_name = repo_name[7:]
repo_dir = tag

View File

@@ -15,14 +15,11 @@ import sqlalchemy as sa
def upgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.add_column('module_builds', sa.Column('scratch', sa.Boolean(), nullable=True))
op.add_column('module_builds', sa.Column('srpms', sa.String(), nullable=True))
# ### end Alembic commands ###
def downgrade():
# ### commands auto generated by Alembic - please adjust! ###
op.drop_column('module_builds', 'srpms')
op.drop_column('module_builds', 'scratch')
# ### end Alembic commands ###
with op.batch_alter_table('module_builds') as b:
b.drop_column('srpms')
b.drop_column('scratch')

View File

@@ -27,7 +27,7 @@
"""
import contextlib
from json import dumps, loads
import json
from collections import OrderedDict
from datetime import datetime
import hashlib
@@ -448,13 +448,13 @@ class ModuleBuild(MBSBase):
raise ValueError('The module\'s modulemd hasn\'t been formatted by MBS')
mmd_formatted_buildrequires = {
dep: info['ref'] for dep, info in mbs_xmd["buildrequires"].items()}
property_json = dumps(OrderedDict(sorted(mmd_formatted_buildrequires.items())))
property_json = json.dumps(OrderedDict(sorted(mmd_formatted_buildrequires.items())))
rv.append(hashlib.sha1(property_json.encode('utf-8')).hexdigest())
# Get the streams of buildrequires and hash it.
mmd_formatted_buildrequires = {
dep: info['stream'] for dep, info in mbs_xmd["buildrequires"].items()}
property_json = dumps(OrderedDict(sorted(mmd_formatted_buildrequires.items())))
property_json = json.dumps(OrderedDict(sorted(mmd_formatted_buildrequires.items())))
build_context = hashlib.sha1(property_json.encode('utf-8')).hexdigest()
rv.append(build_context)
@@ -469,7 +469,7 @@ class ModuleBuild(MBSBase):
# Sort the streams for each module name and also sort the module names.
mmd_requires = {
dep: sorted(list(streams)) for dep, streams in mmd_requires.items()}
property_json = dumps(OrderedDict(sorted(mmd_requires.items())))
property_json = json.dumps(OrderedDict(sorted(mmd_requires.items())))
runtime_context = hashlib.sha1(property_json.encode('utf-8')).hexdigest()
rv.append(runtime_context)
@@ -488,7 +488,8 @@ class ModuleBuild(MBSBase):
@classmethod
def create(cls, session, conf, name, stream, version, modulemd, scmurl, username,
context=None, rebuild_strategy=None, scratch=False, srpms=[], publish_msg=True):
context=None, rebuild_strategy=None, scratch=False, srpms=None,
publish_msg=True, **kwargs):
now = datetime.utcnow()
module = cls(
name=name,
@@ -504,7 +505,8 @@ class ModuleBuild(MBSBase):
# If the rebuild_strategy isn't specified, use the default
rebuild_strategy=rebuild_strategy or conf.rebuild_strategy,
scratch=scratch,
srpms=dumps(srpms) if srpms else '[]',
srpms=json.dumps(srpms or []),
**kwargs
)
# Add a state transition to "init"
mbt = ModuleBuildTrace(state_time=now, state=module.state)
@@ -641,15 +643,15 @@ class ModuleBuild(MBSBase):
buildrequires = xmd['mbs']['buildrequires']
except KeyError:
buildrequires = {}
json = self.short_json()
json.update({
rv = self.short_json()
rv.update({
'component_builds': [build.id for build in self.component_builds],
'koji_tag': self.koji_tag,
'owner': self.owner,
'rebuild_strategy': self.rebuild_strategy,
'scmurl': self.scmurl,
'scratch': self.scratch,
'srpms': loads(self.srpms) if self.srpms else [],
'srpms': json.loads(self.srpms or '[]'),
'siblings': self.siblings,
'state_reason': self.state_reason,
'time_completed': _utc_datetime_to_iso(self.time_completed),
@@ -658,8 +660,8 @@ class ModuleBuild(MBSBase):
'buildrequires': buildrequires,
})
if show_tasks:
json['tasks'] = self.tasks()
return json
rv['tasks'] = self.tasks()
return rv
def extended_json(self, show_state_url=False, api_version=1):
"""
@@ -669,12 +671,12 @@ class ModuleBuild(MBSBase):
SQLAlchemy sessions.
:kwarg api_version: the API version to use when building the state URL
"""
json = self.json(show_tasks=True)
rv = self.json(show_tasks=True)
state_url = None
if show_state_url:
state_url = get_url_for('module_build', api_version=api_version, id=self.id)
json.update({
rv.update({
'base_module_buildrequires': [br.short_json(True) for br in self.buildrequires],
'build_context': self.build_context,
'modulemd': self.modulemd,
@@ -691,7 +693,7 @@ class ModuleBuild(MBSBase):
'state_url': state_url,
})
return json
return rv
def tasks(self):
"""

View File

@@ -62,7 +62,6 @@ def make_simple_stop_condition(session):
done = (
module_build_service.models.BUILD_STATES["failed"],
module_build_service.models.BUILD_STATES["ready"],
# XXX should this one be removed?
module_build_service.models.BUILD_STATES["done"],
)
result = module.state in done

View File

@@ -116,6 +116,7 @@ def done(config, session, msg):
"""Called whenever a module enters the 'done' state.
We currently don't do anything useful, so moving to ready.
Except for scratch module builds, which remain in the done state.
Otherwise the done -> ready state should happen when all
dependent modules were re-built, at least that's the current plan.
"""
@@ -128,8 +129,10 @@ def done(config, session, msg):
# This is ok.. it's a race condition we can ignore.
pass
build.transition(config, state="ready")
session.commit()
# Scratch builds stay in 'done' state, otherwise move to 'ready'
if not build.scratch:
build.transition(config, state="ready")
session.commit()
build_logs.stop(build)
module_build_service.builder.GenericBuilder.clear_cache(build)
@@ -184,7 +187,7 @@ def generate_module_build_koji_tag(build):
log.info('Getting tag for %s:%s:%s', build.name, build.stream, build.version)
if conf.system in ['koji', 'test']:
return generate_koji_tag(build.name, build.stream, build.version, build.context,
scratch=build.scratch)
scratch=build.scratch, scratch_id=build.id)
else:
return '-'.join(['module', build.name, build.stream, build.version])

View File

@@ -105,7 +105,7 @@ def module_build_state_from_msg(msg):
return state
def generate_koji_tag(name, stream, version, context, max_length=256, scratch=False):
def generate_koji_tag(name, stream, version, context, max_length=256, scratch=False, scratch_id=0):
"""Generate a koji tag for a module
Generally, a module's koji tag is in format ``module-N-S-V-C``. However, if
@@ -120,18 +120,24 @@ def generate_koji_tag(name, stream, version, context, max_length=256, scratch=Fa
characters, which is the maximum length of a tag Koji accepts.
:param bool scratch: a flag indicating if the generated tag will be for
a scratch module build
:param int scratch_id: for scratch module builds, a unique build identifier
:return: a Koji tag
:rtype: str
"""
prefix = ('scrmod' if scratch else 'module')
# TODO scrmod: is a unique _suffix_ needed here, too?
if scratch:
prefix = 'scrmod-'
# use unique suffix so same commit can be resubmitted
suffix = '+' + str(scratch_id)
else:
prefix = 'module-'
suffix = ''
nsvc_list = [name, stream, str(version), context]
nsvc_tag = '-'.join([prefix] + nsvc_list)
nsvc_tag = prefix + '-'.join(nsvc_list) + suffix
if len(nsvc_tag) + len('-build') > max_length:
# Fallback to the old format of 'module-<hash>' if the generated koji tag
# name is longer than max_length
nsvc_hash = hashlib.sha1('.'.join(nsvc_list).encode('utf-8')).hexdigest()[:16]
return prefix + '-' + nsvc_hash
return prefix + nsvc_hash + suffix
return nsvc_tag
@@ -211,9 +217,6 @@ def validate_koji_tag(tag_arg_names, pre='', post='-', dict_key='name'):
return validation_decorator
# TODO scrmod: scratch module build components need a unique dist tag prefix/suffix
# to distinguish them and make it possible to build the same NSVC multiple times,
# but what should it be? Is this the place to set that?
def get_rpm_release(module_build):
"""
Generates the dist tag for the specified module
@@ -248,8 +251,11 @@ def get_rpm_release(module_build):
log.warning('Module build {0} does not buildrequire a base module ({1})'
.format(module_build.id, ' or '.join(conf.base_module_names)))
# use alternate prefix for scratch module build components so they can be identified
prefix = ('scrmod+' if module_build.scratch else conf.default_dist_tag_prefix)
return '{prefix}{base_module_stream}{index}+{dist_hash}'.format(
prefix=conf.default_dist_tag_prefix,
prefix=prefix,
base_module_stream=base_module_stream,
index=index,
dist_hash=dist_hash,

View File

@@ -93,7 +93,7 @@ def get_reusable_module(session, module):
previous_module_build = session.query(models.ModuleBuild)\
.filter_by(name=mmd.get_name())\
.filter_by(stream=mmd.get_stream())\
.filter(models.ModuleBuild.state.in_([3, 5]))\
.filter_by(state=models.BUILD_STATES["ready"])\
.filter(models.ModuleBuild.scmurl.isnot(None))\
.filter_by(build_context=module.build_context)\
.order_by(models.ModuleBuild.time_completed.desc())

View File

@@ -163,10 +163,6 @@ def format_mmd(mmd, scmurl, module=None, session=None):
if 'rpms' not in xmd['mbs']:
xmd['mbs']['rpms'] = {}
# Add missing data in RPM components
# TODO scrmod: Does something need to be done here for RPMs that are
# overridden by custom SRPMs in addition to what is currently being
# done in record_component_builds()? Should repository and cache
# properties be set? If so, to what?
for pkgname, pkg in mmd.get_rpm_components().items():
# In case of resubmit of existing module which have been
# cancelled/failed during the init state, the package
@@ -317,6 +313,7 @@ def merge_included_mmd(mmd, included_mmd):
# Set the modified xmd back to the modulemd
mmd.set_xmd(glib.dict_values(xmd))
def get_module_srpm_overrides(module):
"""
Make necessary preparations to use any provided custom SRPMs.
@@ -366,6 +363,7 @@ def get_module_srpm_overrides(module):
return overrides
def record_component_builds(mmd, module, initial_batch=1,
previous_buildorder=None, main_mmd=None, session=None):
# Imported here to allow import of utils in GenericBuilder.

View File

@@ -395,9 +395,6 @@ class BaseHandler(object):
class SCMHandler(BaseHandler):
def __init__(self, request):
super(SCMHandler, self).__init__(request)
def validate(self, skip_branch=False, skip_optional_params=False):
if "scmurl" not in self.data:
log.error('Missing scmurl')

View File

@@ -110,7 +110,7 @@ def clean_database(add_platform_module=True):
import_mmd(db.session, mmd)
def init_data(data_size=10, contexts=False, multiple_stream_versions=False):
def init_data(data_size=10, contexts=False, multiple_stream_versions=False, scratch=False):
"""
Creates data_size * 3 modules in database in different states and
with different component builds. See _populate_data for more info.
@@ -132,10 +132,10 @@ def init_data(data_size=10, contexts=False, multiple_stream_versions=False):
mmd.set_stream(stream)
import_mmd(db.session, mmd)
with make_session(conf) as session:
_populate_data(session, data_size, contexts=contexts)
_populate_data(session, data_size, contexts=contexts, scratch=scratch)
def _populate_data(session, data_size=10, contexts=False):
def _populate_data(session, data_size=10, contexts=False, scratch=False):
num_contexts = 2 if contexts else 1
for index in range(data_size):
for context in range(num_contexts):
@@ -144,6 +144,7 @@ def _populate_data(session, data_size=10, contexts=False):
build_one.stream = '1'
build_one.version = 2 + index
build_one.state = BUILD_STATES['ready']
build_one.scratch = scratch
if contexts:
build_one.stream = str(index)
unique_hash = hashlib.sha1(("%s:%s:%d:%d" % (
@@ -155,7 +156,10 @@ def _populate_data(session, data_size=10, contexts=False):
combined_hashes = '{0}:{1}'.format(unique_hash, unique_hash)
build_one.context = hashlib.sha1(combined_hashes.encode("utf-8")).hexdigest()[:8]
build_one.modulemd = read_staged_data('nginx_mmd')
build_one.koji_tag = 'module-nginx-1.2'
if scratch:
build_one.koji_tag = 'scrmod-nginx-1.2'
else:
build_one.koji_tag = 'module-nginx-1.2'
build_one.scmurl = ('git://pkgs.domain.local/modules/nginx?'
'#ba95886c7a443b36a9ce31abda1f9bef22f2f8c9')
build_one.batch = 2
@@ -206,8 +210,12 @@ def _populate_data(session, data_size=10, contexts=False):
build_two.stream = '1'
build_two.version = 2 + index
build_two.state = BUILD_STATES['done']
build_two.scratch = scratch
build_two.modulemd = read_staged_data('testmodule')
build_two.koji_tag = 'module-postgressql-1.2'
if scratch:
build_two.koji_tag = 'scrmod-postgressql-1.2'
else:
build_two.koji_tag = 'module-postgressql-1.2'
build_two.scmurl = ('git://pkgs.domain.local/modules/postgressql?'
'#aa95886c7a443b36a9ce31abda1f9bef22f2f8c9')
build_two.batch = 2
@@ -257,6 +265,7 @@ def _populate_data(session, data_size=10, contexts=False):
build_three.stream = '4.3.43'
build_three.version = 6 + index
build_three.state = BUILD_STATES['wait']
build_three.scratch = scratch
build_three.modulemd = read_staged_data('testmodule')
build_three.koji_tag = None
build_three.scmurl = ('git://pkgs.domain.local/modules/testmodule?'
@@ -310,7 +319,7 @@ def _populate_data(session, data_size=10, contexts=False):
session.commit()
def scheduler_init_data(tangerine_state=None):
def scheduler_init_data(tangerine_state=None, scratch=False):
""" Creates a testmodule in the building state with all the components in the same batch
"""
clean_database()
@@ -329,10 +338,14 @@ def scheduler_init_data(tangerine_state=None):
build_one.stream = 'master'
build_one.version = 20170109091357
build_one.state = BUILD_STATES['build']
build_one.scratch = scratch
build_one.build_context = 'ac4de1c346dcf09ce77d38cd4e75094ec1c08eb0'
build_one.runtime_context = 'ac4de1c346dcf09ce77d38cd4e75094ec1c08eb0'
build_one.context = '7c29193d'
build_one.koji_tag = 'module-testmodule-master-20170109091357-7c29193d'
if scratch:
build_one.koji_tag = 'scrmod-testmodule-master-20170109091357-7c29193d'
else:
build_one.koji_tag = 'module-testmodule-master-20170109091357-7c29193d'
build_one.scmurl = 'https://src.stg.fedoraproject.org/modules/testmodule.git?#ff1ea79'
if tangerine_state:
build_one.batch = 3

View File

@@ -336,6 +336,21 @@ class TestUtils:
release = module_build_service.utils.get_rpm_release(build_one)
assert release == 'module+f28+2+814cfa39'
def test_get_rpm_release_mse_scratch(self):
init_data(contexts=True, scratch=True)
build_one = models.ModuleBuild.query.get(2)
build_two = models.ModuleBuild.query.get(3)
release_one = module_build_service.utils.get_rpm_release(build_one)
release_two = module_build_service.utils.get_rpm_release(build_two)
assert release_one == "scrmod+2+b8645bbb"
assert release_two == "scrmod+2+17e35784"
def test_get_rpm_release_platform_stream_scratch(self):
scheduler_init_data(1, scratch=True)
build_one = models.ModuleBuild.query.get(2)
release = module_build_service.utils.get_rpm_release(build_one)
assert release == 'scrmod+f28+2+814cfa39'
@pytest.mark.parametrize('scmurl', [
('https://src.stg.fedoraproject.org/modules/testmodule.git'
'?#620ec77321b2ea7b0d67d82992dda3e1d67055b4'),

View File

@@ -1795,10 +1795,10 @@ class TestViews:
assert data['component_builds'] == []
assert data['name'] == 'testmodule'
assert data['scmurl'] is None
# TODO scrmod: assert data['modulemd'] == ???
# generated modulemd is nondeterministic, so just make sure it is set
assert data['modulemd'] is not None
assert data['scratch'] is True
# TODO scrmod: assert data['version'] == ???
# generated version is nondeterministic, so just make sure it is set
assert data['version'] is not None
assert data['time_submitted'] is not None
assert data['time_modified'] is not None