From cc509f2b139607d87a108e916ea7e8166f7c9f4a Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Thu, 17 Apr 2025 16:07:33 -0700 Subject: [PATCH] Add newrev to timer-triggered items If jobs in a periodic pipeline run longer than the timer interval for the pipeline trigger, Zuul will not enqueue a second queue item for the same project-branch. This can be counter-intuitive for users, where a pipeline that runs every 24 hours may turn into a pipeline that runs every 48 hours if a job happens to take 25 hours. To address this, we will now store the branch head sha as the newrev of the ref associated with queue items. This will cause periodic pipelines to behave a bit more like post pipelines, in that multiple items will be allowed for the same project-branch. However, if the branch sha has not changed, we will not enqueue further items (since they will continue to look identical to Zuul). Change-Id: I41891ac3011fb95c4c891c7554e1ea6bec94b56f --- doc/source/drivers/timer.rst | 27 ++++++++++++ doc/source/job-content.rst | 9 ++-- .../notes/periodic-head-217573a1cc31279f.yaml | 27 ++++++++++++ tests/fakegithub.py | 20 ++++++--- tests/fakegitlab.py | 19 +++++++-- tests/fakepagure.py | 10 ++++- tests/fixtures/layouts/idle-dereference.yaml | 26 ++++++++++++ tests/unit/test_connection.py | 1 + tests/unit/test_gerrit.py | 7 ++++ tests/unit/test_git_driver.py | 8 ++++ tests/unit/test_github_driver.py | 10 +++++ tests/unit/test_gitlab_driver.py | 8 ++++ tests/unit/test_pagure_driver.py | 8 ++++ tests/unit/test_scheduler.py | 41 +++++++++++++++++++ tests/unit/test_v3.py | 2 +- zuul/driver/gerrit/gerritsource.py | 6 +++ zuul/driver/git/gitconnection.py | 4 ++ zuul/driver/git/gitsource.py | 4 ++ zuul/driver/github/githubconnection.py | 19 +++++++++ zuul/driver/github/githubsource.py | 3 ++ zuul/driver/gitlab/gitlabconnection.py | 6 +++ zuul/driver/gitlab/gitlabsource.py | 3 ++ zuul/driver/pagure/pagureconnection.py | 19 +++++++-- zuul/driver/pagure/paguresource.py | 3 ++ zuul/driver/timer/__init__.py | 17 +++++--- zuul/driver/timer/timermodel.py | 4 +- zuul/driver/timer/timertrigger.py | 17 +++++--- zuul/source/__init__.py | 4 ++ 28 files changed, 299 insertions(+), 33 deletions(-) create mode 100644 releasenotes/notes/periodic-head-217573a1cc31279f.yaml create mode 100644 tests/fixtures/layouts/idle-dereference.yaml diff --git a/doc/source/drivers/timer.rst b/doc/source/drivers/timer.rst index c643675cff..558c433aab 100644 --- a/doc/source/drivers/timer.rst +++ b/doc/source/drivers/timer.rst @@ -42,6 +42,33 @@ Zuul implements the timer using `apscheduler`_, Please check the words, it is not guaranteed to vary from one run of the timer trigger to the next). + .. attr:: dereference + :default: false + + Whether the branch tip should be dereferenced when enqueued. + + This controls the behavior when the timer trigger for a given + project-branch activates a second or more time for a given + project-branch while a queue item for that project-branch is + still in the pipeline. + + If set to the default value of ``false``, then the triggering + event and queue item will only include the name of the branch; + this means that Zuul will see an identical queue item in the + pipeline and will not enqueue a duplicate entry. + + If set to ``true`` then Zuul will look up the current Git sha of + the tip of each project-branch when enqueueing that + project-branch and include that information in the triggering + event and queue item. If the timer trigger activates a second + time while a given project-branch is still in the pipeline, the + behavior then depends on whether the Git commit sha differs. If + the branch has changed between the two activations, Zuul will + treat the second activation as distinct and enqueue a new item + for the same project-branch (but with a different ``newrev`` + value). If the Git commit sha is the same on both activations, + Zuul will not enqueue a second entry. + .. warning:: Be aware the day-of-week value differs from from cron. The first weekday is Monday (0), and the last is Sunday (6). diff --git a/doc/source/job-content.rst b/doc/source/job-content.rst index feaf7909d8..cd6ce7b313 100644 --- a/doc/source/job-content.rst +++ b/doc/source/job-content.rst @@ -439,9 +439,12 @@ described above) are available: This field is present for the following item types: Branch - If the item was enqueued as the result of a change merging - or being pushed to the branch, the git sha of the new - revision will be included here. + If the item was enqueued as the result of a change merging or + being pushed to the branch, the git sha of the new revision + will be included here. If the item was enqueued due to a + timer and the ``dereference`` flag was set on the timer + trigger, it will contain the git sha of the branch at the + time it was enqueued. Tag If the item was enqueued as the result of a tag being diff --git a/releasenotes/notes/periodic-head-217573a1cc31279f.yaml b/releasenotes/notes/periodic-head-217573a1cc31279f.yaml new file mode 100644 index 0000000000..50b1669bad --- /dev/null +++ b/releasenotes/notes/periodic-head-217573a1cc31279f.yaml @@ -0,0 +1,27 @@ +--- +features: + - | + An option has been added to allow multiple timer-triggered queue + items for the same project-branch may now be enqueued if the + branch head has advanced. + + Previously timer-triggered items only included information about + the project and branch, and therefore if a job on such an item ran + longer than the timer interval, Zuul would not enqueue a second + copy of the item because it would have been seen as identical to + the first (even if the underlying repo had changed. + + If the new ``dereference`` attribute of the timer trigger is set, + Zuul will now include information about the branch head SHA of the + queue item, so that now if a timer trigger fires while a queue + item for the same project-branch is already in the queue, the + second item will be enqueued if the branch has changed since the + first item was enqueued. If the SHA of the branch head is the + same for both items, the second item will not be enqueued. + + When set, the ``newrev`` is now available in the job's ``zuul`` + variables for such queue items. + + The previous behavior remains the default, where derefence is not + set, and ``newrev`` is not available. + diff --git a/tests/fakegithub.py b/tests/fakegithub.py index 0e39e2022a..25047b8f55 100644 --- a/tests/fakegithub.py +++ b/tests/fakegithub.py @@ -482,9 +482,14 @@ class FakeUser(object): class FakeBranch(object): - def __init__(self, fake_repo, branch='master', protected=False): + def __init__(self, fake_repo, github_data, branch='master', + protected=False): self.name = branch self._fake_repo = fake_repo + upstream_root = github_data.fake_github_connection.upstream_root + repo_path = os.path.join(upstream_root, fake_repo.name) + repo = git.Repo(repo_path) + self.sha = repo.heads[branch].commit.hexsha @property def protected(self): @@ -493,7 +498,10 @@ class FakeBranch(object): def as_dict(self): return { 'name': self.name, - 'protected': self.protected + 'protected': self.protected, + 'commit': { + 'sha': self.sha, + } } @@ -686,11 +694,11 @@ class FakeCommit: class FakeRepository(object): def __init__(self, name, data): - self._api = FAKE_BASE_URL - self._branches = [FakeBranch(self)] - self._commits = {} self.data = data self.name = name + self._api = FAKE_BASE_URL + self._branches = [FakeBranch(self, data)] + self._commits = {} # Simple dictionary to store permission values per feature (e.g. # checks, Repository contents, Pull requests, Commit statuses, ...). @@ -755,7 +763,7 @@ class FakeRepository(object): return client.session.get(url, headers) def _create_branch(self, branch): - self._branches.append((FakeBranch(self, branch=branch))) + self._branches.append((FakeBranch(self, self.data, branch=branch))) def _delete_branch(self, branch_name): self._branches = [b for b in self._branches if b.name != branch_name] diff --git a/tests/fakegitlab.py b/tests/fakegitlab.py index f66f90c959..70bb7c51d2 100644 --- a/tests/fakegitlab.py +++ b/tests/fakegitlab.py @@ -33,7 +33,8 @@ import git from git.util import IterableList import requests -FakeGitlabBranch = namedtuple('Branch', ('name', 'protected')) +FakeGitlabBranch = namedtuple('Branch', + ('name', 'protected', 'upstream_root')) class GitlabWebServer(object): @@ -214,7 +215,17 @@ class GitlabWebServer(object): owner, name = project.split('/') if branch in fake_repos[(owner, name)]: protected = fake_repos[(owner, name)][branch].protected - self.send_data({'protected': protected}) + upstream_root = fake_repos[(owner, name)][ + branch].upstream_root + repo_path = os.path.join(upstream_root, owner, name) + repo = git.Repo(repo_path) + sha = repo.heads[branch].commit.hexsha + self.send_data({ + 'protected': protected, + 'commit': { + 'id': sha, + }, + }) else: return self.send_data({}, code=404) @@ -325,14 +336,14 @@ class FakeGitlabConnection(gitlabconnection.GitlabConnection): def addProjectByName(self, project_name): owner, proj = project_name.split('/') repo = self._test_web_server.fake_repos[(owner, proj)] - branch = FakeGitlabBranch('master', False) + branch = FakeGitlabBranch('master', False, self.upstream_root) if 'master' not in repo: repo.append(branch) def protectBranch(self, owner, project, branch, protected=True): if branch in self._test_web_server.fake_repos[(owner, project)]: del self._test_web_server.fake_repos[(owner, project)][branch] - fake_branch = FakeGitlabBranch(branch, protected=protected) + fake_branch = FakeGitlabBranch(branch, protected, self.upstream_root) self._test_web_server.fake_repos[(owner, project)].append(fake_branch) def deleteBranch(self, owner, project, branch): diff --git a/tests/fakepagure.py b/tests/fakepagure.py index 18c179f332..1243276dcd 100644 --- a/tests/fakepagure.py +++ b/tests/fakepagure.py @@ -280,7 +280,7 @@ class FakePagureAPIClient(pagureconnection.PagureAPIClient): return self.gen_error("GET") return pr - def get(self, url): + def get(self, url, params=None): self.log.debug("Getting resource %s ..." % url) match = re.match(r'.+/api/0/(.+)/pull-request/(\d+)$', url) @@ -307,7 +307,13 @@ class FakePagureAPIClient(pagureconnection.PagureAPIClient): match = re.match('.+/api/0/(.+)/git/branches$', url) if match: # project = match.groups()[0] - return {'branches': ['master']}, 200, "", "GET" + if params and params.get('with_commits'): + branches = { + 'master': '16ae2a4df107658b52750063ae203f978cf02ff7' + } + else: + branches = ['master'] + return {'branches': branches}, 200, "", "GET" match = re.match(r'.+/api/0/(.+)/pull-request/(\d+)/diffstats$', url) if match: diff --git a/tests/fixtures/layouts/idle-dereference.yaml b/tests/fixtures/layouts/idle-dereference.yaml new file mode 100644 index 0000000000..708ba48011 --- /dev/null +++ b/tests/fixtures/layouts/idle-dereference.yaml @@ -0,0 +1,26 @@ +- pipeline: + name: periodic + manager: independent + trigger: + timer: + - time: '* * * * * */1' + dereference: true + +- job: + name: base + parent: null + run: playbooks/base.yaml + +- job: + name: project-bitrot + nodeset: + nodes: + - name: static + label: ubuntu-xenial + run: playbooks/project-bitrot.yaml + +- project: + name: org/project + periodic: + jobs: + - project-bitrot diff --git a/tests/unit/test_connection.py b/tests/unit/test_connection.py index 031cc6a522..1796a53685 100644 --- a/tests/unit/test_connection.py +++ b/tests/unit/test_connection.py @@ -1064,6 +1064,7 @@ class TestConnectionsBranchCache(ZuulTestCase): # Ensure that the empty list of branches is valid and is not # seen as an error + self.init_repo("org/newproject") newproject = source.getProject('org/newproject') connection.addProject(newproject) tpc = zuul.model.TenantProjectConfig(newproject) diff --git a/tests/unit/test_gerrit.py b/tests/unit/test_gerrit.py index f1f69d8b3a..0a2d9720f1 100644 --- a/tests/unit/test_gerrit.py +++ b/tests/unit/test_gerrit.py @@ -448,6 +448,13 @@ class TestGerritWeb(ZuulTestCase): # the test will time-out. self.waitUntilSettled() + def test_get_project_branch_sha(self): + # Exercise this method since it's only called from timer + # triggers + source = self.fake_gerrit.source + project = source.getProject('org/project') + self.assertIsNotNone(source.getProjectBranchSha(project, 'master')) + class TestFileComments(AnsibleZuulTestCase): config_file = 'zuul-gerrit-web.conf' diff --git a/tests/unit/test_git_driver.py b/tests/unit/test_git_driver.py index a8dba71327..de57c1bd06 100644 --- a/tests/unit/test_git_driver.py +++ b/tests/unit/test_git_driver.py @@ -190,3 +190,11 @@ class TestGitDriver(ZuulTestCase): self.waitUntilSettled() # Make sure no job as run as ignore-delete is True by default self.assertEqual(len(self.history), 0) + + @simple_layout('layouts/basic-git.yaml', driver='git') + def test_get_project_branch_sha(self): + # Exercise this method since it's only called from timer + # triggers + source = self.scheds.first.sched.connections.getSource('git') + project = source.getProject('org/project') + self.assertIsNotNone(source.getProjectBranchSha(project, 'master')) diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index a69903a58f..7c18ad3535 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -1625,6 +1625,14 @@ class TestGithubDriver(ZuulTestCase): self.assertIsNotNone(other_change.cache_stat) self.assertIs(change, other_change) + @simple_layout("layouts/basic-github.yaml", driver="github") + def test_get_project_branch_sha(self): + # Exercise this method since it's only called from timer + # triggers + source = self.fake_github.source + project = source.getProject('org/project') + self.assertIsNotNone(source.getProjectBranchSha(project, 'master')) + class TestMultiGithubDriver(ZuulTestCase): config_file = 'zuul-multi-github.conf' @@ -1805,6 +1813,7 @@ class TestGithubUnprotectedBranches(ZuulTestCase): # add a spare branch so that the project is not empty after master gets # deleted. + self.create_branch('org/project2', 'feat-x') repo._create_branch('feat-x') self.fake_github.emitEvent( self.fake_github.getPushEvent( @@ -1902,6 +1911,7 @@ class TestGithubUnprotectedBranches(ZuulTestCase): # took place. def test_reconfigure_on_pr_to_new_protected_branch(self): self.create_branch('org/project2', 'release') + self.create_branch('org/project2', 'feature') github = self.fake_github.getGithubClient() repo = github.repo_from_project('org/project2') diff --git a/tests/unit/test_gitlab_driver.py b/tests/unit/test_gitlab_driver.py index d7778c68c5..3416500db3 100644 --- a/tests/unit/test_gitlab_driver.py +++ b/tests/unit/test_gitlab_driver.py @@ -1023,6 +1023,14 @@ class TestGitlabDriver(ZuulTestCase): change = conn.getChange(change_key) self.assertEqual(change.commit_id, A.sha) + @simple_layout("layouts/basic-gitlab.yaml", driver="gitlab") + def test_get_project_branch_sha(self): + # Exercise this method since it's only called from timer + # triggers + source = self.fake_gitlab.source + project = source.getProject('org/project') + self.assertIsNotNone(source.getProjectBranchSha(project, 'master')) + class TestGitlabUnprotectedBranches(ZuulTestCase): config_file = 'zuul-gitlab-driver.conf' diff --git a/tests/unit/test_pagure_driver.py b/tests/unit/test_pagure_driver.py index 8c10b581b3..b07bb2cd3d 100644 --- a/tests/unit/test_pagure_driver.py +++ b/tests/unit/test_pagure_driver.py @@ -687,6 +687,14 @@ class TestPagureDriver(ZuulTestCase): # is reverted and not in changed files to trigger project-test2 self.assertEqual(1, len(self.history)) + @simple_layout("layouts/basic-pagure.yaml", driver="pagure") + def test_get_project_branch_sha(self): + # Exercise this method since it's only called from timer + # triggers + source = self.fake_pagure.source + project = source.getProject('org/project') + self.assertIsNotNone(source.getProjectBranchSha(project, 'master')) + class TestPagureToGerritCRD(ZuulTestCase): config_file = 'zuul-crd-pagure.conf' diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 98d90a3813..7620222a49 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -3207,6 +3207,47 @@ class TestScheduler(ZuulTestCase): self.executor_server.release() self.waitUntilSettled() + def test_timer_branch_updated(self): + # Test that if a branch in updated while a periodic job is + # running, we get a second queue item. + # This test can not use simple_layout because it must start + # with a configuration which does not include a + # timer-triggered job so that we have an opportunity to set + # the hold flag before the first job. + self.executor_server.hold_jobs_in_build = True + # Start timer trigger - also org/project + self.commitConfigUpdate('common-config', + 'layouts/idle-dereference.yaml') + self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) + # The pipeline triggers every second, so we should have seen + # several by now. + time.sleep(5) + self.waitUntilSettled() + + M = self.fake_gerrit.addFakeChange('org/project', 'master', 'M') + M.setMerged() + + time.sleep(5) + self.waitUntilSettled() + + # Stop queuing timer triggered jobs so that the assertions + # below don't race against more jobs being queued. + # Must be in same repo, so overwrite config with another one + self.commitConfigUpdate('common-config', 'layouts/no-timer.yaml') + self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) + self.waitUntilSettled() + # If APScheduler is in mid-event when we remove the job, we + # can end up with one more event firing, so give it an extra + # second to settle. + time.sleep(1) + self.waitUntilSettled() + # There should be two periodic jobs, one before and one after + # the update. + self.assertEqual(2, len(self.builds)) + self.executor_server.release() + self.waitUntilSettled() + self.assertEqual(2, len(self.history)) + def test_new_patchset_dequeues_old_on_head(self): "Test that a new patchset causes the old to be dequeued (at head)" # D -> C (depends on B) -> B (depends on A) -> A -> M diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 95eb7abf81..5d3aeeddeb 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -3613,8 +3613,8 @@ class TestGlobalRepoState(AnsibleZuulTestCase): # of override-checkout repo = github.repo_from_project('org/requiredproject-github') repo._set_branch_protection('master', True) - repo._create_branch('feat-x') self.create_branch('org/requiredproject-github', 'feat-x') + repo._create_branch('feat-x') self.fake_github.emitEvent(self.fake_github.getPushEvent( 'org/requiredproject-github', ref='refs/heads/feat-x')) diff --git a/zuul/driver/gerrit/gerritsource.py b/zuul/driver/gerrit/gerritsource.py index ca210adab3..1da7dad528 100644 --- a/zuul/driver/gerrit/gerritsource.py +++ b/zuul/driver/gerrit/gerritsource.py @@ -96,6 +96,12 @@ class GerritSource(BaseSource): return self.connection.getChange(change_key, refresh=refresh, event=event) + def getProjectBranchSha(self, project, branch_name): + return ( + self.connection.getRefSha(project, f'refs/heads/{branch_name}') + or None + ) + def getChangeByURL(self, url, event): try: parsed = urllib.parse.urlparse(url) diff --git a/zuul/driver/git/gitconnection.py b/zuul/driver/git/gitconnection.py index a6fa71481f..7104b7e5ad 100644 --- a/zuul/driver/git/gitconnection.py +++ b/zuul/driver/git/gitconnection.py @@ -140,6 +140,10 @@ class GitConnection(ZKChangeCacheMixin, BaseConnection): refs if ref.startswith('refs/heads/')] return branches + def getRefSha(self, project, ref): + refs = self.lsRemote(project.name) + return refs.get(ref, '') + def getGitUrl(self, project): return os.path.join(self.baseurl, project.name) diff --git a/zuul/driver/git/gitsource.py b/zuul/driver/git/gitsource.py index bc813cf873..edc3ccaccc 100644 --- a/zuul/driver/git/gitsource.py +++ b/zuul/driver/git/gitsource.py @@ -52,6 +52,10 @@ class GitSource(BaseSource): return self.connection.getChange(change_key, refresh=refresh, event=event) + def getProjectBranchSha(self, project, branch_name): + return (self.connection.getRefSha(project, f'refs/heads/{branch_name}') + or None) + def getChangeByURL(self, url, event): return None diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index b4734284c3..0e4705808b 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -1840,6 +1840,25 @@ class GithubConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): resp = resp.json() return resp['default_branch'] + def getProjectBranchSha(self, project, branch_name): + github = self.getGithubClient(project.name) + url = github.session.build_url('repos', project.name, 'branches', + branch_name) + resp = github.session.get(url) + + if resp.status_code == 403: + self.log.error(str(resp)) + rate_limit = github.rate_limit() + if rate_limit['resources']['core']['remaining'] == 0: + self.log.warning("Rate limit exceeded") + return None + elif resp.status_code == 404: + raise Exception("Got status code 404 when fetching " + "project %s branch %s", project.name, branch_name) + + resp = resp.json() + return resp['commit']['sha'] + def isBranchProtected(self, project_name: str, branch_name: str, zuul_event_id=None) -> Optional[bool]: github = self.getGithubClient( diff --git a/zuul/driver/github/githubsource.py b/zuul/driver/github/githubsource.py index 86d810bfcd..0976386772 100644 --- a/zuul/driver/github/githubsource.py +++ b/zuul/driver/github/githubsource.py @@ -152,6 +152,9 @@ class GithubSource(BaseSource): return super().getProjectDefaultBranch(project, tenant, min_ltime) return default_branch + def getProjectBranchSha(self, project, branch_name): + return self.connection.getProjectBranchSha(project, branch_name) + def getProjectBranchCacheLtime(self): return self.connection._branch_cache.ltime diff --git a/zuul/driver/gitlab/gitlabconnection.py b/zuul/driver/gitlab/gitlabconnection.py index ac8ad539b4..260ab8f844 100644 --- a/zuul/driver/gitlab/gitlabconnection.py +++ b/zuul/driver/gitlab/gitlabconnection.py @@ -634,6 +634,12 @@ class GitlabConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): bi.present = True return valid_flags, list(branch_infos.values()) + def getProjectBranchSha(self, project_name, branch_name, + zuul_event_id=None): + branch = self.gl_client.get_project_branch(project_name, branch_name, + zuul_event_id) + return branch['commit']['id'] + def isBranchProtected(self, project_name, branch_name, zuul_event_id=None): branch = self.gl_client.get_project_branch(project_name, branch_name, diff --git a/zuul/driver/gitlab/gitlabsource.py b/zuul/driver/gitlab/gitlabsource.py index 535165c8a3..1a9ef386c0 100644 --- a/zuul/driver/gitlab/gitlabsource.py +++ b/zuul/driver/gitlab/gitlabsource.py @@ -121,6 +121,9 @@ class GitlabSource(BaseSource): def getProjectBranches(self, project, tenant, min_ltime=-1): return self.connection.getProjectBranches(project, tenant, min_ltime) + def getProjectBranchSha(self, project, branch_name): + return self.connection.getProjectBranchSha(project.name, branch_name) + def getProjectBranchCacheLtime(self): return self.connection._branch_cache.ltime diff --git a/zuul/driver/pagure/pagureconnection.py b/zuul/driver/pagure/pagureconnection.py index 92205c8ded..c5c0be66b6 100644 --- a/zuul/driver/pagure/pagureconnection.py +++ b/zuul/driver/pagure/pagureconnection.py @@ -384,9 +384,9 @@ class PagureAPIClient(): verb, url, code, data )) - def get(self, url): + def get(self, url, params=None): self.log.debug("Getting resource %s ..." % url) - ret = self.session.get(url, headers=self.headers) + ret = self.session.get(url, params=params, headers=self.headers) self.log.debug("GET returned (code: %s): %s" % ( ret.status_code, ret.text)) return ret.json(), ret.status_code, ret.url, 'GET' @@ -405,9 +405,13 @@ class PagureAPIClient(): self._manage_error(*resp) return resp[0]['username'] - def get_project_branches(self): + def get_project_branches(self, with_commits=False): path = '%s/git/branches' % self.project - resp = self.get(self.base_url + path) + if with_commits: + params = dict(with_commits=True) + else: + params = None + resp = self.get(self.base_url + path, params=params) self._manage_error(*resp) return resp[0].get('branches', []) @@ -611,6 +615,13 @@ class PagureConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): for name in branches] return BranchFlag.PRESENT, branch_infos + def getProjectBranchSha(self, project, branch_name): + pagure = self.get_project_api_client(project.name) + branches = pagure.get_project_branches(with_commits=True) + + self.log.info("Got branches with commits for %s" % project.name) + return branches[branch_name] + def isBranchProtected(self, project_name, branch_name, zuul_event_id=None): return True diff --git a/zuul/driver/pagure/paguresource.py b/zuul/driver/pagure/paguresource.py index 009da1a650..748593b9bb 100644 --- a/zuul/driver/pagure/paguresource.py +++ b/zuul/driver/pagure/paguresource.py @@ -125,6 +125,9 @@ class PagureSource(BaseSource): def getProjectBranches(self, project, tenant, min_ltime=-1): return self.connection.getProjectBranches(project, tenant, min_ltime) + def getProjectBranchSha(self, project, branch_name): + return self.connection.getProjectBranchSha(project, branch_name) + def getProjectBranchCacheLtime(self): return self.connection._branch_cache.ltime diff --git a/zuul/driver/timer/__init__.py b/zuul/driver/timer/__init__.py index dd0d95fdb3..7b54634070 100644 --- a/zuul/driver/timer/__init__.py +++ b/zuul/driver/timer/__init__.py @@ -151,12 +151,12 @@ class TimerDriver(Driver, TriggerInterface): self._addJobsInner(tenant, pipeline, cron_args, jitter, timespec, - jobs) + ef.dereference, jobs) self._removeJobs(tenant, jobs) self.tenant_jobs[tenant.name] = jobs def _addJobsInner(self, tenant, pipeline, cron_args, jitter, - timespec, jobs): + timespec, dereference, jobs): # jobs is a dict of args->job that we mutate existing_jobs = self.tenant_jobs.get(tenant.name, {}) for project_name, pcs in tenant.layout.project_configs.items(): @@ -171,7 +171,7 @@ class TimerDriver(Driver, TriggerInterface): try: for branch in tenant.getProjectBranches(project_name): args = (tenant.name, pipeline.name, project_name, - branch, timespec,) + branch, dereference, timespec,) existing_job = existing_jobs.get(args) if jitter: # Resolve jitter here so that it is the same @@ -211,7 +211,7 @@ class TimerDriver(Driver, TriggerInterface): tenant, pipeline, project_name) def _onTrigger(self, tenant_name, pipeline_name, project_name, branch, - timespec): + dereference, timespec): if not self.election_won: return @@ -226,13 +226,13 @@ class TimerDriver(Driver, TriggerInterface): with self.tracer.start_as_current_span( "TimerEvent", attributes=attributes): self._dispatchEvent(tenant_name, pipeline_name, project_name, - branch, timespec) + branch, dereference, timespec) except Exception: self.stop_event.set() self.log.exception("Error when dispatching timer event") def _dispatchEvent(self, tenant_name, pipeline_name, project_name, - branch, timespec): + branch, dereference, timespec): self.log.debug('Got trigger for tenant %s and pipeline %s ' 'project %s branch %s with timespec %s', tenant_name, pipeline_name, project_name, @@ -255,6 +255,11 @@ class TimerDriver(Driver, TriggerInterface): # change cache. change_key = project.source.getChangeKey(event) with self.project_update_locks[project.canonical_name]: + if dereference: + event.newrev = project.source.getProjectBranchSha( + project, branch) + else: + event.newrev = None project.source.getChange(change_key, refresh=True, event=event) log = get_annotated_logger(self.log, event) diff --git a/zuul/driver/timer/timermodel.py b/zuul/driver/timer/timermodel.py index 4b00577c63..fdc293fd7c 100644 --- a/zuul/driver/timer/timermodel.py +++ b/zuul/driver/timer/timermodel.py @@ -17,12 +17,14 @@ from zuul.model import EventFilter, TriggerEvent class TimerEventFilter(EventFilter): - def __init__(self, connection_name, trigger, types=[], timespecs=[]): + def __init__(self, connection_name, trigger, types=[], timespecs=[], + dereference=False): EventFilter.__init__(self, connection_name, trigger) self._types = [x.pattern for x in types] self.types = types self.timespecs = timespecs + self.dereference = dereference def __repr__(self): ret = '