From e35116f7d8f381b615047d4d29510e392669e212 Mon Sep 17 00:00:00 2001 From: gnaponie Date: Aug 28 2019 09:54:06 +0000 Subject: [PATCH 1/3] Filter out non-modular container images when handling modular RHSA When finding out container images containing RPM installed from module, we should ignore all the container images which does *not* install the RPM from module. ref: FACTORY-4979 Signed-off-by: gnaponie --- diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index 7a69f79..2cd97ce 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -777,6 +777,60 @@ class LightBlue(object): image["brew"]["build"], srpm_name_to_nvrs.values())) return ret + def filter_out_non_modular_container_images(self, images, srpm_name_to_nvrs): + """ + Filter out container images which contain a component from a module but + do not contain the module itself. + + :param list images: List of ContainerImage instances. + :param dict srpm_name_to_nvrs: Dict with SRPM name as a key and list + of NVRs as a value. + :rtype: list + :return: List of ContainerImage instances without the filtered images. + """ + ret = [] + for image in images: + if "rpm_manifest" not in image or not image["rpm_manifest"]: + # Do not filter if we are not sure what RPMs are in the image. + ret.append(image) + log.info(("Not filtering out non modular container images" + "because we are not sure what RPMs are in the image.")) + continue + # There is always just single "rpm_manifest". Lightblue returns + # this as a list, because it is reference to + # containerImageRPMManifest. + rpm_manifest = image["rpm_manifest"][0] + if "rpms" not in rpm_manifest: + # Do not filter if we are not sure what RPMs are in the image. + ret.append(image) + log.info(("Not filtering out non modular container images" + "because we are not sure what RPMs are in the image.")) + continue + # Check whether the RPMs contained in the images are installed from module + # and if there are other images that does not install the RPM from module + # so that we can ignore these latter. + image_included = False + rpms = rpm_manifest["rpms"] + for rpm in rpms: + for srpm_nvr in srpm_name_to_nvrs.get(rpm.get("srpm_name"), []): + if (("module+" in srpm_nvr and "module+" in rpm["srpm_nevra"]) or + ("module+" not in srpm_nvr and "module+" not in rpm["srpm_nevra"])): + ret.append(image) + image_included = True + break + if image_included: + break + else: + # Oh-no, the mighty for/else block! + # The else clause executes after the loop completes normally. + # This means that the loop did not encounter a break statement. + # In our case, this means that we filtered out the image. + log.info( + "Will not rebuild %s, because it does not contain " + "RPMs from modules: %r" % ( + image["brew"]["build"], srpm_name_to_nvrs.values())) + return ret + def find_images_with_included_srpms( self, content_sets, srpm_nvrs, repositories, published=True, include_rpms=True): @@ -879,6 +933,7 @@ class LightBlue(object): break images = new_images images = self.filter_out_images_with_lower_srpm_nvr(images, srpm_name_to_nvrs) + images = self.filter_out_non_modular_container_images(images, srpm_name_to_nvrs) return images def get_images_by_nvrs(self, nvrs, published=True, content_sets=None, diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index 19fced0..8cfc43e 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -738,6 +738,43 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): }, ] + self.fake_images_with_modules = [ + { + 'brew': { + 'completion_date': u'20170421T04:27:51.000-0400', + 'build': 'package-name-3-4-12.10', + 'package': 'package-name-1' + }, + "content_sets": ["dummy-content-set-1", + "dummy-content-set-2"], + 'repositories': [ + {'repository': 'product2/repo2', 'published': True, + 'tags': [{"name": "tag2"}]} + ], + 'parsed_data': { + 'files': [ + { + 'key': 'buildfile', + 'content_url': 'http://git.repo.com/cgit/rpms/repo-1/plain/Dockerfile?id=commit_hash1', + 'filename': u'Dockerfile' + } + ], + }, + 'rpm_manifest': [{ + 'rpms': [ + { + "srpm_name": "openssl", + "srpm_nevra": "openssl-1.2.1-2.module+el8.0.0+3248+9d514f3b.src" + }, + { + "srpm_name": "tespackage", + "srpm_nevra": "testpackage-10:1.2.3-1.src" + } + ] + }] + }, + ] + self.fake_container_images = [ ContainerImage.create(data) for data in self.fake_images_with_parsed_data] @@ -746,6 +783,10 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): ContainerImage.create(data) for data in self.fake_images_with_parsed_data_floating_tag] + self.fake_images_with_modules = [ + ContainerImage.create(data) + for data in self.fake_images_with_modules] + self.fake_koji_builds = [{"task_id": 654321}, {"task_id": 123456}] self.fake_koji_task_requests = [ ["git://pkgs.devel.redhat.com/rpms/repo-2#commit_hash2", @@ -1649,6 +1690,31 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): image = lb.find_latest_parent_image("foo", 1) self.assertEqual(image["brew"]["build"], "parent-1-3") + @patch('freshmaker.lightblue.LightBlue.find_container_images') + @patch('os.path.exists') + def test_images_with_modular_container_image( + self, exists, cont_images): + + exists.return_value = True + lb = LightBlue(server_url=self.fake_server_url, + cert=self.fake_cert_file, + private_key=self.fake_private_key) + repositories = { + repo["repository"]: repo for repo in + self.fake_repositories_with_content_sets} + cont_images.return_value = ( + self.fake_images_with_modules) + ret = lb.find_images_with_included_srpms( + ["content-set-1", "content-set-2"], ["openssl-1.2.3-2.module+el8.0.0+3248+9d514f3b.src"], repositories) + self.assertEqual( + [image["brew"]["build"] for image in ret], + ["package-name-3-4-12.10"]) + ret = lb.find_images_with_included_srpms( + ["content-set-1", "content-set-2"], ["openssl-1.2.3-2.el8.0.0+3248+9d514f3b.src"], repositories) + self.assertEqual( + [image["brew"]["build"] for image in ret], + []) + class TestEntityVersion(helpers.FreshmakerTestCase): """Test case for ensuring correct entity version in request""" From 2ad3def23087cb91f55ee0f7fca0ccdf8e9c4648 Mon Sep 17 00:00:00 2001 From: gnaponie Date: Aug 28 2019 09:56:57 +0000 Subject: [PATCH 2/3] Improve filter_out_images_with_lower_srpm_nvr Signed-off-by: gnaponie --- diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index 2cd97ce..cec0175 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -734,6 +734,8 @@ class LightBlue(object): if "rpm_manifest" not in image or not image["rpm_manifest"]: # Do not filter if we are not sure what RPMs are in the image. ret.append(image) + log.info(("Not filtering out images with lower srpm_nvr" + "because we are not sure what RPMs are in the image.")) continue # There is always just single "rpm_manifest". Lightblue returns # this as a list, because it is reference to @@ -742,6 +744,8 @@ class LightBlue(object): if "rpms" not in rpm_manifest: # Do not filter if we are not sure what RPMs are in the image. ret.append(image) + log.info(("Not filtering out images with lower srpm_nvr" + "because we are not sure what RPMs are in the image.")) continue # Check whether all the input SRPMs in the container image are # older or newer and filter the container images in case they are @@ -749,21 +753,20 @@ class LightBlue(object): image_included = False rpms = rpm_manifest["rpms"] for rpm in rpms: - if "srpm_name" in rpm and rpm["srpm_name"] in srpm_name_to_nvrs: - image_srpm_nvr = kobo.rpmlib.parse_nvr(rpm["srpm_nevra"]) - for srpm_nvr in srpm_name_to_nvrs[rpm["srpm_name"]]: - input_srpm_nvr = kobo.rpmlib.parse_nvr(srpm_nvr) - # compare_nvr return values: - # - nvr1 newer than nvr2: 1 - # - same nvrs: 0 - # - nvr1 older: -1 - # We want to rebuild only images with SRPM NVR lower than - # input SRPM NVR, therefore we check for -1. - if kobo.rpmlib.compare_nvr( - image_srpm_nvr, input_srpm_nvr, ignore_epoch=True) == -1: - ret.append(image) - image_included = True - break + image_srpm_nvr = kobo.rpmlib.parse_nvr(rpm["srpm_nevra"]) + for srpm_nvr in srpm_name_to_nvrs.get(rpm.get("srpm_name"), []): + input_srpm_nvr = kobo.rpmlib.parse_nvr(srpm_nvr) + # compare_nvr return values: + # - nvr1 newer than nvr2: 1 + # - same nvrs: 0 + # - nvr1 older: -1 + # We want to rebuild only images with SRPM NVR lower than + # input SRPM NVR, therefore we check for -1. + if kobo.rpmlib.compare_nvr( + image_srpm_nvr, input_srpm_nvr, ignore_epoch=True) == -1: + ret.append(image) + image_included = True + break if image_included: break else: @@ -771,10 +774,9 @@ class LightBlue(object): # The else clause executes after the loop completes normally. # This means that the loop did not encounter a break statement. # In our case, this means that we filtered out the image. - image.log_error( - "Will not rebuild %s, because it does not contain " - "older version of any input package: %r" % ( - image["brew"]["build"], srpm_name_to_nvrs.values())) + log.info("Will not rebuild %s, because it does not contain " + "older version of any input package: %r" % ( + image["brew"]["build"], srpm_name_to_nvrs.values())) return ret def filter_out_non_modular_container_images(self, images, srpm_name_to_nvrs): From ff3f99e38b229c5a206f98a0f061ecf5f08fc5ff Mon Sep 17 00:00:00 2001 From: gnaponie Date: Aug 30 2019 08:26:45 +0000 Subject: [PATCH 3/3] Deduplicate some code in the lightblue module Signed-off-by: gnaponie --- diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index cec0175..bfb7b81 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -474,6 +474,26 @@ class ContainerImage(dict): err = "Cannot resolve the container image: %s" % e self.log_error(err) + def get_rpms(self): + """ + Extracts the RPMs from the Container image. + """ + if "rpm_manifest" not in self or not self["rpm_manifest"]: + # Do not filter if we are not sure what RPMs are in the image. + log.info(("Not filtering out this image because we " + "are not sure what RPMs are in there.")) + return + # There is always just single "rpm_manifest". Lightblue returns + # this as a list, because it is reference to + # containerImageRPMManifest. + rpm_manifest = self["rpm_manifest"][0] + if "rpms" not in rpm_manifest: + # Do not filter if we are not sure what RPMs are in the image. + log.info(("Not filtering out this image because we " + "are not sure what RPMs are in there.")) + return + return rpm_manifest["rpms"] + class LightBlue(object): """Interface to query lightblue""" @@ -731,28 +751,11 @@ class LightBlue(object): """ ret = [] for image in images: - if "rpm_manifest" not in image or not image["rpm_manifest"]: - # Do not filter if we are not sure what RPMs are in the image. - ret.append(image) - log.info(("Not filtering out images with lower srpm_nvr" - "because we are not sure what RPMs are in the image.")) - continue - # There is always just single "rpm_manifest". Lightblue returns - # this as a list, because it is reference to - # containerImageRPMManifest. - rpm_manifest = image["rpm_manifest"][0] - if "rpms" not in rpm_manifest: - # Do not filter if we are not sure what RPMs are in the image. + rpms = image.get_rpms() + if rpms is None: ret.append(image) - log.info(("Not filtering out images with lower srpm_nvr" - "because we are not sure what RPMs are in the image.")) - continue - # Check whether all the input SRPMs in the container image are - # older or newer and filter the container images in case they are - # not older. image_included = False - rpms = rpm_manifest["rpms"] - for rpm in rpms: + for rpm in rpms or []: image_srpm_nvr = kobo.rpmlib.parse_nvr(rpm["srpm_nevra"]) for srpm_nvr in srpm_name_to_nvrs.get(rpm.get("srpm_name"), []): input_srpm_nvr = kobo.rpmlib.parse_nvr(srpm_nvr) @@ -792,28 +795,14 @@ class LightBlue(object): """ ret = [] for image in images: - if "rpm_manifest" not in image or not image["rpm_manifest"]: - # Do not filter if we are not sure what RPMs are in the image. + rpms = image.get_rpms() + if rpms is None: ret.append(image) - log.info(("Not filtering out non modular container images" - "because we are not sure what RPMs are in the image.")) - continue - # There is always just single "rpm_manifest". Lightblue returns - # this as a list, because it is reference to - # containerImageRPMManifest. - rpm_manifest = image["rpm_manifest"][0] - if "rpms" not in rpm_manifest: - # Do not filter if we are not sure what RPMs are in the image. - ret.append(image) - log.info(("Not filtering out non modular container images" - "because we are not sure what RPMs are in the image.")) - continue + image_included = False # Check whether the RPMs contained in the images are installed from module # and if there are other images that does not install the RPM from module # so that we can ignore these latter. - image_included = False - rpms = rpm_manifest["rpms"] - for rpm in rpms: + for rpm in rpms or []: for srpm_nvr in srpm_name_to_nvrs.get(rpm.get("srpm_name"), []): if (("module+" in srpm_nvr and "module+" in rpm["srpm_nevra"]) or ("module+" not in srpm_nvr and "module+" not in rpm["srpm_nevra"])): @@ -1107,16 +1096,8 @@ class LightBlue(object): if srpm_name: tmp = [] for image in images: - if "rpm_manifest" not in image or not image["rpm_manifest"]: - continue - # There can be just single "rpm_manifest". Lightblue returns - # this as a list, because it is reference to - # containerImageRPMManifest. - rpm_manifest = image["rpm_manifest"][0] - if "rpms" not in rpm_manifest: - continue - rpms = rpm_manifest["rpms"] - for rpm in rpms: + rpms = image.get_rpms() + for rpm in rpms or []: if "srpm_name" in rpm and rpm["srpm_name"] == srpm_name: tmp.append(image) break diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index 8cfc43e..da370d7 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -1022,7 +1022,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): repositories = { repo["repository"]: repo for repo in self.fake_repositories_with_content_sets} - cont_images.return_value = self.fake_images_with_parsed_data + cont_images.return_value = self.fake_container_images ret = lb.find_images_with_included_srpms( ["content-set-1", "content-set-2"], ["openssl-1.2.3-2"], repositories)