From 5a0352a79cfc1675fc50f8648075810eee2bc68c Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Feb 13 2018 04:32:11 +0000
Subject: [PATCH 1/4] Dry run module rebuild for GitRPMSpecChangeEvent


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

---

diff --git a/conf/config.py b/conf/config.py
index 7fc1c22..45d4695 100644
--- a/conf/config.py
+++ b/conf/config.py
@@ -242,6 +242,8 @@ class DevConfiguration(BaseConfiguration):
     AUTH_BACKEND = 'noauth'
     AUTH_OPENIDC_USERINFO_URI = 'https://iddev.fedorainfracloud.org/openidc/UserInfo'
 
+    DRY_RUN = True
+
 
 class TestConfiguration(BaseConfiguration):
     LOG_BACKEND = 'console'
diff --git a/freshmaker/handlers/git/rpm_spec_change.py b/freshmaker/handlers/git/rpm_spec_change.py
index c1ce5a5..b15b07d 100644
--- a/freshmaker/handlers/git/rpm_spec_change.py
+++ b/freshmaker/handlers/git/rpm_spec_change.py
@@ -19,11 +19,12 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
-from freshmaker import log, conf, utils
+from freshmaker import db, log, conf, utils
 from freshmaker.types import ArtifactType
 from freshmaker.pdc import PDC
 from freshmaker.handlers import BaseHandler
 from freshmaker.events import GitRPMSpecChangeEvent
+from freshmaker.models import ArtifactBuild
 
 
 class GitRPMSpecChangeHandler(BaseHandler):
@@ -35,10 +36,19 @@ class GitRPMSpecChangeHandler(BaseHandler):
 
         return False
 
+    def _init_max_build_id(self):
+        """Ensure the uniqueness of ArtifactBuild.build_id in dry run mode"""
+        rows = db.session.query(ArtifactBuild.build_id).filter_by(
+            type=ArtifactType.MODULE.value).all()
+        self._max_build_id = max((row[0] for row in rows)) if rows else 0
+
     def handle(self, event):
         """
         Rebuild module when spec file of rpm in module is updated.
         """
+        if conf.dry_run:
+            self._init_max_build_id()
+
         rpm = event.rpm
         branch = event.branch
         rev = event.rev
@@ -51,18 +61,39 @@ class GitRPMSpecChangeHandler(BaseHandler):
                                          component_branch=branch,
                                          active='true')
 
-        for module in modules:
-            name = module['variant_name']
-            version = module['variant_version']
-            if not self.allow_build(ArtifactType.MODULE, name=name, branch=version):
-                log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist",
-                         name, version)
-                continue
-            log.info("Going to rebuild module '%s:%s'.", name, version)
-            commit_msg = "Bump to rebuild because of %s rpm spec update (%s)." % (rpm, rev)
-            rev = utils.bump_distgit_repo('modules', name, branch=version, commit_msg=commit_msg, logger=log)
-            build_id = self.build_module(name, version, rev)
-            if build_id is not None:
-                self.record_build(event, name, ArtifactType.MODULE, build_id)
+        if conf.dry_run:
+            for module in modules:
+                name = module['variant_name']
+                version = module['variant_version']
+                log.info('DRY-RUN: Allow to build fake module %s', module)
+                log.info("DRY-RUN: Going to rebuild module '%s:%s'.", name, version)
+                commit_msg = "Bump to rebuild because of %s rpm spec update (%s)." % (rpm, rev)
+                log.info('DRY-RUN: Bump dist-git repo, modules/%s, branch %s, commit message %s',
+                         name, version, commit_msg)
+                rev = '1234567'
+                log.info('DRY-RUN: Start to build module %s, version %s, rev %s',
+                         name, version, rev)
+
+                build_id = self.build_module(name, version, rev)
+                if build_id is not None:
+                    build_id = self._max_build_id + build_id
+                    self._max_build_id = build_id
+
+                    log.info('DRY-RUN: Record fake MBS module build ID %s', build_id)
+                    build = self.record_build(event, name, ArtifactType.MODULE, build_id)
+                    log.info('DRY-RUN: Recorded build: %s', build)
+        else:
+            for module in modules:
+                name = module['variant_name']
+                version = module['variant_version']
+                if not self.allow_build(ArtifactType.MODULE, name=name, branch=version):
+                    log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist", name, version)
+                    continue
+                log.info("Going to rebuild module '%s:%s'.", name, version)
+                commit_msg = "Bump to rebuild because of %s rpm spec update (%s)." % (rpm, rev)
+                rev = utils.bump_distgit_repo('modules', name, branch=version, commit_msg=commit_msg, logger=log)
+                build_id = self.build_module(name, version, rev)
+                if build_id is not None:
+                    self.record_build(event, name, ArtifactType.MODULE, build_id)
 
         return []
