From b536aef7abe2965b35b2f737f759a21e46909128 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Aug 31 2017 00:51:59 +0000 Subject: [PATCH 1/3] Reword and fix docstring, comment and method name Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index a461282..5d42e8a 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -75,7 +75,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): # Get and record all images to rebuild based on the current # ErrataAdvisoryRPMsSignedEvent event. - builds = self._record_images_to_rebuild(db_event, event) + builds = self._find_and_record_images_to_rebuild(db_event, event) if not builds: log.info('No container images to rebuild for advisory %r', event.errata_name) @@ -108,7 +108,8 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): if ev in seen_extra_events: continue seen_extra_events.append(ev) - builds = self._record_images_to_rebuild(ev, event, builds) + builds = self._find_and_record_images_to_rebuild( + ev, event, builds) repo_urls.append(self._prepare_yum_repo(ev)) # Remove duplicates from repo_urls. @@ -215,7 +216,7 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): """ Logs the information about images to rebuilt using log.info(...). :param builds dict: list of docker images to build as returned by - _record_images_to_rebuild(...). + _find_and_record_images_to_rebuild(...). """ log.info('Found docker images to rebuild in following order:') batch = 0 @@ -245,12 +246,12 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): def _find_events_to_include(self, db_event, builds): """ Find out all unreleased events which built some image which is also - planned to be build as part of current image rebuild. + planned to be built as part of current image rebuild. :param db_event Event: Database representation of ErrataAdvisoryRPMsSignedEvent. :param builds dict: list of docker images to build as returned by - _record_images_to_rebuild(...). + _find_and_record_images_to_rebuild(...). """ events_to_include = [] for ev in Event.get_unreleased(db.session): @@ -269,14 +270,23 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): def _record_batches(self, batches, event, builds=None): """ Records the images from batches to database. - :param batches list: Output of _find_images_to_rebuild(...). - :param event ErrataAdvisoryRPMsSignedEvent: The main event this handler + + :param batches list: Output of LightBlue._find_images_to_rebuild(...). + :param event ErrataAdvisoryRPMsSignedEvent: The event this handler is currently handling. - :param builds dict: list of docker images to build as returned by - _record_images_to_rebuild(...). + :param builds dict: mappings from docker image build NVR to + corresponding ArtifactBuild object, e.g. + ``{brew_build_nvr: ArtifactBuild, ...}``. Previous builds returned + from this method can be passed to this call to be extended by + adding a new mappings after docker image is stored into database. + For the first time to call this method, builds could be None. + :return: a mapping between docker image build NVR and + corresponding ArtifactBuild object representing a future rebuild of + that docker image. It is extended by including those docker images + stored into database. + :rtype: dict """ - - # Used as tmp dict with {brew_buil_id: ArtifactBuild, ...} mapping. + # Used as tmp dict with {brew_build_nvr: ArtifactBuild, ...} mapping. builds = builds or {} for batch in batches: @@ -305,17 +315,20 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): return builds - def _record_images_to_rebuild(self, db_event, event, builds=None): + def _find_and_record_images_to_rebuild(self, db_event, event, builds=None): """ - Finds and records to DB the list of Docker images to rebuild based - on the particular ErrataAdvisoryRPMsSignedEvent. + Finds docker images to rebuild based on the particular + ErrataAdvisoryRPMsSignedEvent and records them into database. :param db_event Event: Database representation of ErrataAdvisoryRPMsSignedEvent. :param event ErrataAdvisoryRPMsSignedEvent: The main event this handler - is currently handling. + is currently handling. Used to store found docker images to + database. :param builds dict: list of docker images to build as returned by - previous calls of _record_images_to_rebuild(...). + previous calls of _find_and_record_images_to_rebuild(...). + :return: mappings extended by and returned from ``_record_batches``. + :rtype: dict """ errata = Errata(conf.errata_tool_server_url) @@ -339,8 +352,8 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): cert=conf.lightblue_certificate, private_key=conf.lightblue_private_key) - # For each RPM build in Errata advisory, find the list of Docker - # images containing this RPM and record it to DB. + # For each RPM package in Errata advisory, find Docker images + # containing this package and record those images into database. builds = builds or {} nvrs = errata.get_builds(errata_id) for nvr in nvrs: diff --git a/freshmaker/handlers/errata/errata_advisory_state_changed.py b/freshmaker/handlers/errata/errata_advisory_state_changed.py index 64f0bcf..68d70d5 100644 --- a/freshmaker/handlers/errata/errata_advisory_state_changed.py +++ b/freshmaker/handlers/errata/errata_advisory_state_changed.py @@ -28,7 +28,15 @@ from freshmaker.handlers import BaseHandler class ErrataAdvisoryStateChangedHandler(BaseHandler): - """Rebuild container when a dependecy container is built in Brew""" + """Mark Errata advisory as released + + When an advisory state is changed to SHIPPED_LIVE, mark it as released in + associated event object of ``ErrataAdvisoryStateChangedHandler``. + + This is used to avoiding generating YUM repository to include RPMs + inlcuded in a SHIPPED_LIVE advisory, because at that state, RPMs will be + available in official YUM repositories. + """ name = 'ErrataAdvisoryStateChangedHandler' @@ -36,16 +44,12 @@ class ErrataAdvisoryStateChangedHandler(BaseHandler): return isinstance(event, ErrataAdvisoryStateChangedEvent) def handle(self, event): - """ - When build container task state changed in brew, update build state in db and - rebuild containers depend on the success build as necessary. - """ - errata_id = event.errata_id state = event.state if state != "SHIPPED_LIVE": - log.debug("Ignoring Errata advisory %d state change to %s, " - "because it is not SHIPPED_LIVE", errata_id, state) + log.debug("Skipping Errata advisory %d to be marked as released, " + "because its state is %s rather than SHIPPED_LIVE.", + errata_id, state) return [] # check db to see whether this advisory exists in db diff --git a/tests/test_errata_advisory_state_changed.py b/tests/test_errata_advisory_state_changed.py index 0282961..51ec0ca 100644 --- a/tests/test_errata_advisory_state_changed.py +++ b/tests/test_errata_advisory_state_changed.py @@ -101,7 +101,7 @@ class TestAllowBuild(unittest.TestCase): db.session.commit() @patch("freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler." - "_record_images_to_rebuild", return_value=[]) + "_find_and_record_images_to_rebuild", return_value=[]) @patch("freshmaker.config.Config.handler_build_whitelist", new_callable=PropertyMock, return_value={ "ErrataAdvisoryRPMsSignedHandler": {"image": [{"advisory_name": "RHSA-.*"}]}}) @@ -116,7 +116,7 @@ class TestAllowBuild(unittest.TestCase): record_images.assert_not_called() @patch("freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler." - "_record_images_to_rebuild", return_value=[]) + "_find_and_record_images_to_rebuild", return_value=[]) @patch("freshmaker.config.Config.handler_build_whitelist", new_callable=PropertyMock, return_value={ "ErrataAdvisoryRPMsSignedHandler": {"image": [{"advisory_name": "RHSA-.*"}]}}) @@ -132,7 +132,7 @@ class TestAllowBuild(unittest.TestCase): record_images.assert_called_once() @patch("freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler." - "_record_images_to_rebuild", return_value=[]) + "_find_and_record_images_to_rebuild", return_value=[]) @patch( "freshmaker.config.Config.handler_build_whitelist", new_callable=PropertyMock, @@ -159,7 +159,7 @@ class TestAllowBuild(unittest.TestCase): record_images.assert_called_once() @patch("freshmaker.handlers.errata.ErrataAdvisoryRPMsSignedHandler." - "_record_images_to_rebuild", return_value=[]) + "_find_and_record_images_to_rebuild", return_value=[]) @patch( "freshmaker.config.Config.handler_build_whitelist", new_callable=PropertyMock, From e6e4180af5aca540e83bb4a2e21c29a05984018b Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Aug 31 2017 00:51:59 +0000 Subject: [PATCH 2/3] Minor enhancements to ArtifactBuild.create * Add more comment to ArtifactBuild.build_id field. * Make parameter build_id optional in method ArtifactBuild.create, as build_id field is nullable. * Make build_id optional in BaseHandler.record_build * Do not pass 0 to parameter build_id when call record_build in ErrataAdvisoryRPMsSignedHandler._record_batches, because NULL will be default. * Fix default value to ArtifactBuild.state. Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py index bdbbcda..3cf9e60 100644 --- a/freshmaker/handlers/__init__.py +++ b/freshmaker/handlers/__init__.py @@ -98,25 +98,28 @@ class BaseHandler(object): namespace=namespace, scratch=conf.koji_container_scratch_build) - def record_build(self, event, name, artifact_type, build_id, dep_on=None, - state=None): + def record_build(self, event, name, artifact_type, + build_id=None, dep_on=None, state=None): """ Record build in db. :param event: instance of an event. :param name: name of the artifact. :param artifact_type: an enum member of ArtifactType. - :param build_id: id of the build in build system. - :param dep_on: the artifact which this one depends on. - :param state: the initial state of build. + :param build_id: id of the real build in a build system. If omitted, + this build has not been built in external build system. + :param dep_on: the artifact which this one depends on. If omitted, no + other artifact is depended on. + :param state: the initial state of build. If omitted, defaults to + ``ArtifactBuildState.BUILD``. :return: recorded build. :rtype: ArtifactBuild. """ ev = models.Event.get_or_create(db.session, event.msg_id, event.search_key, event.__class__) - build = models.ArtifactBuild.create( - db.session, ev, name, artifact_type.name.lower(), build_id, - dep_on, state) + build = models.ArtifactBuild.create(db.session, ev, name, + artifact_type.name.lower(), + build_id, dep_on, state) db.session.commit() return build diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index 5d42e8a..95ea5be 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -301,8 +301,9 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): if image["parent"] else None dep_on = builds[parent_name] if parent_name in builds else None build = self.record_build( - event, name, ArtifactType.IMAGE, 0, dep_on, - ArtifactBuildState.PLANNED.value) + event, name, ArtifactType.IMAGE, + dep_on=dep_on, + state=ArtifactBuildState.PLANNED.value) build_args = {} build_args["repository"] = image["repository"] diff --git a/freshmaker/models.py b/freshmaker/models.py index e89bb1c..9779f7f 100644 --- a/freshmaker/models.py +++ b/freshmaker/models.py @@ -133,20 +133,24 @@ class ArtifactBuild(FreshmakerBase): event_id = db.Column(db.Integer, db.ForeignKey('events.id')) event = relationship("Event", back_populates="builds") - # Id of a build in the build system + # Id of corresponding real build in external build system. Currently, it + # could be ID of a build in MBS or Koji, maybe others in the future. + # build_id may be NULL, which means this build has not been built in + # external build system. build_id = db.Column(db.Integer) # Build args in json format. build_args = db.Column(db.String, nullable=True) @classmethod - def create(cls, session, event, name, type, build_id, dep_on=None, state=None): + def create(cls, session, event, name, type, + build_id=None, dep_on=None, state=None): now = datetime.utcnow() build = cls( name=name, type=type, event=event, - state=state or "build", + state=state or ArtifactBuildState.BUILD.value, build_id=build_id, time_submitted=now, dep_on=dep_on From 797bb7bd984ebf91c7209b54ca20d8f203fa0a74 Mon Sep 17 00:00:00 2001 From: Chenxiong Qi Date: Aug 31 2017 00:51:59 +0000 Subject: [PATCH 3/3] More doc to methods Signed-off-by: Chenxiong Qi --- diff --git a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py index 95ea5be..01b4930 100644 --- a/freshmaker/handlers/errata/errata_advisory_rpms_signed.py +++ b/freshmaker/handlers/errata/errata_advisory_rpms_signed.py @@ -150,18 +150,17 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): else: compose_source = source - odcs = ODCS(conf.odcs_server_url, auth_mech=AuthMech.Kerberos, - verify_ssl=conf.odcs_verify_ssl) - if compose_source is None: - log.error('Builds for errata %d are not the latest build in its ' - 'all tags.', errata_id) + log.error('None of builds %s of advisory %d is the latest build in' + ' its candidate tag.', builds, errata_id) return log.info('Generate new compose for rebuild: ' 'source: %s, source type: %s, packages: %s', compose_source, 'tag', packages) + odcs = ODCS(conf.odcs_server_url, auth_mech=AuthMech.Kerberos, + verify_ssl=conf.odcs_verify_ssl) new_compose = odcs.new_compose(compose_source, 'tag', packages=packages) @@ -195,13 +194,29 @@ class ErrataAdvisoryRPMsSignedHandler(BaseHandler): return new_compose['result_repo'] def _get_packages_for_compose(self, nvr): - """Get RPMs of current build NVR""" + """Get RPMs of current build NVR + + :param str nvr: build NVR. + :return: list of RPM names built from given build. + :rtype: list + """ with koji_service(conf.koji_profile, log) as session: rpms = session.get_build_rpms(nvr) return list(set([rpm['name'] for rpm in rpms])) def _get_compose_source(self, nvr): - """Get tag from which to collect packages to compose""" + """Get tag from which to collect packages to compose + + Try to find *-candidate tag from given build, the NVR. Whatever a + release tag is tagged to the given build, a candidate tag will always + be usable for gathering RPMs from Brew since it is the first tag must + be tagged when package is built in Brew for the first time. + + :param str nvr: build NVR used to find correct tag. + :return: found tag. None is returned if build is not the latest build + of found tag. + :rtype: str + """ with koji_service(conf.koji_profile, log) as service: tag = [tag['name'] for tag in service.session.listTags(nvr) if tag['name'].endswith('-candidate')][0]