From ce0e348eb631e712df730d8e6d0628981c413a96 Mon Sep 17 00:00:00 2001 From: Simon Westphahl Date: Thu, 8 Jul 2021 16:25:01 +0200 Subject: [PATCH] Don't clear connection caches during full reconfig Clearing of connection caches doesn't seem to be needed in practice and is removed as it can't be easily coordinated between multiple schedulers. Change-Id: I663fa1f1752d0dd599af06a7275a14235faaff2f --- tests/unit/test_circular_dependencies.py | 10 ++++++++-- tests/unit/test_configloader.py | 9 +++++++-- tests/unit/test_github_driver.py | 12 ++++++++++-- tests/unit/test_gitlab_driver.py | 11 ++++++++--- tests/unit/test_scheduler.py | 6 ++++++ tests/unit/test_v3.py | 8 ++++++-- tests/unit/test_web.py | 6 ++++++ tests/zuul_client/test_zuulclient.py | 3 +++ zuul/connection/__init__.py | 22 ---------------------- zuul/driver/gerrit/gerritconnection.py | 3 --- zuul/driver/pagure/pagureconnection.py | 3 --- zuul/scheduler.py | 5 ----- 12 files changed, 54 insertions(+), 44 deletions(-) diff --git a/tests/unit/test_circular_dependencies.py b/tests/unit/test_circular_dependencies.py index 846513821a..40c98920e9 100644 --- a/tests/unit/test_circular_dependencies.py +++ b/tests/unit/test_circular_dependencies.py @@ -1388,15 +1388,21 @@ class TestGithubCircularDependencies(ZuulTestCase): 'master', True) github.repo_from_project('gh/project')._set_branch_protection( 'stable/foo', True) + pevent = self.fake_github.getPushEvent(project='gh/project', + ref='refs/heads/stable/foo') + self.fake_github.emitEvent(pevent) self.create_branch('gh/project1', 'stable/bar') github.repo_from_project('gh/project1')._set_branch_protection( 'master', True) github.repo_from_project('gh/project1')._set_branch_protection( 'stable/bar', True) + pevent = self.fake_github.getPushEvent(project='gh/project', + ref='refs/heads/stable/bar') + self.fake_github.emitEvent(pevent) - # Reconfigure to pick up branch protection settings - self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) + # Wait until push events are processed to pick up branch + # protection settings self.waitUntilSettled() A = self.fake_github.openFakePullRequest( diff --git a/tests/unit/test_configloader.py b/tests/unit/test_configloader.py index bf8d2fd61e..a0b4bfad08 100644 --- a/tests/unit/test_configloader.py +++ b/tests/unit/test_configloader.py @@ -434,8 +434,13 @@ class TestTenantConfigBranches(ZuulTestCase): self.log.debug('Creating branches') self.create_branch('common-config', 'stable') self.create_branch('common-config', 'feat_x') - - self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) + self.fake_gerrit.addEvent( + self.fake_gerrit.getFakeBranchCreatedEvent( + 'common-config', 'stable')) + self.fake_gerrit.addEvent( + self.fake_gerrit.getFakeBranchCreatedEvent( + 'common-config', 'feat_x')) + self.waitUntilSettled() # Job must be defined in master self._validate_job(common_job, 'master') diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index 757601fdd7..f6df5eb222 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -1339,7 +1339,10 @@ class TestGithubUnprotectedBranches(ZuulTestCase): github = self.fake_github.getGithubClient() repo = github.repo_from_project('org/project2') repo._set_branch_protection('master', True) - self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) + + pevent = self.fake_github.getPushEvent(project='org/project2', + ref='refs/heads/master') + self.fake_github.emitEvent(pevent) self.waitUntilSettled() tenant = self.scheds.first.sched.abide.tenants.get('tenant-one') @@ -1470,6 +1473,9 @@ class TestGithubUnprotectedBranches(ZuulTestCase): github = self.fake_github.getGithubClient() repo = github.repo_from_project('org/project2') repo._set_branch_protection('master', True) + self.fake_github.emitEvent( + self.fake_github.getPushEvent( + project='org/project2', ref='refs/heads/master')) A = self.fake_github.openFakePullRequest('org/project2', 'master', 'A') A.setMerged("merging A") @@ -1477,8 +1483,10 @@ class TestGithubUnprotectedBranches(ZuulTestCase): # add a spare branch so that the project is not empty after master gets # deleted. repo._create_branch('feat-x') + self.fake_github.emitEvent( + self.fake_github.getPushEvent( + project='org/project2', ref='refs/heads/feat-x')) - self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) self.waitUntilSettled() # record previous tenant reconfiguration time, which may not be set diff --git a/tests/unit/test_gitlab_driver.py b/tests/unit/test_gitlab_driver.py index 8458769ad4..7a6d5e5baa 100644 --- a/tests/unit/test_gitlab_driver.py +++ b/tests/unit/test_gitlab_driver.py @@ -726,7 +726,8 @@ class TestGitlabUnprotectedBranches(ZuulTestCase): # now enable branch protection and trigger reload self.fake_gitlab.protectBranch('org', 'project2', 'master') - self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) + pevent = self.fake_gitlab.getPushEvent(project='org/project2') + self.fake_gitlab.emitEvent(pevent) self.waitUntilSettled() tenant = self.scheds.first.sched.abide.tenants.get('tenant-one') @@ -869,6 +870,9 @@ class TestGitlabUnprotectedBranches(ZuulTestCase): # Prepare repo with an initial commit and enable branch protection self.fake_gitlab.protectBranch('org', 'project2', 'master') + self.fake_gitlab.emitEvent( + self.fake_gitlab.getPushEvent( + project='org/project2', branch='refs/heads/master')) A = self.fake_gitlab.openFakeMergeRequest('org/project2', 'master', 'A') @@ -879,8 +883,9 @@ class TestGitlabUnprotectedBranches(ZuulTestCase): self.create_branch('org/project2', 'feat-x') self.fake_gitlab.protectBranch('org', 'project2', 'feat-x', protected=False) - - self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) + self.fake_gitlab.emitEvent( + self.fake_gitlab.getPushEvent( + project='org/project2', branch='refs/heads/feat-x')) self.waitUntilSettled() # record previous tenant reconfiguration time, which may not be set diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index eddab51fc7..0ed05c914b 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -4072,6 +4072,9 @@ class TestScheduler(ZuulTestCase): # timer-triggered job so that we have an opportunity to set # the hold flag before the first job. self.create_branch('org/project', 'stable') + self.fake_gerrit.addEvent( + self.fake_gerrit.getFakeBranchCreatedEvent( + 'org/project', 'stable')) self.executor_server.hold_jobs_in_build = True self.commitConfigUpdate('common-config', 'layouts/timer-template.yaml') self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) @@ -4122,6 +4125,9 @@ class TestScheduler(ZuulTestCase): # timer-triggered job so that we have an opportunity to set # the hold flag before the first job. self.create_branch('org/project', 'stable') + self.fake_gerrit.addEvent( + self.fake_gerrit.getFakeBranchCreatedEvent( + 'org/project', 'stable')) self.executor_server.hold_jobs_in_build = True self.commitConfigUpdate('common-config', config_file) self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 8b5da16988..cf92890e37 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -2542,6 +2542,8 @@ class TestGlobalRepoState(AnsibleZuulTestCase): github.repo_from_project( 'org/requiringproject-github')._set_branch_protection( 'master', True) + self.fake_github.emitEvent(self.fake_github.getPushEvent( + 'org/requiringproject-github', ref='refs/heads/master')) # Create unprotected branch feat-x. This branch will be the target # of override-checkout @@ -2549,9 +2551,11 @@ class TestGlobalRepoState(AnsibleZuulTestCase): repo._set_branch_protection('master', True) repo._create_branch('feat-x') self.create_branch('org/requiredproject-github', 'feat-x') + self.fake_github.emitEvent(self.fake_github.getPushEvent( + 'org/requiredproject-github', ref='refs/heads/feat-x')) - # Reconfigure to ensure zuul knows about the branch protection - self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) + # Wait until Zuul has processed the push events and knows about + # the branch protection self.waitUntilSettled() A = self.fake_github.openFakePullRequest( diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py index 37932a1cb3..a2a21d9acc 100644 --- a/tests/unit/test_web.py +++ b/tests/unit/test_web.py @@ -1794,6 +1794,9 @@ class TestTenantScopedWebApi(BaseTestWeb): """Test that the admin web interface can dequeue a change""" start_builds = len(self.builds) self.create_branch('org/project', 'stable') + self.fake_gerrit.addEvent( + self.fake_gerrit.getFakeBranchCreatedEvent( + 'org/project', 'stable')) self.executor_server.hold_jobs_in_build = True self.commitConfigUpdate('common-config', 'layouts/timer.yaml') self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) @@ -2538,6 +2541,9 @@ class TestCLIViaWebApi(BaseTestWeb): """Test that the CLI can dequeue a change via REST""" start_builds = len(self.builds) self.create_branch('org/project', 'stable') + self.fake_gerrit.addEvent( + self.fake_gerrit.getFakeBranchCreatedEvent( + 'org/project', 'stable')) self.executor_server.hold_jobs_in_build = True self.commitConfigUpdate('common-config', 'layouts/timer.yaml') self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) diff --git a/tests/zuul_client/test_zuulclient.py b/tests/zuul_client/test_zuulclient.py index a5e9003c7a..038fa16583 100644 --- a/tests/zuul_client/test_zuulclient.py +++ b/tests/zuul_client/test_zuulclient.py @@ -291,6 +291,9 @@ class TestZuulClientAdmin(BaseTestWeb): self.executor_server.hold_jobs_in_build = True start_builds = len(self.builds) self.create_branch('org/project', 'stable') + self.fake_gerrit.addEvent( + self.fake_gerrit.getFakeBranchCreatedEvent( + 'org/project', 'stable')) self.executor_server.hold_jobs_in_build = True self.commitConfigUpdate('common-config', 'layouts/timer.yaml') self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) diff --git a/zuul/connection/__init__.py b/zuul/connection/__init__.py index 1d83772fd7..480ee7daa5 100644 --- a/zuul/connection/__init__.py +++ b/zuul/connection/__init__.py @@ -77,17 +77,6 @@ class BaseConnection(object, metaclass=abc.ABCMeta): def registerScheduler(self, sched) -> None: self.sched = sched - def clearCache(self): - """Clear the cache for this connection. - - This is called immediately prior to performing a full - reconfiguration. The cache should be cleared so that a - full reconfiguration can be used to correct any errors in - cached data. - - """ - pass - def maintainCache(self, relevant): """Make cache contain relevant changes. @@ -236,17 +225,6 @@ class CachedBranchConnection(BaseConnection): cache[project.name] = branches return branches - def clearCache(self) -> None: - """Clear the connection cache for all projects. - - This method is called by the scheduler in order to perform a full - reconfiguration. - """ - self.log.debug("Clearing branch cache for all branches: %s", - self.connection_name) - self._project_branch_cache_exclude_unprotected = {} - self._project_branch_cache_include_unprotected = {} - def clearProjectCache(self, project: Project) -> None: """Clear the connection cache for this project. """ diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py index 078d24c996..97c4728f0f 100644 --- a/zuul/driver/gerrit/gerritconnection.py +++ b/zuul/driver/gerrit/gerritconnection.py @@ -721,9 +721,6 @@ class GerritConnection(BaseConnection): def addProject(self, project: Project) -> None: self.projects[project.name] = project - def clearCache(self): - self._project_branch_cache = {} - def _clearBranchCache(self, project): try: del self._project_branch_cache[project.name] diff --git a/zuul/driver/pagure/pagureconnection.py b/zuul/driver/pagure/pagureconnection.py index 01648aa8c0..e65a329b89 100644 --- a/zuul/driver/pagure/pagureconnection.py +++ b/zuul/driver/pagure/pagureconnection.py @@ -534,9 +534,6 @@ class PagureConnection(BaseConnection): for key in remove: del self._change_cache[key] - def clearCache(self): - self.project_branch_cache = {} - def getWebController(self, zuul_web): return PagureWebController(zuul_web, self) diff --git a/zuul/scheduler.py b/zuul/scheduler.py index a48b53d74d..e52d02d1aa 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -881,11 +881,6 @@ class Scheduler(threading.Thread): self.ansible_manager = AnsibleManager( default_version=default_ansible_version) - if not event.smart: - for connection in self.connections.connections.values(): - self.log.debug("Clear cache for: %s" % connection) - connection.clearCache() - loader = configloader.ConfigLoader( self.connections, self, self.merger, self.keystore) tenant_config, script = self._checkTenantSourceConf(self.config)