#1182 Move module build to done or failed according to Greenwave
Merged 5 years ago by cqi. Opened 5 years ago by cqi.
cqi/fm-orchestrator gating-module-by-greenwave  into  master

@@ -442,6 +442,15 @@ 

      @staticmethod

      @module_build_service.utils.retry(wait_on=(xmlrpclib.ProtocolError, koji.GenericError))

      def get_session(config, login=True):

+         """Create and return a koji.ClientSession object

+ 

+         :param config: the config object returned from :meth:`init_config`.

+         :type config: :class:`Config`

+         :param bool login: whether to log into the session. To login if True

+             is passed, otherwise not to log into session.

+         :return: the Koji session object.

+         :rtype: :class:`koji.ClientSession`

+         """

          koji_config = munch.Munch(koji.read_config(

              profile_name=config.koji_profile,

              user_config=config.koji_config,

@@ -535,6 +535,13 @@ 

                          "redhat-rpm-config", "fedpkg-minimal", "rpm-build", "shadow-utils"],

              'desc': ('The list packages for offline module build RPM buildroot.')

          },

+         'greenwave_decision_context': {

+             'type': str,

+             'default': 'osci_compose_gate_modules',

+             'desc': 'The Greenwave decision context that whose messages should '

+                     'be handled by MBS. By default, MBS handles Greenwave '

+                     'messages for OSCI.',

+         }

      }

  

      def __init__(self, conf_section_obj):

@@ -107,9 +107,11 @@ 

          topic_categories = _messaging_backends['fedmsg']['services']

          categories_re = '|'.join(map(re.escape, topic_categories))

          regex_pattern = re.compile(

-             (r'(?P<category>' + categories_re + r')(?:(?:\.)'

-              r'(?P<object>build|repo|module))?(?:(?:\.)'

-              r'(?P<subobject>state|build))?(?:\.)(?P<event>change|done|end|tag)$'))

+             r'(?P<category>' + categories_re + r')'

+             r'(?:(?:\.)(?P<object>build|repo|module|decision))?'

+             r'(?:(?:\.)(?P<subobject>state|build))?'

+             r'(?:\.)(?P<event>change|done|end|tag|update)$'

+         )

          regex_results = re.search(regex_pattern, topic)

  

          if regex_results:
@@ -169,6 +171,14 @@ 

                  msg_obj = MBSModule(

                      msg_id, msg_inner_msg.get('id'), msg_inner_msg.get('state'))

  

+             elif (category == 'greenwave' and object == 'decision' and

+                     subobject is None and event == 'update'):

+                 msg_obj = GreenwaveDecisionUpdate(

+                     msg_id=msg_id,

+                     decision_context=msg_inner_msg.get('decision_context'),

+                     policies_satisfied=msg_inner_msg.get('policies_satisfied'),

+                     subject_identifier=msg_inner_msg.get('subject_identifier'))

+ 

              # If the message matched the regex and is important to the app,

              # it will be returned

              if msg_obj:
@@ -246,6 +256,17 @@ 

          self.module_build_state = module_build_state

  

  

+ class GreenwaveDecisionUpdate(BaseMessage):

+     """A class representing message send to topic greenwave.decision.update"""

+ 

+     def __init__(self, msg_id, decision_context, policies_satisfied,

+                  subject_identifier):

+         super(GreenwaveDecisionUpdate, self).__init__(msg_id)

+         self.decision_context = decision_context

+         self.policies_satisfied = policies_satisfied

+         self.subject_identifier = subject_identifier

+ 

+ 

  def publish(topic, msg, conf, service):

      """

      Publish a single message to a given backend, and return
@@ -316,7 +337,7 @@ 

  

  _fedmsg_backend = {

      'publish': _fedmsg_publish,

-     'services': ['buildsys', 'mbs'],

+     'services': ['buildsys', 'mbs', 'greenwave'],

      'parser': FedmsgMessageParser(),

      'topic_suffix': '.',

  }

@@ -271,6 +271,18 @@ 

              ]

  

      @staticmethod

+     def get_by_id(session, module_build_id):

+         """Find out a module build by id and return

+ 

+         :param session: SQLAlchemy database session object.

+         :param int module_build_id: the module build id to find out.

+         :return: the found module build. None is returned if no module build

+             with specified id in database.

+         :rtype: :class:`ModuleBuild`

+         """

+         return session.query(ModuleBuild).filter(ModuleBuild.id == module_build_id).first()

+ 

+     @staticmethod

      def get_last_build_in_all_streams(session, name):

          """

          Returns list of all latest ModuleBuilds in "ready" state for all
@@ -541,7 +553,19 @@ 

          return module

  

      def transition(self, conf, state, state_reason=None):

-         """ Record that a build has transitioned state. """

