From 298bf63120fa04a2e6737da21a407a23228f90de Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Fri, 10 Mar 2017 13:31:08 +0100 Subject: [PATCH 1/2] Fix #409 - Remove koji_tag_inherit_from and whitelist all the components we are going to build instead. --- module_build_service/builder.py | 51 ++++++++++++------- module_build_service/config.py | 4 -- .../scheduler/handlers/components.py | 3 +- .../scheduler/handlers/modules.py | 6 ++- .../scheduler/handlers/repos.py | 2 +- tests/test_build/test_build.py | 2 +- tests/test_builder/test_koji.py | 6 ++- tests/test_scheduler/test_module_wait.py | 1 + 8 files changed, 45 insertions(+), 30 deletions(-) diff --git a/module_build_service/builder.py b/module_build_service/builder.py index 151e72ce..03d211e9 100644 --- a/module_build_service/builder.py +++ b/module_build_service/builder.py @@ -319,7 +319,7 @@ class KojiModuleBuilder(GenericBuilder): _build_lock = threading.Lock() @module_build_service.utils.validate_koji_tag('tag_name') - def __init__(self, owner, module, config, tag_name): + def __init__(self, owner, module, config, tag_name, **kwargs): """ :param owner: a string representing who kicked off the builds :param module: string representing module @@ -345,6 +345,7 @@ class KojiModuleBuilder(GenericBuilder): self.module_target = None # A koji target dict self.build_priority = config.koji_build_priority + self.components = kwargs["components"] if "components" in kwargs else [] def __repr__(self): return "" % ( @@ -495,6 +496,8 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules self.module_build_tag = self._koji_create_tag( self.tag_name + "-build", self.arches, perm="admin") + self._koji_whitelist_packages(self.components) + @module_build_service.utils.retry(wait_on=SysCallError, interval=5) def add_groups(): return self._koji_add_groups_to_tag( @@ -766,8 +769,7 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules taginfo = self.koji_session.getTag(tag_name) if not taginfo: - self.koji_session.createTag( - tag_name, parent=conf.koji_tag_inherit_from) + self.koji_session.createTag(tag_name) taginfo = self._get_tag(tag_name) opts = {} @@ -806,23 +808,34 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules user = self.koji_session.getLoggedInUser()['name'] return user - def _koji_whitelist_packages(self, packages): + def _koji_whitelist_packages(self, packages, tags = None): + if not tags: + tags = [self.module_tag, self.module_build_tag] + + # TODO: This has to be done per-package or just without the need + # to pass the `packages` to it depending on the result of + # issue #337. + owner = self._get_component_owner(packages[0]) + if not self.koji_session.getUser(owner): + raise ValueError("Unknown user %s" % owner) + # This will help with potential resubmiting of failed builds - pkglist = dict([(p['package_name'], p['package_id']) for p in self.koji_session.listPackages(tagID=self.module_tag['id'])]) - to_add = [] - for package in packages: - package_id = pkglist.get(package, None) - if not package_id is None: - log.debug("%s Package %s is already whitelisted." % (self, package)) - continue - to_add.append(package) + pkglists = {} + for tag in tags: + pkglists[tag['id']] = dict([(p['package_name'], p['package_id']) for p in self.koji_session.listPackages(tagID=tag['id'])]) - for package in to_add: - owner = self._get_component_owner(package) - if not self.koji_session.getUser(owner): - raise ValueError("Unknown user %s" % owner) + self.koji_session.multicall = True + for tag in tags: + pkglist = pkglists[tag['id']] + to_add = [] + for package in packages: + package_id = pkglist.get(package, None) + if not package_id is None: + log.debug("%s Package %s is already whitelisted." % (self, package)) + continue - self.koji_session.packageListAdd(self.module_tag['name'], package, owner) + self.koji_session.packageListAdd(tag['name'], package, owner) + self.koji_session.multiCall(strict=True) @module_build_service.utils.validate_koji_tag(['build_tag', 'dest_tag']) def _koji_add_target(self, name, build_tag, dest_tag): @@ -906,7 +919,7 @@ class CoprModuleBuilder(GenericBuilder): _build_lock = threading.Lock() @module_build_service.utils.validate_koji_tag('tag_name') - def __init__(self, owner, module, config, tag_name): + def __init__(self, owner, module, config, tag_name, **kwargs): self.owner = owner self.config = config self.tag_name = tag_name @@ -1181,7 +1194,7 @@ mdpolicy=group:primary """ @module_build_service.utils.validate_koji_tag('tag_name') - def __init__(self, owner, module, config, tag_name): + def __init__(self, owner, module, config, tag_name, **kwargs): self.module_str = module self.tag_name = tag_name self.config = config diff --git a/module_build_service/config.py b/module_build_service/config.py index f2238efa..a4e4f899 100644 --- a/module_build_service/config.py +++ b/module_build_service/config.py @@ -167,10 +167,6 @@ class Config(object): 'type': str, 'default': '', 'desc': 'Target to build "module-build-macros" RPM in.'}, - 'koji_tag_inherit_from': { - 'type': str, - 'default': 'module-package-list', - 'desc': 'Tag that new module tags inherit from.'}, 'koji_tag_prefixes': { 'type': list, 'default': ['module'], diff --git a/module_build_service/scheduler/handlers/components.py b/module_build_service/scheduler/handlers/components.py index ab32ec77..c59f5a6a 100644 --- a/module_build_service/scheduler/handlers/components.py +++ b/module_build_service/scheduler/handlers/components.py @@ -79,7 +79,8 @@ def _finalize(config, session, msg, state): module_name = parent.name tag = parent.koji_tag builder = module_build_service.builder.GenericBuilder.create( - parent.owner, module_name, config.system, config, tag_name=tag) + parent.owner, module_name, config.system, config, tag_name=tag, + components=[c.package for c in parent.component_builds]) groups = module_build_service.builder.GenericBuilder.default_buildroot_groups( session, parent) diff --git a/module_build_service/scheduler/handlers/modules.py b/module_build_service/scheduler/handlers/modules.py index 3c656987..ddbbce65 100644 --- a/module_build_service/scheduler/handlers/modules.py +++ b/module_build_service/scheduler/handlers/modules.py @@ -79,7 +79,8 @@ def failed(config, session, msg): if build.koji_tag: builder = module_build_service.builder.GenericBuilder.create( - build.owner, build.name, config.system, config, tag_name=build.koji_tag) + build.owner, build.name, config.system, config, tag_name=build.koji_tag, + components=[c.package for c in build.component_builds]) builder.buildroot_connect(groups) for component in unbuilt_components: @@ -222,7 +223,8 @@ def wait(config, session, msg): build.koji_tag = tag builder = module_build_service.builder.GenericBuilder.create( - build.owner, build.name, config.system, config, tag_name=tag) + build.owner, build.name, config.system, config, tag_name=tag, + components=[c.package for c in build.component_builds]) builder.buildroot_connect(groups) log.debug("Adding dependencies %s into buildroot for module %s" % (dependencies, module_info)) builder.buildroot_add_repos(dependencies) diff --git a/module_build_service/scheduler/handlers/repos.py b/module_build_service/scheduler/handlers/repos.py index 57c97ee3..9bdfb5ab 100644 --- a/module_build_service/scheduler/handlers/repos.py +++ b/module_build_service/scheduler/handlers/repos.py @@ -84,7 +84,7 @@ def done(config, session, msg): builder = module_build_service.builder.GenericBuilder.create( module_build.owner, module_build.name, config.system, config, - tag_name=tag) + tag_name=tag, components=[c.package for c in module_build.component_builds]) builder.buildroot_connect(groups) # Ok, for the subset of builds that did complete successfully, check to diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index 7d879fe3..f32c3bc3 100644 --- a/tests/test_build/test_build.py +++ b/tests/test_build/test_build.py @@ -92,7 +92,7 @@ class TestModuleBuilder(GenericBuilder): on_tag_artifacts_cb = None @module_build_service.utils.validate_koji_tag('tag_name') - def __init__(self, owner, module, config, tag_name): + def __init__(self, owner, module, config, tag_name, **kwargs): self.module_str = module self.tag_name = tag_name self.config = config diff --git a/tests/test_builder/test_koji.py b/tests/test_builder/test_koji.py index e056bea0..e779f9d8 100644 --- a/tests/test_builder/test_koji.py +++ b/tests/test_builder/test_koji.py @@ -65,7 +65,8 @@ class TestKojiBuilder(unittest.TestCase): fake_kmb = FakeKojiModuleBuilder(owner='Moe Szyslak', module='nginx', config=conf, - tag_name='module-nginx-1.2') + tag_name='module-nginx-1.2', + components=[]) fake_kmb.module_target = {'build_tag': 'module-fake_tag'} with self.assertRaises(IOError): @@ -88,7 +89,8 @@ class TestGetKojiClientSession(unittest.TestCase): KojiModuleBuilder(owner=self.owner, module=self.module, config=self.config, - tag_name=self.tag_name) + tag_name=self.tag_name, + components=[]) args, kwargs = mocked_krb_login.call_args self.assertTrue(set([('proxyuser', self.owner)]).issubset(set(kwargs.items()))) diff --git a/tests/test_scheduler/test_module_wait.py b/tests/test_scheduler/test_module_wait.py index e3305afe..cd6563db 100644 --- a/tests/test_scheduler/test_module_wait.py +++ b/tests/test_scheduler/test_module_wait.py @@ -70,6 +70,7 @@ class TestModuleWait(unittest.TestCase): mmd.loads(f) mocked_module_build.mmd.return_value = mmd + mocked_module_build.component_builds = [] from_module_event.return_value = mocked_module_build From 82edbab2b48ffa406d03a1f90b7e92bed8257c42 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Tue, 14 Mar 2017 12:12:53 +0100 Subject: [PATCH 2/2] Use real 'components' arg instead of **kwargs. --- module_build_service/builder.py | 15 +++++++-------- tests/test_build/test_build.py | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/module_build_service/builder.py b/module_build_service/builder.py index 03d211e9..11ebb3ad 100644 --- a/module_build_service/builder.py +++ b/module_build_service/builder.py @@ -319,7 +319,7 @@ class KojiModuleBuilder(GenericBuilder): _build_lock = threading.Lock() @module_build_service.utils.validate_koji_tag('tag_name') - def __init__(self, owner, module, config, tag_name, **kwargs): + def __init__(self, owner, module, config, tag_name, components): """ :param owner: a string representing who kicked off the builds :param module: string representing module @@ -345,7 +345,7 @@ class KojiModuleBuilder(GenericBuilder): self.module_target = None # A koji target dict self.build_priority = config.koji_build_priority - self.components = kwargs["components"] if "components" in kwargs else [] + self.components = components def __repr__(self): return "" % ( @@ -806,6 +806,8 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules def _get_component_owner(self, package): user = self.koji_session.getLoggedInUser()['name'] + if not self.koji_session.getUser(user): + raise ValueError("Unknown user %s" % user) return user def _koji_whitelist_packages(self, packages, tags = None): @@ -816,8 +818,6 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules # to pass the `packages` to it depending on the result of # issue #337. owner = self._get_component_owner(packages[0]) - if not self.koji_session.getUser(owner): - raise ValueError("Unknown user %s" % owner) # This will help with potential resubmiting of failed builds pkglists = {} @@ -829,8 +829,7 @@ chmod 644 %buildroot/%_rpmconfigdir/macros.d/macros.modules pkglist = pkglists[tag['id']] to_add = [] for package in packages: - package_id = pkglist.get(package, None) - if not package_id is None: + if pkglist.get(package, None): log.debug("%s Package %s is already whitelisted." % (self, package)) continue @@ -919,7 +918,7 @@ class CoprModuleBuilder(GenericBuilder): _build_lock = threading.Lock() @module_build_service.utils.validate_koji_tag('tag_name') - def __init__(self, owner, module, config, tag_name, **kwargs): + def __init__(self, owner, module, config, tag_name, components): self.owner = owner self.config = config self.tag_name = tag_name @@ -1194,7 +1193,7 @@ mdpolicy=group:primary """ @module_build_service.utils.validate_koji_tag('tag_name') - def __init__(self, owner, module, config, tag_name, **kwargs): + def __init__(self, owner, module, config, tag_name, components): self.module_str = module self.tag_name = tag_name self.config = config diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index f32c3bc3..67640a00 100644 --- a/tests/test_build/test_build.py +++ b/tests/test_build/test_build.py @@ -92,7 +92,7 @@ class TestModuleBuilder(GenericBuilder): on_tag_artifacts_cb = None @module_build_service.utils.validate_koji_tag('tag_name') - def __init__(self, owner, module, config, tag_name, **kwargs): + def __init__(self, owner, module, config, tag_name, components): self.module_str = module self.tag_name = tag_name self.config = config