Add an upgrade test

This adds a framework for upgrade testing where we split a
functional test in half, running the first half on the previous
commit and the second half on the current commit.  This may allow
us to catch upgrade errors which are otherwise difficult to find
in tests because they require data generated by old/removed code.

This does not run the test, since it operates on the current and
prior commits, only a commit that follows this one can run the
job successfully.

Change-Id: I9d4d4af42fb1f684a88ec5a7e747b132423696f1

.zuul.yaml

@@ -96,6 +96,17 @@
       nox_force_python: "3.11"
       python_version: "3.11"
 
+- job:
+    name: zuul-nox-upgrade
+    parent: zuul-nox
+    run: playbooks/zuul-upgrade/run.yaml
+    failure-output:
+      # This matches stestr output when a test fails
+      # {1} tests.unit.test_blah [5.743446s] ... FAILED
+      - '\{\d+\} (.*?) \[[\d\.]+s\] \.\.\. FAILED'
+    vars:
+      nox_session: upgrade
+
 - job:
     # Zuul cient uses this job so we can't just delete it yet.
     name: zuul-tox-zuul-client

noxfile.py

@@ -96,6 +96,23 @@ def tests(session):
                 *session.posargs)
 
 
+@nox.session(python='3')
+def upgrade(session):
+    set_standard_env_vars(session)
+    session.install('-r', 'requirements.txt',
+                    '-r', 'test-requirements.txt')
+    session.install('-e', '.')
+    session.run_always('zuul-manage-ansible', '-v')
+    procs = max(int(multiprocessing.cpu_count() * 0.75), 1)
+    session.run('stestr', 'run', '--test-path', './tests/upgrade',
+                '--slowest', f'--concurrency={procs}',
+                *session.posargs)
+    # Output the test log to stdout so we have a copy of even the
+    # successful output.  We capture and output instead of just
+    # streaming it so that it's not interleaved.
+    session.run('stestr', 'last', '--all-attachments')
+
+
 @nox.session(python='3')
 def remote(session):
     set_standard_env_vars(session)

playbooks/zuul-upgrade/roles/checkout/tasks/main.yaml

@@ -0,0 +1,4 @@
+- name: Checkout a ref
+  command: 'git checkout {{ ref }}'
+  args:
+    chdir: '{{ zuul.project.src_dir }}'

playbooks/zuul-upgrade/run.yaml

@@ -0,0 +1,11 @@
+- hosts: all
+  roles:
+    - revoke-sudo
+    - role: checkout
+      ref: 'origin/{{ zuul.branch }}'
+    - role: nox
+      nox_extra_args: '-v -- tests.upgrade.test_upgrade_old'
+    - role: checkout
+      ref: '{{ zuul.branch }}'
+    - role: nox
+      nox_extra_args: '-v -- tests.upgrade.test_upgrade_new'

tests/base.py

@@ -1006,6 +1006,7 @@ class RecordingExecutorServer(zuul.executor.server.ExecutorServer):
 
     def stop(self):
         for build in self.running_builds:
+            build.aborted = True
             build.release()
         super(RecordingExecutorServer, self).stop()
 
@@ -1313,7 +1314,7 @@ class FakeNodepool(object):
 
 
 class ChrootedKazooFixture(fixtures.Fixture):
-    def __init__(self, test_id):
+    def __init__(self, test_id, random_databases, delete_databases):
         super(ChrootedKazooFixture, self).__init__()
 
         if 'ZOOKEEPER_2181_TCP' in os.environ:
@@ -1359,22 +1360,28 @@ class ChrootedKazooFixture(fixtures.Fixture):
             self.zookeeper_port = int(port)
 
         self.test_id = test_id
+        self.random_databases = random_databases
+        self.delete_databases = delete_databases
 
     def _setUp(self):