+         """Record that a build has transitioned state.

+ 

+         The history of state transitions are recorded in model

+         ``ModuleBuildTrace``. If transform to a different state, for example

+         from ``build`` to ``done``, message will be sent to configured message

+         bus.

+ 

+         :param conf: MBS config object returned from function :func:`init_config`

+             which contains loaded configs.

+         :type conf: :class:`Config`

+         :param int state: the state value to transition to. Refer to ``BUILD_STATES``.

+         :param str state_reason: optional reason of why to transform to ``state``.

+         """

          now = datetime.utcnow()

          old_state = self.state

          self.state = state

@@ -41,14 +41,17 @@ 

  import six

  import sqlalchemy.exc

  

- from module_build_service.utils import module_build_state_from_msg

  import module_build_service.messaging

  import module_build_service.scheduler.handlers.repos

  import module_build_service.scheduler.handlers.components

  import module_build_service.scheduler.handlers.modules

  import module_build_service.scheduler.handlers.tags

+ import module_build_service.scheduler.handlers.greenwave

  import module_build_service.monitor as monitor

+ 

  from module_build_service import models, log, conf

+ from module_build_service.scheduler.handlers import greenwave

+ from module_build_service.utils import module_build_state_from_msg

  

  

  class MBSConsumer(fedmsg.consumers.FedmsgConsumer):
@@ -129,6 +132,7 @@ 

          # Only one kind of repo change event, though...

          self.on_repo_change = module_build_service.scheduler.handlers.repos.done

          self.on_tag_change = module_build_service.scheduler.handlers.tags.tagged

+         self.on_decision_update = module_build_service.scheduler.handlers.greenwave.decision_update

          self.sanity_check()

  

      def shutdown(self):
@@ -232,6 +236,9 @@ 

          elif type(msg) == module_build_service.messaging.MBSModule:

              handler = self.on_module_change[module_build_state_from_msg(msg)]

              build = models.ModuleBuild.from_module_event(session, msg)

+         elif type(msg) == module_build_service.messaging.GreenwaveDecisionUpdate:

+             handler = self.on_decision_update

+             build = greenwave.get_corresponding_module_build(msg.subject_identifier)

          else:

              return

  

@@ -0,0 +1,95 @@ 

+ # -*- coding: utf-8 -*-

+ # Copyright (c) 2019  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.

+ #

+ # Written by Chenxiong Qi <cqi@redhat.com>

+ 

+ from module_build_service import conf, db, log

+ from module_build_service.builder.KojiModuleBuilder import KojiModuleBuilder

+ from module_build_service.models import ModuleBuild, BUILD_STATES

+ 

+ 

+ def get_corresponding_module_build(nvr):

+     """Find corresponding module build from database and return

+ 

+     :param str nvr: module build NVR. This is the subject_identifier included

+         inside ``greenwave.decision.update`` message.

+     :return: the corresponding module build object. For whatever the reason,

+         if the original module build id cannot be found from the Koji build of

+         ``nvr``, None will be returned.

+     :rtype: :class:`ModuleBuild` or None

+     """

+     koji_session = KojiModuleBuilder.get_session(conf, login=False)

+     build_info = koji_session.getBuild(nvr)

+     if build_info is None:

+         return None

+ 

+     try:

+         module_build_id = build_info['extra']['typeinfo']['module'][

+             'module_build_service_id']

+     except KeyError:

+         # If any of the keys is not present, the NVR is not the one for

+         # handling Greenwave event.

+         return None

+ 

+     return ModuleBuild.get_by_id(db.session, module_build_id)

+ 

+ 

+ def decision_update(config, session, msg):

+     """Move module build to ready or failed according to Greenwave result

+ 

+     :param config: the config object returned from function :func:`init_config`,

+         which is loaded from configuration file.

+     :type config: :class:`Config`

+     :param session: the SQLAlchemy database session object.

+     :param msg: the message object representing a message received from topic

+         ``greenwave.decision.update``.

+     :type msg: :class:`GreenwaveDecisionUpdate`

+     """

+     if msg.decision_context != config.greenwave_decision_context:

+         log.debug('Skip Greenwave message %s as MBS only handles message in '

+                   'decision context %s',

+                   msg.msg_id, msg.decision_context)

+         return

+ 

+     module_build_nvr = msg.subject_identifier

+ 

+     if not msg.policies_satisfied:

+         log.debug('Skip to handle module build %s because it has not satisfied'

+                   ' Greenwave policies.',

+                   module_build_nvr)

+         return

+ 

+     build = get_corresponding_module_build(module_build_nvr)

+ 

+     if build is None:

+         log.debug('No corresponding module build of subject_identifier %s is '

+                   'found.', module_build_nvr)

+         return

+ 

+     if build.state == BUILD_STATES['done']:

+         build.transition(

+             conf, BUILD_STATES['ready'],

+             state_reason='Module build {} has satisfied Greenwave policies.'

+                          .format(module_build_nvr))

