Merge pull request buildbot#8184 from p12tic/copydb-fixes

Fix copydb script

master/buildbot/scripts/copydb.py

@@ -55,12 +55,19 @@ def print_log(*args, **kwargs):
     ):
         config_file = base.getConfigFileFromTac(config['basedir'])
 
+    if not config_file:
+        return 1
+
     with base.captureErrors(
         config_module.ConfigErrors, f"Unable to load '{config_file}' from '{config['basedir']}':"
     ):
         master_src_cfg = base.loadConfig(config, config_file)
         master_dst_cfg = base.loadConfig(config, config_file)
-        master_dst_cfg.db["db_url"] = config["destination_url"]
+
+    if not master_src_cfg or not master_dst_cfg:
+        return 1
+
+    master_dst_cfg.db["db_url"] = config["destination_url"]
 
     print_log(f"Copying database ({master_src_cfg.db['db_url']}) to ({config['destination_url']})")
 
@@ -149,7 +156,7 @@ def thd_write(conn):
                 rows_queue.task_done()
 
     def thd_read(conn):
-        q = sa.select([sa.sql.func.count()]).select_from(table)
+        q = sa.select(sa.sql.func.count()).select_from(table)
         total_count[0] = conn.execute(q).scalar()
 
         result = conn.execute(sa.select(table))

master/buildbot/test/unit/scripts/test_copydb.py

@@ -0,0 +1,176 @@
+# This file is part of Buildbot.  Buildbot is free software: you can
+# redistribute it and/or modify it under the terms of the GNU General Public
+# License as published by the Free Software Foundation, version 2.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
+# details.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc., 51
+# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+#
+# Copyright Buildbot Team Members
+
+import os
+import platform
+import textwrap
+
+from twisted.internet import defer
+from twisted.trial import unittest
+
+from buildbot.config import BuilderConfig
+from buildbot.plugins import schedulers
+from buildbot.process.factory import BuildFactory
+from buildbot.scripts import copydb
+from buildbot.steps.master import MasterShellCommand
+from buildbot.test.reactor import TestReactorMixin
+from buildbot.test.util import db
+from buildbot.test.util import dirs
+from buildbot.test.util import misc
+from buildbot.test.util.integration import RunMasterBase
+from buildbot.util.twisted import async_to_deferred
+
+
+def get_script_config(destination_url='sqlite://', **kwargs):
+    config = {
+        "quiet": False,
+        "basedir": os.path.abspath('basedir'),
+        'destination_url': destination_url,
+    }
+    config.update(kwargs)
+    return config
+
+
+def write_buildbot_tac(path):
+    with open(path, "w", encoding='utf-8') as f:
+        f.write(
+            textwrap.dedent("""
+            from twisted.application import service
+            application = service.Application('buildmaster')
+        """)
+        )
+
+
+class TestCopyDb(misc.StdoutAssertionsMixin, dirs.DirsMixin, TestReactorMixin, unittest.TestCase):
+    def setUp(self):
+        self.setup_test_reactor(auto_tear_down=False)
+        self.setUpDirs('basedir')
+        write_buildbot_tac(os.path.join('basedir', 'buildbot.tac'))
+        self.setUpStdoutAssertions()
+
+    @defer.inlineCallbacks
+    def tearDown(self):
+        self.tearDownDirs()
+        yield self.tear_down_test_reactor()
+
+    def create_master_cfg(self, db_url='sqlite://', extraconfig=""):
+        with open(os.path.join('basedir', 'master.cfg'), "w", encoding='utf-8') as f:
+            f.write(
+                textwrap.dedent(f"""
+                from buildbot.plugins import *
+                c = BuildmasterConfig = dict()
+                c['db_url'] = {db_url!r}
+                c['buildbotNetUsageData'] = None
+                c['multiMaster'] = True  # don't complain for no builders
+                {extraconfig}
+            """)
+            )
+
+    @async_to_deferred
+    async def test_not_basedir(self):
+        res = await copydb._copy_database_in_reactor(get_script_config(basedir='doesntexist'))
+        self.assertEqual(res, 1)
+        tac_path = os.path.join('doesntexist', 'buildbot.tac')
+        self.assertInStdout(f'error reading \'{tac_path}\': No such file or directory')
+
+    @async_to_deferred
+    async def test_bad_config(self):
+        res = await copydb._copy_database_in_reactor(get_script_config(basedir='basedir'))
+        self.assertEqual(res, 1)
+        master_cfg_path = os.path.abspath(os.path.join('basedir', 'master.cfg'))
+        self.assertInStdout(f'configuration file \'{master_cfg_path}\' does not exist')
+
+    @async_to_deferred
+    async def test_bad_config2(self):
+        self.create_master_cfg(extraconfig="++++ # syntaxerror")
+        res = await copydb._copy_database_in_reactor(get_script_config(basedir='basedir'))
+        self.assertEqual(res, 1)
+        self.assertInStdout("encountered a SyntaxError while parsing config file:")
+        # config logs an error via log.err, we must eat it or trial will
+        # complain
+        self.flushLoggedErrors()
+
+
+class TestCopyDbRealDb(misc.StdoutAssertionsMixin, RunMasterBase, dirs.DirsMixin, TestReactorMixin):
+    INITIAL_DB_URL = 'sqlite:///tmp.sqlite'
+
+    def setUp(self):
+        self.setUpDirs('basedir')
+        # self.setUpStdoutAssertions()  # comment out to see stdout from script
+        write_buildbot_tac(os.path.join('basedir', 'buildbot.tac'))
+
+    def tearDown(self):
+        self.tearDownDirs()
+
+    @defer.inlineCallbacks
+    def create_master_config(self):
+        f = BuildFactory()
+        cmd = "dir" if platform.system() in ("Windows", "Microsoft") else "ls"
+        f.addStep(MasterShellCommand(cmd))
+
+        config_dict = {
+            'builders': [
+                BuilderConfig(
+                    name="testy",
+                    workernames=["local1"],
+                    factory=f,
+                ),
+            ],
+            'schedulers': [schedulers.ForceScheduler(name="force", builderNames=["testy"])],
+            # Disable checks about missing scheduler.
+            'multiMaster': True,
+            'db_url': self.INITIAL_DB_URL,
+        }
+        yield self.setup_master(config_dict, basedir='basedir')
+        builder_id = yield self.master.data.updates.findBuilderId('testy')
+
+        return builder_id
+
+    def create_master_config_file(self, db_url):
+        with open(os.path.join('basedir', 'master.cfg'), "w", encoding='utf-8') as f:
+            f.write(
+                textwrap.dedent(f"""
+                from buildbot.plugins import *
+                c = BuildmasterConfig = dict()
+                c['db_url'] = {db_url!r}
+                c['buildbotNetUsageData'] = None
+                c['multiMaster'] = True  # don't complain for no builders
+            """)
+            )
+
+    def resolve_db_url(self):
+        # test may use mysql or pg if configured in env
+        envkey = "BUILDBOT_TEST_DB_URL"
+        if envkey not in os.environ or os.environ[envkey] == 'sqlite://':
+            return "sqlite:///" + os.path.abspath(os.path.join("basedir", "dest.sqlite"))
+        return os.environ[envkey]
+
+    @async_to_deferred
+    async def test_full(self):
+        await self.create_master_config()
+
+        await self.doForceBuild()
+        await self.doForceBuild()
+        await self.doForceBuild()
+
+        await self.tested_master.shutdown()
+
+        self.create_master_config_file(self.INITIAL_DB_URL)
+
+        dest_db_url = db.resolve_test_index_in_db_url(self.resolve_db_url())
+
+        script_config = get_script_config(destination_url=dest_db_url)
+        res = await copydb._copy_database_in_reactor(script_config)
+        self.assertEqual(res, 0)

