PR#309: Limit the LB query using list of container images in case ManualRebuildWithAdvisoryEvent is handled. - freshmaker

freshmaker

#309 Limit the LB query using list of container images in case ManualRebuildWithAdvisoryEvent is handled.

Merged 5 years ago by jkaluza. Opened 5 years ago by jkaluza.

jkaluza/freshmaker rebuild-with-advisory into master

Limit the LB query using list of container images in case ManualRebuildWithAdvisoryEvent is handled.

Jan Kaluza • 5 years ago

cb7df2f

freshmaker/handlers/errata/errata_advisory_rpms_signed.py

file modified

+8 -1

		`@@ -32,6 +32,7 @@`
		`from freshmaker import conf, db, log`
		`from freshmaker.events import ErrataAdvisoryRPMsSignedEvent`
		`from freshmaker.events import ODCSComposeStateChangeEvent`
		`+ from freshmaker.events import ManualRebuildWithAdvisoryEvent`
		`from freshmaker.handlers import ContainerBuildHandler, fail_event_on_handler_exception`
		`from freshmaker.kojiservice import koji_service`
		`from freshmaker.lightblue import LightBlue`
		`@@ -668,6 +669,11 @@`
		`srpm_name = koji.parse_NVR(nvr)['name']`
		`srpm_names.add(srpm_name)`

		`+ # Limit the Lightblue query to particular leaf images if set in Event.`
		`+ leaf_container_images = None`
		`+ if isinstance(self.event, ManualRebuildWithAdvisoryEvent):`
		`+ leaf_container_images = self.event.container_images`
		`+`
		`# For each SRPM name, find out all the containers which include`
		`# this SRPM name.`
		`self.log_info(`
		`@@ -676,7 +682,8 @@`
		`batches = lb.find_images_to_rebuild(`
		`srpm_names, content_sets,`
		`filter_fnc=self._filter_out_not_allowed_builds,`
		`- published=published, release_category=release_category)`
		`+ published=published, release_category=release_category,`
		`+ leaf_container_images=leaf_container_images)`
		`return batches`

		`def _find_build_srpm_name(self, build_nvr):`

freshmaker/lightblue.py

file modified

+84 -5

		`@@ -819,6 +819,73 @@`
		`images = new_images`
		`return images`

		`+ def get_images_by_nvrs(self, nvrs, published=True, content_sets=None,`
		`+ srpm_names=None):`
		`+ """Query lightblue and returns containerImages defined by list of`
		+ `nvrs`.
		`+`
		`+ :param list nvrs: List of NVRs defining the containerImages to return.`
		`+ :param bool published: whether to limit queries to published images`
		`+ :param list content_sets: List of content_sets the image includes RPMs`
		`+ from.`
		`+ :param list srpm_names: list of srpm_name (source rpm name) to look for`
		`+ :return: List of containerImages.`
		`+ :rtype: list of ContainerImages.`
		`+ """`
		`+ image_request = {`
		`+ "objectType": "containerImage",`
		`+ "query": {`
		`+ "$and": [`
		`+ {`
		`+ "$or": [{`
		`+ "field": "brew.build",`
		`+ "op": "=",`
		`+ "rvalue": nvr`
		`+ } for nvr in nvrs]`
		`+ },`
		`+ {`
		`+ "field": "parsed_data.files.*.key",`
		`+ "op": "=",`
		`+ "rvalue": "buildfile"`
		`+ },`
		`+ ]`
		`+ },`
		`+ "projection": self._get_default_projection()`
		`+ }`
		`+`
		`+ if content_sets is not None:`
		`+ image_request["query"]["$and"].append(`
		`+ {`
		`+ "$or": [{`
		`+ "field": "content_sets.*",`
		`+ "op": "=",`
		`+ "rvalue": r`
		`+ } for r in content_sets]`
		`+ }`
		`+ )`
		`+`
		`+ if srpm_names is not None:`
		`+ image_request["query"]["$and"].append(`
		`+ {`
		`+ "$or": [{`
		`+ "field": "rpm_manifest..rpms..srpm_name",`
		`+ "op": "=",`
		`+ "rvalue": srpm_name`
		`+ } for srpm_name in srpm_names]`
		`+ }`
		`+ )`
		`+`
		`+ if published is not None:`
		`+ image_request["query"]["$and"].append(`
		`+ {`
		`+ "field": "repositories.*.published",`
		`+ "op": "=",`
		`+ "rvalue": published`
		`+ })`
		`+`
		`+ images = self.find_container_images(image_request)`
		`+ return images`
		`+`
		`def find_unpublished_image_for_build(self, build):`
		`"""`
		Returns the unpublished variant of Docker image specified by `build`
		`@@ -1014,7 +1081,8 @@`
		`def find_images_with_packages_from_content_set(`
		`self, srpm_names, content_sets, filter_fnc=None,`
		`published=True, deprecated=False,`
		`- release_category="Generally Available"):`
		`+ release_category="Generally Available",`
		`+ leaf_container_images=None):`
		`"""Query lightblue and find containers which contain given`
		`package from one of content sets`

		`@@ -1033,6 +1101,9 @@`
		`repositories`
		`:param str release_category: filter only repositories with specific`
		`release category (options: Deprecated, Generally Available, Beta, Tech Preview)`
		`+ :param list leaf_container_images: List of NVRs of leaf images to`
		`+ consider for the rebuild. If not set, all images found in`
		`+ Lightblue will be considered for rebuild.`

		`:return: a list of dictionaries with three keys - repository, commit and`
		`srpm_nevra. Repository is a name git repository including the`
		`@@ -1045,8 +1116,12 @@`
		`published, deprecated, release_category)`
		`if not repos:`
		`return []`
		`- images = self.find_images_with_included_srpms(`
		`- content_sets, srpm_names, repos, published)`
		`+ if not leaf_container_images:`
		`+ images = self.find_images_with_included_srpms(`
		`+ content_sets, srpm_names, repos, published)`
		`+ else:`
		`+ images = self.get_images_by_nvrs(`
		`+ leaf_container_images, published, content_sets, srpm_names)`

		`# There can be multi-arch images which share the same`
		`# image['brew']['build']. Freshmaker is not interested in the image`
		`@@ -1284,7 +1359,8 @@`

		`def find_images_to_rebuild(`
		`self, srpm_names, content_sets, published=True, deprecated=False,`
		`- release_category="Generally Available", filter_fnc=None):`
		`+ release_category="Generally Available", filter_fnc=None,`
		`+ leaf_container_images=None):`
		`"""`
		`Find images to rebuild through image build layers`

		`@@ -1309,10 +1385,13 @@`
		`will not be considered for a rebuild as well as its parent images.`
		`This function is used to filter out images not allowed by`
		`Freshmaker configuration.`
		`+ :param list leaf_container_images: List of NVRs of leaf images to`
		`+ consider for the rebuild. If not set, all images found in`
		`+ Lightblue will be considered for rebuild.`
		`"""`
		`images = self.find_images_with_packages_from_content_set(`
		`srpm_names, content_sets, filter_fnc, published, deprecated,`
		`- release_category)`
		`+ release_category, leaf_container_images=leaf_container_images)`

		`def _get_images_to_rebuild(image):`
		`"""`

