From 829a569dc82b802443f14c1a6c206ed0ce95a8ba Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Sep 26 2017 08:57:44 +0000 Subject: Fix various issues found by testing Freshmaker with real depending services: - When FRESHMAKER_CONFIG_FILE is defined, load it even when using DEVELOPER_ENV. - Get the git branch name from the Brew build instead of setting it to "unknown" every time. - Set container image release to XX.timestamp instead of XX.YY.timestamp. - Pass models.py Event instance to prepare_yum_repo instead of events.py Event instance. - Remove call of non-existing _build_first_batch method in Errata handler. - Pass full config file to Koji. - Fix bug when Koji build was always 'scratch' build. - Do not include ".eng" in topic suffix for handler, this should be part of TOPIC_PREFIX configuration. --- diff --git a/freshmaker/config.py b/freshmaker/config.py index 8096af5..7ea20b1 100644 --- a/freshmaker/config.py +++ b/freshmaker/config.py @@ -66,6 +66,7 @@ def init_config(app): # package -> /conf/config.py. elif ('FRESHMAKER_DEVELOPER_ENV' in os.environ and + 'FRESHMAKER_CONFIG_FILE' not in os.environ and os.environ['FRESHMAKER_DEVELOPER_ENV'].lower() in ('1', 'on', 'true', 'y', 'yes')): config_section = 'DevConfiguration' if 'FRESHMAKER_CONFIG_FILE' in os.environ: diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index 815a615..65ebaa0 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -242,15 +242,16 @@ class ContainerBuildHandler(BaseHandler): "supported yet", build) return - parent = args["parent"] scm_url = "%s/%s#%s" % (conf.git_base_url, args["repository"], args["commit"]) - release = build.name.split("-")[-1] + "." + str(int(time.time())) - # According to Luiz from OSBS team, it is OK to use "unknown" if - # we don't know the branch name. TODO: Get the branch name from - # Koji in lightblue.py. - branch = "unknown" + branch = args["branch"] target = args["target"] + parent = args["parent"] + + # Set release from XX.YY to XX.$timestamp + version_release = build.name.split("-")[-1] + version = version_release.split(".")[0] + release = str(version) + "." + str(int(time.time())) return self.build_container( scm_url, branch, target, repo_urls=repo_urls, diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index 1fad4d4..ec99658 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -82,7 +82,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): # Generate the ODCS compose with RPMs from the current advisory. repo_urls = [] - repo_urls.append(self._prepare_yum_repo(event)) # noqa + repo_urls.append(self._prepare_yum_repo(db_event)) # noqa # Find out extra events we want to include. These are advisories # which are not released yet and touches some Docker images which @@ -122,9 +122,6 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): for url in repo_urls: log.info(" - %s", url) - # Build first batch of images. - self._build_first_batch(db_event) - return [] def _prepare_yum_repo(self, db_event): @@ -169,7 +166,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): compose_id = new_compose['id'] yum_repourl = new_compose['result_repofile'] - rebuild_event = Event.get(db.session, db_event.msg_id) + rebuild_event = Event.get(db.session, db_event.message_id) rebuild_event.compose_id = compose_id db.session.commit() @@ -307,6 +304,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): build_args["commit"] = image["commit"] build_args["parent"] = parent_name build_args["target"] = image["target"] + build_args["branch"] = image["git_branch"] build.build_args = json.dumps(build_args) db.session.commit() diff --git a/freshmaker/kojiservice.py b/freshmaker/kojiservice.py index 34a39c9..c1efa68 100644 --- a/freshmaker/kojiservice.py +++ b/freshmaker/kojiservice.py @@ -57,7 +57,7 @@ class KojiService(object): def session(self): if not hasattr(self, '_session'): self._session = koji.ClientSession(self.config['server'], - {'krb_rdns': self.config['krb_rdns']}) + self.config) return self._session def krb_login(self, proxyuser=None): @@ -77,7 +77,7 @@ class KojiService(object): build_target = target build_opts = { - 'scratch': False if scratch is None else True, + 'scratch': False if scratch is None else scratch, 'git_branch': branch, } @@ -114,7 +114,7 @@ class KojiService(object): return self.session.getBuild(build_nvr) def get_task_request(self, task_id): - log.info("get_build %r", task_id) + log.info("get_task_request %r", task_id) return self.session.getTaskRequest(task_id) diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index 3f48bb7..51fe7fe 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -133,6 +133,7 @@ class ContainerImage(dict): reponame = None commit = None target = None + git_branch = None # Find the repository name, commit id and koji target form the Koji # build. @@ -142,22 +143,43 @@ class ContainerImage(dict): else: with koji_service(conf.koji_profile, log) as session: build = session.get_build(nvr) - if build: - brew_task = session.get_task_request( - build['task_id']) - source = brew_task[0] - target = brew_task[1] - - m = re.match(r".*/(?P.*)/(?P.*)#(?P.*)", source) - if m: - namespace = m.group("namespace") - container = m.group("container") - reponame = namespace + "/" + container - commit = m.group("commit") + if not build: + err = "Cannot find Koji build with nvr %s in Koji." % nvr + log.error(err) + raise ValueError(err) + + if not build['task_id']: + if ("extra" in build and + "container_koji_task_id" in build["extra"] and + build["extra"]["container_koji_task_id"]): + build['task_id'] = \ + build["extra"]['container_koji_task_id'] + else: + err = "Cannot find task_id or container_koji_task_id " \ + "in the Koji build %r" % build + log.error(err) + raise ValueError(err) + + brew_task = session.get_task_request( + build['task_id']) + source = brew_task[0] + target = brew_task[1] + extra_data = brew_task[2] + if "git_branch" in extra_data: + git_branch = extra_data["git_branch"] + else: + git_branch = "unknown" + + m = re.match(r".*/(?P.*)/(?P.*)#(?P.*)", source) + if m: + namespace = m.group("namespace") + container = m.group("container") + reponame = namespace + "/" + container + commit = m.group("commit") ContainerImage.KOJI_BUILDS_CACHE[nvr] = (reponame, commit, target) data = {"repository": reponame, "commit": commit, "target": target, - "srpm_nevra": srpm_nevra} + "srpm_nevra": srpm_nevra, "git_branch": git_branch} self.update(data) diff --git a/freshmaker/parsers/brew/sign_rpm.py b/freshmaker/parsers/brew/sign_rpm.py index ceff2ac..aec5451 100644 --- a/freshmaker/parsers/brew/sign_rpm.py +++ b/freshmaker/parsers/brew/sign_rpm.py @@ -29,7 +29,7 @@ class BrewSignRpmParser(BaseParser): """Parser parsing message from Brew""" name = "BrewSignRpmParser" - topic_suffixes = ["eng.brew.sign.rpm"] + topic_suffixes = ["brew.sign.rpm"] def can_parse(self, topic, msg): return any([topic.endswith(s) for s in self.topic_suffixes]) diff --git a/freshmaker/parsers/brew/task_state_change.py b/freshmaker/parsers/brew/task_state_change.py index c97e4f8..05f63da 100644 --- a/freshmaker/parsers/brew/task_state_change.py +++ b/freshmaker/parsers/brew/task_state_change.py @@ -34,7 +34,7 @@ class BrewTaskStateChangeParser(BaseParser): """ name = "BrewTaskStateChangeParser" - topic_suffixes = ["eng.brew.task.closed", 'eng.brew.task.failed'] + topic_suffixes = ["brew.task.closed", 'brew.task.failed'] def can_parse(self, topic, msg): return any([topic.endswith(s) for s in self.topic_suffixes]) diff --git a/freshmaker/parsers/errata/state_change.py b/freshmaker/parsers/errata/state_change.py index df2e7f2..4ff8756 100644 --- a/freshmaker/parsers/errata/state_change.py +++ b/freshmaker/parsers/errata/state_change.py @@ -29,7 +29,7 @@ class ErrataAdvisoryStateChangedParser(BaseParser): """ name = "ErrataAdvisoryStateChangedParser" - topic_suffixes = ["eng.errata.activity.status"] + topic_suffixes = ["errata.activity.status"] def can_parse(self, topic, msg): return any([topic.endswith(s) for s in self.topic_suffixes]) diff --git a/freshmaker/parsers/odcs/__init__.py b/freshmaker/parsers/odcs/__init__.py index e69de29..43ae986 100644 --- a/freshmaker/parsers/odcs/__init__.py +++ b/freshmaker/parsers/odcs/__init__.py @@ -0,0 +1,22 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2016 Red Hat, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +from .state_change import ComposeStateChangeParser # noqa diff --git a/freshmaker/parsers/odcs/state_change.py b/freshmaker/parsers/odcs/state_change.py index 6c9c8b4..f260d19 100644 --- a/freshmaker/parsers/odcs/state_change.py +++ b/freshmaker/parsers/odcs/state_change.py @@ -27,7 +27,7 @@ class ComposeStateChangeParser(BaseParser): """Parser parsing odcs.compose.state.change""" name = "ComposeStateChangeEvent" - topic_suffixes = ["eng.odcs.compose.state.change"] + topic_suffixes = ["odcs.state.change"] def can_parse(self, topic, msg): return any([topic.endswith(s) for s in self.topic_suffixes]) diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index da1608d..9a5d074 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -200,7 +200,8 @@ class TestBatches(unittest.TestCase): if parent: parent = {"brew": {"build": parent}} return {'brew': {'build': build}, 'repository': build + '_repo', - 'commit': build + '_123', 'parent': parent, "target": "t1"} + 'commit': build + '_123', 'parent': parent, "target": "t1", + 'git_branch': 'mybranch'} def test_batches_records(self): """ @@ -396,12 +397,12 @@ class TestPrepareYumRepo(unittest.TestCase): errata.return_value.get_builds.return_value = set(["httpd-2.4.15-1.f27"]) - event = Mock(msg_id='msg-id', search_key=12345) + event = Mock(message_id='msg-id', search_key=12345) handler = ErrataAdvisoryRPMsSignedHandler() repo_url = handler._prepare_yum_repo(event) rebuild_event = db.session.query(Event).filter( - Event.message_id == event.msg_id).first() + Event.message_id == event.message_id).first() self.assertEqual(3, rebuild_event.compose_id) _get_compose_source.assert_called_once_with("httpd-2.4.15-1.f27") diff --git a/tests/test_handler.py b/tests/test_handler.py index 2214b8e..8aaaf6d 100644 --- a/tests/test_handler.py +++ b/tests/test_handler.py @@ -108,6 +108,7 @@ class TestBuildFirstBatch(TestCase): "repository": "repo", "target": "target", "commit": "hash", + "branch": "mybranch", "yum_repourl": "http://localhost/composes/latest-odcs-3-1/compose/" "Temporary/odcs-3.repo", }) @@ -162,7 +163,7 @@ class TestBuildFirstBatch(TestCase): 'git://pkgs.fedoraproject.org/repo#hash', 'target', {'scratch': True, 'isolated': True, 'koji_parent_build': u'nvr', - 'git_branch': 'unknown', 'release': AnyStringWith('4.'), + 'git_branch': 'mybranch', 'release': AnyStringWith('4.'), 'yum_repourls': [ 'http://localhost/composes/latest-odcs-3-1/compose/Temporary/odcs-3.repo']}) @@ -216,7 +217,7 @@ class TestBuildFirstBatch(TestCase): 'git://pkgs.fedoraproject.org/repo#hash', 'target', {'scratch': True, 'isolated': True, 'koji_parent_build': u'nvr', - 'git_branch': 'unknown', 'release': AnyStringWith('4.'), + 'git_branch': 'mybranch', 'release': AnyStringWith('4.'), 'yum_repourls': [ 'http://localhost/composes/latest-odcs-3-1/compose/Temporary/odcs-3.repo', 'http://localhost/composes/latest-odcs-4-1/compose/Temporary/odcs-4.repo']}) diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index 29b1f4d..8bc8efa 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -158,7 +158,7 @@ class TestContainerImageObject(unittest.TestCase): get_build.return_value = {"task_id": 123456} get_task_request.return_value = [ - "git://example.com/rpms/repo-1#commit_hash1", "target1"] + "git://example.com/rpms/repo-1#commit_hash1", "target1", {}] image.resolve_commit("openssl") self.assertEqual(image["repository"], "rpms/repo-1") @@ -271,9 +271,9 @@ class TestQueryEntityFromLightBlue(unittest.TestCase): self.fake_koji_builds = [{"task_id": 123456}, {"task_id": 654321}] self.fake_koji_task_requests = [ ["git://pkgs.devel.redhat.com/rpms/repo-1#commit_hash1", - "target1"], + "target1", {"git_branch": "mybranch"}], ["git://pkgs.devel.redhat.com/rpms/repo-2#commit_hash2", - "target2"]] + "target2", {"git_branch": "mybranch"}]] @patch('freshmaker.lightblue.requests.post') def test_find_container_images(self, post): @@ -568,6 +568,7 @@ class TestQueryEntityFromLightBlue(unittest.TestCase): "commit": "commit_hash1", "srpm_nevra": "openssl-0:1.2.3-1.src", "target": "target1", + "git_branch": "mybranch", "brew": { "completion_date": u"20170421T04:27:51.000-0400", "build": "package-name-1-4-12.10", @@ -598,6 +599,7 @@ class TestQueryEntityFromLightBlue(unittest.TestCase): "commit": "commit_hash2", "srpm_nevra": "openssl-1:1.2.3-1.src", "target": "target2", + "git_branch": "mybranch", "brew": { "completion_date": u"20170421T04:27:51.000-0400", "build": "package-name-2-4-12.10", @@ -639,7 +641,7 @@ class TestQueryEntityFromLightBlue(unittest.TestCase): get_build.return_value = {"task_id": 123456} get_task_request.return_value = [ - "git://example.com/rpms/repo-1#commit_hash1", "target1"] + "git://example.com/rpms/repo-1#commit_hash1", "target1", {}] exists.return_value = True cont_images.side_effect = [self.fake_container_images, [], self.fake_container_images]