-        # Make sure the test chroot paths do not conflict
-        random_bits = ''.join(random.choice(string.ascii_lowercase +
-                                            string.ascii_uppercase)
-                              for x in range(8))
+        if self.random_databases:
+            # Make sure the test chroot paths do not conflict
+            random_bits = ''.join(random.choice(string.ascii_lowercase +
+                                                string.ascii_uppercase)
+                                  for x in range(8))
 
-        rand_test_path = '%s_%s_%s' % (random_bits, os.getpid(), self.test_id)
-        self.zookeeper_chroot = f"/test/{rand_test_path}"
+            test_path = '%s_%s_%s' % (random_bits, os.getpid(), self.test_id)
+        else:
+            test_path = self.test_id.split('.')[-1]
+        self.zookeeper_chroot = f"/test/{test_path}"
 
         self.zk_hosts = '%s:%s%s' % (
             self.zookeeper_host,
             self.zookeeper_port,
             self.zookeeper_chroot)
 
-        self.addCleanup(self._cleanup)
+        if self.delete_databases:
+            self.addCleanup(self._cleanup)
 
         # Ensure the chroot path exists and clean up any pre-existing znodes.
         _tmp_client = kazoo.client.KazooClient(
@@ -1386,8 +1393,9 @@ class ChrootedKazooFixture(fixtures.Fixture):
         )
         _tmp_client.start()
 
-        if _tmp_client.exists(self.zookeeper_chroot):
-            _tmp_client.delete(self.zookeeper_chroot, recursive=True)
+        if self.random_databases:
+            if _tmp_client.exists(self.zookeeper_chroot):
+                _tmp_client.delete(self.zookeeper_chroot, recursive=True)
 
         _tmp_client.ensure_path(self.zookeeper_chroot)
         _tmp_client.stop()
@@ -1495,14 +1503,27 @@ class ZuulWebFixture(fixtures.Fixture):
 
 
 class MySQLSchemaFixture(fixtures.Fixture):
-    def setUp(self):
-        super(MySQLSchemaFixture, self).setUp()
+    log = logging.getLogger('zuul.test.MySQLSchemaFixture')
 
-        random_bits = ''.join(random.choice(string.ascii_lowercase +
-                                            string.ascii_uppercase)
-                              for x in range(8))
-        self.name = '%s_%s' % (random_bits, os.getpid())
-        self.passwd = uuid.uuid4().hex
+    def __init__(self, test_id, random_databases, delete_databases):
+        super().__init__()
+        self.test_id = test_id
+        self.random_databases = random_databases
+        self.delete_databases = delete_databases
+
+    def setUp(self):
+        super().setUp()
+
+        if self.random_databases:
+            random_bits = ''.join(random.choice(string.ascii_lowercase +
+                                                string.ascii_uppercase)
+                                  for x in range(8))
+            self.name = '%s_%s' % (random_bits, os.getpid())
+            self.passwd = uuid.uuid4().hex
+        else:
+            self.name = self.test_id.split('.')[-1]
+            self.passwd = self.name
+        self.log.debug("Creating database %s", self.name)
         self.host = os.environ.get('ZUUL_MYSQL_HOST', '127.0.0.1')
         self.port = int(os.environ.get('ZUUL_MYSQL_PORT', 3306))
         db = pymysql.connect(host=self.host,
@@ -1519,6 +1540,12 @@ class MySQLSchemaFixture(fixtures.Fixture):
                 cur.execute("grant all on {name}.* to '{name}'@''".format(
                     name=self.name))
                 cur.execute("flush privileges")
+        except pymysql.err.ProgrammingError as e:
+            if e.args[0] == 1007:
+                # Database exists
+                pass
+            else:
+                raise
         finally:
             db.close()
 
@@ -1530,9 +1557,11 @@ class MySQLSchemaFixture(fixtures.Fixture):
                 port=self.port
             )
         self.addDetail('dburi', testtools.content.text_content(self.dburi))
-        self.addCleanup(self.cleanup)
+        if self.delete_databases:
+            self.addCleanup(self.cleanup)
 
     def cleanup(self):
