From ee823a4ed7804bd46e6d067af3b2115e8be4d8a8 Mon Sep 17 00:00:00 2001 From: Giulia Naponiello Date: Jun 01 2020 09:17:17 +0000 Subject: Perform non-CVE builds Freshmaker now will be able to perform non-CVE builds, when triggered by a user. It will take care of building the requested images in the correct order, and to fill possible gaps between images. ref: FACTORY-5837 Signed-off-by: Giulia Naponiello --- diff --git a/dev_scripts/test_async_rebuild.py b/dev_scripts/test_async_rebuild.py new file mode 100755 index 0000000..b25908f --- /dev/null +++ b/dev_scripts/test_async_rebuild.py @@ -0,0 +1,48 @@ +#!/usr/bin/env python +""" +Example usage: + ./test_async_rebuild.py BRANCH_NAME CSV_IMAGES_LIST + ./test_async_rebuild.py extras-rhel-7.8 etcd-container,rhel-server-container +""" +import os +import sys +from unittest.mock import patch +from logging.config import dictConfig +import fedmsg.config + +# Allow imports from parent directory. +sys.path.insert(1, os.path.join(sys.path[0], '..')) + +# Set the FRESHMAKER_DEVELOPER_ENV variable. +os.environ["FRESHMAKER_DEVELOPER_ENV"] = "1" +os.environ["FRESHMAKER_CONFIG_FILE"] = os.path.join(sys.path[0], "config.py") +os.environ["REQUESTS_CA_BUNDLE"] = "/etc/ssl/certs/ca-bundle.crt" + +from freshmaker import db, app +from freshmaker.events import FreshmakerAsyncManualBuildEvent +from freshmaker.handlers.koji import RebuildImagesOnRPMAdvisoryChange, RebuildImagesOnAsyncManualBuild + +fedmsg_config = fedmsg.config.load_config() +dictConfig(fedmsg_config.get('logging', {'version': 1})) + +if len(sys.argv) < 3: + print("Usage: ./test_async_rebuild.py BRANCH CSV_LIST_IMAGES") + sys.exit(1) + +app_context = app.app_context() +app_context.__enter__() + +db.drop_all() +db.create_all() +db.session.commit() + +branch = sys.argv[1] +images = sys.argv[2].split(',') +kwargs = {} + +event = FreshmakerAsyncManualBuildEvent(msg_id='fake-msg', dist_git_branch=branch, container_images=images, dry_run=True) + +handler = RebuildImagesOnAsyncManualBuild() +with patch("freshmaker.consumer.get_global_consumer"): + with patch("freshmaker.handlers.koji.RebuildImagesOnAsyncManualBuild.start_to_build_images"): + handler.handle(event) diff --git a/freshmaker/config.py b/freshmaker/config.py index 79e936e..3f4654d 100644 --- a/freshmaker/config.py +++ b/freshmaker/config.py @@ -160,7 +160,7 @@ class Config(object): 'parsers': { 'type': list, 'default': [ - 'freshmaker.parsers.internal:FreshmakerAsyncManualbuildParser', + 'freshmaker.parsers.koji:FreshmakerAsyncManualbuildParser', 'freshmaker.parsers.internal:FreshmakerManualRebuildParser', 'freshmaker.parsers.brew:BrewTaskStateChangeParser', 'freshmaker.parsers.errata:ErrataAdvisoryStateChangedParser', diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index b1d2be9..dbbee07 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -524,7 +524,7 @@ class ContainerBuildHandler(BaseHandler): compose_ids = [] for relation in build.composes: compose_ids.append(relation.compose.odcs_compose_id) - if args["renewed_odcs_compose_ids"]: + if args.get("renewed_odcs_compose_ids"): compose_ids += args["renewed_odcs_compose_ids"] for compose_id in compose_ids: diff --git a/freshmaker/handlers/internal/__init__.py b/freshmaker/handlers/internal/__init__.py index 0bc806c..f701278 100644 --- a/freshmaker/handlers/internal/__init__.py +++ b/freshmaker/handlers/internal/__init__.py @@ -23,4 +23,3 @@ from .update_db_on_advisory_change import UpdateDBOnAdvisoryChange # noqa from .generate_advisory_signed_event_on_rpm_sign import GenerateAdvisorySignedEventOnRPMSign # noqa from .update_db_on_odcs_compose_fail import UpdateDBOnODCSComposeFail # noqa from .cancel_event_on_freshmaker_manage_request import CancelEventOnFreshmakerManageRequest # noqa -from .rebuild_images_on_async_manual_build import RebuildOnAsyncManualBuild # noqa \ No newline at end of file diff --git a/freshmaker/handlers/internal/rebuild_images_on_async_manual_build.py b/freshmaker/handlers/internal/rebuild_images_on_async_manual_build.py deleted file mode 100644 index 60eb4fe..0000000 --- a/freshmaker/handlers/internal/rebuild_images_on_async_manual_build.py +++ /dev/null @@ -1,33 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright (c) 2020 Red Hat, Inc. -# -# Permission is hereby granted, free of charge, to any person obtaining a copy -# of this software and associated documentation files (the "Software"), to deal -# in the Software without restriction, including without limitation the rights -# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -# copies of the Software, and to permit persons to whom the Software is -# furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice shall be included in all -# copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -# SOFTWARE. - -from freshmaker.handlers import BaseHandler -from freshmaker.events import FreshmakerAsyncManualBuildEvent - - -class RebuildOnAsyncManualBuild(BaseHandler): - """Rebuild images on async.manual.build""" - - def can_handle(self, event): - return isinstance(event, FreshmakerAsyncManualBuildEvent) - - def handle(self, event): - """ TODO: """ diff --git a/freshmaker/handlers/koji/__init__.py b/freshmaker/handlers/koji/__init__.py index 7f28a9b..3782e87 100644 --- a/freshmaker/handlers/koji/__init__.py +++ b/freshmaker/handlers/koji/__init__.py @@ -22,3 +22,4 @@ from .rebuild_images_on_odcs_compose_done import RebuildImagesOnODCSComposeDone # noqa from .rebuild_images_on_parent_image_build import RebuildImagesOnParentImageBuild # noqa from .rebuild_images_on_rpm_advisory_change import RebuildImagesOnRPMAdvisoryChange # noqa +from .rebuild_images_on_async_manual_build import RebuildImagesOnAsyncManualBuild # noqa diff --git a/freshmaker/handlers/koji/rebuild_images_on_async_manual_build.py b/freshmaker/handlers/koji/rebuild_images_on_async_manual_build.py new file mode 100644 index 0000000..0fc7f92 --- /dev/null +++ b/freshmaker/handlers/koji/rebuild_images_on_async_manual_build.py @@ -0,0 +1,381 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2020 Red Hat, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +import koji +import json + +from freshmaker import conf, db, log +from freshmaker.lightblue import LightBlue +from freshmaker.handlers import ContainerBuildHandler, fail_event_on_handler_exception +from freshmaker.events import FreshmakerAsyncManualBuildEvent +from freshmaker.types import EventState +from freshmaker.models import Event +from freshmaker.kojiservice import koji_service +from freshmaker.types import ArtifactBuildState, ArtifactType +from freshmaker.utils import sorted_by_nvr + + +class RebuildImagesOnAsyncManualBuild(ContainerBuildHandler): + """Rebuild images on async.manual.build""" + + name = 'RebuildImagesOnAsyncManualBuild' + + def can_handle(self, event): + return isinstance(event, FreshmakerAsyncManualBuildEvent) + + @fail_event_on_handler_exception + def handle(self, event): + """ + Rebuilds all container images requested by the user and the tree in between the + requested images, if relationships between them are found. + """ + + if event.dry_run: + self.force_dry_run() + + self.event = event + + db_event = Event.get_or_create_from_event(db.session, event) + self.set_context(db_event) + + # Check if we are allowed to build this image. + if not self.event.is_allowed(self, ArtifactType.IMAGE): + msg = ("This image rebuild is not allowed by internal policy. " + f"message_id: {event.msg_id}") + db_event.transition(EventState.SKIPPED, msg) + db.session.commit() + self.log_info(msg) + return [] + + lb = self.init_lightblue_instance() + # images contains at this point a list of images with all NVR for the same package + images = self._find_images_to_rebuild(lb) + + # Since the input is an image name, and not an NVR, freshmaker won't be able to know + # exactly which one needs to be rebuilt. For this reason Freshmaker asked Lightblue + # all the NVRs that match that image name. Now we need to check which one has the + # dist_git_branch. If more than one is found Freshmaker will choose the one with the + # highest NVR. + images = self.filter_images_based_on_dist_git_branch(images, db_event) + if not images: + return [] + + # If the user requested to rebuild only one image, there's no need to find out all the tree + # it is just more efficient to return that single image + if len(images) == 1: + batches = [images] + else: + images_trees = self.find_images_trees_to_rebuild(images, lb) + to_rebuild = self.filter_out_unrelated_images(images_trees) + batches = self.generate_batches(to_rebuild, images, lb) + + builds = self._record_batches(batches, db_event, lb) + + if not builds: + msg = f"No container images to rebuild for event with message_id {event.msg_id}" + self.log_info(msg) + db_event.transition(EventState.SKIPPED, msg) + db.session.commit() + return [] + + if all([build.state == ArtifactBuildState.FAILED.value + for build in builds.values()]): + db_event.transition( + EventState.COMPLETE, + "No container images to rebuild, all are in failed state.") + db.session.commit() + return [] + + self.start_to_build_images( + db_event.get_image_builds_in_first_batch(db.session)) + + msg = 'Rebuilding %d container images.' % (len(db_event.builds.all())) + db_event.transition(EventState.BUILDING, msg) + + return [] + + def init_lightblue_instance(self): + return LightBlue(server_url=conf.lightblue_server_url, + cert=conf.lightblue_certificate, + private_key=conf.lightblue_private_key, + event_id=self.event.msg_id) + + def filter_out_unrelated_images(self, batches): + """ + Filters out images that are unrelated to the requested images. + Example: + batches = + [ + [image_b, image_a, image_0], + [image_d, image_a, image_0] + ] + Returns: + [ + [image_b], + [image_d] + ] + + :param batches: list of lists, the first item is the image requested and the + following items are its tree, up to the base image. + :return: container images that will actually be rebuilt, because the user requested them + or because they are part of the tree that needs to be rebuilt. + """ + new_batches = [] + for batch in batches: + # We expect the first item in the list to always be in the requested images + # if not, there must be something wrong... maybe we should return an error. + if batch[0]['brew']['package'] not in self.event.container_images: + self.log_info('Unexpected error identifying images to rebuild.') + return [] + filtered_batch = [] + maybe_batch = [] + for image in batch: + maybe_batch.append(image) + if image['brew']['package'] in self.event.container_images: + filtered_batch.extend(maybe_batch) + maybe_batch = [] + new_batches.append(filtered_batch) + return new_batches + + def generate_batches(self, to_rebuild, images, lb): + # Get all the directly affected images so that any parents that are not marked as + # directly affected can be set in _images_to_rebuild_to_batches + directly_affected_nvrs = { + image.nvr for image in images if image.get("directly_affected") + } + # Now generate batches from deduplicated list and return it. + return lb._images_to_rebuild_to_batches(to_rebuild, directly_affected_nvrs) + + def _find_images_to_rebuild(self, lb): + """ + Since the input is an image name, and not an NVR, freshmaker won't be able to know + exactly which one needs to be rebuilt. For this reason Freshmaker will ask Lightblue + all the NVRs that match that image name. It will then check which one has the + dist_git_branch. If more than one is found Freshmaker will choose the one with the highest + NVRArtifactType.IMAGE. + + :param lb LightBlue: LightBlue instance + :return: list of container images matching a certain name. + :rtype: list + """ + + return lb.get_images_by_brew_package(self.event.container_images) + + def get_image_tree(self, lb, image, tree): + """ + This method recursively finds the tree for given image, up to the base image. + At every recursive call it will add one element of the tree. + When the base image is reached, the tree will be completed. + + :param lb LightBlue: LightBlue instance + :param image ContainerImage: image of which we want the tree. + :param tree list: the list of images that we found until now. + :return: list of images, in this order: [parent, grandparent, ..., baseimage] + :rtype: list + """ + parent_nvr = lb.find_parent_brew_build_nvr_from_child(image) + if parent_nvr: + parent = lb.get_images_by_nvrs([parent_nvr], published=None) + if parent: + parent = parent[0] + parent.resolve(lb) + image['parent'] = parent + tree.append(parent) + return self.get_image_tree(lb, parent, tree) + return tree + + def filter_images_based_on_dist_git_branch(self, images, db_event): + """ + Filter images based on the dist-git branch requested by the user. If the images were never + be built for that branch, let's skip the build. + The input images are all the images matching a specific name. In this method we also select + the images with higher NVR for the same name (package). + + :param images list: images to rebuild. + :param db_event Event: event object in the db. + :return: list of images to rebuild. If the event gets skipped, return empty list. + :rtype: list + """ + with koji_service( + conf.koji_profile, log, dry_run=conf.dry_run, + login=False) as session: + + # Sort images by nvr + images = sorted_by_nvr(images, reverse=True) + + # Use a dict to map a package (name) to the highest NVR image. For example: + # {"ubi8-container": ContainerImage, + # "nodejs-12-container": ContainerImage)} + images_to_rebuild = {} + + # In case the user requested to build ['s2i-core-container', 'cnv-libvirt-container'] + # lightblue will return a bunch of NVRs for each name, example: + # * s2i-core-container-1-127 + # * s2i-core-container-1-126 + # * ... + # * cnv-libvirt-container-1.3-1 + # * cnv-libvirt-container-1.2-4 + # * ... + # Since `images` is a list of sorted NVRs, we just need to select the first NVR for + # each package (name). + for image in images: + build = None + git_branch = None + + package = image['brew']['package'] + # if package is already in images_to_rebuild we don't need to keep searching + # since the images were sorted by NVR in the beginning + if package not in images_to_rebuild: + # Let's get the branch for this image + build = session.get_build(image.nvr) + task_id = build.get("extra", {}).get("container_koji_task_id") + if task_id: + task = session.get_task_request(task_id) + # The task_info should always be in the 3rd element + task_info = task[2] + git_branch = task_info.get("git_branch") if len(task_info) else None + + if (build and task_id and git_branch and + self.event.dist_git_branch == git_branch): + images_to_rebuild[package] = image + + if not images_to_rebuild or len(images_to_rebuild) < len(self.event.container_images): + # If we didn't find images to rebuild, or we found less than what the user asked + # it means that some of those images were never been built before for the requested + # branch. In this case we need to throw an error because we won't build something + # that was never built before. + # We cannot return to the API with an error, because the request already completed + # at this point. Let's mark this build as FAILED then. + msg = ("One or more of the requested image was never built before for the " + f"requested branch: {self.event.dist_git_branch}. " + "Cannot build it, please change your request.") + missing_images = set(self.event.container_images) - set(images_to_rebuild.keys()) + if missing_images: + msg += f" Problematic images are {missing_images}" + db_event.transition(EventState.FAILED, msg) + db.session.commit() + self.log_info(msg) + return [] + + # The result needs to be a list + return list(images_to_rebuild.values()) + + def find_images_trees_to_rebuild(self, images_to_rebuild, lb): + """ + At this point images_to_rebuilds contains the images to rebuild requested by the + user. We now need to find out if there's some dependency between these images. + Example: image A is parent image of B, B is parent image of C (A > B > C). The user + requests to build A and C. In this case we'll also have to build B, and we need to + build all of them in the right order (first A, then B, and C in the end). + Let's find out if those images are related to each other. To do that, we find check + all the hierarchy of these images until we don't reach the base image. Then we check + if these images are related (if A appears in the parent tree of C). + + This method, for the given input images, return their trees, after deduplication. + + :param images_to_rebuild list: list of images to rebuild. + :param lb LightBlue: LightBlue instance + :return: list of trees of images. + :rtype: list of lists + """ + # images_trees will be a list of lists, where the first elements in the list will be + # the requested images and the following elements are the elements in the tree up to the + # base image. + images_trees = [] + for image in images_to_rebuild: + images_trees.append([image] + self.get_image_tree(lb, image, [])) + + # Let's remove duplicated images which share the same name and version, but different + # release. + to_rebuild = lb._deduplicate_images_to_rebuild(images_trees) + return to_rebuild + + def _record_batches(self, batches, db_event, lb): + """ + Records the images from batches to the database. + + :param batches list: Output of LightBlue._find_images_to_rebuild(...). + :param db_event: event to handle. + :param lb LightBlue: LightBlue instance + :return: a mapping between image build NVR and corresponding ArtifactBuild + object representing a future rebuild of that. It is extended by including + those images stored into database. + :rtype: dict + """ + # builds tracks all the builds we register in db + builds = {} + + for batch in batches: + for image in batch: + # Reset context to db_event for each iteration before + # the ArtifactBuild is created. + self.set_context(db_event) + + nvr = image["brew"]["build"] + + self.log_debug("Recording %s", nvr) + parent_nvr = image["parent"].nvr \ + if "parent" in image and image["parent"] else None + + if "error" in image and image["error"]: + state_reason = image["error"] + state = ArtifactBuildState.FAILED.value + else: + state_reason = "" + state = ArtifactBuildState.PLANNED.value + + image_name = koji.parse_NVR(image["brew"]["build"])["name"] + parent_nvr = image["parent"].nvr \ + if "parent" in image and image["parent"] else None + dep_on = builds[parent_nvr] if parent_nvr in builds else None + + # We don't need to rebuild the nvr this time. The release value + # will be automatically generated by OSBS. + build = self.record_build( + self.event, image_name, ArtifactType.IMAGE, + dep_on=dep_on, + state=ArtifactBuildState.PLANNED.value, + original_nvr=nvr) + + # Set context to particular build so logging shows this build + # in case of error. + self.set_context(build) + + image.resolve(lb) + build.transition(state, state_reason) + build_args = {} + build_args["repository"] = image['repository'] + build_args["commit"] = image["commit"] + build_args["target"] = ( + self.event.brew_target if self.event.brew_target else image["target"]) + build_args["branch"] = image["git_branch"] + build_args["original_parent"] = parent_nvr, + build_args["arches"] = image["arches"] + build.build_args = json.dumps(build_args) + + db.session.commit() + + builds[nvr] = build + + # Reset context to db_event. + self.set_context(db_event) + + return builds diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index 10850ee..9e41c43 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -1034,6 +1034,36 @@ class LightBlue(object): images = self.filter_out_images_with_higher_srpm_nvr(images, srpm_name_to_nvrs) return images + def get_images_by_brew_package(self, names): + """ + Query Lightblue to get all the images for a specific list of names. + :param names list: list of names we want to find images for. + :return: list of container images matching the requested names. + :rtype: list of ContainerImages + """ + + query = { + "objectType": "containerImage", + "query": { + "$and": [ + { + "field": "repositories.*.published", + "op": "=", + "rvalue": True + }, + { + "$or": [{ + "field": "brew.package", + "op": "=", + "rvalue": name, + } for name in names] + } + ] + }, + "projection": self._get_default_projection(include_rpm_manifest=False) + } + return self.find_container_images(query) + def find_parent_brew_build_nvr_from_child(self, child_image): """ Returns the parent brew build NVR of the input image. If the parent is not found it returns None. diff --git a/freshmaker/parsers/internal/__init__.py b/freshmaker/parsers/internal/__init__.py index 41a271c..1f08864 100644 --- a/freshmaker/parsers/internal/__init__.py +++ b/freshmaker/parsers/internal/__init__.py @@ -21,4 +21,3 @@ from .manual_rebuild import FreshmakerManualRebuildParser # noqa from .freshmaker_manage_request import FreshmakerManageRequestParser # noqa -from .async_manual_build import FreshmakerAsyncManualbuildParser # noqa diff --git a/freshmaker/parsers/internal/async_manual_build.py b/freshmaker/parsers/internal/async_manual_build.py deleted file mode 100644 index 720b27c..0000000 --- a/freshmaker/parsers/internal/async_manual_build.py +++ /dev/null @@ -1,59 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright (c) 2020 Red Hat, Inc. -# -# Permission is hereby granted, free of charge, to any person obtaining a copy -# of this software and associated documentation files (the "Software"), to deal -# in the Software without restriction, including without limitation the rights -# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -# copies of the Software, and to permit persons to whom the Software is -# furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice shall be included in all -# copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -# SOFTWARE. - -import time -from freshmaker.parsers import BaseParser -from freshmaker.events import FreshmakerAsyncManualBuildEvent - - -class FreshmakerAsyncManualbuildParser(BaseParser): - """Parser of event async.manual.build""" - - name = 'FreshmakerAsyncManualbuildParser' - topic_suffixes = ['freshmaker.async.manual.build'] - - def can_parse(self, topic, msg): - return any([topic.endswith(s) for s in self.topic_suffixes]) - - def parse_post_data(self, data): - """ - Method shared between Frontend and Backend to parse the POST data - of async build JSON and generate the BaseEvent representation - of the rebuild request. - - :param dict data: Dict generated from JSON from HTTP POST or parsed - from the UMB message sent from Frontend to Backend. - """ - msg_id = data.get('msg_id', "async_build_%s" % (str(time.time()))) - - event = FreshmakerAsyncManualBuildEvent( - msg_id, data.get('dist_git_branch'), data.get('container_images', []), - freshmaker_event_id=data.get('freshmaker_event_id'), - brew_target=data.get('brew_target'), - dry_run=data.get('dry_run', False), - requester=data.get('requester', None), - requester_metadata_json=data.get("metadata", None)) - - return event - - def parse(self, topic, msg): - inner_msg = msg['msg'] - return self.parse_post_data(inner_msg) diff --git a/freshmaker/parsers/koji/__init__.py b/freshmaker/parsers/koji/__init__.py index db282d1..914ccff 100644 --- a/freshmaker/parsers/koji/__init__.py +++ b/freshmaker/parsers/koji/__init__.py @@ -20,3 +20,4 @@ # SOFTWARE. from .task_state_change import KojiTaskStateChangeParser # noqa +from .async_manual_build import FreshmakerAsyncManualbuildParser # noqa diff --git a/freshmaker/parsers/koji/async_manual_build.py b/freshmaker/parsers/koji/async_manual_build.py new file mode 100644 index 0000000..ac6f733 --- /dev/null +++ b/freshmaker/parsers/koji/async_manual_build.py @@ -0,0 +1,57 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2020 Red Hat, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +import time +from freshmaker.parsers import BaseParser +from freshmaker.events import FreshmakerAsyncManualBuildEvent + + +class FreshmakerAsyncManualbuildParser(BaseParser): + """Parser of event async.manual.build""" + + name = 'FreshmakerAsyncManualbuildParser' + topic_suffixes = ['freshmaker.async.manual.build'] + + def can_parse(self, topic, msg): + return any([topic.endswith(s) for s in self.topic_suffixes]) + + def parse_post_data(self, data): + """ + Method shared between Frontend and Backend to parse the POST data + of async build JSON and generate the BaseEvent representation + of the rebuild request. + + :param dict data: Dict generated from JSON from HTTP POST or parsed + from the UMB message sent from Frontend to Backend. + """ + msg_id = data.get('msg_id', "async_build_%s" % (str(time.time()))) + + return FreshmakerAsyncManualBuildEvent( + msg_id, data.get('dist_git_branch'), data.get('container_images', []), + freshmaker_event_id=data.get('freshmaker_event_id'), + brew_target=data.get('brew_target'), + dry_run=data.get('dry_run', False), + requester=data.get('requester', None), + requester_metadata_json=data.get("metadata", None)) + + def parse(self, topic, msg): + inner_msg = msg['msg'] + return self.parse_post_data(inner_msg) diff --git a/freshmaker/views.py b/freshmaker/views.py index 54e9578..67b3814 100644 --- a/freshmaker/views.py +++ b/freshmaker/views.py @@ -41,7 +41,7 @@ from freshmaker.api_utils import json_error from freshmaker.api_utils import pagination_metadata from freshmaker.auth import login_required, requires_roles, require_scopes, user_has_role from freshmaker.parsers.internal.manual_rebuild import FreshmakerManualRebuildParser -from freshmaker.parsers.internal.async_manual_build import FreshmakerAsyncManualbuildParser +from freshmaker.parsers.koji.async_manual_build import FreshmakerAsyncManualbuildParser from freshmaker.monitor import ( monitor_api, freshmaker_build_api_latency, freshmaker_event_api_latency) from freshmaker.image_verifier import ImageVerifier @@ -607,6 +607,12 @@ class AsyncBuildAPI(MethodView): 'event.'.format(data.get('freshmaker_event_id')), ) + # The '-container' string is optional, the user might have omitted it. But we need it to be + # there for our query. Let's check if it's there, and if it's not, let's add it. + for i, image in enumerate(data.get('container_images', [])): + if not image.endswith('-container'): + data.get('container_images')[i] = f"{image}-container" + # parse the POST data and generate FreshmakerAsyncManualBuildEvent parser = FreshmakerAsyncManualbuildParser() db_event = _create_rebuild_event_from_request(db.session, parser, request) diff --git a/tests/handlers/internal/test_rebuild_images_on_async_manual_build.py b/tests/handlers/internal/test_rebuild_images_on_async_manual_build.py deleted file mode 100644 index 2682a85..0000000 --- a/tests/handlers/internal/test_rebuild_images_on_async_manual_build.py +++ /dev/null @@ -1,34 +0,0 @@ -# -*- coding: utf-8 -*- -# Copyright (c) 2020 Red Hat, Inc. -# -# Permission is hereby granted, free of charge, to any person obtaining a copy -# of this software and associated documentation files (the "Software"), to deal -# in the Software without restriction, including without limitation the rights -# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -# copies of the Software, and to permit persons to whom the Software is -# furnished to do so, subject to the following conditions: -# -# The above copyright notice and this permission notice shall be included in -# all copies or substantial portions of the Software. -# -# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -# SOFTWARE. - -from unittest import TestCase - -from freshmaker.handlers.internal import RebuildOnAsyncManualBuild -from freshmaker.events import FreshmakerAsyncManualBuildEvent - - -class TestRebuildOnAsyncManualBuild(TestCase): - - def test_can_handle_event(self): - event = FreshmakerAsyncManualBuildEvent( - 'msg-id-01', 'repo-branch', ['image1', 'image2']) - handler = RebuildOnAsyncManualBuild() - self.assertTrue(handler.can_handle(event)) diff --git a/tests/handlers/koji/test_rebuild_images_on_async_manual_build.py b/tests/handlers/koji/test_rebuild_images_on_async_manual_build.py new file mode 100644 index 0000000..3c35173 --- /dev/null +++ b/tests/handlers/koji/test_rebuild_images_on_async_manual_build.py @@ -0,0 +1,389 @@ +# -*- coding: utf-8 -*- +# Copyright (c) 2020 Red Hat, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + +from freshmaker import db +from freshmaker.handlers.koji import RebuildImagesOnAsyncManualBuild +from freshmaker.events import FreshmakerAsyncManualBuildEvent +from freshmaker.types import EventState +from freshmaker.models import Event +from freshmaker.lightblue import ContainerImage +from tests import helpers + + +class TestRebuildImagesOnAsyncManualBuild(helpers.ModelsTestCase): + + def setUp(self): + super(TestRebuildImagesOnAsyncManualBuild, self).setUp() + + self.patcher = helpers.Patcher( + 'freshmaker.handlers.koji.RebuildImagesOnAsyncManualBuild.') + # We do not want to send messages to message bus while running tests + self.mock_messaging_publish = self.patcher.patch( + 'freshmaker.messaging.publish') + + # Mocking koji + self.mock_get_build = self.patcher.patch( + 'freshmaker.kojiservice.KojiService.get_build', + return_value={'build_id': 123456, 'extra': {'container_koji_task_id': 21938204}}) + self.mock_get_task_request = self.patcher.patch( + 'freshmaker.kojiservice.KojiService.get_task_request', return_value=[ + 'git://example.com/rpms/repo-1#commit_hash1', + 'test-target', + {'compose_ids': None, + 'git_branch': 'test_branch', + 'scratch': False, + 'signing_intent': None, + 'yum_repourls': [('fake-url.repo')]}]) + + self.mock_allow_build = self.patcher.patch('allow_build', return_value=True) + + # Mocking Lightblue + self.mock_find_images_to_rebuild = self.patcher.patch('_find_images_to_rebuild') + self.mock_lightblue = self.patcher.patch('init_lightblue_instance') + + self.mock_start_to_build_images = self.patcher.patch('start_to_build_images') + self.mock_get_image_builds_in_first_batch = self.patcher.patch( + 'freshmaker.models.Event.get_image_builds_in_first_batch') + + # Structure of the images used for testing: + # image_0 + # + + # | + # + + # image_a + # + + # | + # +----+----+ + # | | + # + + + # image_b image_d + # + + # | + # + + # image_e + + # image_c is unrelated. + + # image_0 is a base image, with no parent + self.image_0 = ContainerImage({ + 'repository': 'repo_1', + 'commit': '1234567', + 'target': 'container-candidate', + 'git_branch': 'test_branch', + 'content_sets': ['image_0_content_set_1', 'image_0_content_set_2'], + 'arches': 'x86_64', + 'brew': { + 'build': 'image-container-1.0-2', + 'package': 'image-container', + }, + 'parent': None, + 'parsed_data': { + 'layers': [ + 'sha512:7890', + 'sha512:5678', + ] + }, + 'published': False, + }) + + self.image_a = ContainerImage({ + 'repository': 'repo_1', + 'commit': '1234567', + 'target': 'container-candidate', + 'git_branch': 'test_branch', + 'content_sets': ['image_a_content_set_1', 'image_a_content_set_2'], + 'arches': 'x86_64', + 'brew': { + 'build': 'image-a-container-1.0-2', + 'package': 'image-a-container', + }, + 'parent': self.image_0, + 'parsed_data': { + 'layers': [ + 'sha512:7890', + 'sha512:5678', + ] + }, + 'published': False, + }) + + # image_b is a child image of image_a + self.image_b = ContainerImage({ + 'repository': 'repo_2', + 'commit': '5678901', + 'target': 'container-candidate', + 'git_branch': 'test_branch', + 'content_sets': ['image_b_content_set_1', 'image_b_content_set_2'], + 'arches': 'x86_64', + 'brew': { + 'build': 'image-b-container-2.14-1', + 'package': 'image-b-container' + }, + 'parent': self.image_a, + 'parsed_data': { + 'layers': [ + 'sha512:f109', + 'sha512:7890', + 'sha512:5678', + ] + }, + 'published': False, + }) + + # image_c is an image unrelated to image_a and image_b + # it also has no parent image. + # image_c has the same name of image_a, that's why it has this name + self.image_c = ContainerImage({ + 'repository': 'repo_1', + 'commit': '1234569', + 'target': 'container-candidate', + 'git_branch': 'test_branch', + 'content_sets': ['image_a_content_set_1', 'image_a_content_set_2'], + 'arches': 'x86_64', + 'brew': { + 'build': 'image-a-container-1.0-3', + 'package': 'image-a-container', + }, + 'parent': None, + 'parsed_data': { + 'layers': [ + 'sha512:7890', + 'sha512:5678', + ] + }, + 'published': False, + }) + + # image_d is a child image of image_a, same as image_b + # so image_d and image_b are unrelated, since they are sibilings + self.image_d = ContainerImage({ + 'repository': 'repo_2', + 'commit': '5678906', + 'target': 'container-candidate', + 'git_branch': 'test_branch', + 'content_sets': ['image_d_content_set_1', 'image_d_content_set_2'], + 'arches': 'x86_64', + 'brew': { + 'build': 'image-d-container-3.3-1', + 'package': 'image-d-container' + }, + 'parent': self.image_a, + 'parsed_data': { + 'layers': [ + 'sha512:f109', + ] + }, + 'published': False, + }) + + # image_e is a child image of image_d + self.image_e = ContainerImage({ + 'repository': 'repo_2', + 'commit': '5678906', + 'target': 'container-candidate', + 'git_branch': 'test_branch', + 'content_sets': ['image_e_content_set_1', 'image_e_content_set_2'], + 'arches': 'x86_64', + 'brew': { + 'build': 'image-e-container-3.3-1', + 'package': 'image-e-container' + }, + 'parent': self.image_d, + 'parsed_data': { + 'layers': [ + 'sha512:f109', + ] + }, + 'published': False, + }) + + def test_can_handle_event(self): + event = FreshmakerAsyncManualBuildEvent( + 'msg-id-01', 'repo-branch', ['image1', 'image2']) + handler = RebuildImagesOnAsyncManualBuild() + self.assertTrue(handler.can_handle(event)) + + def test_building_single_image(self): + """ + This tests the successful build of a single image + """ + self.mock_find_images_to_rebuild.return_value = [self.image_a] + self.mock_find_images_trees_to_rebuild = self.patcher.patch( + 'find_images_trees_to_rebuild', return_value=[[self.image_a]]) + event = FreshmakerAsyncManualBuildEvent( + 'msg-id-123', 'test_branch', ['image-a-container']) + handler = RebuildImagesOnAsyncManualBuild() + handler.handle(event) + + db_event = Event.get(db.session, 'msg-id-123') + self.assertEqual(EventState.BUILDING.value, db_event.state) + self.mock_get_image_builds_in_first_batch.assert_called_once_with(db.session) + self.assertEqual(len(db_event.builds.all()), 1) + self.mock_start_to_build_images.assert_called_once() + + def test_building_related_images_correct_order(self): + """ + This tests the successful build of 2 related images in the correct order. + """ + self.mock_find_images_to_rebuild.return_value = [self.image_a, self.image_b] + self.mock_find_images_trees_to_rebuild = self.patcher.patch( + 'find_images_trees_to_rebuild', return_value=[[self.image_a, self.image_b]]) + self.mock_generate_batches = self.patcher.patch('generate_batches', return_value=[ + [self.image_a], + [self.image_b] + ]) + event = FreshmakerAsyncManualBuildEvent( + 'msg-id-123', 'test_branch', ['image-b-container', 'image-a-container']) + handler = RebuildImagesOnAsyncManualBuild() + handler.handle(event) + + db_event = Event.get(db.session, 'msg-id-123') + self.assertEqual(EventState.BUILDING.value, db_event.state) + self.mock_get_image_builds_in_first_batch.assert_called_once_with(db.session) + self.assertEqual(len(db_event.builds.all()), 2) + self.mock_start_to_build_images.assert_called_once() + + def test_failed_to_build_images_never_built_before(self): + """ + This test checks that trying to build an image that was never built before (for that + branch) will make the build fail. + """ + self.mock_find_images_to_rebuild.return_value = [self.image_a] + self.mock_find_images_trees_to_rebuild = self.patcher.patch( + 'find_images_trees_to_rebuild', return_value=[[self.image_a]]) + event = FreshmakerAsyncManualBuildEvent( + 'msg-id-123', 'another-branch', ['image-a-container']) + handler = RebuildImagesOnAsyncManualBuild() + handler.handle(event) + db_event = Event.get(db.session, 'msg-id-123') + self.assertEqual(EventState.FAILED.value, db_event.state) + + def test_multiple_nvrs_for_the_same_name(self): + """ + This test checks that when for one name more nvrs are returned by lightblue, Freshmaker + will pick the one with higher nvr. + """ + self.mock_find_images_to_rebuild.return_value = [self.image_a, self.image_c] + self.mock_find_images_trees_to_rebuild = self.patcher.patch( + 'find_images_trees_to_rebuild', return_value=[[self.image_c]]) + event = FreshmakerAsyncManualBuildEvent( + 'msg-id-123', 'test_branch', ['image-a-container']) + handler = RebuildImagesOnAsyncManualBuild() + handler.handle(event) + + db_event = Event.get(db.session, 'msg-id-123') + self.assertEqual(EventState.BUILDING.value, db_event.state) + self.mock_get_image_builds_in_first_batch.assert_called_once_with(db.session) + self.mock_start_to_build_images.assert_called_once() + self.assertEqual(len(db_event.builds.all()), 1) + self.assertEqual(db_event.builds.one().original_nvr, 'image-a-container-1.0-3') + + def test_building_sibilings(self): + """ + This test checks that when the users requests to rebuild 2 images that are sibilings + (or other unrelated images) Freshmaker will rebuild them separately, without the need + of rebuilding the parent. + """ + self.mock_find_images_to_rebuild.return_value = [self.image_b, self.image_d] + self.find_images_trees_to_rebuild = self.patcher.patch( + 'find_images_trees_to_rebuild', return_value=[ + [self.image_b, self.image_a, self.image_0], + [self.image_d, self.image_a, self.image_0]]) + event = FreshmakerAsyncManualBuildEvent( + 'msg-id-123', 'test_branch', ['image-b-container', 'image-d-container']) + handler = RebuildImagesOnAsyncManualBuild() + handler.handle(event) + + db_event = Event.get(db.session, 'msg-id-123') + self.assertEqual(EventState.BUILDING.value, db_event.state) + self.mock_get_image_builds_in_first_batch.assert_called_once_with(db.session) + self.assertEqual(len(db_event.builds.all()), 2) + self.mock_start_to_build_images.assert_called_once() + + def test_building_images_with_disconnected_tree(self): + self.mock_find_images_to_rebuild.return_value = [self.image_b, self.image_d, self.image_e] + self.find_images_trees_to_rebuild = self.patcher.patch( + 'find_images_trees_to_rebuild', return_value=[ + [self.image_b, self.image_a, self.image_0], + [self.image_d, self.image_a, self.image_0], + [self.image_e, self.image_d, self.image_a, self.image_0]]) + self.mock_generate_batches = self.patcher.patch('generate_batches', return_value=[ + [self.image_b, self.image_d], + [self.image_e] + ]) + event = FreshmakerAsyncManualBuildEvent( + 'msg-id-123', 'test_branch', + ['image-b-container', 'image-d-container', 'image-e-container']) + handler = RebuildImagesOnAsyncManualBuild() + handler.handle(event) + + db_event = Event.get(db.session, 'msg-id-123') + self.assertEqual(EventState.BUILDING.value, db_event.state) + self.mock_get_image_builds_in_first_batch.assert_called_once_with(db.session) + self.assertEqual(len(db_event.builds.all()), 3) + self.mock_start_to_build_images.assert_called_once() + + def test_intermediate_images_are_build(self): + self.mock_find_images_to_rebuild.return_value = [self.image_b, self.image_d, self.image_0] + self.find_images_trees_to_rebuild = self.patcher.patch( + 'find_images_trees_to_rebuild', return_value=[ + [self.image_0], + [self.image_b, self.image_a, self.image_0], + [self.image_d, self.image_a, self.image_0]]) + self.mock_generate_batches = self.patcher.patch('generate_batches', return_value=[ + [self.image_0], + [self.image_a], + [self.image_b, self.image_d] + ]) + event = FreshmakerAsyncManualBuildEvent( + 'msg-id-123', 'test_branch', + ['image-container', 'image-b-container', 'image-d-container']) + handler = RebuildImagesOnAsyncManualBuild() + handler.handle(event) + + db_event = Event.get(db.session, 'msg-id-123') + self.assertEqual(EventState.BUILDING.value, db_event.state) + self.mock_get_image_builds_in_first_batch.assert_called_once_with(db.session) + self.assertEqual(len(db_event.builds.all()), 4) + self.mock_start_to_build_images.assert_called_once() + + def test_related_images_are_built(self): + self.mock_find_images_to_rebuild.return_value = [self.image_b, self.image_d, self.image_a] + self.find_images_trees_to_rebuild = self.patcher.patch( + 'find_images_trees_to_rebuild', return_value=[ + [self.image_a, self.image_0], + [self.image_b, self.image_a, self.image_0], + [self.image_d, self.image_a, self.image_0]]) + self.mock_generate_batches = self.patcher.patch('generate_batches', return_value=[ + [self.image_a], + [self.image_b, self.image_d] + ]) + event = FreshmakerAsyncManualBuildEvent( + 'msg-id-123', 'test_branch', + ['image-a-container', 'image-b-container', 'image-d-container']) + handler = RebuildImagesOnAsyncManualBuild() + handler.handle(event) + + db_event = Event.get(db.session, 'msg-id-123') + self.assertEqual(EventState.BUILDING.value, db_event.state) + self.mock_get_image_builds_in_first_batch.assert_called_once_with(db.session) + self.assertEqual(len(db_event.builds.all()), 3) + self.mock_start_to_build_images.assert_called_once() diff --git a/tests/test_views.py b/tests/test_views.py index 5bafeee..cbc3f05 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -981,7 +981,7 @@ class TestAsyncBuild(ViewBaseTest): self.client = app.test_client() @patch('freshmaker.messaging.publish') - @patch('freshmaker.parsers.internal.async_manual_build.time.time') + @patch('freshmaker.parsers.koji.async_manual_build.time.time') def test_async_build(self, time, publish): time.return_value = 123 with patch('freshmaker.models.datetime') as datetime_patch: @@ -1007,7 +1007,7 @@ class TestAsyncBuild(ViewBaseTest): u'event_type_id': 14, u'id': 1, u'message_id': 'async_build_123', - u'requested_rebuilds': ['foo-1-1', 'bar-1-1'], + u'requested_rebuilds': ['foo-1-1-container', 'bar-1-1-container'], u'requester': 'root', u'requester_metadata': {}, u'search_key': 'async_build_123', @@ -1023,12 +1023,12 @@ class TestAsyncBuild(ViewBaseTest): { 'msg_id': 'async_build_123', 'dist_git_branch': 'master', - 'container_images': ['foo-1-1', 'bar-1-1'], - 'requester': 'root', + 'container_images': ['foo-1-1-container', 'bar-1-1-container'], + 'requester': 'root' }) @patch('freshmaker.messaging.publish') - @patch('freshmaker.parsers.internal.async_manual_build.time.time') + @patch('freshmaker.parsers.koji.async_manual_build.time.time') def test_async_build_dry_run(self, time, publish): time.return_value = 123 @@ -1049,7 +1049,7 @@ class TestAsyncBuild(ViewBaseTest): { 'msg_id': 'async_build_123', 'dist_git_branch': 'master', - 'container_images': ['foo-1-1', 'bar-1-1'], + 'container_images': ['foo-1-1-container', 'bar-1-1-container'], 'dry_run': True, 'requester': 'root', })