+     else:

+         log.warning('Module build %s is not in done state but Greenwave tells '

+                     'it passes tests in decision context %s',

+                     module_build_nvr, msg.decision_context)

@@ -0,0 +1,179 @@ 

+ # Copyright (c) 2019  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.

+ #

+ # Written by Chenxiong Qi <cqi@redhat.com>

+ 

+ import pytest

+ 

+ from mock import call, patch, Mock

+ from sqlalchemy import func

+ 

+ from module_build_service import conf, db

+ from module_build_service.models import BUILD_STATES, ModuleBuild

+ from module_build_service.scheduler.consumer import MBSConsumer

+ from module_build_service.scheduler.handlers.greenwave import get_corresponding_module_build

+ from module_build_service.scheduler.handlers.greenwave import decision_update

+ from tests import clean_database, make_module

+ 

+ 

+ class TestGetCorrespondingModuleBuild:

+     """Test get_corresponding_module_build"""

+ 

+     def setup_method(self, method):

+         clean_database()

+ 

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

+     def test_module_build_nvr_does_not_exist_in_koji(self, ClientSession):

+         ClientSession.return_value.getBuild.return_value = None

+ 

+         assert get_corresponding_module_build('n-v-r') is None

+ 

+     @pytest.mark.parametrize('build_info', [

+         # Build info does not have key extra

+         {'id': 1000, 'name': 'ed'},

+         # Build info contains key extra, but it is not for the module build

+         {

+             'extra': {'submitter': 'osbs', 'image': {}}

+         },

+         # Key module_build_service_id is missing

+         {

+             'extra': {'typeinfo': {'module': {}}}

+         }

+     ])

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

+     def test_cannot_find_module_build_id_from_build_info(self, ClientSession, build_info):

+         ClientSession.return_value.getBuild.return_value = build_info

+ 

+         assert get_corresponding_module_build('n-v-r') is None

+ 

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

+     def test_corresponding_module_build_id_does_not_exist_in_db(self, ClientSession):

+         fake_module_build_id, = db.session.query(func.max(ModuleBuild.id)).first()

+ 

+         ClientSession.return_value.getBuild.return_value = {

+             'extra': {'typeinfo': {'module': {

+                 'module_build_service_id': fake_module_build_id + 1

+             }}}

+         }

+ 

+         assert get_corresponding_module_build('n-v-r') is None

+ 

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

+     def test_find_the_module_build(self, ClientSession):

+         expected_module_build = (

+             db.session.query(ModuleBuild)

+                       .filter(ModuleBuild.name == 'platform').first()

+         )

+ 

+         ClientSession.return_value.getBuild.return_value = {

+             'extra': {'typeinfo': {'module': {

+                 'module_build_service_id': expected_module_build.id

+             }}}

+         }

+ 

+         build = get_corresponding_module_build('n-v-r')

+ 

+         assert expected_module_build.id == build.id

+         assert expected_module_build.name == build.name

+ 

+ 

+ class TestDecisionUpdateHandler:

+     """Test handler decision_update"""

+ 

+     @patch('module_build_service.scheduler.handlers.greenwave.log')

+     def test_decision_context_is_not_match(self, log):

+         msg = Mock(msg_id='msg-id-1',

+                    decision_context='bodhi_update_push_testing')

+         decision_update(conf, db.session, msg)

+         log.debug.assert_called_once_with(

+             'Skip Greenwave message %s as MBS only handles message in decision'

+             ' context %s',

+             'msg-id-1', 'bodhi_update_push_testing'

+         )

+ 

+     @patch('module_build_service.scheduler.handlers.greenwave.log')

+     def test_not_satisfy_policies(self, log):

+         msg = Mock(msg_id='msg-id-1',

+                    decision_context='osci_compose_gate_modules',

+                    policies_satisfied=False,

+                    subject_identifier='pkg-0.1-1.c1')

+         decision_update(conf, db.session, msg)

+         log.debug.assert_called_once_with(

+             'Skip to handle module build %s because it has not satisfied '

+             'Greenwave policies.',

+             msg.subject_identifier

+         )

+ 

+     @patch('module_build_service.messaging.publish')

+     @patch('module_build_service.builder.KojiModuleBuilder.KojiClientSession')

+     def test_transform_from_done_to_ready(self, ClientSession, publish):

+         clean_database()

+ 

+         # This build should be queried and transformed to ready state

+         module_build = make_module('pkg:0.1:1:c1', requires_list={'platform': 'el8'})

+         module_build.transition(

+             conf, BUILD_STATES['done'], 'Move to done directly for running test.')

+ 

+         # Assert this call below

+         first_publish_call = call(

+             service='mbs',

+             topic='module.state.change',

+             msg=module_build.json(show_tasks=False),

+             conf=conf

+         )

