From 5e3c4c3693d47531e0557820872fc51ea35e897f Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Mar 21 2018 08:04:23 +0000 Subject: Filter out images which are latest in deprecated repositories. --- diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index 2a84083..5423c1f 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -274,9 +274,15 @@ class ContainerImage(dict): # Checking only the first repository is OK, because if an image # is in multiple repositories, the content_sets of all of them # must be the same by definition. - image_content_sets = lb_instance.find_content_sets_for_repository( - self["repositories"][0]["repository"], published, deprecated, - release_category) + # But some older repositories don't have to have the content_sets + # set, so try to iterate over all of them and stop once we find + # some repository which returns some content_sets. + for repository in self["repositories"]: + image_content_sets = lb_instance.find_content_sets_for_repository( + repository["repository"], published, deprecated, + release_category) + if image_content_sets: + break log.info("Container image %s uses following content sets: %r", self["brew"]["build"], image_content_sets) @@ -526,6 +532,8 @@ class LightBlue(object): ret = set() for repo in repos: + if "content_sets" not in repo: + continue ret |= set(repo["content_sets"]) return sorted(list(ret)) @@ -539,6 +547,7 @@ class LightBlue(object): {"field": "parsed_data.layers.*", "include": True, "recursive": True}, {"field": "repositories.*.published", "include": True, "recursive": True}, {"field": "repositories.*.repository", "include": True, "recursive": True}, + {"field": "repositories.*.tags.*.name", "include": True, "recursive": True}, ] def _set_container_image_filters(self, request, published): @@ -601,7 +610,26 @@ class LightBlue(object): } image_request = self._set_container_image_filters( image_request, published) - return self.find_container_images(image_request) + images = self.find_container_images(image_request) + if not images: + return images + + # The image_request returns container images which are in the + # right repository and are latest in *some* repository. But we need + # those images to be latest in one of the `repositories`. It is not + # trivial to generate LB query like this, so filter this client-side + # for now. + expected_repositories = [r["repository"] for r in repositories] + new_images = [] + for image in images: + for repository in image["repositories"]: + tag_names = [tag["name"] for tag in repository["tags"]] + if (repository["repository"] in expected_repositories and + "latest" in tag_names): + new_images.append(image) + images = new_images + + return images def find_unpublished_image_for_build(self, build): """ @@ -834,7 +862,8 @@ class LightBlue(object): content_sets, published, deprecated, release_category) if not repos: return [] - images = self.find_images_with_included_srpm(repos, srpm_name, published) + images = self.find_images_with_included_srpm( + repos, srpm_name, published) # There can be multi-arch images which share the same # image['brew']['build']. Freshmaker is not interested in the image diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index 61f2376..c45bc9b 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -350,7 +350,8 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): 'package': 'package-name-1' }, 'repositories': [ - {'repository': 'product1/repo1', 'published': True} + {'repository': 'product1/repo1', 'published': True, + 'tags': [{"name": "latest"}]} ], 'parsed_data': { 'files': [ @@ -381,7 +382,8 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): 'package': 'package-name-2' }, 'repositories': [ - {'repository': 'product2/repo2', 'published': True} + {'repository': 'product2/repo2', 'published': True, + 'tags': [{"name": "latest"}]} ], 'parsed_data': { 'files': [ @@ -684,8 +686,13 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): }, "projection": lb._get_default_projection() } + cont_images.assert_called_with(expected_image_request) - self.assertEqual(ret, cont_images.return_value) + + # Only the second 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(ret, [cont_images.return_value[1]]) def _filter_fnc(self, image): return image["brew"]["build"].startswith("filtered_") @@ -703,7 +710,11 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): 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({"brew": {"build": "filtered_x-1-23"}})] + ContainerImage.create( + {"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 @@ -716,7 +727,10 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): ret = lb.find_images_with_package_from_content_set( "openssl", ["dummy-content-set-1"], filter_fnc=self._filter_fnc) - self.assertEqual(2, len(ret)) + # 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, [ { @@ -731,7 +745,10 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): "package": "package-name-2" }, 'content_sets': ['dummy-content-set-1', 'dummy-content-set-2'], - 'repositories': [{'repository': 'product2/repo2', 'published': True}], + 'repositories': [ + {'repository': 'product2/repo2', 'published': True, + 'tags': [{"name": "latest"}]} + ], 'parsed_data': { 'files': [ { @@ -759,41 +776,6 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): ] }] }, - { - "repository": "rpms/repo-1", - "commit": "commit_hash1", - "target": "target1", - "git_branch": "mybranch", - "error": None, - "brew": { - "completion_date": u"20170421T04:27:51.000-0400", - "build": "package-name-1-4-12.10", - "package": "package-name-1" - }, - 'repositories': [{'repository': 'product1/repo1', 'published': True}], - 'content_sets': ['dummy-content-set-1', 'dummy-content-set-2'], - '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-0:1.2.3-1.src" - }, - { - "srpm_name": "tespackage", - "srpm_nevra": "testpackage-10:1.2.3-1.src" - } - ] - }] - }, ]) @patch('freshmaker.lightblue.LightBlue.find_content_sets_for_repository')