master/buildbot/test/util/integration.py

@@ -63,16 +63,15 @@ def __init__(self):
         self.is_master_shutdown = False
 
     @async_to_deferred
-    async def create_master(self, case, reactor, config_dict):
+    async def create_master(self, case, reactor, config_dict, basedir=None):
         """
         Create a started ``BuildMaster`` with the given configuration.
         """
-        basedir = FilePath(case.mktemp())
-        basedir.createDirectory()
+        if basedir is None:
+            basedir = case.mktemp()
+        os.makedirs(basedir, exist_ok=True)
         config_dict['buildbotNetUsageData'] = None
-        self.master = BuildMaster(
-            basedir.path, reactor=reactor, config_loader=DictLoader(config_dict)
-        )
+        self.master = BuildMaster(basedir, reactor=reactor, config_loader=DictLoader(config_dict))
 
         if 'db_url' not in config_dict:
             config_dict['db_url'] = 'sqlite://'
@@ -278,6 +277,7 @@ async def setup_master(
         reactor,
         config_dict,
         proto='null',
+        basedir=None,
         start_worker=True,
         **worker_kwargs,
     ):
@@ -308,7 +308,7 @@ async def setup_master(
             ]
             config_dict['protocols'] = config_protocols
 
-        await self.create_master(case, reactor, config_dict)
+        await self.create_master(case, reactor, config_dict, basedir=basedir)
         self.master_config_dict = config_dict
         case.assertFalse(stop.called, "startService tried to stop the reactor; check logs")
 
@@ -393,10 +393,16 @@ class RunMasterBase(unittest.TestCase):
         skip = "buildbot-worker package is not installed"
 
     @defer.inlineCallbacks
-    def setup_master(self, config_dict, startWorker=True, **worker_kwargs):
+    def setup_master(self, config_dict, startWorker=True, basedir=None, **worker_kwargs):
         self.tested_master = TestedRealMaster()
         yield self.tested_master.setup_master(
-            self, reactor, config_dict, proto=self.proto, start_worker=startWorker, **worker_kwargs
+            self,
+            reactor,
+            config_dict,
+            proto=self.proto,
+            basedir=basedir,
+            start_worker=startWorker,
+            **worker_kwargs,
         )
         self.master = self.tested_master.master
         self.master_config_dict = self.tested_master.master_config_dict

newsfragments/copydb.bugfix

@@ -0,0 +1 @@
+Fixed ``copydb`` script when SQLAlchemy 2.x is used.