From f9a35140361747935dea32f95fb30f82ff04639e Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Feb 08 2018 12:48:05 +0000 Subject: Filter out according to srpm_name locally, because server-side filtering is very slow and even timeouts. --- diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index edcc219..c83d540 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -204,20 +204,11 @@ class ContainerImage(dict): Uses the ContainerImage data to resolve the information about commit from which the Docker image has been built. - Sets the "repository, "commit" and "srpm_nevra" keys/values if - available. + Sets the "repository and "commit" keys/values if available. :param str srpm_name: Name of the package because of which the Docker image is rebuilt. """ - srpm_nevra = None - if ("rpm_manifest" in self and len(self['rpm_manifest']) > 0 and - "rpms" in self["rpm_manifest"]): - for rpm in self["rpm_manifest"]['rpms']: - if "srpm_name" in rpm and rpm["srpm_name"] == srpm_name: - srpm_nevra = rpm['srpm_nevra'] - break - # Find the additional data for Container build in Koji. nvr = self["brew"]["build"] try: @@ -228,7 +219,6 @@ class ContainerImage(dict): data = self._get_default_additional_data() data["error"] = err - data["srpm_nevra"] = srpm_nevra self.update(data) def resolve_content_sets( @@ -686,16 +676,36 @@ class LightBlue(object): }, "projection": self._get_default_projection() } - if srpm_name: - query['query']['$and'].append({ - "field": "rpm_manifest.*.rpms.*.srpm_name", - "op": "=", - "rvalue": srpm_name - }) images = self.find_container_images(query) if not images: return None + + # Filter out images which do not contain srpm_name locally, because + # filtering in lightblue takes long time and can even timeout + # server-side. + # We expect just at max 2 images here, published and unpublished, so + # it is not big deal doing so. + 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: + if "srpm_name" in rpm and rpm["srpm_name"] == srpm_name: + tmp.append(image) + break + images = tmp + if not images: + return None + for image in images: # we should prefer published image if 'repositories' in image: diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index ce26c26..a95474d 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -157,7 +157,7 @@ class TestContainerImageObject(helpers.FreshmakerTestCase): 'build': 'package-name-1-4-12.10', 'package': 'package-name-1' }, - 'rpm_manifest': { + 'rpm_manifest': [{ 'rpms': [ { "srpm_name": "openssl", @@ -168,7 +168,7 @@ class TestContainerImageObject(helpers.FreshmakerTestCase): "srpm_nevra": "testpackage-10:1.2.3-1.src" } ] - } + }] }) get_build.return_value = {"task_id": 123456} @@ -179,7 +179,6 @@ class TestContainerImageObject(helpers.FreshmakerTestCase): self.assertEqual(image["repository"], "rpms/repo-1") self.assertEqual(image["commit"], "commit_hash1") self.assertEqual(image["target"], "target1") - self.assertEqual(image["srpm_nevra"], "openssl-0:1.2.3-1.src") @patch('freshmaker.kojiservice.KojiService.get_build') @patch('freshmaker.kojiservice.KojiService.get_task_request') @@ -191,7 +190,7 @@ class TestContainerImageObject(helpers.FreshmakerTestCase): 'build': 'package-name-1-4-12.10', 'package': 'package-name-1' }, - 'rpm_manifest': { + 'rpm_manifest': [{ 'rpms': [ { "srpm_name": "openssl", @@ -202,7 +201,7 @@ class TestContainerImageObject(helpers.FreshmakerTestCase): "srpm_nevra": "testpackage-10:1.2.3-1.src" } ] - } + }] }) get_build.return_value = {} @@ -211,7 +210,6 @@ class TestContainerImageObject(helpers.FreshmakerTestCase): self.assertEqual(image["repository"], None) self.assertEqual(image["commit"], None) self.assertEqual(image["target"], None) - self.assertEqual(image["srpm_nevra"], "openssl-0:1.2.3-1.src") self.assertTrue(image["error"].find( "Cannot find Koji build with nvr package-name-1-4-12.10 in " "Koji.") != -1) @@ -226,7 +224,7 @@ class TestContainerImageObject(helpers.FreshmakerTestCase): 'build': 'package-name-1-4-12.10', 'package': 'package-name-1' }, - 'rpm_manifest': { + 'rpm_manifest': [{ 'rpms': [ { "srpm_name": "openssl", @@ -237,7 +235,7 @@ class TestContainerImageObject(helpers.FreshmakerTestCase): "srpm_nevra": "testpackage-10:1.2.3-1.src" } ] - } + }] }) get_build.return_value = {"task_id": None} @@ -246,7 +244,6 @@ class TestContainerImageObject(helpers.FreshmakerTestCase): self.assertEqual(image["repository"], None) self.assertEqual(image["commit"], None) self.assertEqual(image["target"], None) - self.assertEqual(image["srpm_nevra"], "openssl-0:1.2.3-1.src") self.assertTrue(image["error"].find( "Cannot find task_id or container_koji_task_id in the Koji build " "{'task_id': None}") != -1) @@ -364,7 +361,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): } ], }, - 'rpm_manifest': { + 'rpm_manifest': [{ 'rpms': [ { "srpm_name": "openssl", @@ -375,7 +372,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): "srpm_nevra": "testpackage-10:1.2.3-1.src" } ] - } + }] }, { 'brew': { @@ -400,7 +397,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): } ], }, - 'rpm_manifest': { + 'rpm_manifest': [{ 'rpms': [ { "srpm_name": "openssl", @@ -411,7 +408,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): "srpm_nevra": "testpackage2-10:1.2.3-1.src" } ] - } + }] }, ] @@ -722,7 +719,6 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): { "repository": "rpms/repo-1", "commit": "commit_hash1", - "srpm_nevra": "openssl-0:1.2.3-1.src", "target": "target1", "git_branch": "mybranch", "error": None, @@ -742,7 +738,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): } ] }, - 'rpm_manifest': { + 'rpm_manifest': [{ 'rpms': [ { "srpm_name": "openssl", @@ -753,12 +749,11 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): "srpm_nevra": "testpackage-10:1.2.3-1.src" } ] - } + }] }, { "repository": "rpms/repo-2", "commit": "commit_hash2", - "srpm_nevra": "openssl-1:1.2.3-1.src", "target": "target2", "git_branch": "mybranch", "error": None, @@ -783,7 +778,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): } ] }, - 'rpm_manifest': { + 'rpm_manifest': [{ 'rpms': [ { "srpm_name": "openssl", @@ -794,7 +789,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): "srpm_nevra": "testpackage2-10:1.2.3-1.src" } ] - } + }] } ]) @@ -834,6 +829,50 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): self.assertEqual(ret[0]["brew"]["package"], "package-name-1") self.assertEqual(ret[0]["content_sets"], set(["content-set"])) + @patch('freshmaker.lightblue.LightBlue.find_container_images') + @patch('os.path.exists') + def test_parent_images_no_rpm_manifest(self, exists, cont_images): + exists.return_value = True + images_without_rpm_manifest = [] + for data in self.fake_images_with_parsed_data: + img = ContainerImage.create(data) + del img["rpm_manifest"] + images_without_rpm_manifest.append(img) + + cont_images.side_effect = [images_without_rpm_manifest, [], + images_without_rpm_manifest] + + lb = LightBlue(server_url=self.fake_server_url, + cert=self.fake_cert_file, + private_key=self.fake_private_key) + ret = lb.find_parent_images_with_package( + self.fake_container_images[0], "openssl", + ["layer0", "layer1", "layer2", "layer3"]) + + self.assertEqual(0, len(ret)) + + @patch('freshmaker.lightblue.LightBlue.find_container_images') + @patch('os.path.exists') + def test_parent_images_empty_rpm_manifest(self, exists, cont_images): + exists.return_value = True + images_without_rpm_manifest = [] + for data in self.fake_images_with_parsed_data: + img = ContainerImage.create(data) + img["rpm_manifest"] = [] + images_without_rpm_manifest.append(img) + + cont_images.side_effect = [images_without_rpm_manifest, [], + images_without_rpm_manifest] + + lb = LightBlue(server_url=self.fake_server_url, + cert=self.fake_cert_file, + private_key=self.fake_private_key) + ret = lb.find_parent_images_with_package( + self.fake_container_images[0], "openssl", + ["layer0", "layer1", "layer2", "layer3"]) + + self.assertEqual(0, len(ret)) + @patch('freshmaker.lightblue.LightBlue.find_content_sets_for_repository') @patch('freshmaker.lightblue.LightBlue.find_container_images') @patch('os.path.exists')