diff --git a/freshmaker/mbs.py b/freshmaker/mbs.py
index b58a76d..84111ff 100644
--- a/freshmaker/mbs.py
+++ b/freshmaker/mbs.py
@@ -23,7 +23,7 @@
 
 import requests
 
-from freshmaker import log
+from freshmaker import conf, log
 
 
 class MBS(object):
@@ -62,8 +62,12 @@ class MBS(object):
         body = {'scmurl': scm_url, 'branch': branch}
         url = "%s/module-build-service/1/module-builds/" % self.base_url
 
-        resp = requests.request("POST", url, headers=headers, json=body)
-        data = resp.json()
+        if conf.dry_run:
+            log.info('DRY-RUN: Request MBS to rebuild: %s', ('POST', url, headers, body))
+            data = {'id': 1}
+        else:
+            resp = requests.request("POST", url, headers=headers, json=body)
+            data = resp.json()
         if 'id' in data:
             log.info("Triggered build of %s, MBS build_id=%s", scm_url, data['id'])
             return data['id']
diff --git a/tests/test_git_rpm_spec_change_handler.py b/tests/test_git_rpm_spec_change_handler.py
index 568e21c..b0c9e7d 100644
--- a/tests/test_git_rpm_spec_change_handler.py
+++ b/tests/test_git_rpm_spec_change_handler.py
@@ -68,18 +68,17 @@ class GitRPMSpecChangeHandlerTest(helpers.ModelsTestCase):
 
     @mock.patch('freshmaker.handlers.git.rpm_spec_change.PDC')
     @mock.patch('freshmaker.handlers.git.rpm_spec_change.utils')
-    @mock.patch('freshmaker.handlers.git.rpm_spec_change.conf')
+    @mock.patch.object(freshmaker.handlers.git.rpm_spec_change.conf,
+                       'git_base_url', new='git://pkgs.fedoraproject.org')
     @mock.patch.object(freshmaker.conf, 'handler_build_whitelist', new={
         'GitRPMSpecChangeHandler': {
             'module': [{'name': 'testmodule'}, {'branch': 'master'}]
         }
     })
-    def test_can_rebuild_modules_has_rpm_included(self, conf, utils, PDC):
+    def test_can_rebuild_modules_has_rpm_included(self, utils, PDC):
         """
         Test handler can rebuild modules which include the rpm.
         """
-        conf.git_base_url = "git://pkgs.fedoraproject.org"
-
         m = helpers.DistGitMessage('rpms', 'bash', 'master', '123')
         m.add_changed_file('bash.spec', 1, 1)
         msg = m.produce()

From 30372ecebaff3984b69f46191ed301b1e1f5e46a Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Feb 13 2018 04:39:49 +0000
Subject: [PATCH 2/4] Dry run module rebuild for GitModuleMetadataChangeEvent


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

---

diff --git a/freshmaker/handlers/__init__.py b/freshmaker/handlers/__init__.py
index e5db5e8..178f9db 100644
--- a/freshmaker/handlers/__init__.py
+++ b/freshmaker/handlers/__init__.py
@@ -33,7 +33,7 @@ from freshmaker.kojiservice import koji_service, parse_NVR
 from freshmaker.mbs import MBS
 from freshmaker.models import ArtifactBuildState
 from freshmaker.types import EventState
