From 351d8139e2f595285a8586d952c3cbe3aa207666 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Feb 02 2018 07:45:26 +0000 Subject: Deduplicate the images to rebuild found in Lightblue by keeping only the ones with highest release. --- diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index 0a7a552..edcc219 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -856,6 +856,73 @@ class LightBlue(object): image.resolve_commit(srpm_name) return images + def _deduplicate_images_to_rebuild(self, to_rebuild): + """ + Deduplicates the images to rebuild in `to_rebuild` in-place. + + The `to_rebuild` list is a list in following format: + [ + [child_image, parent_of_child_image, parent_of_parent, ...], + ... + ] + + This methods goes through all the images in `to_rebuild` list and + changes the list in a way that only single image with the highest + release will exist for the given image name-version. + + For example, if there are three images in a list - foo-1-2, foo-1-3 + and foo-2-2, the foo-1-3 will be used instead of foo-1-2 on every + occurence in a list, because the NVR is higher than NVR of foo-1-2. + The foo-2-2 will be kept unchanged in a list, because it is the + single record for the foo image in version 2. + """ + # Temporary dict mapping the NVR of image to coordinates in the + # `to_rebuild` list. For example + # nvr_to_coordinates["nvr"] = [0, 3] means that the image with + # nvr "nvr" is 4th image in the to_rebuild[0] list. + nvr_to_coordinates = {} + # Temporary dict mapping the NV to list of NVRs. The List of NVRs + # is always sorted descending. + nv_to_nvrs = {} + # Temporary dict mapping the NVR to image. + nvr_to_image = {} + + # Constructs the temporary dicts as desribed above. + for image_id, images in enumerate(to_rebuild): + for parent_id, image in enumerate(images): + nvr = image["brew"]["build"] + parsed_nvr = koji.parse_NVR(nvr) + nv = "%s-%s" % (parsed_nvr["name"], parsed_nvr["version"]) + if nv not in nv_to_nvrs: + nv_to_nvrs[nv] = [] + nv_to_nvrs[nv].append(nvr) + nvr_to_coordinates[nvr] = [image_id, parent_id] + nvr_to_image[nvr] = image + + # Sort the lists in nv_to_nvrs dict. + for nv in nv_to_nvrs.keys(): + nv_to_nvrs[nv].sort(reverse=True) + + # Iterate through list of NVs. + for nvrs in nv_to_nvrs.values(): + # Since nv_to_nvrs is sorted, nvrs[0] is always the NVR + # with highest release for given NV. + latest_nvr = nvrs[0] + # Now replace all others NVR with the highest one. + for nvr in nvrs[1:]: + # At first replace the image in to_rebuid based + # on the coordinates from temp dict. + image_id, parent_id = nvr_to_coordinates[nvr] + to_rebuild[image_id][parent_id] = nvr_to_image[latest_nvr] + + # And in case this image is not the the leaf image, also replace + # the ["parent"] record for the child image to point to the image + # with highest NVR. + if parent_id != 0: + to_rebuild[image_id][parent_id - 1]["parent"] = nvr_to_image[latest_nvr] + + return to_rebuild + def find_images_to_rebuild( self, srpm_name, content_sets, published=True, deprecated=False, release_category="Generally Available", filter_fnc=None): @@ -935,18 +1002,23 @@ class LightBlue(object): to_rebuild.append(rebuild_list) # The to_rebuild list now contains all the images which need to be - # rebuilt, but there are lot of duplicates there - for example for - # every RHSCL Docker image, there is s2i-base image (their shared - # parent image). - # Therefore, group the same parent images from the same inheritance - # level to not build them multiple times for each image, but just once. - - # Using dict for each batch to remove duplicate images + # rebuilt, but there are lot of duplicates there. + + # At first remove duplicated images which share the same namd and + # version, but different release. + to_rebuild = self._deduplicate_images_to_rebuild(to_rebuild) + + # Now create the batches with images. We still might find duplicate + # images in batch. For example if the image A depends on X and also + # image B depends on X, the X would be in the first batch twice. + # We remove duplicates like that using the dict for each batch with + # Brew build NVR as a key and add the image to the batch only when + # it is not in this dict. batches = [{} for i in range(max_len)] for image_rebuild_list in to_rebuild: for image, batch in zip(reversed(image_rebuild_list), batches): - image_key = '{0}_{1}'.format(image['repository'], - image['commit']) + image_key = image["brew"]["build"] + if image_key not in batch: batch[image_key] = image diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index 0d53f14..268bbf5 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -940,37 +940,37 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): }) leaf_image1 = ContainerImage.create({ - 'brew': {'build': 'leaf-image-1'}, + 'brew': {'build': 'leaf-image-1-1'}, 'parsed_data': {'layers': ['fake layer']}, 'repository': 'repo-1', 'commit': 'leaf-image1-commit', }) leaf_image2 = ContainerImage.create({ - 'brew': {'build': 'leaf-image-2'}, + 'brew': {'build': 'leaf-image-2-1'}, 'parsed_data': {'layers': ['fake layer']}, 'repository': 'repo-1', 'commit': 'leaf-image2-commit', }) leaf_image3 = ContainerImage.create({ - 'brew': {'build': 'leaf-image-3'}, + 'brew': {'build': 'leaf-image-3-1'}, 'parsed_data': {'layers': ['fake layer']}, 'repository': 'repo-1', 'commit': 'leaf-image3-commit', }) leaf_image4 = ContainerImage.create({ - 'brew': {'build': 'leaf-image-4'}, + 'brew': {'build': 'leaf-image-4-1'}, 'parsed_data': {'layers': ['fake layer']}, 'repository': 'repo-1', 'commit': 'leaf-image4-commit', }) leaf_image5 = ContainerImage.create({ - 'brew': {'build': 'leaf-image-5'}, + 'brew': {'build': 'leaf-image-5-1'}, 'parsed_data': {'layers': ['fake layer']}, 'repository': 'repo-1', 'commit': 'leaf-image5-commit', }) leaf_image6 = ContainerImage.create({ - 'brew': {'build': 'leaf-image-6'}, + 'brew': {'build': 'leaf-image-6-1'}, 'parsed_data': {'layers': ['fake layer']}, 'repository': 'repo-1', 'commit': 'leaf-image6-commit', @@ -1004,10 +1004,10 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): [leaf_image3] ] - self.assertEqual( - expected_batches, - [sorted(images, key=lambda image: image['brew']['build']) - for images in batches]) + returned_batches = [sorted(images, key=lambda image: image['brew']['build']) + for images in batches] + + self.assertEqual(expected_batches, returned_batches) @patch('freshmaker.lightblue.LightBlue.find_images_with_package_from_content_set') @patch('freshmaker.lightblue.LightBlue.find_unpublished_image_for_build') @@ -1121,3 +1121,70 @@ class TestEntityVersion(helpers.FreshmakerTestCase): call('find/containerRepository/', {}), call('find/containerImage/', {}), ]) + + +class TestDeduplicateImagesToRebuild(helpers.FreshmakerTestCase): + + def setUp(self): + super(TestDeduplicateImagesToRebuild, self).setUp() + self.fake_server_url = 'lightblue.localhost' + self.fake_cert_file = 'path/to/cert' + self.fake_private_key = 'path/to/private-key' + + self.os_path_exists_patcher = patch("os.path.exists") + self.os_path_exists_patcher.start() + + self.lb = LightBlue(server_url=self.fake_server_url, + cert=self.fake_cert_file, + private_key=self.fake_private_key) + + def tearDown(self): + super(TestDeduplicateImagesToRebuild, self).tearDown() + self.os_path_exists_patcher.stop() + + def _create_img(self, nvr): + return ContainerImage.create({ + 'brew': {'build': nvr} + }) + + def _create_imgs(self, nvrs): + images = [] + for nvr in nvrs: + image = self._create_img(nvr) + if images: + images[len(images) - 1]['parent'] = image + images.append(image) + return images + + def test_use_highest_nvr(self): + httpd = self._create_imgs([ + "httpd-2.4-12", + "s2i-base-1-3", + "s2i-core-1-2", + "rhel-server-docker-7.4-125", + ]) + + perl = self._create_imgs([ + "perl-5.7-1", + "s2i-base-1-1", + "s2i-core-1-1", + "rhel-server-docker-7.4-150", + ]) + + expected_images = [ + self._create_imgs([ + "httpd-2.4-12", + "s2i-base-1-3", + "s2i-core-1-2", + "rhel-server-docker-7.4-150", + ]), + self._create_imgs([ + "perl-5.7-1", + "s2i-base-1-3", + "s2i-core-1-2", + "rhel-server-docker-7.4-150", + ]) + ] + + ret = self.lb._deduplicate_images_to_rebuild([httpd, perl]) + self.assertEqual(ret, expected_images)