From 54145e0fd9813c417f57de602201eaf9da16f2a2 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Wed, 10 Jan 2018 16:07:41 -0800 Subject: [PATCH] Add cross-source tests Change-Id: Iaf31211d12a2c8ce3b4a2860e079748f7e705aba Story: 2001334 Task: 5885 --- tests/base.py | 1 + .../playbooks/nonvoting-project-merge.yaml | 2 + .../playbooks/nonvoting-project-test1.yaml | 2 + .../playbooks/nonvoting-project-test2.yaml | 2 + .../playbooks/project-merge.yaml | 2 + .../common-config/playbooks/project-post.yaml | 2 + .../playbooks/project-test1.yaml | 2 + .../playbooks/project-test2.yaml | 2 + .../playbooks/project-testfile.yaml | 2 + .../project1-project2-integration.yaml | 2 + .../cross-source/git/common-config/zuul.yaml | 168 ++++ .../cross-source/git/gerrit_project1/README | 1 + .../cross-source/git/github_project2/README | 1 + tests/fixtures/config/cross-source/main.yaml | 11 + tests/fixtures/zuul-gerrit-github.conf | 35 + tests/unit/test_cross_crd.py | 950 ++++++++++++++++++ zuul/driver/gerrit/gerritsource.py | 4 +- zuul/driver/github/githubconnection.py | 8 +- zuul/driver/github/githubsource.py | 2 + zuul/manager/dependent.py | 4 +- 20 files changed, 1198 insertions(+), 5 deletions(-) create mode 100644 tests/fixtures/config/cross-source/git/common-config/playbooks/nonvoting-project-merge.yaml create mode 100644 tests/fixtures/config/cross-source/git/common-config/playbooks/nonvoting-project-test1.yaml create mode 100644 tests/fixtures/config/cross-source/git/common-config/playbooks/nonvoting-project-test2.yaml create mode 100644 tests/fixtures/config/cross-source/git/common-config/playbooks/project-merge.yaml create mode 100644 tests/fixtures/config/cross-source/git/common-config/playbooks/project-post.yaml create mode 100644 tests/fixtures/config/cross-source/git/common-config/playbooks/project-test1.yaml create mode 100644 tests/fixtures/config/cross-source/git/common-config/playbooks/project-test2.yaml create mode 100644 tests/fixtures/config/cross-source/git/common-config/playbooks/project-testfile.yaml create mode 100644 tests/fixtures/config/cross-source/git/common-config/playbooks/project1-project2-integration.yaml create mode 100644 tests/fixtures/config/cross-source/git/common-config/zuul.yaml create mode 100644 tests/fixtures/config/cross-source/git/gerrit_project1/README create mode 100644 tests/fixtures/config/cross-source/git/github_project2/README create mode 100644 tests/fixtures/config/cross-source/main.yaml create mode 100644 tests/fixtures/zuul-gerrit-github.conf create mode 100644 tests/unit/test_cross_crd.py diff --git a/tests/base.py b/tests/base.py index e688abd023..64f66579c6 100755 --- a/tests/base.py +++ b/tests/base.py @@ -641,6 +641,7 @@ class FakeGithubPullRequest(object): self.is_merged = False self.merge_message = None self.state = 'open' + self.url = 'https://%s/%s/pull/%s' % (github.server, project, number) self._createPRRef() self._addCommitToRepo(files=files) self._updateTimeStamp() diff --git a/tests/fixtures/config/cross-source/git/common-config/playbooks/nonvoting-project-merge.yaml b/tests/fixtures/config/cross-source/git/common-config/playbooks/nonvoting-project-merge.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/cross-source/git/common-config/playbooks/nonvoting-project-merge.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/cross-source/git/common-config/playbooks/nonvoting-project-test1.yaml b/tests/fixtures/config/cross-source/git/common-config/playbooks/nonvoting-project-test1.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/cross-source/git/common-config/playbooks/nonvoting-project-test1.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/cross-source/git/common-config/playbooks/nonvoting-project-test2.yaml b/tests/fixtures/config/cross-source/git/common-config/playbooks/nonvoting-project-test2.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/cross-source/git/common-config/playbooks/nonvoting-project-test2.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/cross-source/git/common-config/playbooks/project-merge.yaml b/tests/fixtures/config/cross-source/git/common-config/playbooks/project-merge.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/cross-source/git/common-config/playbooks/project-merge.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/cross-source/git/common-config/playbooks/project-post.yaml b/tests/fixtures/config/cross-source/git/common-config/playbooks/project-post.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/cross-source/git/common-config/playbooks/project-post.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/cross-source/git/common-config/playbooks/project-test1.yaml b/tests/fixtures/config/cross-source/git/common-config/playbooks/project-test1.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/cross-source/git/common-config/playbooks/project-test1.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/cross-source/git/common-config/playbooks/project-test2.yaml b/tests/fixtures/config/cross-source/git/common-config/playbooks/project-test2.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/cross-source/git/common-config/playbooks/project-test2.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/cross-source/git/common-config/playbooks/project-testfile.yaml b/tests/fixtures/config/cross-source/git/common-config/playbooks/project-testfile.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/cross-source/git/common-config/playbooks/project-testfile.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/cross-source/git/common-config/playbooks/project1-project2-integration.yaml b/tests/fixtures/config/cross-source/git/common-config/playbooks/project1-project2-integration.yaml new file mode 100644 index 0000000000..f679dceaef --- /dev/null +++ b/tests/fixtures/config/cross-source/git/common-config/playbooks/project1-project2-integration.yaml @@ -0,0 +1,2 @@ +- hosts: all + tasks: [] diff --git a/tests/fixtures/config/cross-source/git/common-config/zuul.yaml b/tests/fixtures/config/cross-source/git/common-config/zuul.yaml new file mode 100644 index 0000000000..abdc34afa6 --- /dev/null +++ b/tests/fixtures/config/cross-source/git/common-config/zuul.yaml @@ -0,0 +1,168 @@ +- pipeline: + name: check + manager: independent + trigger: + gerrit: + - event: patchset-created + github: + - event: pull_request + action: edited + success: + gerrit: + Verified: 1 + github: {} + failure: + gerrit: + Verified: -1 + github: {} + +- pipeline: + name: gate + manager: dependent + success-message: Build succeeded (gate). + require: + github: + label: approved + gerrit: + approval: + - Approved: 1 + trigger: + gerrit: + - event: comment-added + approval: + - Approved: 1 + github: + - event: pull_request + action: edited + - event: pull_request + action: labeled + label: approved + success: + gerrit: + Verified: 2 + submit: true + github: + merge: true + failure: + gerrit: + Verified: -2 + github: {} + start: + gerrit: + Verified: 0 + github: {} + precedence: high + +- pipeline: + name: post + manager: independent + trigger: + gerrit: + - event: ref-updated + ref: ^(?!refs/).*$ + precedence: low + +- job: + name: base + parent: null + +- job: + name: project-merge + hold-following-changes: true + nodeset: + nodes: + - name: controller + label: label1 + run: playbooks/project-merge.yaml + +- job: + name: project-test1 + attempts: 4 + nodeset: + nodes: + - name: controller + label: label1 + run: playbooks/project-test1.yaml + +- job: + name: project-test1 + branches: stable + nodeset: + nodes: + - name: controller + label: label2 + run: playbooks/project-test1.yaml + +- job: + name: project-post + nodeset: + nodes: + - name: static + label: ubuntu-xenial + run: playbooks/project-post.yaml + +- job: + name: project-test2 + nodeset: + nodes: + - name: controller + label: label1 + run: playbooks/project-test2.yaml + +- job: + name: project1-project2-integration + nodeset: + nodes: + - name: controller + label: label1 + run: playbooks/project1-project2-integration.yaml + +- job: + name: project-testfile + files: + - .*-requires + run: playbooks/project-testfile.yaml + +- project: + name: gerrit/project1 + check: + jobs: + - project-merge + - project-test1: + dependencies: project-merge + - project-test2: + dependencies: project-merge + - project1-project2-integration: + dependencies: project-merge + gate: + queue: integrated + jobs: + - project-merge + - project-test1: + dependencies: project-merge + - project-test2: + dependencies: project-merge + - project1-project2-integration: + dependencies: project-merge + +- project: + name: github/project2 + check: + jobs: + - project-merge + - project-test1: + dependencies: project-merge + - project-test2: + dependencies: project-merge + - project1-project2-integration: + dependencies: project-merge + gate: + queue: integrated + jobs: + - project-merge + - project-test1: + dependencies: project-merge + - project-test2: + dependencies: project-merge + - project1-project2-integration: + dependencies: project-merge diff --git a/tests/fixtures/config/cross-source/git/gerrit_project1/README b/tests/fixtures/config/cross-source/git/gerrit_project1/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/cross-source/git/gerrit_project1/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/cross-source/git/github_project2/README b/tests/fixtures/config/cross-source/git/github_project2/README new file mode 100644 index 0000000000..9daeafb986 --- /dev/null +++ b/tests/fixtures/config/cross-source/git/github_project2/README @@ -0,0 +1 @@ +test diff --git a/tests/fixtures/config/cross-source/main.yaml b/tests/fixtures/config/cross-source/main.yaml new file mode 100644 index 0000000000..bf85c33b25 --- /dev/null +++ b/tests/fixtures/config/cross-source/main.yaml @@ -0,0 +1,11 @@ +- tenant: + name: tenant-one + source: + gerrit: + config-projects: + - common-config + untrusted-projects: + - gerrit/project1 + github: + untrusted-projects: + - github/project2 diff --git a/tests/fixtures/zuul-gerrit-github.conf b/tests/fixtures/zuul-gerrit-github.conf new file mode 100644 index 0000000000..d3cbf7b259 --- /dev/null +++ b/tests/fixtures/zuul-gerrit-github.conf @@ -0,0 +1,35 @@ +[gearman] +server=127.0.0.1 + +[statsd] +# note, use 127.0.0.1 rather than localhost to avoid getting ipv6 +# see: https://github.com/jsocol/pystatsd/issues/61 +server=127.0.0.1 + +[scheduler] +tenant_config=main.yaml + +[merger] +git_dir=/tmp/zuul-test/merger-git +git_user_email=zuul@example.com +git_user_name=zuul + +[executor] +git_dir=/tmp/zuul-test/executor-git + +[connection gerrit] +driver=gerrit +server=review.example.com +user=jenkins +sshkey=fake_id_rsa_path + +[connection github] +driver=github +webhook_token=0000000000000000000000000000000000000000 + +[connection smtp] +driver=smtp +server=localhost +port=25 +default_from=zuul@example.com +default_to=you@example.com diff --git a/tests/unit/test_cross_crd.py b/tests/unit/test_cross_crd.py new file mode 100644 index 0000000000..7d68989ab3 --- /dev/null +++ b/tests/unit/test_cross_crd.py @@ -0,0 +1,950 @@ +#!/usr/bin/env python + +# Copyright 2012 Hewlett-Packard Development Company, L.P. +# Copyright 2018 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from tests.base import ( + ZuulTestCase, +) + + +class TestGerritToGithubCRD(ZuulTestCase): + config_file = 'zuul-gerrit-github.conf' + tenant_config_file = 'config/cross-source/main.yaml' + + def test_crd_gate(self): + "Test cross-repo dependencies" + A = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'A') + B = self.fake_github.openFakePullRequest('github/project2', 'master', + 'B') + + A.addApproval('Code-Review', 2) + + AM2 = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', + 'AM2') + AM1 = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', + 'AM1') + AM2.setMerged() + AM1.setMerged() + + # A -> AM1 -> AM2 + # A Depends-On: B + # M2 is here to make sure it is never queried. If it is, it + # means zuul is walking down the entire history of merged + # changes. + + A.setDependsOn(AM1, 1) + AM1.setDependsOn(AM2, 1) + + A.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + A.subject, B.url) + + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'NEW') + self.assertFalse(B.is_merged) + + for connection in self.connections.connections.values(): + connection.maintainCache([]) + + self.executor_server.hold_jobs_in_build = True + B.addLabel('approved') + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertEqual(AM2.queried, 0) + self.assertEqual(A.data['status'], 'MERGED') + self.assertTrue(B.is_merged) + self.assertEqual(A.reported, 2) + self.assertEqual(len(B.comments), 2) + + changes = self.getJobFromHistory( + 'project-merge', 'gerrit/project1').changes + self.assertEqual(changes, '1,%s 1,1' % B.head_sha) + + def test_crd_branch(self): + "Test cross-repo dependencies in multiple branches" + + self.create_branch('github/project2', 'mp') + A = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'A') + B = self.fake_github.openFakePullRequest('github/project2', 'master', + 'B') + C1 = self.fake_github.openFakePullRequest('github/project2', 'mp', + 'C1') + + A.addApproval('Code-Review', 2) + + # A Depends-On: B+C1 + A.data['commitMessage'] = '%s\n\nDepends-On: %s\nDepends-On: %s\n' % ( + A.subject, B.url, C1.url) + + self.executor_server.hold_jobs_in_build = True + B.addLabel('approved') + C1.addLabel('approved') + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'MERGED') + self.assertTrue(B.is_merged) + self.assertTrue(C1.is_merged) + self.assertEqual(A.reported, 2) + self.assertEqual(len(B.comments), 2) + self.assertEqual(len(C1.comments), 2) + + changes = self.getJobFromHistory( + 'project-merge', 'gerrit/project1').changes + self.assertEqual(changes, '1,%s 2,%s 1,1' % + (B.head_sha, C1.head_sha)) + + def test_crd_gate_reverse(self): + "Test reverse cross-repo dependencies" + A = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'A') + B = self.fake_github.openFakePullRequest('github/project2', 'master', + 'B') + A.addApproval('Code-Review', 2) + + # A Depends-On: B + + A.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + A.subject, B.url) + + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'NEW') + self.assertFalse(B.is_merged) + + self.executor_server.hold_jobs_in_build = True + A.addApproval('Approved', 1) + self.fake_github.emitEvent(B.addLabel('approved')) + self.waitUntilSettled() + + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'MERGED') + self.assertTrue(B.is_merged) + self.assertEqual(A.reported, 2) + self.assertEqual(len(B.comments), 2) + + changes = self.getJobFromHistory( + 'project-merge', 'gerrit/project1').changes + self.assertEqual(changes, '1,%s 1,1' % + (B.head_sha,)) + + def test_crd_cycle(self): + "Test cross-repo dependency cycles" + A = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'A') + msg = "Depends-On: %s" % (A.data['url'],) + B = self.fake_github.openFakePullRequest('github/project2', 'master', + 'B', body=msg) + A.addApproval('Code-Review', 2) + B.addLabel('approved') + + # A -> B -> A (via commit-depends) + + A.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + A.subject, B.url) + + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.assertEqual(A.reported, 0) + self.assertEqual(len(B.comments), 0) + self.assertEqual(A.data['status'], 'NEW') + self.assertFalse(B.is_merged) + + def test_crd_gate_unknown(self): + "Test unknown projects in dependent pipeline" + self.init_repo("github/unknown", tag='init') + A = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'A') + B = self.fake_github.openFakePullRequest('github/unknown', 'master', + 'B') + A.addApproval('Code-Review', 2) + + # A Depends-On: B + A.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + A.subject, B.url) + + event = B.addLabel('approved') + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + # Unknown projects cannot share a queue with any other + # since they don't have common jobs with any other (they have no jobs). + # Changes which depend on unknown project changes + # should not be processed in dependent pipeline + self.assertEqual(A.data['status'], 'NEW') + self.assertFalse(B.is_merged) + self.assertEqual(A.reported, 0) + self.assertEqual(len(B.comments), 0) + self.assertEqual(len(self.history), 0) + + # Simulate change B being gated outside this layout Set the + # change merged before submitting the event so that when the + # event triggers a gerrit query to update the change, we get + # the information that it was merged. + B.setMerged('merged') + self.fake_github.emitEvent(event) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # Now that B is merged, A should be able to be enqueued and + # merged. + self.fake_gerrit.addEvent(A.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'MERGED') + self.assertEqual(A.reported, 2) + self.assertTrue(B.is_merged) + self.assertEqual(len(B.comments), 0) + + def test_crd_check(self): + "Test cross-repo dependencies in independent pipelines" + self.executor_server.hold_jobs_in_build = True + self.gearman_server.hold_jobs_in_queue = True + A = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'A') + B = self.fake_github.openFakePullRequest( + 'github/project2', 'master', 'B') + + # A Depends-On: B + A.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + A.subject, B.url) + + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.gearman_server.hold_jobs_in_queue = False + self.gearman_server.release() + self.waitUntilSettled() + + self.executor_server.release('.*-merge') + self.waitUntilSettled() + + self.assertTrue(self.builds[0].hasChanges(A, B)) + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'NEW') + self.assertFalse(B.is_merged) + self.assertEqual(A.reported, 1) + self.assertEqual(len(B.comments), 0) + + changes = self.getJobFromHistory( + 'project-merge', 'gerrit/project1').changes + self.assertEqual(changes, '1,%s 1,1' % + (B.head_sha,)) + + tenant = self.sched.abide.tenants.get('tenant-one') + self.assertEqual(len(tenant.layout.pipelines['check'].queues), 0) + + def test_crd_check_duplicate(self): + "Test duplicate check in independent pipelines" + self.executor_server.hold_jobs_in_build = True + A = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'A') + B = self.fake_github.openFakePullRequest( + 'github/project2', 'master', 'B') + + # A Depends-On: B + A.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + A.subject, B.url) + tenant = self.sched.abide.tenants.get('tenant-one') + check_pipeline = tenant.layout.pipelines['check'] + + # Add two dependent changes... + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertEqual(len(check_pipeline.getAllItems()), 2) + + # ...make sure the live one is not duplicated... + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertEqual(len(check_pipeline.getAllItems()), 2) + + # ...but the non-live one is able to be. + self.fake_github.emitEvent(B.getPullRequestEditedEvent()) + self.waitUntilSettled() + self.assertEqual(len(check_pipeline.getAllItems()), 3) + + # Release jobs in order to avoid races with change A jobs + # finishing before change B jobs. + self.orderedRelease() + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'NEW') + self.assertFalse(B.is_merged) + self.assertEqual(A.reported, 1) + self.assertEqual(len(B.comments), 1) + + changes = self.getJobFromHistory( + 'project-merge', 'gerrit/project1').changes + self.assertEqual(changes, '1,%s 1,1' % + (B.head_sha,)) + + changes = self.getJobFromHistory( + 'project-merge', 'github/project2').changes + self.assertEqual(changes, '1,%s' % + (B.head_sha,)) + self.assertEqual(len(tenant.layout.pipelines['check'].queues), 0) + + self.assertIn('Build succeeded', A.messages[0]) + + def _test_crd_check_reconfiguration(self, project1, project2): + "Test cross-repo dependencies re-enqueued in independent pipelines" + + self.gearman_server.hold_jobs_in_queue = True + A = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'A') + B = self.fake_github.openFakePullRequest( + 'github/project2', 'master', 'B') + + # A Depends-On: B + A.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + A.subject, B.url) + + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.sched.reconfigure(self.config) + + # Make sure the items still share a change queue, and the + # first one is not live. + tenant = self.sched.abide.tenants.get('tenant-one') + self.assertEqual(len(tenant.layout.pipelines['check'].queues), 1) + queue = tenant.layout.pipelines['check'].queues[0] + first_item = queue.queue[0] + for item in queue.queue: + self.assertEqual(item.queue, first_item.queue) + self.assertFalse(first_item.live) + self.assertTrue(queue.queue[1].live) + + self.gearman_server.hold_jobs_in_queue = False + self.gearman_server.release() + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'NEW') + self.assertFalse(B.is_merged) + self.assertEqual(A.reported, 1) + self.assertEqual(len(B.comments), 0) + + changes = self.getJobFromHistory( + 'project-merge', 'gerrit/project1').changes + self.assertEqual(changes, '1,%s 1,1' % + (B.head_sha,)) + self.assertEqual(len(tenant.layout.pipelines['check'].queues), 0) + + def test_crd_check_reconfiguration(self): + self._test_crd_check_reconfiguration('org/project1', 'org/project2') + + def test_crd_undefined_project(self): + """Test that undefined projects in dependencies are handled for + independent pipelines""" + # It's a hack for fake github, + # as it implies repo creation upon the creation of any change + self.init_repo("github/unknown", tag='init') + self._test_crd_check_reconfiguration('gerrit/project1', + 'github/unknown') + + def test_crd_check_transitive(self): + "Test transitive cross-repo dependencies" + # Specifically, if A -> B -> C, and C gets a new patchset and + # A gets a new patchset, ensure the test of A,2 includes B,1 + # and C,2 (not C,1 which would indicate stale data in the + # cache for B). + A = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'A') + C = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'C') + # B Depends-On: C + msg = "Depends-On: %s" % (C.data['url'],) + B = self.fake_github.openFakePullRequest( + 'github/project2', 'master', 'B', body=msg) + + # A Depends-On: B + A.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + A.subject, B.url) + + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertEqual(self.history[-1].changes, '2,1 1,%s 1,1' % + (B.head_sha,)) + + self.fake_github.emitEvent(B.getPullRequestEditedEvent()) + self.waitUntilSettled() + self.assertEqual(self.history[-1].changes, '2,1 1,%s' % + (B.head_sha,)) + + self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertEqual(self.history[-1].changes, '2,1') + + C.addPatchset() + self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(2)) + self.waitUntilSettled() + self.assertEqual(self.history[-1].changes, '2,2') + + A.addPatchset() + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(2)) + self.waitUntilSettled() + self.assertEqual(self.history[-1].changes, '2,2 1,%s 1,2' % + (B.head_sha,)) + + def test_crd_check_unknown(self): + "Test unknown projects in independent pipeline" + self.init_repo("github/unknown", tag='init') + A = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'A') + B = self.fake_github.openFakePullRequest( + 'github/unknown', 'master', 'B') + # A Depends-On: B + A.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + A.subject, B.url) + + # Make sure zuul has seen an event on B. + self.fake_github.emitEvent(B.getPullRequestEditedEvent()) + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + self.assertEqual(A.data['status'], 'NEW') + self.assertEqual(A.reported, 1) + self.assertFalse(B.is_merged) + self.assertEqual(len(B.comments), 0) + + def test_crd_cycle_join(self): + "Test an updated change creates a cycle" + A = self.fake_github.openFakePullRequest( + 'github/project2', 'master', 'A') + + self.fake_github.emitEvent(A.getPullRequestEditedEvent()) + self.waitUntilSettled() + self.assertEqual(len(A.comments), 1) + + # Create B->A + B = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'B') + B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + B.subject, A.url) + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + # Dep is there so zuul should have reported on B + self.assertEqual(B.reported, 1) + + # Update A to add A->B (a cycle). + A.editBody('Depends-On: %s\n' % (B.data['url'])) + self.fake_github.emitEvent(A.getPullRequestEditedEvent()) + self.waitUntilSettled() + + # Dependency cycle injected so zuul should not have reported again on A + self.assertEqual(len(A.comments), 1) + + # Now if we update B to remove the depends-on, everything + # should be okay. B; A->B + + B.addPatchset() + B.data['commitMessage'] = '%s\n' % (B.subject,) + self.fake_github.emitEvent(A.getPullRequestEditedEvent()) + self.waitUntilSettled() + + # Cycle was removed so now zuul should have reported again on A + self.assertEqual(len(A.comments), 2) + + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(2)) + self.waitUntilSettled() + self.assertEqual(B.reported, 2) + + +class TestGithubToGerritCRD(ZuulTestCase): + config_file = 'zuul-gerrit-github.conf' + tenant_config_file = 'config/cross-source/main.yaml' + + def test_crd_gate(self): + "Test cross-repo dependencies" + A = self.fake_github.openFakePullRequest('github/project2', 'master', + 'A') + B = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'B') + + B.addApproval('Code-Review', 2) + + # A Depends-On: B + A.editBody('Depends-On: %s\n' % (B.data['url'])) + + event = A.addLabel('approved') + self.fake_github.emitEvent(event) + self.waitUntilSettled() + + self.assertFalse(A.is_merged) + self.assertEqual(B.data['status'], 'NEW') + + for connection in self.connections.connections.values(): + connection.maintainCache([]) + + self.executor_server.hold_jobs_in_build = True + B.addApproval('Approved', 1) + self.fake_github.emitEvent(event) + self.waitUntilSettled() + + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertTrue(A.is_merged) + self.assertEqual(B.data['status'], 'MERGED') + self.assertEqual(len(A.comments), 2) + self.assertEqual(B.reported, 2) + + changes = self.getJobFromHistory( + 'project-merge', 'github/project2').changes + self.assertEqual(changes, '1,1 1,%s' % A.head_sha) + + def test_crd_branch(self): + "Test cross-repo dependencies in multiple branches" + + self.create_branch('gerrit/project1', 'mp') + A = self.fake_github.openFakePullRequest('github/project2', 'master', + 'A') + B = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'B') + C1 = self.fake_gerrit.addFakeChange('gerrit/project1', 'mp', 'C1') + + B.addApproval('Code-Review', 2) + C1.addApproval('Code-Review', 2) + + # A Depends-On: B+C1 + A.editBody('Depends-On: %s\nDepends-On: %s\n' % ( + B.data['url'], C1.data['url'])) + + self.executor_server.hold_jobs_in_build = True + B.addApproval('Approved', 1) + C1.addApproval('Approved', 1) + self.fake_github.emitEvent(A.addLabel('approved')) + self.waitUntilSettled() + + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + self.assertTrue(A.is_merged) + self.assertEqual(B.data['status'], 'MERGED') + self.assertEqual(C1.data['status'], 'MERGED') + self.assertEqual(len(A.comments), 2) + self.assertEqual(B.reported, 2) + self.assertEqual(C1.reported, 2) + + changes = self.getJobFromHistory( + 'project-merge', 'github/project2').changes + self.assertEqual(changes, '1,1 2,1 1,%s' % + (A.head_sha,)) + + def test_crd_gate_reverse(self): + "Test reverse cross-repo dependencies" + A = self.fake_github.openFakePullRequest('github/project2', 'master', + 'A') + B = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'B') + B.addApproval('Code-Review', 2) + + # A Depends-On: B + A.editBody('Depends-On: %s\n' % (B.data['url'],)) + + self.fake_github.emitEvent(A.addLabel('approved')) + self.waitUntilSettled() + + self.assertFalse(A.is_merged) + self.assertEqual(B.data['status'], 'NEW') + + self.executor_server.hold_jobs_in_build = True + A.addLabel('approved') + self.fake_gerrit.addEvent(B.addApproval('Approved', 1)) + self.waitUntilSettled() + + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.release('.*-merge') + self.waitUntilSettled() + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertTrue(A.is_merged) + self.assertEqual(B.data['status'], 'MERGED') + self.assertEqual(len(A.comments), 2) + self.assertEqual(B.reported, 2) + + changes = self.getJobFromHistory( + 'project-merge', 'github/project2').changes + self.assertEqual(changes, '1,1 1,%s' % + (A.head_sha,)) + + def test_crd_cycle(self): + "Test cross-repo dependency cycles" + A = self.fake_github.openFakePullRequest('github/project2', 'master', + 'A') + B = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'B') + B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + B.subject, A.url) + + B.addApproval('Code-Review', 2) + B.addApproval('Approved', 1) + + # A -> B -> A (via commit-depends) + A.editBody('Depends-On: %s\n' % (B.data['url'],)) + + self.fake_github.emitEvent(A.addLabel('approved')) + self.waitUntilSettled() + + self.assertEqual(len(A.comments), 0) + self.assertEqual(B.reported, 0) + self.assertFalse(A.is_merged) + self.assertEqual(B.data['status'], 'NEW') + + def test_crd_gate_unknown(self): + "Test unknown projects in dependent pipeline" + self.init_repo("gerrit/unknown", tag='init') + A = self.fake_github.openFakePullRequest('github/project2', 'master', + 'A') + B = self.fake_gerrit.addFakeChange('gerrit/unknown', 'master', 'B') + B.addApproval('Code-Review', 2) + + # A Depends-On: B + A.editBody('Depends-On: %s\n' % (B.data['url'],)) + + B.addApproval('Approved', 1) + event = A.addLabel('approved') + self.fake_github.emitEvent(event) + self.waitUntilSettled() + + # Unknown projects cannot share a queue with any other + # since they don't have common jobs with any other (they have no jobs). + # Changes which depend on unknown project changes + # should not be processed in dependent pipeline + self.assertFalse(A.is_merged) + self.assertEqual(B.data['status'], 'NEW') + self.assertEqual(len(A.comments), 0) + self.assertEqual(B.reported, 0) + self.assertEqual(len(self.history), 0) + + # Simulate change B being gated outside this layout Set the + # change merged before submitting the event so that when the + # event triggers a gerrit query to update the change, we get + # the information that it was merged. + B.setMerged() + self.fake_gerrit.addEvent(B.addApproval('Approved', 1)) + self.waitUntilSettled() + self.assertEqual(len(self.history), 0) + + # Now that B is merged, A should be able to be enqueued and + # merged. + self.fake_github.emitEvent(event) + self.waitUntilSettled() + + self.assertTrue(A.is_merged) + self.assertEqual(len(A.comments), 2) + self.assertEqual(B.data['status'], 'MERGED') + self.assertEqual(B.reported, 0) + + def test_crd_check(self): + "Test cross-repo dependencies in independent pipelines" + self.executor_server.hold_jobs_in_build = True + self.gearman_server.hold_jobs_in_queue = True + A = self.fake_github.openFakePullRequest('github/project2', 'master', + 'A') + B = self.fake_gerrit.addFakeChange( + 'gerrit/project1', 'master', 'B') + + # A Depends-On: B + A.editBody('Depends-On: %s\n' % (B.data['url'],)) + + self.fake_github.emitEvent(A.getPullRequestEditedEvent()) + self.waitUntilSettled() + + self.gearman_server.hold_jobs_in_queue = False + self.gearman_server.release() + self.waitUntilSettled() + + self.executor_server.release('.*-merge') + self.waitUntilSettled() + + self.assertTrue(self.builds[0].hasChanges(A, B)) + + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertFalse(A.is_merged) + self.assertEqual(B.data['status'], 'NEW') + self.assertEqual(len(A.comments), 1) + self.assertEqual(B.reported, 0) + + changes = self.getJobFromHistory( + 'project-merge', 'github/project2').changes + self.assertEqual(changes, '1,1 1,%s' % + (A.head_sha,)) + + tenant = self.sched.abide.tenants.get('tenant-one') + self.assertEqual(len(tenant.layout.pipelines['check'].queues), 0) + + def test_crd_check_duplicate(self): + "Test duplicate check in independent pipelines" + self.executor_server.hold_jobs_in_build = True + A = self.fake_github.openFakePullRequest('github/project2', 'master', + 'A') + B = self.fake_gerrit.addFakeChange( + 'gerrit/project1', 'master', 'B') + + # A Depends-On: B + A.editBody('Depends-On: %s\n' % (B.data['url'],)) + tenant = self.sched.abide.tenants.get('tenant-one') + check_pipeline = tenant.layout.pipelines['check'] + + # Add two dependent changes... + self.fake_github.emitEvent(A.getPullRequestEditedEvent()) + self.waitUntilSettled() + self.assertEqual(len(check_pipeline.getAllItems()), 2) + + # ...make sure the live one is not duplicated... + self.fake_github.emitEvent(A.getPullRequestEditedEvent()) + self.waitUntilSettled() + self.assertEqual(len(check_pipeline.getAllItems()), 2) + + # ...but the non-live one is able to be. + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertEqual(len(check_pipeline.getAllItems()), 3) + + # Release jobs in order to avoid races with change A jobs + # finishing before change B jobs. + self.orderedRelease() + self.executor_server.hold_jobs_in_build = False + self.executor_server.release() + self.waitUntilSettled() + + self.assertFalse(A.is_merged) + self.assertEqual(B.data['status'], 'NEW') + self.assertEqual(len(A.comments), 1) + self.assertEqual(B.reported, 1) + + changes = self.getJobFromHistory( + 'project-merge', 'github/project2').changes + self.assertEqual(changes, '1,1 1,%s' % + (A.head_sha,)) + + changes = self.getJobFromHistory( + 'project-merge', 'gerrit/project1').changes + self.assertEqual(changes, '1,1') + self.assertEqual(len(tenant.layout.pipelines['check'].queues), 0) + + self.assertIn('Build succeeded', A.comments[0]) + + def _test_crd_check_reconfiguration(self, project1, project2): + "Test cross-repo dependencies re-enqueued in independent pipelines" + + self.gearman_server.hold_jobs_in_queue = True + A = self.fake_github.openFakePullRequest('github/project2', 'master', + 'A') + B = self.fake_gerrit.addFakeChange( + 'gerrit/project1', 'master', 'B') + + # A Depends-On: B + A.editBody('Depends-On: %s\n' % (B.data['url'],)) + + self.fake_github.emitEvent(A.getPullRequestEditedEvent()) + self.waitUntilSettled() + + self.sched.reconfigure(self.config) + + # Make sure the items still share a change queue, and the + # first one is not live. + tenant = self.sched.abide.tenants.get('tenant-one') + self.assertEqual(len(tenant.layout.pipelines['check'].queues), 1) + queue = tenant.layout.pipelines['check'].queues[0] + first_item = queue.queue[0] + for item in queue.queue: + self.assertEqual(item.queue, first_item.queue) + self.assertFalse(first_item.live) + self.assertTrue(queue.queue[1].live) + + self.gearman_server.hold_jobs_in_queue = False + self.gearman_server.release() + self.waitUntilSettled() + + self.assertFalse(A.is_merged) + self.assertEqual(B.data['status'], 'NEW') + self.assertEqual(len(A.comments), 1) + self.assertEqual(B.reported, 0) + + changes = self.getJobFromHistory( + 'project-merge', 'github/project2').changes + self.assertEqual(changes, '1,1 1,%s' % + (A.head_sha,)) + self.assertEqual(len(tenant.layout.pipelines['check'].queues), 0) + + def test_crd_check_reconfiguration(self): + self._test_crd_check_reconfiguration('org/project1', 'org/project2') + + def test_crd_undefined_project(self): + """Test that undefined projects in dependencies are handled for + independent pipelines""" + # It's a hack for fake gerrit, + # as it implies repo creation upon the creation of any change + self.init_repo("gerrit/unknown", tag='init') + self._test_crd_check_reconfiguration('github/project2', + 'gerrit/unknown') + + def test_crd_check_transitive(self): + "Test transitive cross-repo dependencies" + # Specifically, if A -> B -> C, and C gets a new patchset and + # A gets a new patchset, ensure the test of A,2 includes B,1 + # and C,2 (not C,1 which would indicate stale data in the + # cache for B). + A = self.fake_github.openFakePullRequest('github/project2', 'master', + 'A') + B = self.fake_gerrit.addFakeChange('gerrit/project1', 'master', 'B') + C = self.fake_github.openFakePullRequest('github/project2', 'master', + 'C') + + # B Depends-On: C + B.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + B.subject, C.url) + + # A Depends-On: B + A.editBody('Depends-On: %s\n' % (B.data['url'],)) + + self.fake_github.emitEvent(A.getPullRequestEditedEvent()) + self.waitUntilSettled() + self.assertEqual(self.history[-1].changes, '2,%s 1,1 1,%s' % + (C.head_sha, A.head_sha)) + + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertEqual(self.history[-1].changes, '2,%s 1,1' % + (C.head_sha,)) + + self.fake_github.emitEvent(C.getPullRequestEditedEvent()) + self.waitUntilSettled() + self.assertEqual(self.history[-1].changes, '2,%s' % + (C.head_sha,)) + + new_c_head = C.head_sha + C.addCommit() + old_c_head = C.head_sha + self.assertNotEqual(old_c_head, new_c_head) + self.fake_github.emitEvent(C.getPullRequestEditedEvent()) + self.waitUntilSettled() + self.assertEqual(self.history[-1].changes, '2,%s' % + (C.head_sha,)) + + new_a_head = A.head_sha + A.addCommit() + old_a_head = A.head_sha + self.assertNotEqual(old_a_head, new_a_head) + self.fake_github.emitEvent(A.getPullRequestEditedEvent()) + self.waitUntilSettled() + self.assertEqual(self.history[-1].changes, '2,%s 1,1 1,%s' % + (C.head_sha, A.head_sha,)) + + def test_crd_check_unknown(self): + "Test unknown projects in independent pipeline" + self.init_repo("gerrit/unknown", tag='init') + A = self.fake_github.openFakePullRequest('github/project2', 'master', + 'A') + B = self.fake_gerrit.addFakeChange( + 'gerrit/unknown', 'master', 'B') + + # A Depends-On: B + A.editBody('Depends-On: %s\n' % (B.data['url'],)) + + # Make sure zuul has seen an event on B. + self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1)) + self.fake_github.emitEvent(A.getPullRequestEditedEvent()) + self.waitUntilSettled() + + self.assertFalse(A.is_merged) + self.assertEqual(len(A.comments), 1) + self.assertEqual(B.data['status'], 'NEW') + self.assertEqual(B.reported, 0) + + def test_crd_cycle_join(self): + "Test an updated change creates a cycle" + A = self.fake_gerrit.addFakeChange( + 'gerrit/project1', 'master', 'A') + + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + self.assertEqual(A.reported, 1) + + # Create B->A + B = self.fake_github.openFakePullRequest('github/project2', 'master', + 'B') + B.editBody('Depends-On: %s\n' % (A.data['url'],)) + self.fake_github.emitEvent(B.getPullRequestEditedEvent()) + self.waitUntilSettled() + + # Dep is there so zuul should have reported on B + self.assertEqual(len(B.comments), 1) + + # Update A to add A->B (a cycle). + A.data['commitMessage'] = '%s\n\nDepends-On: %s\n' % ( + A.subject, B.url) + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + # Dependency cycle injected so zuul should not have reported again on A + self.assertEqual(A.reported, 1) + + # Now if we update B to remove the depends-on, everything + # should be okay. B; A->B + + B.addCommit() + B.editBody('') + self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1)) + self.waitUntilSettled() + + # Cycle was removed so now zuul should have reported again on A + self.assertEqual(A.reported, 2) + + self.fake_github.emitEvent(B.getPullRequestEditedEvent()) + self.waitUntilSettled() + self.assertEqual(len(B.comments), 2) diff --git a/zuul/driver/gerrit/gerritsource.py b/zuul/driver/gerrit/gerritsource.py index 9a75b3eb05..9e327b93a5 100644 --- a/zuul/driver/gerrit/gerritsource.py +++ b/zuul/driver/gerrit/gerritsource.py @@ -70,13 +70,15 @@ class GerritSource(BaseSource): return change def getChangesDependingOn(self, change, projects): + changes = [] + if not change.uris: + return changes queries = set() for uri in change.uris: queries.add('message:%s' % uri) query = '(' + ' OR '.join(queries) + ')' results = self.connection.simpleQuery(query) seen = set() - changes = [] for result in results: for match in find_dependency_headers(result['commitMessage']): found = False diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index 1957cc2477..a7aefe0cd5 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -646,9 +646,12 @@ class GithubConnection(BaseConnection): return self._github def maintainCache(self, relevant): + remove = set() for key, change in self._change_cache.items(): if change not in relevant: - del self._change_cache[key] + remove.add(key) + for key in remove: + del self._change_cache[key] def getChange(self, event, refresh=False): """Get the change representing an event.""" @@ -661,7 +664,6 @@ class GithubConnection(BaseConnection): change.uris = [ '%s/%s/pull/%s' % (self.server, project, change.number), ] - change.updated_at = self._ghTimestampToDate(event.updated_at) change.source_event = event change.is_current_patchset = (change.pr.get('head').get('sha') == event.patch_number) @@ -789,6 +791,8 @@ class GithubConnection(BaseConnection): change.labels = change.pr.get('labels') # ensure message is at least an empty string change.message = change.pr.get('body') or '' + change.updated_at = self._ghTimestampToDate( + change.pr.get('updated_at')) if history is None: history = [] diff --git a/zuul/driver/github/githubsource.py b/zuul/driver/github/githubsource.py index 9834727d79..33f8f7cae3 100644 --- a/zuul/driver/github/githubsource.py +++ b/zuul/driver/github/githubsource.py @@ -46,6 +46,8 @@ class GithubSource(BaseSource): if not change.number: # Not a pull request, considering merged. return True + # We don't need to perform another query because the API call + # to perform the merge will ensure this is updated. return change.is_merged def canMerge(self, change, allow_needs): diff --git a/zuul/manager/dependent.py b/zuul/manager/dependent.py index 23c2cdb753..20b376d6a8 100644 --- a/zuul/manager/dependent.py +++ b/zuul/manager/dependent.py @@ -166,7 +166,6 @@ class DependentPipelineManager(PipelineManager): # Return true if okay to proceed enqueing this change, # false if the change should not be enqueued. self.log.debug("Checking for changes needed by %s:" % change) - source = change.project.source if (hasattr(change, 'commit_needs_changes') and (change.refresh_deps or change.commit_needs_changes is None)): self.updateCommitDependencies(change, change_queue) @@ -201,7 +200,8 @@ class DependentPipelineManager(PipelineManager): self.log.debug(" Needed change is already ahead " "in the queue") continue - if source.canMerge(needed_change, self.getSubmitAllowNeeds()): + if needed_change.project.source.canMerge( + needed_change, self.getSubmitAllowNeeds()): self.log.debug(" Change %s is needed" % needed_change) if needed_change not in changes_needed: changes_needed.append(needed_change)