+ 

+         db.session.refresh(module_build)

+ 

+         ClientSession.return_value.getBuild.return_value = {

+             'extra': {'typeinfo': {'module': {

+                 'module_build_service_id': module_build.id

+             }}}

+         }

+ 

+         msg = {

+             'msg_id': 'msg-id-1',

+             'topic': 'org.fedoraproject.prod.greenwave.decision.update',

+             'msg': {

+                 'decision_context': 'osci_compose_gate_modules',

+                 'policies_satisfied': True,

+                 'subject_identifier': 'pkg-0.1-1.c1'

+             }

+         }

+         hub = Mock(config={

+             'validate_signatures': False

+         })

+         consumer = MBSConsumer(hub)

+         consumer.consume(msg)

+ 

+         # Load module build again to check its state is moved correctly

+         module_build = (

+             db.session.query(ModuleBuild)

+                       .filter(ModuleBuild.id == module_build.id).first()

+         )

+ 

+         assert BUILD_STATES['ready'] == module_build.state

+ 

+         publish.assert_has_calls([

+             first_publish_call,

+             call(service='mbs',

+                  topic='module.state.change',

+                  msg=module_build.json(show_tasks=False),

+                  conf=conf),

+         ])

Signed-off-by: Chenxiong Qi cqi@redhat.com

Can you handle the case when NVR does not exist in Koji (some OSCI error...)?

Maybe we should use log.debug here, because this might produce lot of messages in log.

I don't remember right now what's in the FACTORY ticket describing this feature, but we should keep the build in done here definitely and do not move it to failed.

The reason is that Greenwave policy contains multiple tests - let's say 2 for example. When first test passes, Greenwave sends a message that 1/2 tests passed and msg.policies_satisfied would be False. Then after few minutes, second test passes and this time msg.policies_satisfied would be True. We therefore need to keep the module build in done until all the tests passes. It is OK to keep the build in done forever.

With this in mind, you can even change this handler to skip all the messages with msg.policies_satisfied == False early in this handler to not query Koji every time for our greenwave_decision_context.

I have to think about this more, but current code would query Koji on every greenwave.decision.update message. Ideally, we should query Koji only if the handler is going to do something important with module build.

I'm thinking that maybe we could move this code to greenwave.py as some new method and add also the conditions there to not call it if msg.policies_satisfied == False or msg.decision_context != conf.greenwave_decision_context and return empty build in that case with some log.debug().

We could that method also from the greenwave.decision_update to remove code duplicity.

rebased onto 2df880ca5015c261582fa2130c5b6f5221494a27

5 years ago

I have to think about this more, but current code would query Koji on every greenwave.decision.update message. Ideally, we should query Koji only if the handler is going to do something important with module build.

I'm thinking that maybe we could move this code to greenwave.py as some new method and add also the conditions there to not call it if msg.policies_satisfied == False or msg.decision_context != conf.greenwave_decision_context and return empty build in that case with some log.debug().

We could that method also from the greenwave.decision_update to remove code duplicity.

There is a conflict here. If variable build is not set to a queried module build, handler function will not be called. This is why these lines are here to query build info from Koji, find out module_build_id from that result, and call ModuleBuild.get_by_id to set build eventually.

What does it mean by the "empty build"?

I'll try to move these lines into greenwave.py to avoid duplicate.

Maybe we should use log.debug here, because this might produce lot of messages in log.

Done.

Can you handle the case when NVR does not exist in Koji (some OSCI error...)?

Done.

I don't remember right now what's in the FACTORY ticket describing this feature, but we should keep the build in done here definitely and do not move it to failed.

Right. The ticket is just for moving issue to ready if it is in done.

The reason is that Greenwave policy contains multiple tests - let's say 2 for example. When first test passes, Greenwave sends a message that 1/2 tests passed and msg.policies_satisfied would be False. Then after few minutes, second test passes and this time msg.policies_satisfied would be True. We therefore need to keep the module build in done until all the tests passes. It is OK to keep the build in done forever.

With this in mind, you can even change this handler to skip all the messages with msg.policies_satisfied == False early in this handler to not query Koji every time for our greenwave_decision_context.

Done.

rebased onto 7cbdd6e43a030e0f47d8330987fe0dd73cae9724

5 years ago

rebased onto 8f4b6927e8eaca23e24f10be688483618fddcd07

5 years ago

rebased onto 320ce4fc50ad4ab23c015b3d09e4d6853c8ccc4e

5 years ago

@cqi Looks like this needs a rebase, then it can be merged.

rebased onto 61bf2137d7242b69844bd1d591432e690093f51c

5 years ago

pretty please pagure-ci rebuild

5 years ago

rebased onto 0ee8018

5 years ago

Pretty please pagure-ci rebuild

Pull-Request has been merged by cqi

5 years ago