From 508ced96d56e9228fa7af515652bd67d05ddad34 Mon Sep 17 00:00:00 2001 From: Jan Kaluza Date: Nov 08 2018 12:49:50 +0000 Subject: Request ODCS compose with all RPMs installed in the *unshipped* container image. So far, Freshmaker rebuilds just shipped container images, but for release-driver use-case, it needs to be able to rebuild also unshipped container images. Such images can contain unshipped RPMs, so our normal way of getting them from Pulp repositories defined by content sets will not work - Pulp repos don't contain unshipped RPMs. In this PR, we therefore request ODCS compose with all the RPMs installed in the unshipped container image and use it together with Pulp repos. Using this approach, unshipped container image can get even the so-far unshipped RPMs. --- diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index 7c4f446..875671f 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -39,6 +39,14 @@ from freshmaker.odcsclient import create_odcs_client from freshmaker.odcsclient import COMPOSE_STATES +class ODCSComposeNotReady(Exception): + """ + Raised when ODCS compose is still generating and therefore not ready + to be used to build an image. + """ + pass + + def fail_event_on_handler_exception(func): """ Decorator which marks the models.Event associated with handler by @@ -196,7 +204,8 @@ class BaseHandler(object): elif type(db_object) == ArtifactBuild: self._db_event_id = db_object.event.id self._db_artifact_build_id = db_object.id - self._log_prefix = "%s: " % str(db_object.event) + # Prefix logs with " ():". + self._log_prefix = "%s (%s): " % (str(db_object.event), str(db_object)) else: raise ProgrammingError( "Unsupported context type passed to BaseHandler.set_context()") @@ -480,9 +489,28 @@ class ContainerBuildHandler(BaseHandler): # cases, we want to convert compose_ids to repository URLs. Otherwise, # just pass compose_ids to OSBS via Koji. if repo_urls: - repo_urls += [self.odcs_get_compose( - compose_id)['result_repofile'] - for compose_id in compose_ids] + for compose_id in compose_ids: + odcs_compose = self.odcs_get_compose(compose_id) + if odcs_compose["state"] in [COMPOSE_STATES['wait'], + COMPOSE_STATES['generating']]: + # In case the ODCS compose is still generating, raise an + # exception. + msg = ("Compose %s is not in done state. Waiting with " + "rebuild." % (str(compose_id))) + self.log_info(msg) + raise ODCSComposeNotReady(msg) + elif odcs_compose["state"] != COMPOSE_STATES["done"]: + # In case the compose is not in 'done' state, mark the + # build as failed. We should never get expired here, + # because the compose has been submitted just a minutes + # ago. If we get "expired" here, it is sign of an issue, + # because that means we are trying to build quite old + # image. + build.transition( + ArtifactBuildState.FAILED.value, + "Compose %s is not in 'done' state." % str(compose_id)) + return + repo_urls.append(odcs_compose["result_repofile"]) compose_ids = [] return self.build_container( @@ -538,7 +566,12 @@ class ContainerBuildHandler(BaseHandler): def build_image(build): self.set_context(build) repo_urls = self.get_repo_urls(build) - build.build_id = self.build_image_artifact_build(build, repo_urls) + try: + build.build_id = self.build_image_artifact_build(build, repo_urls) + except ODCSComposeNotReady: + # We skip this image for now. It will be built once the ODCS + # compose is finished. + return if build.build_id: build.transition( ArtifactBuildState.BUILD.value, diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index 8839342..aab67cd 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -25,6 +25,7 @@ import json import koji import requests +import kobo.rpmlib from six.moves import cStringIO from six.moves import configparser @@ -136,16 +137,11 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): # Log what we are going to rebuild self._check_images_to_rebuild(db_event, builds) - - if event.advisory.state == 'SHIPPED_LIVE': - # As mentioned above, no need to wait for the event of new compose - # is generated in ODCS, so we can start to rebuild the first batch - # from here immediately. - self.start_to_build_images( - db_event.get_image_builds_in_first_batch(db.session)) + self.start_to_build_images( + db_event.get_image_builds_in_first_batch(db.session)) msg = ('Waiting for composes to finish in order to start to ' - 'schedule base images for rebuild.') + 'schedule images for rebuild.') db_event.transition(EventState.BUILDING, msg) return [] @@ -314,6 +310,53 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): return new_compose + def _prepare_odcs_compose_with_image_rpms(self, image): + """ + Request a compose from ODCS for builds included in Errata advisory + + Run a compose in ODCS to contain required RPMs for rebuilding images + later. + + :param dict image: Container image representation as returned by + LightBlue class. + :return: a mapping returned from ODCS that represents the request + compose. + :rtype: dict + """ + + if not image.get('rpm_manifest'): + self.log_warn('"rpm_manifest" not set in image.') + return + + rpm_manifest = image["rpm_manifest"][0] + if not rpm_manifest.get('rpms'): + return + + builds = set() + packages = set() + for rpm in rpm_manifest["rpms"]: + parsed_nvr = kobo.rpmlib.parse_nvra(rpm["srpm_nevra"]) + srpm_nvr = "%s-%s-%s" % (parsed_nvr["name"], parsed_nvr["version"], + parsed_nvr["release"]) + builds.add(srpm_nvr) + parsed_nvr = kobo.rpmlib.parse_nvra(rpm["nvra"]) + packages.add(parsed_nvr["name"]) + + if not self.dry_run: + with krb_context(): + new_compose = create_odcs_client().new_compose( + "", 'build', packages=packages, builds=builds, + sigkeys=conf.odcs_sigkeys, flags=["no_deps"]) + else: + new_compose = self._fake_odcs_new_compose( + "", 'build', packages=packages, + builds=builds) + + self.log_info("Started generating ODCS 'build' type compose %d." % ( + new_compose["id"])) + + return new_compose + def _get_base_image_build_target(self, image): dockerfile = image.dockerfile image_build_conf_url = dockerfile['content_url'].replace( @@ -491,6 +534,9 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): stored into database. :rtype: dict """ + db_event = Event.get_or_create( + db.session, event.msg_id, event.search_key, event.__class__) + # Used as tmp dict with {brew_build_nvr: ArtifactBuild, ...} mapping. builds = builds or {} @@ -500,6 +546,10 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): 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"] if nvr in builds: self.log_debug("Skipping recording build %s, " @@ -539,6 +589,10 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): original_nvr=nvr, rebuilt_nvr=rebuilt_nvr) + # Set context to particular build so logging shows this build + # in case of error. + self.set_context(build) + build.transition(state, state_reason) build_args = {} @@ -577,6 +631,18 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): build.add_composes(db.session, [db_compose]) db.session.commit() + # Unpublished images can contain unreleased RPMs, so generate + # the ODCS compose with all the RPMs in the image to allow + # installation of possibly unreleased RPMs. + if not image["published"]: + compose = self._prepare_odcs_compose_with_image_rpms(image) + if compose: + db_compose = Compose(odcs_compose_id=compose['id']) + db.session.add(db_compose) + db.session.commit() + build.add_composes(db.session, [db_compose]) + db.session.commit() + # TODO: uncomment following code after boot.iso compose is # deployed in ODCS server. # if image.is_base_image: @@ -598,6 +664,9 @@ class ErrataAdvisoryRPMsSignedHandler(ContainerBuildHandler): builds[nvr] = build + # Reset context to db_event. + self.set_context(db_event) + return builds def _filter_out_not_allowed_builds(self, image): diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index fb4853e..5977596 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -181,6 +181,7 @@ class ContainerImage(dict): "error": None, "arches": None, "odcs_compose_ids": None, + "published": None, } @region.cache_on_arguments() @@ -492,6 +493,16 @@ class ContainerImage(dict): "is suspicious.", self["brew"]["build"]) self.update({"content_sets": []}) + def resolve_published(self, lb_instance): + # Get the published version of this image to find out if the image + # was actually published. + image = lb_instance.get_images_by_nvrs( + self["brew"]["build"], published=True) + if image: + self["published"] = True + else: + self["published"] = False + def resolve(self, lb_instance, children=None): """ Resolves the Container image - populates additional metadata by @@ -501,6 +512,7 @@ class ContainerImage(dict): """ self.resolve_commit() self.resolve_content_sets(lb_instance, children) + self.resolve_published(lb_instance) class LightBlue(object): diff --git a/tests/test_errata_advisory_rpms_signed_handler.py b/tests/test_errata_advisory_rpms_signed_handler.py index 5fac2b2..1561899 100644 --- a/tests/test_errata_advisory_rpms_signed_handler.py +++ b/tests/test_errata_advisory_rpms_signed_handler.py @@ -95,6 +95,7 @@ class TestErrataAdvisoryRPMsSignedHandler(helpers.ModelsTestCase): }, "generate_pulp_repos": True, "odcs_compose_ids": None, + "published": False, }) self.image_b = ContainerImage({ 'repository': 'repo_2', @@ -115,6 +116,7 @@ class TestErrataAdvisoryRPMsSignedHandler(helpers.ModelsTestCase): }, "generate_pulp_repos": True, "odcs_compose_ids": None, + "published": False, }) self.image_c = ContainerImage({ 'repository': 'repo_2', @@ -136,6 +138,7 @@ class TestErrataAdvisoryRPMsSignedHandler(helpers.ModelsTestCase): }, "generate_pulp_repos": True, "odcs_compose_ids": None, + "published": False, }) self.image_d = ContainerImage({ 'repository': 'repo_2', @@ -157,6 +160,7 @@ class TestErrataAdvisoryRPMsSignedHandler(helpers.ModelsTestCase): }, "generate_pulp_repos": True, "odcs_compose_ids": None, + "published": False, }) self.image_e = ContainerImage({ 'repository': 'repo_2', @@ -178,6 +182,7 @@ class TestErrataAdvisoryRPMsSignedHandler(helpers.ModelsTestCase): }, "generate_pulp_repos": True, "odcs_compose_ids": None, + "published": False, }) self.image_f = ContainerImage({ 'repository': 'repo_2', @@ -199,6 +204,7 @@ class TestErrataAdvisoryRPMsSignedHandler(helpers.ModelsTestCase): }, "generate_pulp_repos": True, "odcs_compose_ids": None, + "published": False, }) # For simplicify, mocking _find_images_to_rebuild to just return one # batch, which contains images found for rebuild from parent to @@ -375,7 +381,7 @@ class TestErrataAdvisoryRPMsSignedHandler(helpers.ModelsTestCase): handler.handle(self.rhsa_event) prepare_yum_repos_for_rebuilds.assert_called_once() - start_to_build_images.assert_not_called() + start_to_build_images.assert_called_once() db_event = Event.get(db.session, self.rhsa_event.msg_id) self.assertEqual(EventState.BUILDING.value, db_event.state) diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index 9f9b185..3686f0f 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -323,6 +323,7 @@ class TestBatches(helpers.ModelsTestCase): "generate_pulp_repos": True, "arches": "x86_64", "odcs_compose_ids": [10, 11], + "published": False, }) @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.create_odcs_client') @@ -648,6 +649,71 @@ class TestPrepareYumRepo(helpers.ModelsTestCase): self.assertTrue(build.state_reason.endswith( "of advisory 123 is the latest build in its candidate tag.")) + def _get_fake_container_image(self): + return { + u'rpm_manifest': [ + {u'rpms': [{u'architecture': u'noarch', + u'gpg': u'199e2f91fd431d51', + u'name': u'apache-commons-lang', + u'nvra': u'apache-commons-lang-2.6-15.el7.noarch', + u'release': u'15.el7', + u'srpm_name': u'apache-commons-lang', + u'srpm_nevra': u'apache-commons-lang-0:2.6-15.el7.src', + u'summary': u'Provides a host of helper utilities for the java.lang API', + u'version': u'2.6'}, + {u'architecture': u'noarch', + u'gpg': u'199e2f91fd431d51', + u'name': u'avalon-logkit', + u'nvra': u'avalon-logkit-2.1-14.el7.noarch', + u'release': u'14.el7', + u'srpm_name': u'avalon-logkit', + u'srpm_nevra': u'avalon-logkit-0:2.1-14.el7.src', + u'summary': u'Java logging toolkit', + u'version': u'2.1'}]}]} + + @patch('freshmaker.handlers.errata.errata_advisory_rpms_signed.' + 'create_odcs_client') + @patch('time.sleep') + def test_prepare_odcs_compose_with_image_rpms( + self, sleep, create_odcs_client): + odcs = create_odcs_client.return_value + odcs.new_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": 0, + "state_name": "wait", + } + + image = self._get_fake_container_image() + + handler = ErrataAdvisoryRPMsSignedHandler() + compose = handler._prepare_odcs_compose_with_image_rpms(image) + + db.session.refresh(self.ev) + self.assertEqual(3, compose['id']) + + # Ensure new_compose is called to request a new compose + odcs.new_compose.assert_called_once_with( + '', 'build', builds=set(['avalon-logkit-2.1-14.el7', 'apache-commons-lang-2.6-15.el7']), + flags=['no_deps'], packages=set([u'avalon-logkit', u'apache-commons-lang']), sigkeys=[]) + + def test_prepare_odcs_compose_with_image_rpms_no_rpm_manifest(self): + handler = ErrataAdvisoryRPMsSignedHandler() + + compose = handler._prepare_odcs_compose_with_image_rpms({}) + self.assertEqual(compose, None) + + compose = handler._prepare_odcs_compose_with_image_rpms( + {"rpm_manifest": []}) + self.assertEqual(compose, None) + + compose = handler._prepare_odcs_compose_with_image_rpms( + {"rpm_manifest": [{"rpms": []}]}) + self.assertEqual(compose, None) + class TestErrataAdvisoryStateChangedHandler(helpers.ModelsTestCase): @@ -894,6 +960,7 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): "generate_pulp_repos": True, "arches": "x86_64", "odcs_compose_ids": None, + "published": False, })], [ContainerImage({ "brew": { @@ -937,6 +1004,7 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): "generate_pulp_repos": True, "arches": "x86_64", "odcs_compose_ids": None, + "published": False, })] ] @@ -983,6 +1051,7 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): "generate_pulp_repos": False, "arches": "x86_64", "odcs_compose_ids": None, + "published": False, })] ] @@ -1019,7 +1088,8 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): "commit": "123456789", "target": "target-candidate", "git_branch": "rhel-7", - "error": None + "error": None, + "published": False, })], [ContainerImage({ "brew": { @@ -1059,7 +1129,8 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): "commit": "987654321", "target": "target-candidate", "git_branch": "rhel-7", - "error": None + "error": None, + "published": False, })] ] @@ -1114,6 +1185,7 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): "arches": "x86_64", "generate_pulp_repos": True, "odcs_compose_ids": None, + "published": False, })], [ContainerImage({ "brew": { @@ -1157,6 +1229,7 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): "arches": "x86_64", "generate_pulp_repos": True, "odcs_compose_ids": None, + "published": False, })] ] @@ -1203,6 +1276,7 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): "error": "Some error occurs while getting this image.", "arches": "x86_64", "odcs_compose_ids": None, + "published": False, })] ] @@ -1239,6 +1313,7 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): "error": "Some error occurs while getting this image.", "arches": "x86_64", "odcs_compose_ids": None, + "published": False, })] ] @@ -1275,6 +1350,7 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): "error": "Some error occured.", "arches": "x86_64", "odcs_compose_ids": None, + "published": False, })], [ContainerImage({ "brew": { @@ -1317,6 +1393,7 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): "error": "Some error occured too.", "arches": "x86_64", "odcs_compose_ids": None, + "published": False, })] ] @@ -1357,6 +1434,7 @@ class TestRecordBatchesImages(helpers.ModelsTestCase): "error": "Some error occured.", "arches": "x86_64", "odcs_compose_ids": None, + "published": False, })], ] diff --git a/tests/test_handler.py b/tests/test_handler.py index 68b94f2..92d58ca 100644 --- a/tests/test_handler.py +++ b/tests/test_handler.py @@ -29,7 +29,7 @@ import freshmaker from freshmaker import db from freshmaker.events import ErrataAdvisoryRPMsSignedEvent -from freshmaker.handlers import ContainerBuildHandler +from freshmaker.handlers import ContainerBuildHandler, ODCSComposeNotReady from freshmaker.models import ( ArtifactBuild, ArtifactBuildState, ArtifactBuildCompose, Compose, Event, EVENT_TYPES @@ -37,6 +37,7 @@ from freshmaker.models import ( from freshmaker.errors import UnprocessableEntity, ProgrammingError from freshmaker.types import ArtifactType, EventState from freshmaker.config import any_, all_ +from freshmaker.odcsclient import COMPOSE_STATES from tests import helpers @@ -152,6 +153,7 @@ class TestGetRepoURLs(helpers.ModelsTestCase): return { "id": compose_id, "result_repofile": "http://localhost/%d.repo" % compose_id, + "state": COMPOSE_STATES["done"], } self.patch_odcs_get_compose = patch( @@ -225,6 +227,22 @@ class TestGetRepoURLs(helpers.ModelsTestCase): arch_override='x86_64', compose_ids=[], isolated=True, koji_parent_build=None, release='2', repo_urls=repo_urls) + @patch("freshmaker.handlers.ContainerBuildHandler.build_container") + def test_build_image_artifact_build_repo_urls_compose_not_ready( + self, build_container): + + def mocked_odcs_get_compose(compose_id): + return { + "id": compose_id, + "result_repofile": "http://localhost/%d.repo" % compose_id, + "state": COMPOSE_STATES["generating"], + } + self.odcs_get_compose.side_effect = mocked_odcs_get_compose + + with self.assertRaises(ODCSComposeNotReady): + handler = MyHandler() + handler.build_image_artifact_build(self.build_1, ["http://localhost/x.repo"]) + class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase): """Test BaseHandler.allow_build""" diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index de1e8fe..e5b25c1 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -1015,6 +1015,7 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): "error": None, "arches": None, "odcs_compose_ids": None, + "published": True, "brew": { "completion_date": u"20170421T04:27:51.000-0400", "build": "package-name-2-4-12.10", @@ -1055,12 +1056,14 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): }, ]) + @patch('freshmaker.lightblue.ContainerImage.resolve_published') @patch('freshmaker.lightblue.LightBlue.find_container_images') @patch('os.path.exists') @patch('freshmaker.kojiservice.KojiService.get_build') @patch('freshmaker.kojiservice.KojiService.get_task_request') - def test_parent_images_with_package(self, get_task_request, get_build, - exists, cont_images): + def test_parent_images_with_package( + self, get_task_request, get_build, exists, cont_images, + resolve_published): get_build.return_value = {"task_id": 123456} get_task_request.return_value = [ @@ -1134,13 +1137,14 @@ class TestQueryEntityFromLightBlue(helpers.FreshmakerTestCase): self.assertEqual(0, len(ret)) + @patch('freshmaker.lightblue.ContainerImage.resolve_published') @patch('freshmaker.lightblue.LightBlue.find_container_images') @patch('os.path.exists') @patch('freshmaker.kojiservice.KojiService.get_build') @patch('freshmaker.kojiservice.KojiService.get_task_request') def test_parent_images_with_package_last_parent_content_sets( - self, get_task_request, get_build, exists, cont_images): - + self, get_task_request, get_build, exists, cont_images, + resolve_published): get_build.return_value = {"task_id": 123456} get_task_request.return_value = [ "git://example.com/rpms/repo-1#commit_hash1", "target1", {}]