From 07d9fe0549df68cad09249b4cb20d2036c1e5e6c Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Jan 28 2019 12:23:06 +0000 Subject: Use ArtifactBuild.dep_on to find out the rebuilt_nvr of the parent image. Currently, we use `build_args["parent"]` to find out the rebuilt_nvr of parent image. This has an issue in case we want to change the rebuilt_nvr in the middle of a rebuild. In such case, we would need to change it in two places - in the parent image and also in all its children images. With this commit, we just take rebuilt_nvr only from the parent image and do not store it also in children images. Therefore the possible change of rebuilt_nvr is much easier. The `build_args["parent"]` has been renamed to "original_parent" to better express its meaning. --- diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index 0c5a86a..a156989 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -464,7 +464,16 @@ class ContainerBuildHandler(BaseHandler): args["commit"]) branch = args["branch"] target = args["target"] - parent = args["parent"] + + # If this container image depends on another container image + # we are going to rebuild, use the new NVR of that image + # as a dependency. Otherwise fallback to build_args, which means + # the parent is not rebuilt by Freshmaker, but we just take existing + # parent from Koji. + if build.dep_on: + parent = build.dep_on.rebuilt_nvr + else: + parent = args["original_parent"] # If set to None, then OSBS defaults to using the arches # of the build tag associated with the target. diff --git a/freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py b/freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py index 8afccce..3062e05 100644 --- a/freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py +++ b/freshmaker/handlers/koji/rebuild_images_on_rpm_advisory_change.py @@ -155,8 +155,11 @@ class RebuildImagesOnRPMAdvisoryChange(ContainerBuildHandler): ((build.dep_on and build.dep_on.original_nvr in printed) or (not build.dep_on and batch == 0))): args = json.loads(build.build_args) - based_on = "based on %s" % args["parent"] \ - if args["parent"] else "base image" + if build.dep_on: + based_on = "based on %s" % build.dep_on.rebuilt_nvr + else: + based_on = "based on %s" % args["original_parent"] \ + if args["original_parent"] else "base image" self.log_info( ' - %s#%s (%s)' % (args["repository"], args["commit"], based_on)) @@ -224,12 +227,6 @@ class RebuildImagesOnRPMAdvisoryChange(ContainerBuildHandler): if "parent" in image and image["parent"] 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"] state = ArtifactBuildState.FAILED.value @@ -262,7 +259,7 @@ class RebuildImagesOnRPMAdvisoryChange(ContainerBuildHandler): build_args = {} build_args["repository"] = image["repository"] build_args["commit"] = image["commit"] - build_args["parent"] = parent_nvr + build_args["original_parent"] = parent_nvr build_args["target"] = image["target"] build_args["branch"] = image["git_branch"] build_args["arches"] = image["arches"] diff --git a/freshmaker/models.py b/freshmaker/models.py index 2cbb5b8..6bfad66 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -565,6 +565,7 @@ class ArtifactBuild(FreshmakerBase): "state_name": ArtifactBuildState(self.state).name, "state_reason": self.state_reason, "dep_on": self.dep_on.name if self.dep_on else None, + "dep_on_id": self.dep_on.id if self.dep_on else None, "time_submitted": _utc_datetime_to_iso(self.time_submitted), "time_completed": _utc_datetime_to_iso(self.time_completed), "event_id": self.event_id, diff --git a/tests/handlers/internal/test_update_db_on_advisory_change.py b/tests/handlers/internal/test_update_db_on_advisory_change.py index d9e6046..02392b7 100644 --- a/tests/handlers/internal/test_update_db_on_advisory_change.py +++ b/tests/handlers/internal/test_update_db_on_advisory_change.py @@ -365,8 +365,8 @@ class TestBatches(helpers.ModelsTestCase): args = json.loads(build.build_args) self.assertEqual(args["repository"], build.name + "_repo") self.assertEqual(args["commit"], build.name + "_123") - self.assertEqual(args["parent"], - build.dep_on.rebuilt_nvr if build.dep_on else None) + self.assertEqual(args["original_parent"], + build.dep_on.original_nvr if build.dep_on else None) self.assertEqual(args["renewed_odcs_compose_ids"], [10, 11]) @@ -378,7 +378,7 @@ class TestCheckImagesToRebuild(helpers.ModelsTestCase): super(TestCheckImagesToRebuild, self).setUp() build_args = json.dumps({ - "parent": "nvr", + "original_parent": "nvr", "repository": "repo", "target": "target", "commit": "hash", diff --git a/tests/test_handler.py b/tests/test_handler.py index b195811..4a36963 100644 --- a/tests/test_handler.py +++ b/tests/test_handler.py @@ -116,7 +116,7 @@ class TestGetRepoURLs(helpers.ModelsTestCase): build_args = {} build_args["repository"] = "repo" build_args["commit"] = "hash" - build_args["parent"] = None + build_args["original_parent"] = None build_args["target"] = "target" build_args["branch"] = "branch" build_args["arches"] = "x86_64"