From 6f8437f27e7ed96cea507cdac3c362014934b41d Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Oct 09 2017 08:19:02 +0000 Subject: [PATCH 1/2] Generate the .repo file with Pulp repositories which are needed for Container image to be rebuild. The Lightblue code now sets 'content_sets' list for every image we are going to rebuild. This list contains all the content-sets the image needs to be available during the build. While recording the images, we ask ODCS to generate the .repo file with these content_sets. Later we use that .repo file as extra repo file for container rebuild. --- diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index a5ae417..aeae4d6 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -32,9 +32,11 @@ from freshmaker.mbs import MBS from freshmaker.models import ArtifactBuildState from freshmaker.types import ArtifactType from freshmaker.models import ArtifactBuild, Event +from freshmaker.utils import krb_context from freshmaker.odcsclient import ODCS from freshmaker.odcsclient import AuthMech +from freshmaker.odcsclient import COMPOSE_STATES class BaseHandler(object): @@ -235,7 +237,26 @@ class ContainerBuildHandler(BaseHandler): scm_url, branch, target, repo_urls=repo_urls, isolated=True, release=release, koji_parent_build=parent) - def get_repo_urls(self, db_event): + def odcs_get_compose(self, compose_id): + """ + Returns the information from the ODCS server about compose with id + `compose_id`. In DRY_RUN mode, returns fake compose information + without contacting the ODCS server. + """ + if conf.dry_run: + compose = {} + compose['id'] = compose_id + compose['result_repofile'] = "http://localhost/%d.repo" % ( + compose['id']) + compose['state'] = COMPOSE_STATES['done'] + return compose + + odcs = ODCS(conf.odcs_server_url, auth_mech=AuthMech.Kerberos, + verify_ssl=conf.odcs_verify_ssl) + with krb_context(): + return odcs.get_compose(compose_id) + + def get_repo_urls(self, db_event, build): """ Returns list of URLs to ODCS repositories which should be used to rebuild the container image for this event. @@ -250,10 +271,15 @@ class ContainerBuildHandler(BaseHandler): # Use compose ids to get the repofile URLs. repo_urls = [] for compose_id in compose_ids: - odcs = ODCS(conf.odcs_server_url, auth_mech=AuthMech.Kerberos, - verify_ssl=conf.odcs_verify_ssl) - compose = odcs.get_compose(compose_id) + compose = self.odcs_get_compose(compose_id) + repo_urls.append(compose["result_repofile"]) + + # Add PULP compose repo url. + if build.build_args and "odcs_pulp_compose_id" in build.build_args: + args = json.loads(build.build_args) + compose = self.odcs_get_compose(args["odcs_pulp_compose_id"]) repo_urls.append(compose["result_repofile"]) + return repo_urls def _build_first_batch(self, db_event): @@ -262,13 +288,12 @@ class ContainerBuildHandler(BaseHandler): depend on other images. """ - repo_urls = self.get_repo_urls(db_event) - builds = db.session.query(ArtifactBuild).filter_by( type=ArtifactType.IMAGE.value, event_id=db_event.id, dep_on=None).all() for build in builds: + repo_urls = self.get_repo_urls(db_event, build) build.build_id = self.build_image_artifact_build(build, repo_urls) if build.build_id: build.transition( diff --git a/freshmaker/handlers/brew/container_task_state_change.py b/freshmaker/handlers/brew/container_task_state_change.py index 8a0a546..efb153c 100644 --- a/freshmaker/handlers/brew/container_task_state_change.py +++ b/freshmaker/handlers/brew/container_task_state_change.py @@ -63,8 +63,8 @@ class BrewContainerTaskStateChangeHandler(ContainerBuildHandler): planned_builds = db.session.query(ArtifactBuild).filter_by(type=ArtifactType.IMAGE.value, state=ArtifactBuildState.PLANNED.value, dep_on=found_build).all() - repo_urls = self.get_repo_urls(found_build.event) for build in planned_builds: + repo_urls = self.get_repo_urls(found_build.event, build) log.info("Build %r depends on build %r" % (build, found_build)) build.build_id = self.build_image_artifact_build(build, repo_urls) build.state = ArtifactBuildState.BUILD.value diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index f6794b4..e48f395 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -38,7 +38,7 @@ from freshmaker.errata import Errata from freshmaker.types import ArtifactType, ArtifactBuildState from freshmaker.models import Event from freshmaker.consumer import work_queue_put -from freshmaker.utils import krb_context +from freshmaker.utils import krb_context, retry from odcs.client.odcs import ODCS from odcs.client.odcs import AuthMech @@ -217,6 +217,57 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): return yum_repourl + def _prepare_pulp_repo(self, db_event, content_sets): + """ + Prepares .repo file containing the repositories matching + the content_sets by creating new ODCS compose of PULP type. + + This currently blocks until the compose is done or failed. + + :param db_event: models.Event instance associated with this build. + :param list content_sets: List of content sets. + :rtype: dict + :return: ODCS compose dictionary. + """ + log.info('Generating new PULP type compose for content_sets: %r', + content_sets) + + odcs = ODCS(conf.odcs_server_url, auth_mech=AuthMech.Kerberos, + verify_ssl=conf.odcs_verify_ssl) + if not conf.dry_run: + with krb_context(): + new_compose = odcs.new_compose( + ' '.join(content_sets), 'pulp') + + # Pulp composes in ODCS takes just few seconds, because ODCS + # only generates the .repo file after single query to Pulp. + # TODO: Freshmaker is currently not designed to handle + # multiple ODCS composes per rebuild Event and since these + # composes are done in no-time normally, it is OK here to + # block. It would still be nice to redesign that part of + # Freshmaker to do things "right". + @retry(timeout=60, interval=2) + def wait_for_compose(compose_id): + ret = odcs.get_compose(compose_id) + if ret["state_name"] == "done": + return True + elif ret["state_name"] == "failed": + return False + log.info("Waiting for Pulp compose to finish: %r", ret) + raise Exception("ODCS compose not finished.") + + done = wait_for_compose(new_compose["id"]) + if not done: + db_event.builds_transition( + ArtifactBuildState.FAILED.value, "Cannot generate " + "ODCS PULP compose for content_sets %r" + % (content_sets)) + else: + new_compose = self._fake_odcs_new_compose( + content_sets, 'pulp') + + return new_compose + def _get_packages_for_compose(self, nvr): """Get RPMs of current build NVR @@ -360,12 +411,15 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): build.transition(state, state_reason) + compose = self._prepare_pulp_repo(event, image["content_sets"]) + build_args = {} build_args["repository"] = image["repository"] build_args["commit"] = image["commit"] build_args["parent"] = parent_name build_args["target"] = image["target"] build_args["branch"] = image["git_branch"] + build_args["odcs_pulp_compose_id"] = compose["id"] build.build_args = json.dumps(build_args) db.session.commit() diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index b71c155..4f53289 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -382,6 +382,64 @@ class LightBlue(object): } return self.find_container_repositories(repo_request) + def find_content_sets_for_repository(self, + repository, + published=True, + deprecated=False, + release_category="Generally Available"): + """ + Query lightblue and find content sets which are used for Container + image in repository `repository` + + :param str repository: name of the repository for which the content + sets will be returned + :param bool published: whether to limit queries to published + repositories + :param bool deprecated: set to True to limit results to deprecated + repositories + :param str release_category: filter only repositories with specific + release category (options: Deprecated, Generally Available, Beta, Tech Preview) + """ + repo_request = { + "objectType": "containerRepository", + "query": { + "$and": [ + { + "field": "repository", + "op": "=", + "rvalue": repository + }, + { + "field": "published", + "op": "=", + "rvalue": published + }, + { + "field": "deprecated", + "op": "=", + "rvalue": deprecated + }, + { + "field": "release_categories.*", + "op": "=", + "rvalue": release_category + } + ] + }, + "projection": [ + {"field": "content_sets", "include": True, "recursive": True} + ] + } + repos = self.find_container_repositories(repo_request) + if not repos: + return set() + + ret = set() + for repo in repos: + ret |= set(repo["content_sets"]) + + return ret + def _get_default_projection(self): return [ {"field": "brew", "include": True, "recursive": True}, @@ -390,6 +448,7 @@ class LightBlue(object): {"field": "parsed_data.rpm_manifest.*.srpm_name", "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}, ] def find_images_with_included_srpm(self, repositories, srpm_name, @@ -644,6 +703,16 @@ class LightBlue(object): images = [image for image in images if not filter_fnc(image)] for image in images: + # Find out the content_sets this image uses and store it as + # "content_sets" key in image. + # 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 = self.find_content_sets_for_repository( + image["repositories"][0]["repository"]) + log.info("Container image %s uses following content sets: %r", + image["brew"]["build"], image_content_sets) + image.update({"content_sets": image_content_sets}) image.resolve_commit(srpm_name) return images diff --git a/freshmaker/odcsclient.py b/freshmaker/odcsclient.py index 4eaf69a..a14cb18 100644 --- a/freshmaker/odcsclient.py +++ b/freshmaker/odcsclient.py @@ -30,3 +30,4 @@ # it would import freshmaker.handlers.odcs, so instead, we import it here # and in freshmaker.handler do "from freshmaker.odcsclient import ODCS". from odcs.client.odcs import * # noqa +from odcs.common.types import * # noqa diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index b86f4f5..39d654c 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -266,12 +266,22 @@ class TestBatches(unittest.TestCase): parent = {"brew": {"build": parent}} return {'brew': {'build': build}, 'repository': build + '_repo', 'commit': build + '_123', 'parent': parent, "target": "t1", - 'git_branch': 'mybranch', "error": error} + 'git_branch': 'mybranch', "error": error, + "content_sets": ["first-content-set"]} - def test_batches_records(self): + @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS.new_compose') + @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.ODCS.get_compose') + @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.krb_context') + def test_batches_records(self, krb_context, get_compose, new_compose): """ Tests that batches are properly recorded in DB. """ + + compose = {'id': 2, 'result_repofile': 'http://localhost/2.repo', + 'state_name': 'done'} + new_compose.return_value = compose + get_compose.return_value = compose + # Creates following tree: # shared_parent # |- child1_parent3 diff --git a/tests/test_handler.py b/tests/test_handler.py index fc51b3a..7526e6d 100644 --- a/tests/test_handler.py +++ b/tests/test_handler.py @@ -115,6 +115,7 @@ class TestBuildFirstBatch(TestCase): "branch": "mybranch", "yum_repourl": "http://localhost/composes/latest-odcs-3-1/compose/" "Temporary/odcs-3.repo", + "odcs_pulp_compose_id": 15, }) self.db_event = Event.get_or_create( @@ -160,15 +161,20 @@ class TestBuildFirstBatch(TestCase): Tests that only PLANNED images without a parent are submitted to build system. """ - ODCS.return_value.get_compose.return_value = { - "id": 3, - "result_repo": "http://localhost/composes/latest-odcs-3-1/compose/Temporary", - "result_repofile": "http://localhost/composes/latest-odcs-3-1/compose/Temporary/odcs-3.repo", - "source": "f26", - "source_type": 1, - "state": 2, - "state_name": "done", - } + + def _fake_get_compose(compose_id): + return { + "id": compose_id, + "result_repo": "http://localhost/composes/latest-odcs-%d-1/compose/Temporary" % compose_id, + "result_repofile": "http://localhost/composes/latest-odcs-%d-1/compose/Temporary/odcs-%s.repo" % (compose_id, compose_id), + "source": "f26", + "source_type": 1, + "state": 2, + "state_name": "done", + } + + ODCS.return_value.get_compose = _fake_get_compose + mock_session = ClientSession.return_value mock_session.buildContainer.return_value = 123 @@ -181,7 +187,8 @@ class TestBuildFirstBatch(TestCase): {'scratch': True, 'isolated': True, 'koji_parent_build': u'nvr', 'git_branch': 'mybranch', 'release': AnyStringWith('4.'), 'yum_repourls': [ - 'http://localhost/composes/latest-odcs-3-1/compose/Temporary/odcs-3.repo']}) + 'http://localhost/composes/latest-odcs-3-1/compose/Temporary/odcs-3.repo', + 'http://localhost/composes/latest-odcs-15-1/compose/Temporary/odcs-15.repo']}) db.session.refresh(self.db_event) for build in self.db_event.builds: diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index cd9de58..df1ddaf 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -281,6 +281,9 @@ class TestQueryEntityFromLightBlue(unittest.TestCase): 'build': 'package-name-1-4-12.10', 'package': 'package-name-1' }, + 'repositories': [ + {'repository': 'product1/repo1', 'published': True} + ], 'parsed_data': { 'files': [ { @@ -307,6 +310,9 @@ class TestQueryEntityFromLightBlue(unittest.TestCase): 'build': 'package-name-2-4-12.10', 'package': 'package-name-2' }, + 'repositories': [ + {'repository': 'product2/repo2', 'published': True} + ], 'parsed_data': { 'files': [ { @@ -650,6 +656,8 @@ class TestQueryEntityFromLightBlue(unittest.TestCase): "build": "package-name-1-4-12.10", "package": "package-name-1" }, + 'repositories': [{'repository': 'product1/repo1', 'published': True}], + 'content_sets': set(['dummy-content-set-1', 'dummy-content-set-2']), 'parsed_data': { 'files': [ { @@ -682,6 +690,8 @@ class TestQueryEntityFromLightBlue(unittest.TestCase): "build": "package-name-2-4-12.10", "package": "package-name-2" }, + 'content_sets': set(['dummy-content-set-1', 'dummy-content-set-2']), + 'repositories': [{'repository': 'product2/repo2', 'published': True}], 'parsed_data': { 'files': [ { From 0caf27d803678cce7af531369eb5a03d64e1b7ea Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Oct 10 2017 06:31:27 +0000 Subject: [PATCH 2/2] Link to Freshmaker issue 114 in TODO comment --- diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index e48f395..ec6385c 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -246,6 +246,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): # composes are done in no-time normally, it is OK here to # block. It would still be nice to redesign that part of # Freshmaker to do things "right". + # This is tracked here: https://pagure.io/freshmaker/issue/114 @retry(timeout=60, interval=2) def wait_for_compose(compose_id): ret = odcs.get_compose(compose_id)