-from freshmaker.models import ArtifactBuild, Event
+from freshmaker.models import ArtifactBuild, ArtifactType, Event
 from freshmaker.utils import krb_context, get_rebuilt_nvr
 from freshmaker.errors import UnprocessableEntity, ProgrammingError
 from freshmaker.odcsclient import create_odcs_client
@@ -117,6 +117,15 @@ class BaseHandler(object):
         self._db_artifact_build_id = None
         self._log_prefix = ""
 
+        if conf.dry_run:
+            self._init_max_build_id()
+
+    def _init_max_build_id(self):
+        """Ensure the uniqueness of ArtifactBuild.build_id in dry run mode"""
+        rows = db.session.query(ArtifactBuild.build_id).filter_by(
+            type=ArtifactType.MODULE.value).all()
+        self._max_build_id = max((row[0] for row in rows)) if rows else 0
+
     def _log(self, log_fnc, msg, *args, **kwargs):
         """
         Logs the message `msg` using `log_fnc`, passing msg, *args and **kwargs
@@ -153,6 +162,10 @@ class BaseHandler(object):
         """
         return self._log(log.error, msg, *args, **kwargs)
 
+    def log_dry_run(self, msg, *args, **kwargs):
+        """Wrap log.info by adding prefix DRY-RUN: """
+        return self._log(log.info, 'DRY-RUN: {}'.format(msg), *args, **kwargs)
+
     @property
     def current_db_event_id(self):
         return self._db_event_id
diff --git a/freshmaker/handlers/git/module_metadata_change.py b/freshmaker/handlers/git/module_metadata_change.py
index 5575df0..4be2b5d 100644
--- a/freshmaker/handlers/git/module_metadata_change.py
+++ b/freshmaker/handlers/git/module_metadata_change.py
@@ -22,7 +22,7 @@
 # Written by Jan Kaluza <jkaluza@redhat.com>
 
 
-from freshmaker import log
+from freshmaker import conf, log
 from freshmaker.types import ArtifactType
 from freshmaker.handlers import BaseHandler
 from freshmaker.events import GitModuleMetadataChangeEvent
@@ -40,13 +40,28 @@ class GitModuleMetadataChangeHandler(BaseHandler):
     def handle(self, event):
         log.info("Triggering rebuild of module %s:%s, metadata updated (%s).",
                  event.module, event.branch, event.rev)
-        if not self.allow_build(ArtifactType.MODULE, name=event.module, branch=event.branch):
-            log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist",
-                     event.module, event.branch)
-            return []
-
-        build_id = self.build_module(event.module, event.branch, event.rev)
-        if build_id is not None:
-            self.record_build(event, event.module, ArtifactType.MODULE, build_id)
+
+        if conf.dry_run:
+            self.log_dry_run('Allow build module: name %s, branch %s',
+                             event.module, event.branch)
+        else:
+            if not self.allow_build(ArtifactType.MODULE, name=event.module, branch=event.branch):
+                log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist",
+                         event.module, event.branch)
+                return []
+
+        if conf.dry_run:
+            self.log_dry_run('Build module: (%s, %s, %s)',
+                             event.module, event.branch, event.rev)
+            build_id = self.build_module(event.module, event.branch, event.rev)
+            if build_id is not None:
+                build_id = self._max_build_id + build_id
+                self._max_build_id = build_id
+                build = self.record_build(event, event.module, ArtifactType.MODULE, build_id)
+                self.log_dry_run('Recorded build: %s', build)
+        else:
+            build_id = self.build_module(event.module, event.branch, event.rev)
+            if build_id is not None:
+                self.record_build(event, event.module, ArtifactType.MODULE, build_id)
 
         return []
