diff options
46 files changed, 971 insertions, 247 deletions
diff --git a/doc/source/developer/model-changelog.rst b/doc/source/developer/model-changelog.rst index b80979362..f78b3f0a0 100644 --- a/doc/source/developer/model-changelog.rst +++ b/doc/source/developer/model-changelog.rst @@ -112,3 +112,10 @@ Version 12 :Prior Zuul version: 8.0.1 :Description: Adds job_versions and build_versions to BuildSet. Affects schedulers. + +Version 13 +---------- +:Prior Zuul version: 8.2.0 +:Description: Stores only the necessary event info as part of a queue item + instead of the full trigger event. + Affects schedulers. diff --git a/doc/source/examples/etc_nodepool/nodepool.yaml b/doc/source/examples/etc_nodepool/nodepool.yaml index 1c1830635..105b0ef54 100644 --- a/doc/source/examples/etc_nodepool/nodepool.yaml +++ b/doc/source/examples/etc_nodepool/nodepool.yaml @@ -7,7 +7,7 @@ zookeeper-tls: ca: /var/certs/certs/cacert.pem labels: - - name: ubuntu-focal + - name: ubuntu-jammy providers: - name: static-vms @@ -16,7 +16,7 @@ providers: - name: main nodes: - name: node - labels: ubuntu-focal + labels: ubuntu-jammy host-key: "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOgHJYejINIKzUiuSJ2MN8uPc+dfFrZ9JH1hLWS8gI+g" python-path: /usr/bin/python3 username: root diff --git a/doc/source/examples/node-Dockerfile b/doc/source/examples/node-Dockerfile index ff74aa592..b588bcf2c 100644 --- a/doc/source/examples/node-Dockerfile +++ b/doc/source/examples/node-Dockerfile @@ -1,4 +1,4 @@ -FROM docker.io/ubuntu:20.04 +FROM docker.io/ubuntu:22.04 RUN apt-get update \ && DEBIAN_FRONTEND="noninteractive" apt-get -y install \ diff --git a/doc/source/examples/zuul-config/zuul.d/jobs.yaml b/doc/source/examples/zuul-config/zuul.d/jobs.yaml index 8ad979e46..bb9822f48 100644 --- a/doc/source/examples/zuul-config/zuul.d/jobs.yaml +++ b/doc/source/examples/zuul-config/zuul.d/jobs.yaml @@ -3,5 +3,5 @@ parent: null nodeset: nodes: - - name: ubuntu-focal - label: ubuntu-focal + - name: ubuntu-jammy + label: ubuntu-jammy diff --git a/doc/source/examples/zuul-config/zuul.d/jobs2.yaml b/doc/source/examples/zuul-config/zuul.d/jobs2.yaml index a6ed1a633..c7b4a6878 100644 --- a/doc/source/examples/zuul-config/zuul.d/jobs2.yaml +++ b/doc/source/examples/zuul-config/zuul.d/jobs2.yaml @@ -18,5 +18,5 @@ timeout: 1800 nodeset: nodes: - - name: ubuntu-focal - label: ubuntu-focal + - name: ubuntu-jammy + label: ubuntu-jammy diff --git a/doc/source/job-content.rst b/doc/source/job-content.rst index d6bb07683..643632d5b 100644 --- a/doc/source/job-content.rst +++ b/doc/source/job-content.rst @@ -669,6 +669,68 @@ of item. - shell: echo example when: zuul_success | bool +.. var:: nodepool + + Information about each host from Nodepool is supplied in the + `nodepool` host variable. Availability of values varies based on + the node and the driver that supplied it. Values may be ``null`` + if they are not applicable. + + .. var:: label + + The nodepool label of this node. + + .. var:: az + + The availability zone in which this node was placed. + + .. var:: cloud + + The name of the cloud in which this node was created. + + .. var:: provider + + The name of the nodepool provider of this node. + + .. var:: region + + The name of the nodepool provider's region. + + .. var:: host_id + + The cloud's host identification for this node's hypervisor. + + .. var:: external_id + + The cloud's identifier for this node. + + .. var:: slot + + If the node supports running multiple jobs on the node, a unique + numeric ID for the subdivision of the node assigned to this job. + This may be used to avoid build directory collisions. + + .. var:: interface_ip + + The best IP address to use to contact the node as determined by + the cloud provider and nodepool. + + .. var:: public_ipv4 + + A public IPv4 address of the node. + + .. var:: private_ipv4 + + A private IPv4 address of the node. + + .. var:: public_ipv6 + + A public IPv6 address of the node. + + .. var:: private_ipv6 + + A private IPv6 address of the node. + Change Items ~~~~~~~~~~~~ diff --git a/releasenotes/notes/elasticsearch-filter-zuul-returned-vars-4a883813481cc313.yaml b/releasenotes/notes/elasticsearch-filter-zuul-returned-vars-4a883813481cc313.yaml new file mode 100644 index 000000000..70b388b16 --- /dev/null +++ b/releasenotes/notes/elasticsearch-filter-zuul-returned-vars-4a883813481cc313.yaml @@ -0,0 +1,6 @@ +--- +deprecations: + - | + The Elasticsearch reporter now filters `zuul` data from the job returned vars. + The job returned vars under the `zuul` key are meant for Zuul and may include large + amount of data such as file comments. diff --git a/releasenotes/notes/handle-existing-commits-with-cherry-pick-e1a979c2e7ed1a78.yaml b/releasenotes/notes/handle-existing-commits-with-cherry-pick-e1a979c2e7ed1a78.yaml new file mode 100644 index 000000000..dd5c502d2 --- /dev/null +++ b/releasenotes/notes/handle-existing-commits-with-cherry-pick-e1a979c2e7ed1a78.yaml @@ -0,0 +1,14 @@ +--- +fixes: + - | + The `cherry-pick` merge mode will now silently skip commits that have + already been applied to the tree when cherry-picking, instead of failing + with an error. + + The exception to this is if the source of the cherry-pick is an empty + commit, in which case it is always kept. + + Skipping commits that have already been applied is important in a pipeline + triggered by the Gerrit `change-merged` event (like the `deploy` pipeline), + since the scheduler would previously try to cherry-pick the change on top + of the commit that just merged and fail. diff --git a/releasenotes/notes/nodepool-slot-2061128253e50580.yaml b/releasenotes/notes/nodepool-slot-2061128253e50580.yaml new file mode 100644 index 000000000..c7ba3e1dc --- /dev/null +++ b/releasenotes/notes/nodepool-slot-2061128253e50580.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + The :var:`nodepool.slot` variable has been added to host vars. + This is supplied by the nodepool static and metastatic drivers + starting with version 8.0.0. It may be used to avoid build + directory collisions on nodes that run more than one job. diff --git a/tests/base.py b/tests/base.py index dcd316fab..290d51934 100644 --- a/tests/base.py +++ b/tests/base.py @@ -384,7 +384,7 @@ class FakeGerritChange(object): def __init__(self, gerrit, number, project, branch, subject, status='NEW', upstream_root=None, files={}, parent=None, merge_parents=None, merge_files=None, - topic=None): + topic=None, empty=False): self.gerrit = gerrit self.source = gerrit self.reported = 0 @@ -429,7 +429,7 @@ class FakeGerritChange(object): self.addMergePatchset(parents=merge_parents, merge_files=merge_files) else: - self.addPatchset(files=files, parent=parent) + self.addPatchset(files=files, parent=parent, empty=empty) if merge_parents: self.data['parents'] = merge_parents elif parent: @@ -503,9 +503,11 @@ class FakeGerritChange(object): repo.heads['master'].checkout() return r - def addPatchset(self, files=None, large=False, parent=None): + def addPatchset(self, files=None, large=False, parent=None, empty=False): self.latest_patchset += 1 - if not files: + if empty: + files = {} + elif not files: fn = '%s-%s' % (self.branch.replace('/', '_'), self.number) data = ("test %s %s %s\n" % (self.branch, self.number, self.latest_patchset)) @@ -1330,7 +1332,7 @@ class FakeGerritConnection(gerritconnection.GerritConnection): def addFakeChange(self, project, branch, subject, status='NEW', files=None, parent=None, merge_parents=None, - merge_files=None, topic=None): + merge_files=None, topic=None, empty=False): """Add a change to the fake Gerrit.""" self.change_number += 1 c = FakeGerritChange(self, self.change_number, project, branch, @@ -1338,7 +1340,7 @@ class FakeGerritConnection(gerritconnection.GerritConnection): status=status, files=files, parent=parent, merge_parents=merge_parents, merge_files=merge_files, - topic=topic) + topic=topic, empty=empty) self.changes[self.change_number] = c return c @@ -2118,6 +2120,21 @@ class FakeGitlabConnection(gitlabconnection.GitlabConnection): yield self._test_web_server.options['community_edition'] = False + @contextmanager + def enable_delayed_complete_mr(self, complete_at): + self._test_web_server.options['delayed_complete_mr'] = complete_at + yield + self._test_web_server.options['delayed_complete_mr'] = 0 + + @contextmanager + def enable_uncomplete_mr(self): + self._test_web_server.options['uncomplete_mr'] = True + orig = self.gl_client.get_mr_wait_factor + self.gl_client.get_mr_wait_factor = 0.1 + yield + self.gl_client.get_mr_wait_factor = orig + self._test_web_server.options['uncomplete_mr'] = False + class GitlabChangeReference(git.Reference): _common_path_default = "refs/merge-requests" diff --git a/tests/fakegithub.py b/tests/fakegithub.py index 725c083e2..25dcb15da 100644 --- a/tests/fakegithub.py +++ b/tests/fakegithub.py @@ -730,7 +730,7 @@ class FakeGithubSession(object): 'message': 'Merge not allowed because of fake reason', } return FakeResponse(data, 405, 'Method not allowed') - pr.setMerged(json["commit_message"]) + pr.setMerged(json.get("commit_message", "")) return FakeResponse({"merged": True}, 200) return FakeResponse(None, 404) diff --git a/tests/fakegitlab.py b/tests/fakegitlab.py index 294887af0..e4a3e1ac8 100644 --- a/tests/fakegitlab.py +++ b/tests/fakegitlab.py @@ -21,6 +21,7 @@ import re import socketserver import threading import urllib.parse +import time from git.util import IterableList @@ -32,12 +33,17 @@ class GitlabWebServer(object): self.merge_requests = merge_requests self.fake_repos = defaultdict(lambda: IterableList('name')) # A dictionary so we can mutate it - self.options = dict(community_edition=False) + self.options = dict( + community_edition=False, + delayed_complete_mr=0, + uncomplete_mr=False) + self.stats = {"get_mr": 0} def start(self): merge_requests = self.merge_requests fake_repos = self.fake_repos options = self.options + stats = self.stats class Server(http.server.SimpleHTTPRequestHandler): log = logging.getLogger("zuul.test.GitlabWebServer") @@ -146,6 +152,7 @@ class GitlabWebServer(object): self.wfile.write(data) def get_mr(self, project, mr): + stats["get_mr"] += 1 mr = self._get_mr(project, mr) data = { 'target_branch': mr.branch, @@ -162,13 +169,20 @@ class GitlabWebServer(object): 'labels': mr.labels, 'merged_at': mr.merged_at.strftime('%Y-%m-%dT%H:%M:%S.%fZ') if mr.merged_at else mr.merged_at, - 'diff_refs': { + 'merge_status': mr.merge_status, + } + if options['delayed_complete_mr'] and \ + time.monotonic() < options['delayed_complete_mr']: + diff_refs = None + elif options['uncomplete_mr']: + diff_refs = None + else: + diff_refs = { 'base_sha': mr.base_sha, 'head_sha': mr.sha, 'start_sha': 'c380d3acebd181f13629a25d2e2acca46ffe1e00' - }, - 'merge_status': mr.merge_status, - } + } + data['diff_refs'] = diff_refs self.send_data(data) def get_mr_approvals(self, project, mr): diff --git a/tests/fixtures/config/elasticsearch-driver/git/common-config/playbooks/test.yaml b/tests/fixtures/config/elasticsearch-driver/git/common-config/playbooks/test.yaml index 75e5b6934..7902f19d0 100644 --- a/tests/fixtures/config/elasticsearch-driver/git/common-config/playbooks/test.yaml +++ b/tests/fixtures/config/elasticsearch-driver/git/common-config/playbooks/test.yaml @@ -2,4 +2,6 @@ tasks: - zuul_return: data: + zuul: + log_url: some-log-url foo: 'bar' 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 index 8d241f17e..201338c5c 100644 --- 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 @@ -1,8 +1,13 @@ - job: name: project-test1 +- queue: + name: project-test-queue + allow-circular-dependencies: true + - project: name: org/project + queue: project-test-queue check: jobs: - project-test1 diff --git a/tests/fixtures/config/in-repo-dir/git/org_project2/.zuul.yaml b/tests/fixtures/config/in-repo-dir/git/org_project2/.zuul.yaml index b86588e42..0c3d89309 100644 --- a/tests/fixtures/config/in-repo-dir/git/org_project2/.zuul.yaml +++ b/tests/fixtures/config/in-repo-dir/git/org_project2/.zuul.yaml @@ -1,5 +1,10 @@ +- queue: + name: project2-test-queue + allow-circular-dependencies: true + - project: name: org/project2 + queue: project2-test-queue check: jobs: - project2-private-extra-file diff --git a/tests/fixtures/config/in-repo-dir/git/org_project3/.extra-3.yaml b/tests/fixtures/config/in-repo-dir/git/org_project3/.extra-3.yaml new file mode 100644 index 000000000..d0098aca7 --- /dev/null +++ b/tests/fixtures/config/in-repo-dir/git/org_project3/.extra-3.yaml @@ -0,0 +1,2 @@ +- job: + name: project3-private-extra-file diff --git a/tests/fixtures/config/in-repo-dir/git/org_project3/.zuul.yaml b/tests/fixtures/config/in-repo-dir/git/org_project3/.zuul.yaml new file mode 100644 index 000000000..4d760354f --- /dev/null +++ b/tests/fixtures/config/in-repo-dir/git/org_project3/.zuul.yaml @@ -0,0 +1,11 @@ +- queue: + name: project3-test-queue + allow-circular-dependencies: true + +- project: + name: org/project3 + queue: project3-test-queue + check: + jobs: + - project3-private-extra-file + - project3-private-extra-dir diff --git a/tests/fixtures/config/in-repo-dir/git/org_project3/README b/tests/fixtures/config/in-repo-dir/git/org_project3/README new file mode 100644 index 000000000..9daeafb98 --- /dev/null +++ b/tests/fixtures/config/in-repo-dir/git/org_project3/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/in-repo-dir/git/org_project3/extra-3.d/jobs-private.yaml b/tests/fixtures/config/in-repo-dir/git/org_project3/extra-3.d/jobs-private.yaml new file mode 100644 index 000000000..d62ad658f --- /dev/null +++ b/tests/fixtures/config/in-repo-dir/git/org_project3/extra-3.d/jobs-private.yaml @@ -0,0 +1,2 @@ +- job: + name: project3-private-extra-dir diff --git a/tests/fixtures/config/in-repo-dir/main.yaml b/tests/fixtures/config/in-repo-dir/main.yaml index bf1679769..f4ea575d3 100644 --- a/tests/fixtures/config/in-repo-dir/main.yaml +++ b/tests/fixtures/config/in-repo-dir/main.yaml @@ -12,6 +12,10 @@ extra-config-paths: - .extra.yaml - extra.d/ + - org/project3: + extra-config-paths: + - .extra-3.yaml + - extra-3.d/ - tenant: name: tenant-two diff --git a/tests/fixtures/layouts/deps-by-topic.yaml b/tests/fixtures/layouts/deps-by-topic.yaml index 3824c5c2c..e7e8fc465 100644 --- a/tests/fixtures/layouts/deps-by-topic.yaml +++ b/tests/fixtures/layouts/deps-by-topic.yaml @@ -47,24 +47,27 @@ run: playbooks/run.yaml - job: - name: test-job + name: check-job + +- job: + name: gate-job - project: name: org/project1 queue: integrated check: jobs: - - test-job + - check-job gate: jobs: - - test-job + - gate-job - project: name: org/project2 queue: integrated check: jobs: - - test-job + - check-job gate: jobs: - - test-job + - gate-job diff --git a/tests/unit/test_circular_dependencies.py b/tests/unit/test_circular_dependencies.py index f534b2596..a3f9dda33 100644 --- a/tests/unit/test_circular_dependencies.py +++ b/tests/unit/test_circular_dependencies.py @@ -2267,8 +2267,8 @@ class TestGerritCircularDependencies(ZuulTestCase): self.assertEqual(B.patchsets[-1]["approvals"][0]["value"], "1") self.assertHistory([ - dict(name="test-job", result="SUCCESS", changes="2,1 1,1"), - dict(name="test-job", result="SUCCESS", changes="1,1 2,1"), + dict(name="check-job", result="SUCCESS", changes="2,1 1,1"), + dict(name="check-job", result="SUCCESS", changes="1,1 2,1"), ], ordered=False) A.addPatchset() @@ -2277,10 +2277,10 @@ class TestGerritCircularDependencies(ZuulTestCase): self.assertHistory([ # Original check run - dict(name="test-job", result="SUCCESS", changes="2,1 1,1"), - dict(name="test-job", result="SUCCESS", changes="1,1 2,1"), + dict(name="check-job", result="SUCCESS", changes="2,1 1,1"), + dict(name="check-job", result="SUCCESS", changes="1,1 2,1"), # Second check run - dict(name="test-job", result="SUCCESS", changes="2,1 1,2"), + dict(name="check-job", result="SUCCESS", changes="2,1 1,2"), ], ordered=False) def test_deps_by_topic_multi_tenant(self): @@ -2368,16 +2368,85 @@ class TestGerritCircularDependencies(ZuulTestCase): self.executor_server.release() self.waitUntilSettled() - # A quirk: at the end of this process, the first change in - # Gerrit has a complete run because the process of updating it - # involves a new patchset that is enqueued. Compare to the - # same test in GitHub. self.assertHistory([ dict(name="project-job", result="ABORTED", changes="1,1"), dict(name="project-job", result="ABORTED", changes="1,1 2,1"), + dict(name="project-job", result="SUCCESS", changes="1,2 2,1"), dict(name="project-job", result="SUCCESS", changes="2,1 1,2"), ], ordered=False) + @simple_layout('layouts/deps-by-topic.yaml') + def test_dependency_refresh_by_topic_check(self): + # Test that when two changes are put into a cycle, the + # dependencies are refreshed and items already in pipelines + # are updated. + self.executor_server.hold_jobs_in_build = True + + # This simulates the typical workflow where a developer + # uploads changes one at a time. + # The first change: + A = self.fake_gerrit.addFakeChange('org/project1', "master", "A", + topic='test-topic') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + # Now that it has been uploaded, upload the second change + # in the same topic. + B = self.fake_gerrit.addFakeChange('org/project2', "master", "B", + topic='test-topic') + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertHistory([ + dict(name="check-job", result="ABORTED", changes="1,1"), + dict(name="check-job", result="SUCCESS", changes="2,1 1,1"), + dict(name="check-job", result="SUCCESS", changes="1,1 2,1"), + ], ordered=False) + + @simple_layout('layouts/deps-by-topic.yaml') + def test_dependency_refresh_by_topic_gate(self): + # Test that when two changes are put into a cycle, the + # dependencies are refreshed and items already in pipelines + # are updated. + self.executor_server.hold_jobs_in_build = True + + # This simulates a workflow where a developer adds a change to + # a cycle already in gate. + A = self.fake_gerrit.addFakeChange('org/project1', "master", "A", + topic='test-topic') + B = self.fake_gerrit.addFakeChange('org/project2', "master", "B", + topic='test-topic') + A.addApproval("Code-Review", 2) + B.addApproval("Code-Review", 2) + A.addApproval("Approved", 1) + self.fake_gerrit.addEvent(B.addApproval("Approved", 1)) + self.waitUntilSettled() + + # Add a new change to the cycle. + C = self.fake_gerrit.addFakeChange('org/project1', "master", "C", + topic='test-topic') + self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + # At the end of this process, the gate jobs should be aborted + # because the new dpendency showed up. + self.assertEqual(A.data["status"], "NEW") + self.assertEqual(B.data["status"], "NEW") + self.assertEqual(C.data["status"], "NEW") + self.assertHistory([ + dict(name="gate-job", result="ABORTED", changes="1,1 2,1"), + dict(name="gate-job", result="ABORTED", changes="1,1 2,1"), + dict(name="check-job", result="SUCCESS", changes="2,1 1,1 3,1"), + ], ordered=False) + class TestGithubCircularDependencies(ZuulTestCase): config_file = "zuul-gerrit-github.conf" @@ -2607,13 +2676,11 @@ class TestGithubCircularDependencies(ZuulTestCase): self.executor_server.release() self.waitUntilSettled() - # A quirk: at the end of this process, the second PR in GitHub - # has a complete run because the process of updating the first - # change is not disruptive to the second. Compare to the same - # test in Gerrit. self.assertHistory([ dict(name="project-job", result="ABORTED", changes=f"{A.number},{A.head_sha}"), dict(name="project-job", result="SUCCESS", changes=f"{A.number},{A.head_sha} {B.number},{B.head_sha}"), + dict(name="project-job", result="SUCCESS", + changes=f"{B.number},{B.head_sha} {A.number},{A.head_sha}"), ], ordered=False) diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index b51639952..f241147eb 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -27,10 +27,11 @@ import jwt import testtools from zuul.zk import ZooKeeperClient +from zuul.zk.locks import SessionAwareLock from zuul.cmd.client import parse_cutoff from tests.base import BaseTestCase, ZuulTestCase -from tests.base import FIXTURE_DIR +from tests.base import FIXTURE_DIR, iterate_timeout from kazoo.exceptions import NoNodeError @@ -362,81 +363,112 @@ class TestOnlineZKOperations(ZuulTestCase): def assertSQLState(self): pass - def test_delete_pipeline_check(self): - self.executor_server.hold_jobs_in_build = True - A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A') - self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) - self.waitUntilSettled() - - config_file = os.path.join(self.test_root, 'zuul.conf') - with open(config_file, 'w') as f: - self.config.write(f) - - # Make sure the pipeline exists - self.getZKTree('/zuul/tenant/tenant-one/pipeline/check/item') - p = subprocess.Popen( - [os.path.join(sys.prefix, 'bin/zuul-admin'), - '-c', config_file, - 'delete-pipeline-state', - 'tenant-one', 'check', - ], - stdout=subprocess.PIPE) - out, _ = p.communicate() - self.log.debug(out.decode('utf8')) - # Make sure it's deleted - with testtools.ExpectedException(NoNodeError): - self.getZKTree('/zuul/tenant/tenant-one/pipeline/check/item') - - self.executor_server.hold_jobs_in_build = False - self.executor_server.release() - B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B') - self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + def _test_delete_pipeline(self, pipeline): + sched = self.scheds.first.sched + tenant = sched.abide.tenants['tenant-one'] + # Force a reconfiguration due to a config change (so that the + # tenant trigger event queue gets a minimum timestamp set) + file_dict = {'zuul.yaml': ''} + M = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files=file_dict) + M.setMerged() + self.fake_gerrit.addEvent(M.getChangeMergedEvent()) self.waitUntilSettled() - self.assertHistory([ - dict(name='project-merge', result='SUCCESS', changes='1,1'), - dict(name='project-merge', result='SUCCESS', changes='2,1'), - dict(name='project-test1', result='SUCCESS', changes='2,1'), - dict(name='project-test2', result='SUCCESS', changes='2,1'), - ], ordered=False) - def test_delete_pipeline_gate(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)) + if pipeline == 'check': + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + else: + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) self.waitUntilSettled() - config_file = os.path.join(self.test_root, 'zuul.conf') - with open(config_file, 'w') as f: - self.config.write(f) - - # Make sure the pipeline exists - self.getZKTree('/zuul/tenant/tenant-one/pipeline/gate/item') - p = subprocess.Popen( - [os.path.join(sys.prefix, 'bin/zuul-admin'), - '-c', config_file, - 'delete-pipeline-state', - 'tenant-one', 'gate', - ], - stdout=subprocess.PIPE) - out, _ = p.communicate() - self.log.debug(out.decode('utf8')) - # Make sure it's deleted - with testtools.ExpectedException(NoNodeError): - self.getZKTree('/zuul/tenant/tenant-one/pipeline/gate/item') + # Lock the check pipeline so we don't process the event we're + # about to submit (it should go into the pipeline trigger event + # queue and stay there while we delete the pipeline state). + # This way we verify that events arrived before the deletion + # still work. + plock = SessionAwareLock( + self.zk_client.client, + f"/zuul/locks/pipeline/{tenant.name}/{pipeline}") + plock.acquire(blocking=True, timeout=None) + try: + self.log.debug('Got pipeline lock') + # Add a new event while our old last reconfigure time is + # in place. + B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B') + if pipeline == 'check': + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + else: + B.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(B.addApproval('Approved', 1)) + + # Wait until it appears in the pipeline trigger event queue + self.log.debug('Waiting for event') + for x in iterate_timeout(30, 'trigger event queue has events'): + if sched.pipeline_trigger_events[ + tenant.name][pipeline].hasEvents(): + break + self.log.debug('Got event') + except Exception: + plock.release() + raise + # Grab the run handler lock so that we will continue to avoid + # further processing of the event after we release the + # pipeline lock (which the delete command needs to acquire). + sched.run_handler_lock.acquire() + try: + plock.release() + self.log.debug('Got run lock') + config_file = os.path.join(self.test_root, 'zuul.conf') + with open(config_file, 'w') as f: + self.config.write(f) + + # Make sure the pipeline exists + self.getZKTree( + f'/zuul/tenant/{tenant.name}/pipeline/{pipeline}/item') + # Save the old layout uuid + tenant = sched.abide.tenants[tenant.name] + old_layout_uuid = tenant.layout.uuid + self.log.debug('Deleting pipeline state') + p = subprocess.Popen( + [os.path.join(sys.prefix, 'bin/zuul-admin'), + '-c', config_file, + 'delete-pipeline-state', + tenant.name, pipeline, + ], + stdout=subprocess.PIPE) + # Delete the pipeline state + out, _ = p.communicate() + self.log.debug(out.decode('utf8')) + # Make sure it's deleted + with testtools.ExpectedException(NoNodeError): + self.getZKTree( + f'/zuul/tenant/{tenant.name}/pipeline/{pipeline}/item') + finally: + sched.run_handler_lock.release() self.executor_server.hold_jobs_in_build = False self.executor_server.release() - B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B') - B.addApproval('Code-Review', 2) - self.fake_gerrit.addEvent(B.addApproval('Approved', 1)) self.waitUntilSettled() self.assertHistory([ - dict(name='project-merge', result='SUCCESS', changes='1,1'), dict(name='project-merge', result='SUCCESS', changes='2,1'), - dict(name='project-test1', result='SUCCESS', changes='2,1'), - dict(name='project-test2', result='SUCCESS', changes='2,1'), + dict(name='project-merge', result='SUCCESS', changes='3,1'), + dict(name='project-test1', result='SUCCESS', changes='3,1'), + dict(name='project-test2', result='SUCCESS', changes='3,1'), ], ordered=False) + tenant = sched.abide.tenants[tenant.name] + new_layout_uuid = tenant.layout.uuid + self.assertEqual(old_layout_uuid, new_layout_uuid) + self.assertEqual(tenant.layout.pipelines[pipeline].state.layout_uuid, + old_layout_uuid) + + def test_delete_pipeline_check(self): + self._test_delete_pipeline('check') + + def test_delete_pipeline_gate(self): + self._test_delete_pipeline('gate') class TestDBPruneParse(BaseTestCase): diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py index fb46aa7d1..3060f5673 100644 --- a/tests/unit/test_github_driver.py +++ b/tests/unit/test_github_driver.py @@ -1430,7 +1430,9 @@ class TestGithubDriver(ZuulTestCase): repo._set_branch_protection( 'master', contexts=['tenant-one/check', 'tenant-one/gate']) - A = self.fake_github.openFakePullRequest('org/project', 'master', 'A') + pr_description = "PR description" + A = self.fake_github.openFakePullRequest('org/project', 'master', 'A', + body_text=pr_description) self.fake_github.emitEvent(A.getPullRequestOpenedEvent()) self.waitUntilSettled() @@ -1448,6 +1450,9 @@ class TestGithubDriver(ZuulTestCase): merges = [report for report in self.fake_github.github_data.reports if report[2] == 'merge'] assert (len(merges) == 1 and merges[0][3] == 'squash') + # Assert that we won't duplicate the PR title in the merge + # message description. + self.assertEqual(A.merge_message, pr_description) @simple_layout('layouts/basic-github.yaml', driver='github') def test_invalid_event(self): @@ -1741,6 +1746,41 @@ class TestGithubUnprotectedBranches(ZuulTestCase): # branch self.assertLess(old, new) + def test_base_branch_updated(self): + self.create_branch('org/project2', 'feature') + github = self.fake_github.getGithubClient() + repo = github.repo_from_project('org/project2') + repo._set_branch_protection('master', True) + + # Make sure Zuul picked up and cached the configured branches + self.scheds.execute(lambda app: app.sched.reconfigure(app.config)) + self.waitUntilSettled() + + github_connection = self.scheds.first.connections.connections['github'] + tenant = self.scheds.first.sched.abide.tenants.get('tenant-one') + project = github_connection.source.getProject('org/project2') + + # Verify that only the master branch is considered protected + branches = github_connection.getProjectBranches(project, tenant) + self.assertEqual(branches, ["master"]) + + A = self.fake_github.openFakePullRequest('org/project2', 'master', + 'A') + # Fake an event from a pull-request that changed the base + # branch from "feature" to "master". The PR is already + # using "master" as base, but the event still references + # the old "feature" branch. + event = A.getPullRequestOpenedEvent() + event[1]["pull_request"]["base"]["ref"] = "feature" + + self.fake_github.emitEvent(event) + self.waitUntilSettled() + + # Make sure we are still only considering "master" to be + # protected. + branches = github_connection.getProjectBranches(project, tenant) + self.assertEqual(branches, ["master"]) + # This test verifies that a PR is considered in case it was created for # a branch just has been set to protected before a tenant reconfiguration # took place. diff --git a/tests/unit/test_gitlab_driver.py b/tests/unit/test_gitlab_driver.py index 6c4d4eeb9..e04a3b5d6 100644 --- a/tests/unit/test_gitlab_driver.py +++ b/tests/unit/test_gitlab_driver.py @@ -126,6 +126,27 @@ class TestGitlabDriver(ZuulTestCase): self.assertTrue(A.approved) @simple_layout('layouts/basic-gitlab.yaml', driver='gitlab') + def test_merge_request_opened_imcomplete(self): + + now = time.monotonic() + complete_at = now + 3 + with self.fake_gitlab.enable_delayed_complete_mr(complete_at): + description = "This is the\nMR description." + A = self.fake_gitlab.openFakeMergeRequest( + 'org/project', 'master', 'A', description=description) + self.fake_gitlab.emitEvent( + A.getMergeRequestOpenedEvent(), project='org/project') + self.waitUntilSettled() + + self.assertEqual('SUCCESS', + self.getJobFromHistory('project-test1').result) + + self.assertEqual('SUCCESS', + self.getJobFromHistory('project-test2').result) + + self.assertTrue(self.fake_gitlab._test_web_server.stats["get_mr"] > 2) + + @simple_layout('layouts/basic-gitlab.yaml', driver='gitlab') def test_merge_request_updated(self): A = self.fake_gitlab.openFakeMergeRequest('org/project', 'master', 'A') @@ -407,7 +428,7 @@ class TestGitlabDriver(ZuulTestCase): @simple_layout('layouts/basic-gitlab.yaml', driver='gitlab') def test_pull_request_with_dyn_reconf(self): - + path = os.path.join(self.upstream_root, 'org/project') zuul_yaml = [ {'job': { 'name': 'project-test3', @@ -424,11 +445,13 @@ class TestGitlabDriver(ZuulTestCase): playbook = "- hosts: all\n tasks: []" A = self.fake_gitlab.openFakeMergeRequest( - 'org/project', 'master', 'A') + 'org/project', 'master', 'A', + base_sha=git.Repo(path).head.object.hexsha) A.addCommit( {'.zuul.yaml': yaml.dump(zuul_yaml), 'job.yaml': playbook} ) + A.addCommit({"dummy.file": ""}) self.fake_gitlab.emitEvent(A.getMergeRequestOpenedEvent()) self.waitUntilSettled() @@ -440,6 +463,40 @@ class TestGitlabDriver(ZuulTestCase): self.getJobFromHistory('project-test3').result) @simple_layout('layouts/basic-gitlab.yaml', driver='gitlab') + def test_pull_request_with_dyn_reconf_alt(self): + with self.fake_gitlab.enable_uncomplete_mr(): + zuul_yaml = [ + {'job': { + 'name': 'project-test3', + 'run': 'job.yaml' + }}, + {'project': { + 'check': { + 'jobs': [ + 'project-test3' + ] + } + }} + ] + playbook = "- hosts: all\n tasks: []" + A = self.fake_gitlab.openFakeMergeRequest( + 'org/project', 'master', 'A') + A.addCommit( + {'.zuul.yaml': yaml.dump(zuul_yaml), + 'job.yaml': playbook} + ) + A.addCommit({"dummy.file": ""}) + self.fake_gitlab.emitEvent(A.getMergeRequestOpenedEvent()) + self.waitUntilSettled() + + self.assertEqual('SUCCESS', + self.getJobFromHistory('project-test1').result) + self.assertEqual('SUCCESS', + self.getJobFromHistory('project-test2').result) + self.assertEqual('SUCCESS', + self.getJobFromHistory('project-test3').result) + + @simple_layout('layouts/basic-gitlab.yaml', driver='gitlab') def test_ref_updated_and_tenant_reconfigure(self): self.waitUntilSettled() diff --git a/tests/unit/test_model_upgrade.py b/tests/unit/test_model_upgrade.py index a5a49bed4..c6cdee7ea 100644 --- a/tests/unit/test_model_upgrade.py +++ b/tests/unit/test_model_upgrade.py @@ -293,6 +293,33 @@ class TestModelUpgrade(ZuulTestCase): result='SUCCESS', changes='1,1'), ], ordered=False) + @model_version(12) + def test_model_12_13(self): + # Initially queue items will still have the full trigger event + # stored in Zookeeper. The trigger event will be converted to + # an event info object after the model API update. + self.executor_server.hold_jobs_in_build = True + A = self.fake_gerrit.addFakeChange('org/project1', 'master', 'A') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertEqual(len(self.builds), 1) + + # Upgrade our component + self.model_test_component_info.model_api = 13 + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertHistory([ + dict(name='project-merge', result='SUCCESS', changes='1,1'), + dict(name='project-test1', result='SUCCESS', changes='1,1'), + dict(name='project-test2', result='SUCCESS', changes='1,1'), + dict(name='project1-project2-integration', + result='SUCCESS', changes='1,1'), + ], ordered=False) + class TestGithubModelUpgrade(ZuulTestCase): config_file = 'zuul-github-driver.conf' diff --git a/tests/unit/test_scheduler.py b/tests/unit/test_scheduler.py index 172ed34dc..131034f17 100644 --- a/tests/unit/test_scheduler.py +++ b/tests/unit/test_scheduler.py @@ -7440,6 +7440,85 @@ class TestSchedulerMerges(ZuulTestCase): result = self._test_project_merge_mode('cherry-pick') self.assertEqual(result, expected_messages) + def test_project_merge_mode_cherrypick_redundant(self): + # A redundant commit (that is, one that has already been applied to the + # working tree) should be skipped + self.executor_server.keep_jobdir = False + project = 'org/project-cherry-pick' + files = { + "foo.txt": "ABC", + } + A = self.fake_gerrit.addFakeChange(project, 'master', 'A', files=files) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.executor_server.hold_jobs_in_build = True + B = self.fake_gerrit.addFakeChange(project, 'master', 'B', files=files) + B.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(B.addApproval('Approved', 1)) + self.waitUntilSettled() + + build = self.builds[-1] + path = os.path.join(build.jobdir.src_root, 'review.example.com', + project) + repo = git.Repo(path) + repo_messages = [c.message.strip() for c in repo.iter_commits()] + repo_messages.reverse() + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + expected_messages = [ + 'initial commit', + 'add content from fixture', + 'A-1', + ] + self.assertHistory([ + dict(name='project-test1', result='SUCCESS', changes='1,1'), + dict(name='project-test1', result='SUCCESS', changes='2,1'), + ]) + self.assertEqual(A.data['status'], 'MERGED') + self.assertEqual(B.data['status'], 'MERGED') + self.assertEqual(repo_messages, expected_messages) + + def test_project_merge_mode_cherrypick_empty(self): + # An empty commit (that is, one that doesn't modify any files) should + # be preserved + self.executor_server.keep_jobdir = False + project = 'org/project-cherry-pick' + self.executor_server.hold_jobs_in_build = True + A = self.fake_gerrit.addFakeChange(project, 'master', 'A', empty=True) + A.addApproval('Code-Review', 2) + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + build = self.builds[-1] + path = os.path.join(build.jobdir.src_root, 'review.example.com', + project) + repo = git.Repo(path) + repo_messages = [c.message.strip() for c in repo.iter_commits()] + repo_messages.reverse() + + changed_files = list(repo.commit("HEAD").diff(repo.commit("HEAD~1"))) + self.assertEqual(changed_files, []) + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + expected_messages = [ + 'initial commit', + 'add content from fixture', + 'A-1', + ] + self.assertHistory([ + dict(name='project-test1', result='SUCCESS', changes='1,1'), + ]) + self.assertEqual(A.data['status'], 'MERGED') + self.assertEqual(repo_messages, expected_messages) + def test_project_merge_mode_cherrypick_branch_merge(self): "Test that branches can be merged together in cherry-pick mode" self.create_branch('org/project-merge-branches', 'mp') diff --git a/tests/unit/test_v3.py b/tests/unit/test_v3.py index 004ede862..22d9518a9 100644 --- a/tests/unit/test_v3.py +++ b/tests/unit/test_v3.py @@ -3367,6 +3367,42 @@ class TestExtraConfigInDependent(ZuulTestCase): changes='2,1 1,1'), ], ordered=False) + def test_extra_config_in_bundle_change(self): + # Test that jobs defined in a extra-config-paths in a repo should be + # loaded in a bundle with changes from different repos. + + # Add an empty zuul.yaml here so we are triggering dynamic layout load + A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A', + files={'zuul.yaml': ''}) + B = self.fake_gerrit.addFakeChange('org/project2', 'master', 'B', + files={'zuul.yaml': ''}) + C = self.fake_gerrit.addFakeChange('org/project3', 'master', 'C', + files={'zuul.yaml': ''}) + # A B form a bundle, and A depends on C + A.data['commitMessage'] = '%s\n\nDepends-On: %s\nDepends-On: %s\n' % ( + A.subject, B.data['url'], C.data['url']) + B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + B.subject, A.data['url']) + + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + # Jobs in both changes should be success + self.assertHistory([ + dict(name='project2-private-extra-file', result='SUCCESS', + changes='3,1 1,1 2,1'), + dict(name='project2-private-extra-dir', result='SUCCESS', + changes='3,1 1,1 2,1'), + dict(name='project-test1', result='SUCCESS', + changes='3,1 2,1 1,1'), + dict(name='project3-private-extra-file', result='SUCCESS', + changes='3,1'), + dict(name='project3-private-extra-dir', result='SUCCESS', + changes='3,1'), + ], ordered=False) + class TestGlobalRepoState(AnsibleZuulTestCase): config_file = 'zuul-connections-gerrit-and-github.conf' diff --git a/web/src/containers/FilterToolbar.jsx b/web/src/containers/FilterToolbar.jsx index 328d60d85..bcf74c062 100644 --- a/web/src/containers/FilterToolbar.jsx +++ b/web/src/containers/FilterToolbar.jsx @@ -13,6 +13,7 @@ // under the License. import React, { useState } from 'react' +import { useDispatch } from 'react-redux' import PropTypes from 'prop-types' import { Button, @@ -32,12 +33,14 @@ import { } from '@patternfly/react-core' import { FilterIcon, SearchIcon } from '@patternfly/react-icons' +import { addNotification } from '../actions/notifications' import { FilterSelect } from './filters/Select' import { FilterTernarySelect } from './filters/TernarySelect' import { FilterCheckbox } from './filters/Checkbox' function FilterToolbar(props) { + const dispatch = useDispatch() const [isCategoryDropdownOpen, setIsCategoryDropdownOpen] = useState(false) const [currentCategory, setCurrentCategory] = useState( props.filterCategories[0].title @@ -58,15 +61,22 @@ function FilterToolbar(props) { } function handleInputSend(event, category) { - const { onFilterChange, filters } = props + const { onFilterChange, filters, filterInputValidation } = props // In case the event comes from a key press, only accept "Enter" if (event.key && event.key !== 'Enter') { return } - // Ignore empty values - if (!inputValue) { + const validationResult = filterInputValidation(category.key, inputValue) + if (!validationResult.success) { + dispatch(addNotification( + { + text: validationResult.message, + type: 'error', + status: '', + url: '', + })) return } @@ -250,6 +260,7 @@ FilterToolbar.propTypes = { onFilterChange: PropTypes.func.isRequired, filters: PropTypes.object.isRequired, filterCategories: PropTypes.array.isRequired, + filterInputValidation: PropTypes.func.isRequired, } function getChipsFromFilters(filters, category) { diff --git a/web/src/pages/Builds.jsx b/web/src/pages/Builds.jsx index b89bcd678..f1c449ec0 100644 --- a/web/src/pages/Builds.jsx +++ b/web/src/pages/Builds.jsx @@ -195,6 +195,28 @@ class BuildsPage extends React.Component { this.updateData(filters) } } + + filterInputValidation = (filterKey, filterValue) => { + // Input value should not be empty for all cases + if (!filterValue) { + return { + success: false, + message: 'Input should not be empty' + } + } + + // For change filter, it must be an integer + if (filterKey === 'change' && isNaN(filterValue)) { + return { + success: false, + message: 'Change must be an integer (do not include revision)' + } + } + + return { + success: true + } + } handleFilterChange = (newFilters) => { const { location, history } = this.props @@ -261,6 +283,7 @@ class BuildsPage extends React.Component { filterCategories={this.filterCategories} onFilterChange={this.handleFilterChange} filters={filters} + filterInputValidation={this.filterInputValidation} /> <Pagination toggleTemplate={({ firstIndex, lastIndex, itemCount }) => ( diff --git a/web/src/pages/Buildsets.jsx b/web/src/pages/Buildsets.jsx index 98d86d640..938309034 100644 --- a/web/src/pages/Buildsets.jsx +++ b/web/src/pages/Buildsets.jsx @@ -148,6 +148,28 @@ class BuildsetsPage extends React.Component { } } + filterInputValidation = (filterKey, filterValue) => { + // Input value should not be empty for all cases + if (!filterValue) { + return { + success: false, + message: 'Input should not be empty' + } + } + + // For change filter, it must be an integer + if (filterKey === 'change' && isNaN(filterValue)) { + return { + success: false, + message: 'Change must be an integer (do not include revision)' + } + } + + return { + success: true + } + } + handleFilterChange = (newFilters) => { const { location, history } = this.props const { filters, itemCount } = this.state @@ -213,6 +235,7 @@ class BuildsetsPage extends React.Component { filterCategories={this.filterCategories} onFilterChange={this.handleFilterChange} filters={filters} + filterInputValidation={this.filterInputValidation} /> <Pagination toggleTemplate={({ firstIndex, lastIndex, itemCount }) => ( diff --git a/zuul/cmd/client.py b/zuul/cmd/client.py index 031b10a1e..62e51ac3f 100755 --- a/zuul/cmd/client.py +++ b/zuul/cmd/client.py @@ -30,16 +30,14 @@ import time import textwrap import requests import urllib.parse -from uuid import uuid4 import zuul.cmd from zuul.lib.config import get_default -from zuul.model import SystemAttributes, PipelineState +from zuul.model import SystemAttributes, PipelineState, PipelineChangeList from zuul.zk import ZooKeeperClient from zuul.lib.keystorage import KeyStorage -from zuul.zk.locks import tenant_write_lock +from zuul.zk.locks import tenant_read_lock, pipeline_lock from zuul.zk.zkobject import ZKContext -from zuul.zk.layout import LayoutState, LayoutStateStore from zuul.zk.components import COMPONENT_REGISTRY @@ -1029,28 +1027,18 @@ class Client(zuul.cmd.ZuulApp): safe_tenant = urllib.parse.quote_plus(args.tenant) safe_pipeline = urllib.parse.quote_plus(args.pipeline) COMPONENT_REGISTRY.create(zk_client) - with tenant_write_lock(zk_client, args.tenant) as lock: + self.log.info('get tenant') + with tenant_read_lock(zk_client, args.tenant): path = f'/zuul/tenant/{safe_tenant}/pipeline/{safe_pipeline}' - layout_uuid = None - zk_client.client.delete( - f'/zuul/tenant/{safe_tenant}/pipeline/{safe_pipeline}', - recursive=True) - with ZKContext(zk_client, lock, None, self.log) as context: - ps = PipelineState.new(context, _path=path, - layout_uuid=layout_uuid) - # Force everyone to make a new layout for this tenant in - # order to rebuild the shared change queues. - layout_state = LayoutState( - tenant_name=args.tenant, - hostname='admin command', - last_reconfigured=int(time.time()), - last_reconfigure_event_ltime=-1, - uuid=uuid4().hex, - branch_cache_min_ltimes={}, - ltime=ps._zstat.last_modified_transaction_id, - ) - tenant_layout_state = LayoutStateStore(zk_client, lambda: None) - tenant_layout_state[args.tenant] = layout_state + self.log.info('get pipe') + with pipeline_lock( + zk_client, args.tenant, args.pipeline + ) as plock: + self.log.info('got locks') + zk_client.client.delete(path, recursive=True) + with ZKContext(zk_client, plock, None, self.log) as context: + PipelineState.new(context, _path=path, layout_uuid=None) + PipelineChangeList.new(context) sys.exit(0) diff --git a/zuul/configloader.py b/zuul/configloader.py index fe22fe0f8..4f472cb4e 100644 --- a/zuul/configloader.py +++ b/zuul/configloader.py @@ -2151,21 +2151,26 @@ class TenantParser(object): for future in futures: future.result() - try: - self._processCatJobs(abide, tenant, loading_errors, jobs, - min_ltimes) - except Exception: - self.log.exception("Error processing cat jobs, canceling") - for job in jobs: + for i, job in enumerate(jobs, start=1): + try: try: - self.log.debug("Canceling cat job %s", job) + self._processCatJob(abide, tenant, loading_errors, job, + min_ltimes) + except TimeoutError: self.merger.cancel(job) - except Exception: - self.log.exception("Unable to cancel job %s", job) - if not ignore_cat_exception: - raise - if not ignore_cat_exception: - raise + raise + except Exception: + self.log.exception("Error processing cat job") + if not ignore_cat_exception: + # Cancel remaining jobs + for cancel_job in jobs[i:]: + self.log.debug("Canceling cat job %s", cancel_job) + try: + self.merger.cancel(cancel_job) + except Exception: + self.log.exception( + "Unable to cancel job %s", cancel_job) + raise def _cacheTenantYAMLBranch(self, abide, tenant, loading_errors, min_ltimes, tpc, project, branch, jobs): @@ -2234,49 +2239,48 @@ class TenantParser(object): job.source_context = source_context jobs.append(job) - def _processCatJobs(self, abide, tenant, loading_errors, jobs, min_ltimes): + def _processCatJob(self, abide, tenant, loading_errors, job, min_ltimes): # Called at the end of _cacheTenantYAML after all cat jobs # have been submitted - for job in jobs: - self.log.debug("Waiting for cat job %s" % (job,)) - res = job.wait(self.merger.git_timeout) - if not res: - # We timed out - raise Exception("Cat job %s timed out; consider setting " - "merger.git_timeout in zuul.conf" % (job,)) - if not job.updated: - raise Exception("Cat job %s failed" % (job,)) - self.log.debug("Cat job %s got files %s" % - (job, job.files.keys())) - - self._updateUnparsedBranchCache(abide, tenant, job.source_context, - job.files, loading_errors, - job.ltime, min_ltimes) - - # Save all config files in Zookeeper (not just for the current tpc) - files_cache = self.unparsed_config_cache.getFilesCache( - job.source_context.project_canonical_name, - job.source_context.branch) - with self.unparsed_config_cache.writeLock( - job.source_context.project_canonical_name): - # Prevent files cache ltime from going backward - if files_cache.ltime >= job.ltime: - self.log.info( - "Discarding job %s result since the files cache was " - "updated in the meantime", job) - continue - # Since the cat job returns all required config files - # for ALL tenants the project is a part of, we can - # clear the whole cache and then populate it with the - # updated content. - files_cache.clear() - for fn, content in job.files.items(): - # Cache file in Zookeeper - if content is not None: - files_cache[fn] = content - files_cache.setValidFor(job.extra_config_files, - job.extra_config_dirs, - job.ltime) + self.log.debug("Waiting for cat job %s" % (job,)) + res = job.wait(self.merger.git_timeout) + if not res: + # We timed out + raise TimeoutError(f"Cat job {job} timed out; consider setting " + "merger.git_timeout in zuul.conf") + if not job.updated: + raise Exception("Cat job %s failed" % (job,)) + self.log.debug("Cat job %s got files %s" % + (job, job.files.keys())) + + self._updateUnparsedBranchCache(abide, tenant, job.source_context, + job.files, loading_errors, + job.ltime, min_ltimes) + + # Save all config files in Zookeeper (not just for the current tpc) + files_cache = self.unparsed_config_cache.getFilesCache( + job.source_context.project_canonical_name, + job.source_context.branch) + with self.unparsed_config_cache.writeLock( + job.source_context.project_canonical_name): + # Prevent files cache ltime from going backward + if files_cache.ltime >= job.ltime: + self.log.info( + "Discarding job %s result since the files cache was " + "updated in the meantime", job) + return + # Since the cat job returns all required config files + # for ALL tenants the project is a part of, we can + # clear the whole cache and then populate it with the + # updated content. + files_cache.clear() + for fn, content in job.files.items(): + # Cache file in Zookeeper + if content is not None: + files_cache[fn] = content + files_cache.setValidFor(job.extra_config_files, + job.extra_config_dirs, + job.ltime) def _updateUnparsedBranchCache(self, abide, tenant, source_context, files, loading_errors, ltime, min_ltimes): diff --git a/zuul/driver/elasticsearch/reporter.py b/zuul/driver/elasticsearch/reporter.py index 7802cb609..e5e90e052 100644 --- a/zuul/driver/elasticsearch/reporter.py +++ b/zuul/driver/elasticsearch/reporter.py @@ -103,7 +103,9 @@ class ElasticsearchReporter(BaseReporter): build_doc['job_vars'] = job.variables if self.index_returned_vars: - build_doc['job_returned_vars'] = build.result_data + rdata = build.result_data.copy() + rdata.pop('zuul', None) + build_doc['job_returned_vars'] = rdata docs.append(build_doc) diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py index 276365e1d..f871671aa 100644 --- a/zuul/driver/gerrit/gerritconnection.py +++ b/zuul/driver/gerrit/gerritconnection.py @@ -161,6 +161,8 @@ class GerritEventConnector(threading.Thread): IGNORED_EVENTS = ( 'cache-eviction', # evict-cache plugin + 'fetch-ref-replicated', + 'fetch-ref-replication-scheduled', 'ref-replicated', 'ref-replication-scheduled', 'ref-replication-done' diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index 182c83bae..f5f81fcdb 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -81,6 +81,10 @@ ANNOTATION_LEVELS = { "warning": "warning", "error": "failure", } +# The maximum size for the 'message' field is 64 KB. Since it's unclear +# from the Github docs if the unit is KiB or KB we'll use KB to be on +# the safe side. +ANNOTATION_MAX_MESSAGE_SIZE = 64 * 1000 EventTuple = collections.namedtuple( "EventTuple", [ @@ -439,7 +443,11 @@ class GithubEventProcessor(object): # branch is now protected. if hasattr(event, "branch") and event.branch: protected = None - if change: + # Only use the `branch_protected` flag if the + # target branch of change and event are the same. + # The base branch could have changed in the + # meantime. + if change and change.branch == event.branch: # PR based events already have the information if the # target branch is protected so take the information # from there. @@ -2428,7 +2436,9 @@ class GithubConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): raw_annotation = { "path": fn, "annotation_level": annotation_level, - "message": comment["message"], + "message": comment["message"].encode( + "utf8")[:ANNOTATION_MAX_MESSAGE_SIZE].decode( + "utf8", "ignore"), "start_line": start_line, "end_line": end_line, "start_column": start_column, diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py index 1f44303bd..396516038 100644 --- a/zuul/driver/github/githubreporter.py +++ b/zuul/driver/github/githubreporter.py @@ -193,13 +193,13 @@ class GithubReporter(BaseReporter): self.log.warning('Merge mode %s not supported by Github', mode) raise MergeFailure('Merge mode %s not supported by Github' % mode) - merge_mode = self.merge_modes[merge_mode] project = item.change.project.name pr_number = item.change.number sha = item.change.patchset log.debug('Reporting change %s, params %s, merging via API', item.change, self.config) - message = self._formatMergeMessage(item.change) + message = self._formatMergeMessage(item.change, merge_mode) + merge_mode = self.merge_modes[merge_mode] for i in [1, 2]: try: @@ -319,10 +319,13 @@ class GithubReporter(BaseReporter): self.connection.unlabelPull(project, pr_number, label, zuul_event_id=item.event) - def _formatMergeMessage(self, change): + def _formatMergeMessage(self, change, merge_mode): message = [] - if change.title: - message.append(change.title) + # For squash merges we don't need to add the title to the body + # as it will already be set as the commit subject. + if merge_mode != model.MERGER_SQUASH_MERGE: + if change.title: + message.append(change.title) if change.body_text: message.append(change.body_text) merge_message = "\n\n".join(message) diff --git a/zuul/driver/gitlab/gitlabconnection.py b/zuul/driver/gitlab/gitlabconnection.py index 1515db8df..da423f085 100644 --- a/zuul/driver/gitlab/gitlabconnection.py +++ b/zuul/driver/gitlab/gitlabconnection.py @@ -281,6 +281,7 @@ class GitlabAPIClient(): self.api_token = api_token self.keepalive = keepalive self.disable_pool = disable_pool + self.get_mr_wait_factor = 2 self.headers = {'Authorization': 'Bearer %s' % ( self.api_token)} @@ -342,11 +343,36 @@ class GitlabAPIClient(): # https://docs.gitlab.com/ee/api/merge_requests.html#get-single-mr def get_mr(self, project_name, number, zuul_event_id=None): - path = "/projects/%s/merge_requests/%s" % ( - quote_plus(project_name), number) - resp = self.get(self.baseurl + path, zuul_event_id=zuul_event_id) - self._manage_error(*resp, zuul_event_id=zuul_event_id) - return resp[0] + log = get_annotated_logger(self.log, zuul_event_id) + attempts = 0 + + def _get_mr(): + path = "/projects/%s/merge_requests/%s" % ( + quote_plus(project_name), number) + resp = self.get(self.baseurl + path, zuul_event_id=zuul_event_id) + self._manage_error(*resp, zuul_event_id=zuul_event_id) + return resp[0] + + # The Gitlab API might not return a complete MR description as + # some attributes are updated asynchronously. This loop ensures + # we query the API until all async attributes are available or until + # a defined delay is reached. + while True: + attempts += 1 + mr = _get_mr() + # The diff_refs attribute is updated asynchronously + if all(map(lambda k: mr.get(k, None), ['diff_refs'])): + return mr + if attempts > 4: + log.warning( + "Fetched MR %s#%s with imcomplete data" % ( + project_name, number)) + return mr + wait_delay = attempts * self.get_mr_wait_factor + log.info( + "Will retry to fetch %s#%s due to imcomplete data " + "(in %s seconds) ..." % (project_name, number, wait_delay)) + time.sleep(wait_delay) # https://docs.gitlab.com/ee/api/branches.html#list-repository-branches def get_project_branches(self, project_name, exclude_unprotected, @@ -667,8 +693,12 @@ class GitlabConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection): change.ref = "refs/merge-requests/%s/head" % change.number change.branch = change.mr['target_branch'] change.is_current_patchset = (change.mr['sha'] == change.patchset) - change.base_sha = change.mr['diff_refs'].get('base_sha') - change.commit_id = change.mr['diff_refs'].get('head_sha') + change.commit_id = event.patch_number + diff_refs = change.mr.get("diff_refs", {}) + if diff_refs: + change.base_sha = diff_refs.get('base_sha') + else: + change.base_sha = None change.owner = change.mr['author'].get('username') # Files changes are not part of the Merge Request data # See api/merge_requests.html#get-single-mr-changes diff --git a/zuul/driver/mqtt/mqttconnection.py b/zuul/driver/mqtt/mqttconnection.py index 7f221282f..4a028ba23 100644 --- a/zuul/driver/mqtt/mqttconnection.py +++ b/zuul/driver/mqtt/mqttconnection.py @@ -64,6 +64,12 @@ class MQTTConnection(BaseConnection): def onLoad(self, zk_client, component_registry): self.log.debug("Starting MQTT Connection") + + # If the connection was not loaded by a scheduler, but by e.g. + # zuul-web, we want to stop here. + if not self.sched: + return + try: self.client.connect( self.connection_config.get('server', 'localhost'), @@ -76,10 +82,11 @@ class MQTTConnection(BaseConnection): self.client.loop_start() def onStop(self): - self.log.debug("Stopping MQTT Connection") - self.client.loop_stop() - self.client.disconnect() - self.connected = False + if self.connected: + self.log.debug("Stopping MQTT Connection") + self.client.loop_stop() + self.client.disconnect() + self.connected = False def publish(self, topic, message, qos, zuul_event_id): log = get_annotated_logger(self.log, zuul_event_id) diff --git a/zuul/executor/server.py b/zuul/executor/server.py index b0e3d69f6..6dbf62de0 100644 --- a/zuul/executor/server.py +++ b/zuul/executor/server.py @@ -1931,6 +1931,7 @@ class AnsibleJob(object): region=node.region, host_id=node.host_id, external_id=getattr(node, 'external_id', None), + slot=node.slot, interface_ip=node.interface_ip, public_ipv4=node.public_ipv4, private_ipv4=node.private_ipv4, diff --git a/zuul/manager/__init__.py b/zuul/manager/__init__.py index 36361df11..c3d082a47 100644 --- a/zuul/manager/__init__.py +++ b/zuul/manager/__init__.py @@ -243,7 +243,7 @@ class PipelineManager(metaclass=ABCMeta): and self.useDependenciesByTopic(change.project)) if (update_commit_dependencies or update_topic_dependencies): - self.updateCommitDependencies(change, None, event=None) + self.updateCommitDependencies(change, event=None) self._change_cache[change.cache_key] = change resolved_changes.append(change) return resolved_changes @@ -285,11 +285,18 @@ class PipelineManager(metaclass=ABCMeta): return True return False - def isAnyVersionOfChangeInPipeline(self, change): - # Checks any items in the pipeline + def isChangeRelevantToPipeline(self, change): + # Checks if any version of the change or its deps matches any + # item in the pipeline. for change_key in self.pipeline.change_list.getChangeKeys(): if change.cache_stat.key.isSameChange(change_key): return True + if isinstance(change, model.Change): + for dep_change_ref in change.getNeedsChanges( + self.useDependenciesByTopic(change.project)): + dep_change_key = ChangeKey.fromReference(dep_change_ref) + if change.cache_stat.key.isSameChange(dep_change_key): + return True return False def isChangeAlreadyInQueue(self, change, change_queue): @@ -315,7 +322,7 @@ class PipelineManager(metaclass=ABCMeta): to_refresh.add(item.change) for existing_change in to_refresh: - self.updateCommitDependencies(existing_change, None, event) + self.updateCommitDependencies(existing_change, event) def reportEnqueue(self, item): if not self.pipeline.state.disabled: @@ -516,7 +523,8 @@ class PipelineManager(metaclass=ABCMeta): def addChange(self, change, event, quiet=False, enqueue_time=None, ignore_requirements=False, live=True, - change_queue=None, history=None, dependency_graph=None): + change_queue=None, history=None, dependency_graph=None, + skip_presence_check=False): log = get_annotated_logger(self.log, event) log.debug("Considering adding change %s" % change) @@ -531,7 +539,9 @@ class PipelineManager(metaclass=ABCMeta): # If we are adding a live change, check if it's a live item # anywhere in the pipeline. Otherwise, we will perform the # duplicate check below on the specific change_queue. - if live and self.isChangeAlreadyInPipeline(change): + if (live and + self.isChangeAlreadyInPipeline(change) and + not skip_presence_check): log.debug("Change %s is already in pipeline, ignoring" % change) return True @@ -564,7 +574,7 @@ class PipelineManager(metaclass=ABCMeta): # to date and this is a noop; otherwise, we need to refresh # them anyway. if isinstance(change, model.Change): - self.updateCommitDependencies(change, None, event) + self.updateCommitDependencies(change, event) with self.getChangeQueue(change, event, change_queue) as change_queue: if not change_queue: @@ -590,8 +600,10 @@ class PipelineManager(metaclass=ABCMeta): log.debug("History after enqueuing changes ahead: %s", history) if self.isChangeAlreadyInQueue(change, change_queue): - log.debug("Change %s is already in queue, ignoring" % change) - return True + if not skip_presence_check: + log.debug("Change %s is already in queue, ignoring", + change) + return True cycle = [] if isinstance(change, model.Change): @@ -625,7 +637,7 @@ class PipelineManager(metaclass=ABCMeta): if enqueue_time: item.enqueue_time = enqueue_time item.live = live - self.reportStats(item, added=True) + self.reportStats(item, trigger_event=event) item.quiet = quiet if item.live: @@ -857,7 +869,7 @@ class PipelineManager(metaclass=ABCMeta): self.pipeline.tenant.name][other_pipeline.name].put( event, needs_result=False) - def updateCommitDependencies(self, change, change_queue, event): + def updateCommitDependencies(self, change, event): log = get_annotated_logger(self.log, event) must_update_commit_deps = ( @@ -1448,16 +1460,17 @@ class PipelineManager(metaclass=ABCMeta): item.bundle and item.bundle.updatesConfig(tenant) and tpc is not None ): - extra_config_files = set(tpc.extra_config_files) - extra_config_dirs = set(tpc.extra_config_dirs) - # Merge extra_config_files and extra_config_dirs of the - # dependent change - for item_ahead in item.items_ahead: - tpc_ahead = tenant.project_configs.get( - item_ahead.change.project.canonical_name) - if tpc_ahead: - extra_config_files.update(tpc_ahead.extra_config_files) - extra_config_dirs.update(tpc_ahead.extra_config_dirs) + # Collect extra config files and dirs of required changes. + extra_config_files = set() + extra_config_dirs = set() + for merger_item in item.current_build_set.merger_items: + source = self.sched.connections.getSource( + merger_item["connection"]) + project = source.getProject(merger_item["project"]) + tpc = tenant.project_configs.get(project.canonical_name) + if tpc: + extra_config_files.update(tpc.extra_config_files) + extra_config_dirs.update(tpc.extra_config_dirs) ready = self.scheduleMerge( item, @@ -1554,6 +1567,7 @@ class PipelineManager(metaclass=ABCMeta): log.info("Dequeuing change %s because " "it can no longer merge" % item.change) self.cancelJobs(item) + quiet_dequeue = False if item.isBundleFailing(): item.setDequeuedBundleFailing('Bundle is failing') elif not meets_reqs: @@ -1565,7 +1579,28 @@ class PipelineManager(metaclass=ABCMeta): else: msg = f'Change {clist} is needed.' item.setDequeuedNeedingChange(msg) - if item.live: + # If all the dependencies are already in the pipeline + # (but not ahead of this change), then we probably + # just added updated versions of them, possibly + # updating a cycle. In that case, attempt to + # re-enqueue this change with the updated deps. + if (item.live and + all([self.isChangeAlreadyInPipeline(c) + for c in needs_changes])): + # Try enqueue, if that succeeds, keep this dequeue quiet + try: + log.info("Attempting re-enqueue of change %s", + item.change) + quiet_dequeue = self.addChange( + item.change, item.event, + enqueue_time=item.enqueue_time, + quiet=True, + skip_presence_check=True) + except Exception: + log.exception("Unable to re-enqueue change %s " + "which is missing dependencies", + item.change) + if item.live and not quiet_dequeue: try: self.reportItem(item) except exceptions.MergeFailure: @@ -2197,7 +2232,7 @@ class PipelineManager(metaclass=ABCMeta): log.error("Reporting item %s received: %s", item, ret) return action, (not ret) - def reportStats(self, item, added=False): + def reportStats(self, item, trigger_event=None): if not self.sched.statsd: return try: @@ -2236,18 +2271,21 @@ class PipelineManager(metaclass=ABCMeta): if dt: self.sched.statsd.timing(key + '.resident_time', dt) self.sched.statsd.incr(key + '.total_changes') - if added and hasattr(item.event, 'arrived_at_scheduler_timestamp'): + if ( + trigger_event + and hasattr(trigger_event, 'arrived_at_scheduler_timestamp') + ): now = time.time() - arrived = item.event.arrived_at_scheduler_timestamp + arrived = trigger_event.arrived_at_scheduler_timestamp processing = (now - arrived) * 1000 - elapsed = (now - item.event.timestamp) * 1000 + elapsed = (now - trigger_event.timestamp) * 1000 self.sched.statsd.timing( basekey + '.event_enqueue_processing_time', processing) self.sched.statsd.timing( basekey + '.event_enqueue_time', elapsed) self.reportPipelineTiming('event_enqueue_time', - item.event.timestamp) + trigger_event.timestamp) except Exception: self.log.exception("Exception reporting pipeline stats") diff --git a/zuul/merger/merger.py b/zuul/merger/merger.py index e4688a1b7..1df833bc5 100644 --- a/zuul/merger/merger.py +++ b/zuul/merger/merger.py @@ -595,14 +595,32 @@ class Repo(object): log = get_annotated_logger(self.log, zuul_event_id) repo = self.createRepoObject(zuul_event_id) self.fetch(ref, zuul_event_id=zuul_event_id) - if len(repo.commit("FETCH_HEAD").parents) > 1: + fetch_head = repo.commit("FETCH_HEAD") + if len(fetch_head.parents) > 1: args = ["-s", "resolve", "FETCH_HEAD"] log.debug("Merging %s with args %s instead of cherry-picking", ref, args) repo.git.merge(*args) else: log.debug("Cherry-picking %s", ref) - repo.git.cherry_pick("FETCH_HEAD") + # Git doesn't have an option to ignore commits that are already + # applied to the working tree when cherry-picking, so pass the + # --keep-redundant-commits option, which will cause it to make an + # empty commit + repo.git.cherry_pick("FETCH_HEAD", keep_redundant_commits=True) + + # If the newly applied commit is empty, it means either: + # 1) The commit being cherry-picked was empty, in which the empty + # commit should be kept + # 2) The commit being cherry-picked was already applied to the + # tree, in which case the empty commit should be backed out + head = repo.commit("HEAD") + parent = head.parents[0] + if not any(head.diff(parent)) and \ + any(fetch_head.diff(fetch_head.parents[0])): + log.debug("%s was already applied. Removing it", ref) + self._checkout(repo, parent) + return repo.head.commit def merge(self, ref, strategy=None, zuul_event_id=None): diff --git a/zuul/model.py b/zuul/model.py index 1d82b5f2c..5be5923a5 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -1408,6 +1408,7 @@ class Node(ConfigObject): self.private_ipv6 = None self.connection_port = 22 self.connection_type = None + self.slot = None self._keys = [] self.az = None self.provider = None @@ -4666,6 +4667,37 @@ class BuildSet(zkobject.ZKObject): return Attributes(uuid=self.uuid) +class EventInfo: + + def __init__(self): + self.zuul_event_id = None + self.timestamp = time.time() + self.span_context = None + + @classmethod + def fromEvent(cls, event): + tinfo = cls() + tinfo.zuul_event_id = event.zuul_event_id + tinfo.timestamp = event.timestamp + tinfo.span_context = event.span_context + return tinfo + + @classmethod + def fromDict(cls, d): + tinfo = cls() + tinfo.zuul_event_id = d["zuul_event_id"] + tinfo.timestamp = d["timestamp"] + tinfo.span_context = d["span_context"] + return tinfo + + def toDict(self): + return { + "zuul_event_id": self.zuul_event_id, + "timestamp": self.timestamp, + "span_context": self.span_context, + } + + class QueueItem(zkobject.ZKObject): """Represents the position of a Change in a ChangeQueue. @@ -4700,7 +4732,7 @@ class QueueItem(zkobject.ZKObject): live=True, # Whether an item is intended to be processed at all layout_uuid=None, _cached_sql_results={}, - event=None, # The trigger event that lead to this queue item + event=None, # Info about the event that lead to this queue item # Additional container for connection specifig information to be # used by reporters throughout the lifecycle @@ -4722,6 +4754,9 @@ class QueueItem(zkobject.ZKObject): def new(klass, context, **kw): obj = klass() obj._set(**kw) + if COMPONENT_REGISTRY.model_api >= 13: + obj._set(event=obj.event and EventInfo.fromEvent(obj.event)) + data = obj._trySerialize(context) obj._save(context, data, create=True) files_state = (BuildSet.COMPLETE if obj.change.files is not None @@ -4750,10 +4785,18 @@ class QueueItem(zkobject.ZKObject): return (tenant, pipeline, uuid) def serialize(self, context): - if isinstance(self.event, TriggerEvent): - event_type = "TriggerEvent" + if COMPONENT_REGISTRY.model_api < 13: + if isinstance(self.event, TriggerEvent): + event_type = "TriggerEvent" + else: + event_type = self.event.__class__.__name__ else: - event_type = self.event.__class__.__name__ + event_type = "EventInfo" + if not isinstance(self.event, EventInfo): + # Convert our local trigger event to a trigger info + # object. This will only happen on the transition to + # model API version 13. + self._set(event=EventInfo.fromEvent(self.event)) data = { "uuid": self.uuid, @@ -4795,14 +4838,18 @@ class QueueItem(zkobject.ZKObject): # child objects. self._set(uuid=data["uuid"]) - event_type = data["event"]["type"] - if event_type == "TriggerEvent": - event_class = ( - self.pipeline.manager.sched.connections.getTriggerEventClass( - data["event"]["data"]["driver_name"]) - ) + if COMPONENT_REGISTRY.model_api < 13: + event_type = data["event"]["type"] + if event_type == "TriggerEvent": + event_class = ( + self.pipeline.manager.sched.connections + .getTriggerEventClass( + data["event"]["data"]["driver_name"]) + ) + else: + event_class = EventTypeIndex.event_type_mapping.get(event_type) else: - event_class = EventTypeIndex.event_type_mapping.get(event_type) + event_class = EventInfo if event_class is None: raise NotImplementedError( diff --git a/zuul/model_api.py b/zuul/model_api.py index ccb12077d..0244296dd 100644 --- a/zuul/model_api.py +++ b/zuul/model_api.py @@ -14,4 +14,4 @@ # When making ZK schema changes, increment this and add a record to # doc/source/developer/model-changelog.rst -MODEL_API = 12 +MODEL_API = 13 diff --git a/zuul/scheduler.py b/zuul/scheduler.py index e646c09b8..cd15a878c 100644 --- a/zuul/scheduler.py +++ b/zuul/scheduler.py @@ -2519,9 +2519,26 @@ class Scheduler(threading.Thread): event.span_context = tracing.getSpanContext(span) for pipeline in tenant.layout.pipelines.values(): + # For most kinds of dependencies, it's sufficient to check + # if this change is already in the pipeline, because the + # only way to update a dependency cycle is to update one + # of the changes in it. However, dependencies-by-topic + # can have changes added to the cycle without updating any + # of the existing changes in the cycle. That means in + # order to detect whether a new change is added to an + # existing cycle in the pipeline, we need to know all of + # the dependencies of the new change, and check if *they* + # are in the pipeline. Therefore, go ahead and update our + # dependencies here so they are available for comparison + # against the pipeline contents. This front-loads some + # work that otherwise would happen in the pipeline + # manager, but the result of the work goes into the change + # cache, so it's not wasted; it's just less parallelized. + if isinstance(change, Change): + pipeline.manager.updateCommitDependencies(change, event) if ( pipeline.manager.eventMatches(event, change) - or pipeline.manager.isAnyVersionOfChangeInPipeline(change) + or pipeline.manager.isChangeRelevantToPipeline(change) ): self.pipeline_trigger_events[tenant.name][ pipeline.name diff --git a/zuul/zk/job_request_queue.py b/zuul/zk/job_request_queue.py index 175c57b90..7c85ae95e 100644 --- a/zuul/zk/job_request_queue.py +++ b/zuul/zk/job_request_queue.py @@ -609,7 +609,7 @@ class JobRequestQueue(ZooKeeperSimpleBase): self.kazoo_client.delete(lock_path, recursive=True) except Exception: self.log.exception( - "Unable to delete lock %s", path) + "Unable to delete lock %s", lock_path) except Exception: self.log.exception("Error cleaning up locks %s", self) |