tests/test_errata_advisory_rpms_signed_handler.py

file modified

+35 -6

		`@@ -226,10 +226,11 @@`

		`def test_can_handle_manual_rebuild_with_advisory_event(self):`
		`event = ManualRebuildWithAdvisoryEvent(`
		`- "123", ["foo-container", "bar-container"],`
		`+ "123",`
		`ErrataAdvisory(123, "RHBA-2017", "REL_PREP", [],`
		`security_impact="",`
		`- product_short_name="product"))`
		`+ product_short_name="product"),`
		`+ ["foo-container", "bar-container"])`
		`handler = ErrataAdvisoryRPMsSignedHandler()`
		`ret = handler.can_handle(event)`
		`self.assertTrue(ret)`
		`@@ -632,6 +633,12 @@`
		`ErrataAdvisory(123, "RHBA-2017", "REL_PREP", [],`
		`security_impact="",`
		`product_short_name="product"))`
		`+ self.manual_event = ManualRebuildWithAdvisoryEvent(`
		`+ "123",`
		`+ ErrataAdvisory(123, "RHBA-2017", "REL_PREP", [],`
		`+ security_impact="",`
		`+ product_short_name="product"),`
		`+ ["foo", "bar"])`
		`self.handler = ErrataAdvisoryRPMsSignedHandler()`
		`self.handler.event = self.event`

		`@@ -652,7 +659,8 @@`
		`self.find_images_to_rebuild.assert_called_once_with(`
		`set(['httpd']), ['content-set-1'],`
		`filter_fnc=self.handler._filter_out_not_allowed_builds,`
		`- published=True, release_category='Generally Available')`
		`+ published=True, release_category='Generally Available',`
		`+ leaf_container_images=None)`

		`@patch.object(freshmaker.conf, 'handler_build_whitelist', new={`
		`'ErrataAdvisoryRPMsSignedHandler': {`
		`@@ -668,7 +676,8 @@`
		`self.find_images_to_rebuild.assert_called_once_with(`
		`set(['httpd']), ['content-set-1'],`
		`filter_fnc=self.handler._filter_out_not_allowed_builds,`
		`- published=True, release_category='Generally Available')`
		`+ published=True, release_category='Generally Available',`
		`+ leaf_container_images=None)`

		`@patch.object(freshmaker.conf, 'handler_build_whitelist', new={`
		`'ErrataAdvisoryRPMsSignedHandler': {`
		`@@ -686,7 +695,8 @@`
		`self.find_images_to_rebuild.assert_called_once_with(`
		`set(['httpd']), ['content-set-1'],`
		`filter_fnc=self.handler._filter_out_not_allowed_builds,`
		`- published=None, release_category=None)`
		`+ published=None, release_category=None,`
		`+ leaf_container_images=None)`

		`@patch.object(freshmaker.conf, 'handler_build_whitelist', new={`
		`'ErrataAdvisoryRPMsSignedHandler': {`
		`@@ -702,4 +712,23 @@`
		`self.find_images_to_rebuild.assert_called_once_with(`
		`set(['httpd']), ['content-set-1'],`
		`filter_fnc=self.handler._filter_out_not_allowed_builds,`
		`- published=True, release_category='Generally Available')`
		`+ published=True, release_category='Generally Available',`
		`+ leaf_container_images=None)`
		`+`
		`+ @patch.object(freshmaker.conf, 'handler_build_whitelist', new={`
		`+ 'ErrataAdvisoryRPMsSignedHandler': {`
		`+ 'image': {'advisory_name': 'RHBA-*',`
		`+ 'published': True}`
		`+ }`
		`+ })`
		`+ @patch('os.path.exists', return_value=True)`
		`+ def test_manual_event_leaf_container_images(self, exists):`
		`+ self.handler.event = self.manual_event`
		`+ for x in self.handler._find_images_to_rebuild(123456):`
		`+ pass`
		`+`
		`+ self.find_images_to_rebuild.assert_called_once_with(`
		`+ set(['httpd']), ['content-set-1'],`
		`+ filter_fnc=self.handler._filter_out_not_allowed_builds,`
		`+ published=True, release_category='Generally Available',`
		`+ leaf_container_images=["foo", "bar"])`