diff --git a/freshmaker/handlers/git/rpm_spec_change.py b/freshmaker/handlers/git/rpm_spec_change.py
index b15b07d..c85bb92 100644
--- a/freshmaker/handlers/git/rpm_spec_change.py
+++ b/freshmaker/handlers/git/rpm_spec_change.py
@@ -19,12 +19,11 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
-from freshmaker import db, log, conf, utils
+from freshmaker import log, conf, utils
 from freshmaker.types import ArtifactType
 from freshmaker.pdc import PDC
 from freshmaker.handlers import BaseHandler
 from freshmaker.events import GitRPMSpecChangeEvent
-from freshmaker.models import ArtifactBuild
 
 
 class GitRPMSpecChangeHandler(BaseHandler):
@@ -36,19 +35,10 @@ class GitRPMSpecChangeHandler(BaseHandler):
 
         return False
 
-    def _init_max_build_id(self):
-        """Ensure the uniqueness of ArtifactBuild.build_id in dry run mode"""
-        rows = db.session.query(ArtifactBuild.build_id).filter_by(
-            type=ArtifactType.MODULE.value).all()
-        self._max_build_id = max((row[0] for row in rows)) if rows else 0
-
     def handle(self, event):
         """
         Rebuild module when spec file of rpm in module is updated.
         """
-        if conf.dry_run:
-            self._init_max_build_id()
-
         rpm = event.rpm
         branch = event.branch
         rev = event.rev
@@ -65,23 +55,23 @@ class GitRPMSpecChangeHandler(BaseHandler):
             for module in modules:
                 name = module['variant_name']
                 version = module['variant_version']
-                log.info('DRY-RUN: Allow to build fake module %s', module)
-                log.info("DRY-RUN: Going to rebuild module '%s:%s'.", name, version)
+                self.log_dry_run('Allow to build fake module %s', module)
+                self.log_dry_run("Going to rebuild module '%s:%s'.", name, version)
                 commit_msg = "Bump to rebuild because of %s rpm spec update (%s)." % (rpm, rev)
-                log.info('DRY-RUN: Bump dist-git repo, modules/%s, branch %s, commit message %s',
-                         name, version, commit_msg)
+                self.log_dry_run('Bump dist-git repo, modules/%s, branch %s, commit message %s',
+                                 name, version, commit_msg)
                 rev = '1234567'
-                log.info('DRY-RUN: Start to build module %s, version %s, rev %s',
-                         name, version, rev)
+                self.log_dry_run('Start to build module %s, version %s, rev %s',
+                                 name, version, rev)
 
                 build_id = self.build_module(name, version, rev)
                 if build_id is not None:
                     build_id = self._max_build_id + build_id
                     self._max_build_id = build_id
 
-                    log.info('DRY-RUN: Record fake MBS module build ID %s', build_id)
+                    self.log_dry_run('Record fake MBS module build ID %s', build_id)
                     build = self.record_build(event, name, ArtifactType.MODULE, build_id)
-                    log.info('DRY-RUN: Recorded build: %s', build)
+                    self.log_dry_run('Recorded build: %s', build)
         else:
             for module in modules:
                 name = module['variant_name']
diff --git a/tests/test_handler.py b/tests/test_handler.py
index 0c9b3c8..ff5f535 100644
--- a/tests/test_handler.py
+++ b/tests/test_handler.py
@@ -221,12 +221,13 @@ class TestGetRepoURLs(helpers.ModelsTestCase):
 class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase):
     """Test BaseHandler.allow_build"""
 
-    @patch('freshmaker.handlers.conf')
-    def test_allow_build_in_whitelist(self, conf):
+    @patch('freshmaker.handlers.conf.handler_build_whitelist',
+           new_callable=PropertyMock)
+    def test_allow_build_in_whitelist(self, handler_build_whitelist):
         """ Test if artifact is in the handlers whitelist """
         whitelist_rules = {"image": [{'name': "test"}]}
         handler = MyHandler()
-        conf.handler_build_whitelist.get.return_value = whitelist_rules
+        handler_build_whitelist.get.return_value = whitelist_rules
         container = {"name": "test", "branch": "branch"}
 
         allow = handler.allow_build(ArtifactType.IMAGE,
@@ -234,12 +235,13 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase):
                                     branch=container["branch"])
         assert allow
 