+        self.log.debug("Deleting database %s", self.name)
         db = pymysql.connect(host=self.host,
                              port=self.port,
                              user="openstack_citest",
@@ -1549,14 +1578,23 @@ class MySQLSchemaFixture(fixtures.Fixture):
 
 
 class PostgresqlSchemaFixture(fixtures.Fixture):
-    def setUp(self):
-        super(PostgresqlSchemaFixture, self).setUp()
+    def __init__(self, test_id, random_databases, delete_databases):
+        super().__init__()
+        self.test_id = test_id
+        self.random_databases = random_databases
+        self.delete_databases = delete_databases
 
-        # Postgres lowercases user and table names during creation but not
-        # during authentication. Thus only use lowercase chars.
-        random_bits = ''.join(random.choice(string.ascii_lowercase)
-                              for x in range(8))
-        self.name = '%s_%s' % (random_bits, os.getpid())
+    def setUp(self):
+        super().setUp()
+
+        if self.random_databases:
+            # Postgres lowercases user and table names during creation but not
+            # during authentication. Thus only use lowercase chars.
+            random_bits = ''.join(random.choice(string.ascii_lowercase)
+                                  for x in range(8))
+            self.name = '%s_%s' % (random_bits, os.getpid())
+        else:
+            self.name = self.test_id.split('.')[-1]
         self.passwd = uuid.uuid4().hex
         self.host = os.environ.get('ZUUL_POSTGRES_HOST', '127.0.0.1')
         db = psycopg2.connect(host=self.host,
@@ -1574,7 +1612,8 @@ class PostgresqlSchemaFixture(fixtures.Fixture):
             name=self.name, passwd=self.passwd, host=self.host)
 
         self.addDetail('dburi', testtools.content.text_content(self.dburi))
-        self.addCleanup(self.cleanup)
+        if self.delete_databases:
+            self.addCleanup(self.cleanup)
 
     def cleanup(self):
         db = psycopg2.connect(host=self.host,
@@ -1629,6 +1668,12 @@ def cpu_times(self):
 class BaseTestCase(testtools.TestCase):
     log = logging.getLogger("zuul.test")
     wait_timeout = 90
+    # These can be unset to use predictable database fixtures that
+    # persist across an upgrade functional test run.
+    random_databases = True
+    delete_databases = True
+    use_tmpdir = True
+    always_attach_logs = False
 
     def attachLogs(self, *args):
         def reader():
@@ -1681,7 +1726,10 @@ class BaseTestCase(testtools.TestCase):
             if (os.environ.get('OS_LOG_CAPTURE') == 'True' or
                 os.environ.get('OS_LOG_CAPTURE') == '1'):
                 self._log_stream = StringIO()
-                self.addOnException(self.attachLogs)
+                if self.always_attach_logs:
+                    self.addCleanup(self.attachLogs)
+                else:
+                    self.addOnException(self.attachLogs)
             else:
                 self._log_stream = sys.stdout
         else:
@@ -1738,8 +1786,10 @@ class BaseTestCase(testtools.TestCase):
 
     def setupZK(self):
         self.zk_chroot_fixture = self.useFixture(
-            ChrootedKazooFixture(self.id())
-        )
+            ChrootedKazooFixture(self.id(),
+                                 self.random_databases,
+                                 self.delete_databases,
+                                 ))
 
     def getZKWatches(self):
         # TODO: The client.command method used here returns only the
@@ -2018,6 +2068,8 @@ class ZuulTestCase(BaseTestCase):
     validate_tenants = None
     wait_for_init = None
     scheduler_count = SCHEDULER_COUNT
+    init_repos = True
+    load_change_db = False
 
     def __getattr__(self, name):
         """Allows to access fake connections the old way, e.g., using
@@ -2056,13 +2108,16 @@ class ZuulTestCase(BaseTestCase):
         self.setupZK()
         self.fake_nodepool = FakeNodepool(self.zk_chroot_fixture)
 
-        if not KEEP_TEMPDIRS:
-            tmp_root = self.useFixture(fixtures.TempDir(
-                rootdir=os.environ.get("ZUUL_TEST_ROOT"))
-            ).path
+        if self.use_tmpdir:
+            if not KEEP_TEMPDIRS:
+                tmp_root = self.useFixture(fixtures.TempDir(
+                    rootdir=os.environ.get("ZUUL_TEST_ROOT")
+                )).path
+            else:
+                tmp_root = tempfile.mkdtemp(
+                    dir=os.environ.get("ZUUL_TEST_ROOT", None))
         else:
-            tmp_root = tempfile.mkdtemp(
-                dir=os.environ.get("ZUUL_TEST_ROOT", None))
+            tmp_root = os.environ.get("ZUUL_TEST_ROOT", '/tmp')
         self.test_root = os.path.join(tmp_root, "zuul-test")
         self.upstream_root = os.path.join(self.test_root, "upstream")
         self.merger_src_root = os.path.join(self.test_root, "merger-git")
@@ -2072,14 +2127,14 @@ class ZuulTestCase(BaseTestCase):
         self.executor_state_root = os.path.join(self.test_root, "executor-lib")
         self.jobdir_root = os.path.join(self.test_root, "builds")
 
-        if os.path.exists(self.test_root):
+        if os.path.exists(self.test_root) and self.init_repos:
             shutil.rmtree(self.test_root)
-        os.makedirs(self.test_root)
-        os.makedirs(self.upstream_root)
-        os.makedirs(self.state_root)
-        os.makedirs(self.merger_state_root)
-        os.makedirs(self.executor_state_root)
-        os.makedirs(self.jobdir_root)
+        os.makedirs(self.test_root, exist_ok=True)
+        os.makedirs(self.upstream_root, exist_ok=True)
+        os.makedirs(self.state_root, exist_ok=True)
+        os.makedirs(self.merger_state_root, exist_ok=True)
+        os.makedirs(self.executor_state_root, exist_ok=True)
+        os.makedirs(self.jobdir_root, exist_ok=True)
 
         # Make per test copy of Configuration.
         self.config = self.setup_config(self.config_file)
@@ -2140,6 +2195,8 @@ class ZuulTestCase(BaseTestCase):
         gerritconnection.GerritEventConnector.delay = 0.0
 
         self.changes = FakeChangeDB()
+        if self.load_change_db:
+            self.loadChangeDB()
 
         self.additional_event_queues = []
         self.zk_client = ZooKeeperClient.fromConfig(self.config)
@@ -2273,12 +2330,14 @@ class ZuulTestCase(BaseTestCase):
         def _setup_fixture(config, section_name):
             if (config.get(section_name, 'dburi') ==
                     '$MYSQL_FIXTURE_DBURI$'):
-                f = MySQLSchemaFixture()
+                f = MySQLSchemaFixture(self.id(), self.random_databases,
+                                       self.delete_databases)
                 self.useFixture(f)
                 config.set(section_name, 'dburi', f.dburi)
             elif (config.get(section_name, 'dburi') ==
                   '$POSTGRESQL_FIXTURE_DBURI$'):
-                f = PostgresqlSchemaFixture()
+                f = PostgresqlSchemaFixture(self.id(), self.random_databases,
+                                            self.delete_databases)
                 self.useFixture(f)
                 config.set(section_name, 'dburi', f.dburi)
 
@@ -2315,15 +2374,17 @@ class ZuulTestCase(BaseTestCase):
                         config.remove_option('scheduler', cfg_attr)
 
             if tenant_config:
-                git_path = os.path.join(
-                    os.path.dirname(
-                        os.path.join(FIXTURE_DIR, tenant_config)),
-                    'git')
-                if os.path.exists(git_path):
-                    for reponame in os.listdir(git_path):
-                        project = reponame.replace('_', '/')
-                        self.copyDirToRepo(project,
-                                           os.path.join(git_path, reponame))
+                if self.init_repos:
+                    git_path = os.path.join(
+                        os.path.dirname(
+                            os.path.join(FIXTURE_DIR, tenant_config)),
+                        'git')
+                    if os.path.exists(git_path):
+                        for reponame in os.listdir(git_path):
+                            project = reponame.replace('_', '/')
+                            self.copyDirToRepo(
+                                project,
+                                os.path.join(git_path, reponame))
         # Make test_root persist after ansible run for .flag test
         config.set('executor', 'trusted_rw_paths', self.test_root)
 
@@ -2355,10 +2416,11 @@ class ZuulTestCase(BaseTestCase):
                 if name.startswith('^'):
                     continue
                 untrusted_projects.append(name)
-                self.init_repo(name)
-                self.addCommitToRepo(name, 'initial commit',
-                                     files={'README': ''},
-                                     branch='master', tag='init')
+                if self.init_repos:
+                    self.init_repo(name)
+                    self.addCommitToRepo(name, 'initial commit',
+                                         files={'README': ''},
+                                         branch='master', tag='init')
             if 'job' in item:
                 if 'run' in item['job']:
                     files['%s' % item['job']['run']] = ''
@@ -2385,9 +2447,11 @@ class ZuulTestCase(BaseTestCase):
         config.set('scheduler', 'tenant_config',
                    os.path.join(FIXTURE_DIR, f.name))
 
-        self.init_repo('org/common-config')
-        self.addCommitToRepo('org/common-config', 'add content from fixture',
-                             files, branch='master', tag='init')
+        if self.init_repos:
+            self.init_repo('org/common-config')
+            self.addCommitToRepo('org/common-config',
+                                 'add content from fixture',
+                                 files, branch='master', tag='init')
 
         return True
 

tests/unit/test_connection.py

@@ -484,12 +484,15 @@ class TestRequiredSQLConnection(BaseTestCase):
             if self.config.get(section_name, 'driver') == 'sql':
                 if (self.config.get(section_name, 'dburi') ==
                         '$MYSQL_FIXTURE_DBURI$'):
-                    f = MySQLSchemaFixture()
+                    f = MySQLSchemaFixture(self.id(), self.random_databases,
+                                           self.delete_databases)
                     self.useFixture(f)
                     self.config.set(section_name, 'dburi', f.dburi)
                 elif (self.config.get(section_name, 'dburi') ==
                       '$POSTGRESQL_FIXTURE_DBURI$'):
-                    f = PostgresqlSchemaFixture()
+                    f = PostgresqlSchemaFixture(self.id(),
+                                                self.random_databases,
+                                                self.delete_databases)
                     self.useFixture(f)
                     self.config.set(section_name, 'dburi', f.dburi)
         self.connections = zuul.lib.connections.ConnectionRegistry()

tests/unit/test_database.py

@@ -46,7 +46,8 @@ class TestMysqlDatabase(DBBaseTestCase):
     def setUp(self):
         super().setUp()
 
-        f = MySQLSchemaFixture()
+        f = MySQLSchemaFixture(self.id(), self.random_databases,
+                               self.delete_databases)
         self.useFixture(f)
 
         config = dict(dburi=f.dburi)
@@ -445,7 +446,8 @@ class TestPostgresqlDatabase(DBBaseTestCase):
     def setUp(self):
         super().setUp()
 
-        f = PostgresqlSchemaFixture()
+        f = PostgresqlSchemaFixture(self.id(), self.random_databases,
+                                    self.delete_databases)
         self.useFixture(f)
         self.db = f
 

tests/unit/test_scheduler.py

@@ -534,12 +534,6 @@ class TestScheduler(ZuulTestCase):
         for build in self.history:
             self.assertTrue(build.parameters['zuul']['voting'])
 
-        # TODO: remove after we have tests that really exercise this;
-        # for now this verifies we can save and load the changedb (if
-        # popuplated by gerrit changes).
-        self.saveChangeDB()
-        self.loadChangeDB()
-
     def test_zk_profile(self):
         command_socket = self.scheds.first.sched.config.get(
             'scheduler', 'command_socket')

tests/upgrade/test_upgrade_new.py

@@ -0,0 +1,55 @@
+# Copyright 2024 Acme Gating, LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from tests.base import ZuulTestCase, iterate_timeout
+
+
+class TestUpgradeNew(ZuulTestCase):
+    tenant_config_file = "config/single-tenant/main.yaml"
+    scheduler_count = 1
+    random_databases = False
+    delete_databases = True
+    use_tmpdir = False
+    init_repos = False
+    load_change_db = True
+    always_attach_logs = True
+
+    def assertFinalState(self):
+        # In this test, we expect to shut down in a non-final state,
+        # so skip these checks.
+        pass
+
+    def test_upgrade(self):
+        A = self.fake_gerrit.changes[1]
+
+        # Resume builds
+        for _ in iterate_timeout(60, 'wait for build'):
+            if self.history:
+                break
+        self.waitUntilSettled()
+
+        # This was performed in the previous run
+        # dict(name='project-merge', result='SUCCESS', changes='1,1'),
+        self.assertHistory([
+            dict(name='project-test1', result='SUCCESS', changes='1,1'),
+            dict(name='project-test2', result='SUCCESS', changes='1,1'),
+        ], ordered=False)
+        self.assertEqual(len(self.builds), 0)
+
+        self.assertEqual(A.data['status'], 'MERGED')
+        self.assertEqual(A.reported, 2)
+        # Make sure all 3 jobs are in the report.
+        self.assertIn('project-merge', A.messages[-1])
+        self.assertIn('project-test1', A.messages[-1])
+        self.assertIn('project-test2', A.messages[-1])

tests/upgrade/test_upgrade_old.py

@@ -0,0 +1,68 @@
+# Copyright 2024 Acme Gating, LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from tests.base import ZuulTestCase
+
+
+class TestUpgradeOld(ZuulTestCase):
+    tenant_config_file = "config/single-tenant/main.yaml"
+    scheduler_count = 1
+    random_databases = False
+    delete_databases = False
+    use_tmpdir = False
+    init_repos = True
+    load_change_db = False
+    always_attach_logs = True
+
+    def assertFinalState(self):
+        # In this test, we expect to shut down in a non-final state,
+        # so skip these checks.
+        pass
+
+    def shutdown(self):
+        # Shutdown the scheduler now before it gets any aborted events
+        self.scheds.execute(lambda app: app.sched.stop())
+        self.scheds.execute(lambda app: app.sched.join())
+        # Then release the executor jobs and stop the executors
+        self.executor_server.hold_jobs_in_build = False
+        self.executor_server.release()
+        self.scheds.execute(lambda app: app.sched.executor.stop())
+        if self.merge_server:
+            self.merge_server.stop()
+            self.merge_server.join()
+
+        self.executor_server.stop()
+        self.executor_server.join()
+        self.statsd.stop()
+        self.statsd.join()
+        self.fake_nodepool.stop()
+        self.zk_client.disconnect()
+        self.printHistory()
+
+    def test_upgrade(self):
+        self.executor_server.hold_jobs_in_build = True
+        A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
+        A.addApproval('Code-Review', 2)
+        self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
+        self.waitUntilSettled()
+
+        self.assertHistory([])
+        self.assertEqual(len(self.builds), 1)
+        self.executor_server.release()
+        self.waitUntilSettled()
+        self.assertHistory([
+            dict(name='project-merge', result='SUCCESS', changes='1,1'),
+        ], ordered=False)
+        self.assertEqual(len(self.builds), 2)
+        self.saveChangeDB()