tests/test_lightblue.py

file modified

+38

		`@@ -1360,6 +1360,44 @@`
		`"openssl",`
		`["dummy-content-set-1"])`

		`+ @patch('freshmaker.lightblue.ContainerImage.resolve')`
		`+ @patch('freshmaker.lightblue.LightBlue.find_container_repositories')`
		`+ @patch('freshmaker.lightblue.LightBlue.find_container_images')`
		`+ @patch('os.path.exists')`
		`+ def test_images_with_content_set_packages_leaf_container_images(`
		`+ self, exists, cont_images, cont_repos, resolve):`
		`+`
		`+ exists.return_value = True`
		`+ cont_images.return_value = self.fake_container_images`
		`+ cont_repos.return_value = self.fake_repositories_with_content_sets`
		`+`
		`+ lb = LightBlue(server_url=self.fake_server_url,`
		`+ cert=self.fake_cert_file,`
		`+ private_key=self.fake_private_key)`
		`+ lb.find_images_with_packages_from_content_set(`
		`+ ["openssl"], ["dummy-content-set"],`
		`+ leaf_container_images=["foo", "bar"])`
		`+ cont_images.assert_called_once_with(`
		`+ {'query': {`
		`+ '$and': [`
		`+ {'$or': [`
		`+ {'field': 'brew.build', 'rvalue': 'foo', 'op': '='},`
		`+ {'field': 'brew.build', 'rvalue': 'bar', 'op': '='}]},`
		`+ {'field': 'parsed_data.files.*.key', 'rvalue': 'buildfile', 'op': '='},`
		`+ {'$or': [{'field': 'content_sets.*', 'rvalue': 'dummy-content-set', 'op': '='}]},`
		`+ {'$or': [{'field': 'rpm_manifest..rpms..srpm_name', 'rvalue': 'openssl', 'op': '='}]},`
		`+ {'field': 'repositories.*.published', 'rvalue': True, 'op': '='}]},`
		`+ 'projection': [{'field': 'brew', 'include': True, 'recursive': True},`
		`+ {'field': 'parsed_data.files', 'include': True, 'recursive': True},`
		`+ {'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},`
		`+ {'field': 'content_sets', 'include': True, 'recursive': True},`
		`+ {'field': 'rpm_manifest.*.rpms', 'include': True, 'recursive': True},`
		`+ {'field': 'rpm_manifest..rpms..srpm_name', 'include': True, 'recursive': True}],`
		`+ 'objectType': 'containerImage'})`
		`+`

		`class TestEntityVersion(helpers.FreshmakerTestCase):`
		`"""Test case for ensuring correct entity version in request"""`

jkaluza commented 5 years ago

In previous PR, I made it possible to generate and handle ManualRebuildWithAdvisoryEvent in the ErrataAdvisoryRPMSSignedHandler which is responsible for building updated container images as a result of advisory update.

The ManualRebuildWithAdvisoryEvent contains container_images list with NVRs of container images to rebuild. This list is so far not respected by the handler - it would rebuild all the images no matter which ones have been marked for rebuild when triggering manual rebuild.

This PR changes the ErrataAdvisoryRPMsSignedHandler to respect container_images list - it passes this list down to Lightblue class which uses it to get just those container images from Lightblue instead of querying all images in lightblue.