-    @patch('freshmaker.handlers.conf')
-    def test_allow_build_not_in_whitelist(self, conf):
+    @patch('freshmaker.handlers.conf.handler_build_whitelist',
+           new_callable=PropertyMock)
+    def test_allow_build_not_in_whitelist(self, handler_build_whitelist):
         """ Test if artifact is not in the handlers whitelist """
         whitelist_rules = {"image": [{'name': "test1"}]}
         handler = MyHandler()
-        conf.handler_build_whitelist.get.return_value = whitelist_rules
+        handler_build_whitelist.get.return_value = whitelist_rules
         container = {"name": "test", "branch": "branch"}
 
         allow = handler.allow_build(ArtifactType.IMAGE,
@@ -247,13 +249,13 @@ class TestAllowBuildBasedOnWhitelist(helpers.FreshmakerTestCase):
                                     branch=container["branch"])
         assert not allow
 
-    @patch('freshmaker.handlers.conf')
-    def test_allow_build_regex_exception(self, conf):
+    @patch('freshmaker.handlers.conf.handler_build_whitelist')
+    def test_allow_build_regex_exception(self, handler_build_whitelist):
         """ If there is a regex error, method will raise UnprocessableEntity error """
 
         whitelist_rules = {"image": [{'name': "te(st"}]}
         handler = MyHandler()
-        conf.handler_build_whitelist.get.return_value = whitelist_rules
+        handler_build_whitelist.get.return_value = whitelist_rules
         container = {"name": "test", "branch": "branch"}
 
         with self.assertRaises(UnprocessableEntity):

From b0d94ac5250f82dd61ed8bee8503c5f138b37b25 Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Feb 13 2018 07:27:00 +0000
Subject: [PATCH 3/4] Do not bypass allow_build check during dry run in GitModuleMetadataChangeHandler


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

---

diff --git a/freshmaker/handlers/git/module_metadata_change.py b/freshmaker/handlers/git/module_metadata_change.py
index 4be2b5d..8c865ae 100644
--- a/freshmaker/handlers/git/module_metadata_change.py
+++ b/freshmaker/handlers/git/module_metadata_change.py
@@ -41,14 +41,10 @@ class GitModuleMetadataChangeHandler(BaseHandler):
         log.info("Triggering rebuild of module %s:%s, metadata updated (%s).",
                  event.module, event.branch, event.rev)
 
-        if conf.dry_run:
-            self.log_dry_run('Allow build module: name %s, branch %s',
-                             event.module, event.branch)
-        else:
-            if not self.allow_build(ArtifactType.MODULE, name=event.module, branch=event.branch):
-                log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist",
-                         event.module, event.branch)
-                return []
+        if not self.allow_build(ArtifactType.MODULE, name=event.module, branch=event.branch):
+            log.info("Skip rebuild of %s:%s as it's not allowed by configured whitelist",
+                     event.module, event.branch)
+            return []
 
         if conf.dry_run:
             self.log_dry_run('Build module: (%s, %s, %s)',

From d0f00b4ecbfeabaeaf99d02c6e485fc23ae88a8a Mon Sep 17 00:00:00 2001
From: Chenxiong Qi <cqi@redhat.com>
Date: Feb 13 2018 07:28:31 +0000
Subject: [PATCH 4/4] Do not set DRY_RUN explicitly in dev configuration section


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

---

diff --git a/conf/config.py b/conf/config.py
index 45d4695..7fc1c22 100644
--- a/conf/config.py
+++ b/conf/config.py
@@ -242,8 +242,6 @@ class DevConfiguration(BaseConfiguration):
     AUTH_BACKEND = 'noauth'
     AUTH_OPENIDC_USERINFO_URI = 'https://iddev.fedorainfracloud.org/openidc/UserInfo'
 
-    DRY_RUN = True
-
 
 class TestConfiguration(BaseConfiguration):
     LOG_BACKEND = 'console'

