From 336eb7abca3876c65fed0b256ab16421b561eed6 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Mar 25 2020 01:14:30 +0000 Subject: Merge #495 `Refactor find latest image by its build NV` --- diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index d7e5efb..aa039ef 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -147,7 +147,11 @@ class ContainerImage(dict): return image def __hash__(self): - return hash((self['brew']['build'])) + return hash((self.nvr)) + + @property + def nvr(self): + return self['brew']['build'] def log_error(self, err): """ @@ -157,7 +161,7 @@ class ContainerImage(dict): """ prefix = "" if 'brew' in self and 'build' in self['brew']: - prefix = self['brew']['build'] + ": " + prefix = self.nvr + ": " log.error("%s%s", prefix, err) if 'error' not in self or not self['error']: self['error'] = str(err) @@ -191,8 +195,7 @@ class ContainerImage(dict): dockerfile = [file for file in self['parsed_data']['files'] if file['filename'] == 'Dockerfile'] if not dockerfile: - log.warning('Image %s does not contain a Dockerfile.', - self['brew']['build']) + log.warning('Image %s does not contain a Dockerfile.', self.nvr) return None return dockerfile[0] @@ -885,7 +888,7 @@ class LightBlue(object): ret = [] for image in images: if not content_sets & set(image["content_sets"]): - log.info(f"Will not rebuild {image['brew']['build']} because its content_sets " + log.info(f"Will not rebuild {image.nvr} because its content_sets " "({image['content_sets']}) are not related to the requested content_sets" " ({content_sets})") else: @@ -1406,6 +1409,7 @@ class LightBlue(object): the given image - can be used for comparisons if needed :rtype: list """ + repos = self.find_all_container_repositories(published, release_categories) if not repos: return [] @@ -1422,30 +1426,26 @@ class LightBlue(object): # architecture, it is only interested in NVR, so group the images # by the same image['brew']['build'] and include just first one in the # image list. - sorted_images = sorted_by_nvr( - images, get_nvr=lambda image: image['brew']['build'], reverse=True) + sorted_images = sorted_by_nvr(images, reverse=True) images = [] - for k, v in groupby(sorted_images, key=lambda x: x['brew']['build']): + for k, v in groupby(sorted_images, key=lambda item: item.nvr): images.append(next(v)) # In case we query for unpublished images, we need to return just # the latest NVR for given name-version, otherwise images would # contain all the versions which ever containing the srpm_name. if not published: - # Sort images by brew build NVR descending - sorted_images = sorted_by_nvr( - images, get_nvr=lambda image: image['brew']['build'], reverse=True) - # Iterate over all the images and only keep the very first one - # with the given name-version - this is the latest one. - images = [] - seen_name_versions = [] - for image in sorted_images: - parsed_build = koji.parse_NVR(image["brew"]["build"]) - nv = "%s-%s" % (parsed_build["name"], parsed_build["version"]) - if nv not in seen_name_versions: - images.append(image) - seen_name_versions.append(nv) + def _name_version_key(item): + nvr = koji.parse_NVR(item.nvr) + return f"{nvr['name']}-{nvr['version']}" + + images = [ + next(grouped_images) for _, grouped_images in groupby( + sorted_by_nvr(images, reverse=True), + key=_name_version_key + ) + ] # Filter out images based on the filter_fnc. if filter_fnc: @@ -1726,8 +1726,7 @@ class LightBlue(object): # This `srpm_name` is not in image. continue - unpublished = self.find_unpublished_image_for_build( - image['brew']['build']) + unpublished = self.find_unpublished_image_for_build(image.nvr) if not unpublished: image.log_error( "Cannot find unpublished version of image, Lightblue " diff --git a/freshmaker/utils.py b/freshmaker/utils.py index 6a658cf..52036d7 100644 --- a/freshmaker/utils.py +++ b/freshmaker/utils.py @@ -59,6 +59,9 @@ def sorted_by_nvr(lst, get_nvr=None, reverse=False): if get_nvr: nvr1 = get_nvr(item1) nvr2 = get_nvr(item2) + elif hasattr(item1, 'nvr') and hasattr(item2, 'nvr'): + nvr1 = item1.nvr + nvr2 = item2.nvr else: nvr1 = item1 nvr2 = item2 diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index f071a55..5886fa6 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -1446,6 +1446,111 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): @patch('freshmaker.kojiservice.KojiService.get_build') @patch('freshmaker.kojiservice.KojiService.get_task_request') @patch('os.path.exists') + def test_images_with_content_set_packages_unpublished( + self, exists, koji_task_request, koji_get_build, cont_images, cont_repos + ): + exists.return_value = True + cont_repos.return_value = self.fake_repositories_with_content_sets + + # "filtered_x-1-23" image will be filtered by filter_fnc. + cont_images.return_value = self.fake_container_images + [ + ContainerImage.create( + { + "content_sets": ["dummy-content-set-1"], + "brew": {"build": "filtered_x-1-23"}, + "repositories": [ + { + "repository": "product/repo1", + "published": True, + "tags": [{"name": "latest"}] + } + ] + } + )] + # Include the images for second time to ensure that they will be + # returned only once. This can happen when the image is multiarch. + cont_images.return_value += self.fake_container_images + + koji_task_request.side_effect = self.fake_koji_task_requests + koji_get_build.side_effect = self.fake_koji_builds + + lb = LightBlue(server_url=self.fake_server_url, + cert=self.fake_cert_file, + private_key=self.fake_private_key) + + ret = lb.find_images_with_packages_from_content_set( + set(["openssl-1.2.3-3"]), ["dummy-content-set-1"], + filter_fnc=self._filter_fnc, + published=False + ) + + # Only the first image should be returned, because the first one + # is in repository "product1/repo1", but we have asked for images + # in repository "product/repo1". + self.assertEqual(1, len(ret)) + self.assertEqual(ret, + [ + { + "latest_released": True, + "generate_pulp_repos": True, + "repository": "rpms/repo-2", + "commit": "commit_hash2", + "target": "target2", + "git_branch": "mybranch", + "error": None, + "arches": None, + "odcs_compose_ids": None, + "parent_build_id": None, + "parent_image_builds": None, + "multi_arch_rpm_manifest": {}, + "published": True, + "brew": { + "completion_date": u"20170421T04:27:51.000-0400", + "build": "package-name-2-4-12.10", + "package": "package-name-2" + }, + 'content_sets': ["dummy-content-set-1"], + 'content_sets_source': 'lightblue_container_image', + 'directly_affected': True, + "release_categories": ["Generally Available"], + 'repositories': [ + {'repository': 'product2/repo2', 'published': True, + 'tags': [{"name": "latest"}]} + ], + 'parsed_data': { + 'files': [ + { + 'key': 'buildfile', + 'content_url': 'http://git.repo.com/cgit/rpms/repo-2/plain/Dockerfile?id=commit_hash2', + 'filename': 'Dockerfile' + }, + { + 'key': 'bogusfile', + 'content_url': 'bogus_test_url', + 'filename': 'bogus.file' + } + ] + }, + 'rpm_manifest': [{ + 'rpms': [ + { + "srpm_name": "openssl", + "srpm_nevra": "openssl-1:1.2.3-1.src" + }, + { + "srpm_name": "tespackage2", + "srpm_nevra": "testpackage2-10:1.2.3-1.src" + } + ] + }] + }, + ]) + + @patch('freshmaker.lightblue.LightBlue.find_container_repositories') + @patch('freshmaker.lightblue.LightBlue.find_container_images') + @patch('freshmaker.kojiservice.KojiService.get_build') + @patch('freshmaker.kojiservice.KojiService.get_task_request') + @patch('os.path.exists') def test_images_with_content_set_packages_beta( self, exists, koji_task_request, koji_get_build, cont_images, cont_repos): exists.return_value = True