From c65e5594992ddf9fcfa4630aba55d4e7d695653b Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: May 24 2018 09:40:40 +0000 Subject: Handle situation when released image depends on unreleased image foo, and we also want to rebuild release version of image foo. --- diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index ea29ee1..d20765a 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -950,6 +950,9 @@ class LightBlue(object): image.resolve_content_sets(self, None, published, deprecated, release_category) image.resolve_commit() + # Images returned by this method are latest released images, so + # mark them like that. + image["latest_released"] = True return images def _deduplicate_images_to_rebuild(self, to_rebuild): @@ -982,6 +985,8 @@ class LightBlue(object): nv_to_nvrs = {} # Temporary dict mapping the NVR to image. nvr_to_image = {} + # Temporary dict mapping NV to latest released NVR for that NV. + nv_to_latest_released_nvr = {} # Constructs the temporary dicts as desribed above. for image_id, images in enumerate(to_rebuild): @@ -997,28 +1002,59 @@ class LightBlue(object): nvr_to_coordinates[nvr] = [] nvr_to_coordinates[nvr].append([image_id, parent_id]) nvr_to_image[nvr] = image + if "latest_released" in image and image["latest_released"]: + nv_to_latest_released_nvr[nv] = nvr # Sort the lists in nv_to_nvrs dict. for nv in nv_to_nvrs.keys(): nv_to_nvrs[nv] = sorted_by_nvr(nv_to_nvrs[nv], reverse=True) + # There might be container image NVRs which are not released yet, + # but some released image is already built on top of them. + # The issue is that such unreleased container image won't be in + # its containerRepository and therefore won't have proper + # content_sets set. + # In this case, we copy the content_sets from the released image. + # This might bring issue in case the content_sets changed + # dramaticaly between released and unreleased release of such + # image, but it's still the best guess we can do. + # This is also used only as fallback in case "content_sets.yml" + # does not exists in the dist-git repo, which should be rare + # situation. + latest_content_sets = [] + for nvr in reversed(nv_to_nvrs[nv]): + image = nvr_to_image[nvr] + if ("repositories" not in image or + len(image["repositories"]) == 0): + image["content_sets"] = latest_content_sets + else: + latest_content_sets = image["content_sets"] + # 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:]: + for nv, nvrs in nv_to_nvrs.items(): + # We want to replace NVRs which are lower than the latest released + # NVR with latest released NVR. If there are some higher NVRs, we + # want to keep them, because we don't want to rebuild the image + # against older NVR than the one it is currently built against. + if nv in nv_to_latest_released_nvr: + latest_released_nvr = nv_to_latest_released_nvr[nv] + else: + latest_released_nvr = nvrs[0] + # The latest_released_nvr_index points to the latest released NVR + # in the `nvrs` list. Because `nvrs` list is desc sorted, every NVR + # with higher index is lower and therefore we need to replace it. + latest_released_nvr_index = nvrs.index(latest_released_nvr) + for nvr in nvrs[latest_released_nvr_index + 1:]: for image_id, parent_id in nvr_to_coordinates[nvr]: # At first replace the image in to_rebuid based # on the coordinates from temp dict. - to_rebuild[image_id][parent_id] = nvr_to_image[latest_nvr] + to_rebuild[image_id][parent_id] = nvr_to_image[latest_released_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] + to_rebuild[image_id][parent_id - 1]["parent"] = nvr_to_image[latest_released_nvr] return to_rebuild diff --git a/tests/test_handler.py b/tests/test_handler.py index 4c6e073..3cae20a 100644 --- a/tests/test_handler.py +++ b/tests/test_handler.py @@ -22,8 +22,6 @@ # # Written by Chenxiong Qi -from mock import patch, PropertyMock - import freshmaker from freshmaker import db @@ -51,56 +49,6 @@ class MyHandler(ContainerBuildHandler): """Implement BaseHandler method""" -class TestKrbContextPreparedForBuildContainer(helpers.FreshmakerTestCase): - """Test krb_context for BaseHandler.build_container""" - - def setUp(self): - super(TestKrbContextPreparedForBuildContainer, self).setUp() - self.koji_service = patch('freshmaker.kojiservice.KojiService') - self.koji_service.start() - - def tearDown(self): - super(TestKrbContextPreparedForBuildContainer, self).tearDown() - self.koji_service.stop() - - @patch('freshmaker.utils.conf') - @patch('freshmaker.utils.krbContext') - @patch("freshmaker.config.Config.krb_auth_principal", - new_callable=PropertyMock, return_value="user@example.com") - def test_prepare_with_keytab(self, auth_principal, krbContext, conf): - conf.krb_auth_use_keytab = True - conf.krb_auth_principal = 'freshmaker/hostname@REALM' - conf.krb_auth_client_keytab = '/etc/freshmaker.keytab' - conf.krb_auth_ccache_file = '/tmp/freshmaker_cc' - - handler = MyHandler() - handler.build_container('image-name', 'f26', '1234') - - krbContext.assert_called_once_with( - using_keytab=True, - principal='freshmaker/hostname@REALM', - keytab_file='/etc/freshmaker.keytab', - ccache_file='/tmp/freshmaker_cc', - ) - - @patch('freshmaker.utils.conf') - @patch('freshmaker.utils.krbContext') - @patch("freshmaker.config.Config.krb_auth_principal", - new_callable=PropertyMock, return_value="user@example.com") - def test_prepare_with_normal_user_credential(self, auth_principal, krbContext, conf): - conf.krb_auth_use_keytab = False - conf.krb_auth_principal = 'somebody@REALM' - conf.krb_auth_ccache_file = '/tmp/freshmaker_cc' - - handler = MyHandler() - handler.build_container('image-name', 'f26', '1234') - - krbContext.assert_called_once_with( - principal='somebody@REALM', - ccache_file='/tmp/freshmaker_cc', - ) - - class TestContext(helpers.ModelsTestCase): """Test setting context of handler""" diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index 96d6d55..1f692fd 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -678,7 +678,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): }, ] }, - "projection": lb._get_default_projection() + "projection": lb._get_default_projection(srpm_names=["openssl"]) } cont_images.assert_called_with(expected_image_request) @@ -728,6 +728,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): self.assertEqual(ret, [ { + "latest_released": True, "repository": "rpms/repo-2", "commit": "commit_hash2", "target": "target2", @@ -1174,18 +1175,109 @@ class TestDeduplicateImagesToRebuild(helpers.FreshmakerTestCase): def _create_img(self, nvr): return ContainerImage.create({ - 'brew': {'build': nvr} + 'brew': {'build': nvr}, + 'content_sets': [], }) def _create_imgs(self, nvrs): images = [] - for nvr in nvrs: - image = self._create_img(nvr) + for data in nvrs: + if type(data) == list: + nvr = data[0] + image = self._create_img(nvr) + image.update(data[1]) + else: + image = self._create_img(data) if images: images[len(images) - 1]['parent'] = image images.append(image) return images + def test_copy_content_sets(self): + httpd = self._create_imgs([ + "httpd-2.4-12", + "s2i-base-1-10", + "s2i-core-1-11", + "rhel-server-docker-7.4-125", + ]) + + perl = self._create_imgs([ + "perl-5.7-1", + ["s2i-base-1-2", { + "content_sets": ["foo"], + "repositories": [{ + "repository": "product/repo1", + "content_sets": ["foo"] + }]}], + "s2i-core-1-2", + "rhel-server-docker-7.4-150", + ]) + + expected_images = [ + self._create_imgs([ + "httpd-2.4-12", + ["s2i-base-1-10", {"content_sets": ["foo"]}], + "s2i-core-1-11", + "rhel-server-docker-7.4-150", + ]), + self._create_imgs([ + "perl-5.7-1", + ["s2i-base-1-10", {"content_sets": ["foo"]}], + "s2i-core-1-11", + "rhel-server-docker-7.4-150", + ]) + ] + + ret = self.lb._deduplicate_images_to_rebuild([httpd, perl]) + self.assertEqual(ret, expected_images) + + def test_use_highest_latest_released_nvr(self): + httpd = self._create_imgs([ + "httpd-2.4-12", + "s2i-base-1-10", + "s2i-core-1-11", + "rhel-server-docker-7.4-125", + ]) + + perl = self._create_imgs([ + "perl-5.7-1", + ["s2i-base-1-2", {"latest_released": True}], + "s2i-core-1-2", + "rhel-server-docker-7.4-150", + ]) + + foo = self._create_imgs([ + "foo-5.7-1", + "s2i-base-1-1", + "s2i-core-1-2", + "rhel-server-docker-7.4-150", + ]) + + expected_images = [ + self._create_imgs([ + "httpd-2.4-12", + "s2i-base-1-10", + "s2i-core-1-11", + "rhel-server-docker-7.4-150", + ]), + self._create_imgs([ + "perl-5.7-1", + ["s2i-base-1-2", {"latest_released": True}], + "s2i-core-1-11", + "rhel-server-docker-7.4-150", + ]), + self._create_imgs([ + "foo-5.7-1", + ["s2i-base-1-2", {"latest_released": True}], + "s2i-core-1-11", + "rhel-server-docker-7.4-150", + ]) + ] + + self.maxDiff = None + ret = self.lb._deduplicate_images_to_rebuild([httpd, perl, foo]) + self.assertEqual(ret, expected_images) + def test_use_highest_nvr(self): httpd = self._create_imgs([ "httpd-2.4-12",