diff --git a/releasenotes/notes/config-cache-regression-471cd339965b247e.yaml b/releasenotes/notes/config-cache-regression-471cd339965b247e.yaml new file mode 100644 index 0000000000..d8148d4f89 --- /dev/null +++ b/releasenotes/notes/config-cache-regression-471cd339965b247e.yaml @@ -0,0 +1,15 @@ +--- +upgrade: + - | + An error was found in a change related to Zuul's internal + configuration cache which could cause Zuul to use cached in-repo + configuration files which no longer exist. If a ``zuul.yaml`` (or + ``zuul.d/*`` or any related variant) file was deleted or renamed, + Zuul would honor that change immediately, but would attempt to + load both the old and new contents from its cache upon the next + restart. + + This error was introduced in version 4.8.0. + + If upgrading from 4.8.0, run ``zuul-scheduler full-reconfigure`` + in order to correctly update the cache. diff --git a/tests/base.py b/tests/base.py index 202500cd60..f425669469 100644 --- a/tests/base.py +++ b/tests/base.py @@ -4324,8 +4324,8 @@ class SchedulerTestManager: def __setitem__(self, key: int, value: SchedulerTestApp): raise Exception("Not implemented, use create method!") - def __delitem__(self, key, value): - raise Exception("Not implemented!") + def __delitem__(self, key): + del self.instances[key] def __iter__(self): return iter(self.instances) @@ -4595,11 +4595,7 @@ class ZuulTestCase(BaseTestCase): self.builds = self.executor_server.running_builds self.scheds = SchedulerTestManager(self.validate_tenants) - self.scheds.create( - self.log, self.config, self.changes, - self.additional_event_queues, self.upstream_root, self.rpcclient, - self.poller_events, self.git_url_with_auth, - self.fake_sql, self.addCleanup, self.validate_tenants) + self.createScheduler() self.merge_server = None @@ -4611,6 +4607,13 @@ class ZuulTestCase(BaseTestCase): self.scheds.execute( lambda app: app.start(self.validate_tenants)) + def createScheduler(self): + self.scheds.create( + self.log, self.config, self.changes, + self.additional_event_queues, self.upstream_root, self.rpcclient, + self.poller_events, self.git_url_with_auth, + self.fake_sql, self.addCleanup, self.validate_tenants) + def __event_queues(self, matcher) -> List[Queue]: # TODO (felix): Can be removed when the nodes provisioned events are # switched to ZooKeeper. diff --git a/tests/fixtures/config/in-repo-dir/git/common-config/playbooks/job.yaml b/tests/fixtures/config/in-repo-dir/git/common-config/playbooks/job.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/in-repo-dir/git/common-config/playbooks/job.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/in-repo-dir/git/common-config/zuul.d/config.yaml b/tests/fixtures/config/in-repo-dir/git/common-config/zuul.d/config.yaml new file mode 100644 index 0000000000..ee51d8685d --- /dev/null +++ b/tests/fixtures/config/in-repo-dir/git/common-config/zuul.d/config.yaml @@ -0,0 +1,22 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + success: + gerrit: + Verified: 1 + failure: + gerrit: + Verified: -1 + +- job: + name: base + parent: null + run: playbooks/job.yaml + +- project: + name: common-config + check: + jobs: [] diff --git a/tests/fixtures/config/in-repo-dir/git/org_project/README b/tests/fixtures/config/in-repo-dir/git/org_project/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/in-repo-dir/git/org_project/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/in-repo-dir/git/org_project/zuul.d/project.yaml b/tests/fixtures/config/in-repo-dir/git/org_project/zuul.d/project.yaml new file mode 100644 index 0000000000..8d241f17ea --- /dev/null +++ b/tests/fixtures/config/in-repo-dir/git/org_project/zuul.d/project.yaml @@ -0,0 +1,8 @@ +- job: + name: project-test1 + +- project: + name: org/project + check: + jobs: + - project-test1 diff --git a/tests/fixtures/config/in-repo-dir/main.yaml b/tests/fixtures/config/in-repo-dir/main.yaml new file mode 100644 index 0000000000..208e274b13 --- /dev/null +++ b/tests/fixtures/config/in-repo-dir/main.yaml @@ -0,0 +1,8 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - org/project diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index e1fcf37f7e..2f3df70848 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -2441,6 +2441,60 @@ class TestInRepoConfig(ZuulTestCase): A.messages[0], "A should have debug info") +class TestInRepoConfigDir(ZuulTestCase): + # Like TestInRepoConfig, but the fixture test files are in zuul.d + tenant_config_file = 'config/in-repo-dir/main.yaml' + + def test_file_move(self): + # Tests that a zuul config file can be renamed + in_repo_conf = textwrap.dedent( + """ + - job: + name: project-test2 + + - project: + name: org/project + check: + jobs: + - project-test2 + """) + file_dict = {'zuul.d/project.yaml': None, + 'zuul.d/newfile.yaml': in_repo_conf} + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files=file_dict) + A.setMerged() + self.fake_gerrit.addEvent(A.getChangeMergedEvent()) + self.waitUntilSettled() + + B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B') + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertHistory([ + dict(name='project-test2', result='SUCCESS', changes='2,1'), + ], ordered=True) + + self.scheds[0].sched.stop() + self.scheds[0].sched.join() + del self.scheds[0] + self.log.debug("Restarting scheduler") + self.createScheduler() + self.scheds[0].start(self.validate_tenants) + + self.waitUntilSettled() + + # The fake gerrit was lost with the scheduler shutdown; + # restore the state we care about: + self.fake_gerrit.change_number = 2 + + C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C') + self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertHistory([ + dict(name='project-test2', result='SUCCESS', changes='2,1'), + dict(name='project-test2', result='SUCCESS', changes='3,1'), + ], ordered=True) + + class TestGlobalRepoState(AnsibleZuulTestCase): config_file = 'zuul-connections-gerrit-and-github.conf' tenant_config_file = 'config/global-repo-state/main.yaml' diff --git a/zuul/configloader.py b/zuul/configloader.py index 220cf86af7..4f246ec60e 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -1787,8 +1787,10 @@ class TenantParser(object): pb_ltime = min_ltimes[project.canonical_name][branch] if files_cache.isValidFor(tpc, pb_ltime): self.log.debug( - "Using files from cache for project %s @%s", - project.canonical_name, branch) + "Using files from cache for project " + "%s @%s: %s", + project.canonical_name, branch, + list(files_cache.keys())) branch_cache = abide.getUnparsedBranchCache( project.canonical_name, branch) if branch_cache.isValidFor(tpc, files_cache.ltime): @@ -1842,6 +1844,22 @@ class TenantParser(object): # Cache file in Zookeeper if content is not None: files_cache[fn] = content + # If there are any files in the cache which match our + # set of requested files but aren't in the set + # returned by the cat job, then they must have been + # deleted. Remove them from the cache so that the + # file list in the cache is correct. + ok_files = ('zuul.yaml', '.zuul.yaml') + tpc.extra_config_files + ok_dirs = ('zuul.d', '.zuul.d') + tpc.extra_config_dirs + for fn in files_cache.keys(): + if ((fn in ok_files) or + any([fn.startswith(d + '/') for d in ok_dirs])): + if fn not in job.files: + self.log.debug( + "Removing file from cache for project " + "%s @%s: %s", + project.canonical_name, branch, fn) + del files_cache[fn] files_cache.setValidFor(job.extra_config_files, job.extra_config_dirs, job.ltime) @@ -2405,10 +2423,10 @@ class ConfigLoader(object): if fn.startswith(".zuul.d/"): fns2.append(fn) for ef in tpc.extra_config_files: - if fn.startswith(ef): + if fn == ef: fns3.append(fn) for ed in tpc.extra_config_dirs: - if fn.startswith(ed): + if fn.startswith(ed + '/'): fns4.append(fn) fns = (["zuul.yaml"] + sorted(fns1) + [".zuul.yaml"] + sorted(fns2) + fns3 + sorted(fns4))