From 61af77bcd997dce98d486daff118c25636fd6aee Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Oct 10 2017 11:36:08 +0000 Subject: Track ArtifactBuild's original_nvr and rebuilt_nvr. Fixes bad koji_parent_nvr value when rebuilding container images. - Add `original_nvr` and `rebuilt_nvr` to ArtifactBuild. - Change the ErrataAdvisoryRPMSSigned handler to set `build.name` to name part of NVR, `build.original_nvr` to NVR and to generate `build.rebuilt_nvr` using new `utils.get_rebuilt_nvr()` method. - Properly set the NVR of parent images to `build.rebuilt_nvr` instead of using original NVR. --- diff --git a/freshmaker/api_utils.py b/freshmaker/api_utils.py index 58278c0..7b08cac 100644 --- a/freshmaker/api_utils.py +++ b/freshmaker/api_utils.py @@ -86,7 +86,8 @@ def filter_artifact_builds(flask_request): else: raise ValueError('An invalid state was supplied') - for key in ['name', 'event_id', 'dep_on_id', 'build_id']: + for key in ['name', 'event_id', 'dep_on_id', 'build_id', 'original_nvr', + 'rebuilt_nvr']: if flask_request.args.get(key, None): search_query[key] = flask_request.args[key] diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index aeae4d6..3bcab33 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -24,15 +24,14 @@ import abc import json import re -import time from freshmaker import conf, log, db, models -from freshmaker.kojiservice import koji_service +from freshmaker.kojiservice import koji_service, parse_NVR from freshmaker.mbs import MBS from freshmaker.models import ArtifactBuildState from freshmaker.types import ArtifactType from freshmaker.models import ArtifactBuild, Event -from freshmaker.utils import krb_context +from freshmaker.utils import krb_context, get_rebuilt_nvr from freshmaker.odcsclient import ODCS from freshmaker.odcsclient import AuthMech @@ -76,7 +75,8 @@ class BaseHandler(object): return mbs.build_module(name, branch, rev) def record_build(self, event, name, artifact_type, - build_id=None, dep_on=None, state=None): + build_id=None, dep_on=None, state=None, + original_nvr=None, rebuilt_nvr=None): """ Record build in db. @@ -89,14 +89,19 @@ class BaseHandler(object): other artifact is depended on. :param state: the initial state of build. If omitted, defaults to ``ArtifactBuildState.BUILD``. + :param original_nvr: The original NVR of artifact. + :param rebuilt_nvr: The NVR of newly rebuilt artifact. :return: recorded build. :rtype: ArtifactBuild. """ + ev = models.Event.get_or_create(db.session, event.msg_id, event.search_key, event.__class__) build = models.ArtifactBuild.create(db.session, ev, name, artifact_type.name.lower(), - build_id, dep_on, state) + build_id, dep_on, state, + original_nvr, rebuilt_nvr) + db.session.commit() return build @@ -228,10 +233,17 @@ class ContainerBuildHandler(BaseHandler): 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())) + if not build.rebuilt_nvr and build.original_nvr: + build.rebuilt_nvr = get_rebuilt_nvr( + build.type, build.original_nvr) + + if not build.rebuilt_nvr: + build.transition( + ArtifactBuildState.FAILED.value, + "Container image does not have rebuilt_nvr set.") + return + + release = parse_NVR(build.rebuilt_nvr)["release"] 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 ec6385c..7999471 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -38,7 +38,7 @@ from freshmaker.errata import Errata from freshmaker.types import ArtifactType, ArtifactBuildState from freshmaker.models import Event from freshmaker.consumer import work_queue_put -from freshmaker.utils import krb_context, retry +from freshmaker.utils import krb_context, retry, get_rebuilt_nvr from odcs.client.odcs import ODCS from odcs.client.odcs import AuthMech @@ -382,15 +382,21 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): for batch in batches: for image in batch: - name = image["brew"]["build"] - if name in builds: + nvr = image["brew"]["build"] + if nvr in builds: log.debug("Skipping recording build %s, " - "it is already in db", name) + "it is already in db", nvr) continue - log.debug("Recording %s", name) - parent_name = image["parent"]["brew"]["build"] \ + log.debug("Recording %s", nvr) + parent_nvr = image["parent"]["brew"]["build"] \ if image["parent"] else None - dep_on = builds[parent_name] if parent_name in builds else None + dep_on = builds[parent_nvr] if parent_nvr in builds else None + + # If this container image depends on another container image + # we are going to rebuild, use the new NVR of that image + # as a dependency instead of the original one. + if dep_on: + parent_nvr = dep_on.rebuilt_nvr if "error" in image and image["error"]: state_reason = image["error"] @@ -405,10 +411,15 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): state_reason = "" state = ArtifactBuildState.PLANNED.value + rebuilt_nvr = get_rebuilt_nvr(ArtifactType.IMAGE.value, nvr) + image_name = koji.parse_NVR(image["brew"]["build"])["name"] + build = self.record_build( - event, name, ArtifactType.IMAGE, + event, image_name, ArtifactType.IMAGE, dep_on=dep_on, - state=ArtifactBuildState.PLANNED.value) + state=ArtifactBuildState.PLANNED.value, + original_nvr=nvr, + rebuilt_nvr=rebuilt_nvr) build.transition(state, state_reason) @@ -417,14 +428,14 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): build_args = {} build_args["repository"] = image["repository"] build_args["commit"] = image["commit"] - build_args["parent"] = parent_name + build_args["parent"] = parent_nvr build_args["target"] = image["target"] build_args["branch"] = image["git_branch"] build_args["odcs_pulp_compose_id"] = compose["id"] build.build_args = json.dumps(build_args) db.session.commit() - builds[name] = build + builds[nvr] = build return builds diff --git a/freshmaker/kojiservice.py b/freshmaker/kojiservice.py index 067da6b..bcd96df 100644 --- a/freshmaker/kojiservice.py +++ b/freshmaker/kojiservice.py @@ -23,6 +23,12 @@ import koji +# Unfortunatelly we want to use "parse_NVR" provided by koji +# in freshmaker.handlers __init__.py. We cannot "import koji" there, because +# it would import freshmaker.handlers.koji, so instead, we import it here +# and in freshmaker.handler do "from freshmaker.kojiservice import parse_NVR". +from koji import parse_NVR # noqa + import contextlib import re from freshmaker import log, conf diff --git a/freshmaker/migrations/versions/d6de0409fc74_.py b/freshmaker/migrations/versions/d6de0409fc74_.py new file mode 100644 index 0000000..a00a19d --- /dev/null +++ b/freshmaker/migrations/versions/d6de0409fc74_.py @@ -0,0 +1,24 @@ +"""Add original_nvr and rebuilt_nvr columns. + +Revision ID: d6de0409fc74 +Revises: 3f56425964cf +Create Date: 2017-10-10 12:57:59.802658 + +""" + +# revision identifiers, used by Alembic. +revision = 'd6de0409fc74' +down_revision = '3f56425964cf' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('artifact_builds', sa.Column('original_nvr', sa.String(), nullable=True)) + op.add_column('artifact_builds', sa.Column('rebuilt_nvr', sa.String(), nullable=True)) + + +def downgrade(): + op.drop_column('artifact_builds', 'rebuilt_nvr') + op.drop_column('artifact_builds', 'original_nvr') diff --git a/freshmaker/models.py b/freshmaker/models.py index 7d6290d..5f9450c 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -164,6 +164,8 @@ class ArtifactBuild(FreshmakerBase): __tablename__ = "artifact_builds" id = db.Column(db.Integer, primary_key=True) name = db.Column(db.String, nullable=False) + original_nvr = db.Column(db.String, nullable=True) + rebuilt_nvr = db.Column(db.String, nullable=True) type = db.Column(db.Integer) state = db.Column(db.Integer, nullable=False) state_reason = db.Column(db.String, nullable=True) @@ -190,10 +192,14 @@ class ArtifactBuild(FreshmakerBase): @classmethod def create(cls, session, event, name, type, - build_id=None, dep_on=None, state=None): + build_id=None, dep_on=None, state=None, + original_nvr=None, rebuilt_nvr=None): + now = datetime.utcnow() build = cls( name=name, + original_nvr=original_nvr, + rebuilt_nvr=rebuilt_nvr, type=type, event=event, state=state or ArtifactBuildState.BUILD.value, @@ -271,6 +277,8 @@ class ArtifactBuild(FreshmakerBase): return { "id": self.id, "name": self.name, + "original_nvr": self.original_nvr, + "rebuilt_nvr": self.rebuilt_nvr, "type": self.type, "type_name": ArtifactType(self.type).name, "state": self.state, diff --git a/freshmaker/utils.py b/freshmaker/utils.py index 3723a93..ccc40d3 100644 --- a/freshmaker/utils.py +++ b/freshmaker/utils.py @@ -30,11 +30,36 @@ import subprocess import sys import tempfile import time +import koji from freshmaker import conf +from freshmaker.types import ArtifactType from krbcontext import krbContext +def get_rebuilt_nvr(artifact_type, nvr): + """ + Returns the new NVR of artifact which should be used when rebuilding + the artifact. + + :param ArtifactType artifact_type: Type of the rebuilt artifact. + :param str nvr: Original NVR of artifact. + + :rtype: str + :return: newly generated NVR + """ + rebuilt_nvr = None + if artifact_type == ArtifactType.IMAGE.value: + # Set release from XX.YY to XX.$timestamp + parsed_nvr = koji.parse_NVR(nvr) + r_version = parsed_nvr["release"].split(".")[0] + release = str(r_version) + "." + str(int(time.time())) + rebuilt_nvr = "%s-%s-%s" % (parsed_nvr["name"], parsed_nvr["version"], + release) + + return rebuilt_nvr + + def krb_context(): if conf.krb_auth_use_keytab: krb_ctx_opts = { diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index 39d654c..8e3e82a 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -263,11 +263,11 @@ class TestBatches(unittest.TestCase): def _mock_build(self, build, parent=None, error=None): if parent: - parent = {"brew": {"build": parent}} - return {'brew': {'build': build}, 'repository': build + '_repo', - 'commit': build + '_123', 'parent': parent, "target": "t1", - 'git_branch': 'mybranch', "error": error, - "content_sets": ["first-content-set"]} + parent = {"brew": {"build": parent + "-1-1.25"}} + return {'brew': {'build': build + "-1-1.25"}, + 'repository': build + '_repo', 'commit': build + '_123', + 'parent': parent, "target": "t1", 'git_branch': 'mybranch', + "error": error, "content_sets": ["first-content-set"]} @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS.new_compose') @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS.get_compose') @@ -323,9 +323,9 @@ class TestBatches(unittest.TestCase): self.assertEqual(build.state, ArtifactBuildState.PLANNED.value) self.assertEqual(build.type, ArtifactType.IMAGE.value) - image = images[build.name] + image = images[build.original_nvr] if image['parent']: - self.assertEqual(build.dep_on.name, image['parent']['brew']['build']) + self.assertEqual(build.dep_on.original_nvr, image['parent']['brew']['build']) else: self.assertEqual(build.dep_on, None) @@ -333,7 +333,7 @@ class TestBatches(unittest.TestCase): self.assertEqual(args["repository"], build.name + "_repo") self.assertEqual(args["commit"], build.name + "_123") self.assertEqual(args["parent"], - build.dep_on.name if build.dep_on else None) + build.dep_on.rebuilt_nvr if build.dep_on else None) class TestGetPackagesForCompose(unittest.TestCase): diff --git a/tests/test_handler.py b/tests/test_handler.py index 7526e6d..96bd083 100644 --- a/tests/test_handler.py +++ b/tests/test_handler.py @@ -124,27 +124,32 @@ class TestBuildFirstBatch(TestCase): self.db_event.compose_id = 3 p1 = ArtifactBuild.create(db.session, self.db_event, "parent1-1-4", "image", - state=ArtifactBuildState.PLANNED.value) + state=ArtifactBuildState.PLANNED.value, + original_nvr="parent1-1-4") p1.build_args = build_args b = ArtifactBuild.create(db.session, self.db_event, "parent1_child1", "image", state=ArtifactBuildState.PLANNED.value, - dep_on=p1) + dep_on=p1, + original_nvr="parent1_child1-1-4") b.build_args = build_args # Not in PLANNED state. b = ArtifactBuild.create(db.session, self.db_event, "parent3", "image", - state=ArtifactBuildState.BUILD.value) + state=ArtifactBuildState.BUILD.value, + original_nvr="parent3-1-4") b.build_args = build_args # No build args b = ArtifactBuild.create(db.session, self.db_event, "parent4", "image", - state=ArtifactBuildState.PLANNED.value) + state=ArtifactBuildState.PLANNED.value, + original_nvr="parent4-1-4") db.session.commit() # No parent - base image b = ArtifactBuild.create(db.session, self.db_event, "parent5", "image", - state=ArtifactBuildState.PLANNED.value) + state=ArtifactBuildState.PLANNED.value, + original_nvr="parent5-1-4") b.build_args = build_args b.build_args = b.build_args.replace("nvr", "")