zuul
/
zuul
Code Issues Proposed changes

Merge "Don't call the merger for non-live items"

This commit is contained in:
Zuul 2019-03-18 17:44:04 +00:00 committed by Gerrit Code Review
parent 78e101ba94 2b3a041e61
commit 09130c8a78
5 changed files with 308 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
tests/base.py
View File

@ -1556,6 +1556,19 @@ class RecordingAnsibleJob(zuul.executor.server.AnsibleJob):
return hosts
class RecordingMergeClient(zuul.merger.client.MergeClient):
def __init__(self, config, sched):
super().__init__(config, sched)
self.history = {}
def submitJob(self, name, data, build_set,
precedence=zuul.model.PRECEDENCE_NORMAL):
self.history.setdefault(name, [])
self.history[name].append((data, build_set))
return super().submitJob(name, data, build_set, precedence)
class RecordingExecutorServer(zuul.executor.server.ExecutorServer):
"""An Ansible executor to be used in tests.
@ -2518,8 +2531,7 @@ class ZuulTestCase(BaseTestCase):
self.executor_client = zuul.executor.client.ExecutorClient(
self.config, self.sched)
self.merge_client = zuul.merger.client.MergeClient(
self.config, self.sched)
self.merge_client = RecordingMergeClient(self.config, self.sched)
self.merge_server = None
self.nodepool = zuul.nodepool.Nodepool(self.sched)
self.zk = zuul.zk.ZooKeeper()

114
tests/unit/test_merger_repo.py
View File

@ -21,6 +21,7 @@ import git
import testtools
from zuul.merger.merger import Repo
from zuul.model import MERGER_MERGE_RESOLVE
from tests.base import ZuulTestCase, FIXTURE_DIR, simple_layout
@ -329,3 +330,116 @@ class TestMergerWithAuthUrl(ZuulTestCase):
# the urls should differ
self.assertNotEqual(first_url, second_url)
class TestMerger(ZuulTestCase):
tenant_config_file = 'config/single-tenant/main.yaml'
@staticmethod
def _item_from_fake_change(fake_change):
return dict(
number=fake_change.number,
patchset=1,
ref=fake_change.patchsets[0]['ref'],
connection='gerrit',
branch=fake_change.branch,
project=fake_change.project,
buildset_uuid='fake-uuid',
merge_mode=MERGER_MERGE_RESOLVE,
)
def test_merge_multiple_items(self):
"""
Tests that the merger merges and returns the requested file changes per
change and in the correct order.
"""
merger = self.executor_server.merger
files = ['zuul.yaml', '.zuul.yaml']
dirs = ['zuul.d', '.zuul.d']
# Simple change A
file_dict_a = {'zuul.d/a.yaml': 'a'}
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
files=file_dict_a)
item_a = self._item_from_fake_change(A)
# Simple change B
file_dict_b = {'zuul.d/b.yaml': 'b'}
B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B',
files=file_dict_b)
item_b = self._item_from_fake_change(B)
# Simple change C on top of A
file_dict_c = {'zuul.d/a.yaml': 'a-with-c'}
C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C',
files=file_dict_c,
parent=A.patchsets[0]['ref'])
item_c = self._item_from_fake_change(C)
# Change in different project
file_dict_d = {'zuul.d/a.yaml': 'a-in-project1'}
D = self.fake_gerrit.addFakeChange('org/project1', 'master', 'D',
files=file_dict_d)
item_d = self._item_from_fake_change(D)
# Merge A
result = merger.mergeChanges([item_a], files=files, dirs=dirs)
self.assertIsNotNone(result)
hexsha, read_files, repo_state, ret_recent, orig_commit = result
self.assertEqual(len(read_files), 1)
self.assertEqual(read_files[0]['project'], 'org/project')
self.assertEqual(read_files[0]['branch'], 'master')
self.assertEqual(read_files[0]['files']['zuul.d/a.yaml'], 'a')
# Merge A -> B
result = merger.mergeChanges([item_a, item_b], files=files, dirs=dirs)
self.assertIsNotNone(result)
hexsha, read_files, repo_state, ret_recent, orig_commit = result
self.assertEqual(len(read_files), 2)
self.assertEqual(read_files[0]['project'], 'org/project')
self.assertEqual(read_files[0]['branch'], 'master')
self.assertEqual(read_files[0]['files']['zuul.d/a.yaml'], 'a')
self.assertEqual(read_files[1]['project'], 'org/project')
self.assertEqual(read_files[1]['branch'], 'master')
self.assertEqual(read_files[1]['files']['zuul.d/b.yaml'], 'b')
# Merge A -> B -> C
result = merger.mergeChanges([item_a, item_b, item_c], files=files,
dirs=dirs)
self.assertIsNotNone(result)
hexsha, read_files, repo_state, ret_recent, orig_commit = result
self.assertEqual(len(read_files), 3)
self.assertEqual(read_files[0]['project'], 'org/project')
self.assertEqual(read_files[0]['branch'], 'master')
self.assertEqual(read_files[0]['files']['zuul.d/a.yaml'], 'a')
self.assertEqual(read_files[1]['project'], 'org/project')
self.assertEqual(read_files[1]['branch'], 'master')
self.assertEqual(read_files[1]['files']['zuul.d/b.yaml'], 'b')
self.assertEqual(read_files[2]['project'], 'org/project')
self.assertEqual(read_files[2]['branch'], 'master')
self.assertEqual(read_files[2]['files']['zuul.d/a.yaml'],
'a-with-c')
# Merge A -> B -> C -> D
result = merger.mergeChanges([item_a, item_b, item_c, item_d],
files=files, dirs=dirs)
self.assertIsNotNone(result)
hexsha, read_files, repo_state, ret_recent, orig_commit = result
self.assertEqual(len(read_files), 4)
self.assertEqual(read_files[0]['project'], 'org/project')
self.assertEqual(read_files[0]['branch'], 'master')
self.assertEqual(read_files[0]['files']['zuul.d/a.yaml'], 'a')
self.assertEqual(read_files[1]['project'], 'org/project')
self.assertEqual(read_files[1]['branch'], 'master')
self.assertEqual(read_files[1]['files']['zuul.d/b.yaml'], 'b')
self.assertEqual(read_files[2]['project'], 'org/project')
self.assertEqual(read_files[2]['branch'], 'master')
self.assertEqual(read_files[2]['files']['zuul.d/a.yaml'],
'a-with-c')
self.assertEqual(read_files[3]['project'], 'org/project1')
self.assertEqual(read_files[3]['branch'], 'master')
self.assertEqual(read_files[3]['files']['zuul.d/a.yaml'],
'a-in-project1')

167
tests/unit/test_v3.py
View File

@ -2078,6 +2078,173 @@ class TestInRepoConfig(ZuulTestCase):
A.messages[0], "A should have debug info")
class TestNonLiveMerges(ZuulTestCase):
config_file = 'zuul-connections-gerrit-and-github.conf'
tenant_config_file = 'config/in-repo/main.yaml'
def test_non_live_merges(self):
"""
This test checks that we don't do merges for non-live queue items.
The scenario tests three scenarios:
* Simple dependency chain:
A -> B -> C
* Dependency chain that includes a merge conflict:
A -> B -> B_conflict (conflicts with A) -> C_conflict
* Dependency chain with broken config in the middls:
A_invalid (broken config) -> B_after_invalid (repairs broken config(
"""
in_repo_conf_a = textwrap.dedent(
"""
- job:
name: project-test1
run: playbooks/project-test.yaml
- project:
name: org/project
check:
jobs:
- project-test1
""")
in_repo_playbook = textwrap.dedent(
"""
- hosts: all
tasks: []
""")
file_dict_a = {'.zuul.yaml': in_repo_conf_a,
'playbooks/project-test.yaml': in_repo_playbook}
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A',
files=file_dict_a)
in_repo_conf_b = textwrap.dedent(
"""
- job:
name: project-test1
run: playbooks/project-test.yaml
- job:
name: project-test2
run: playbooks/project-test.yaml
- project:
name: org/project
check:
jobs:
- project-test1
- project-test2
""")
file_dict_b = {'.zuul.yaml': in_repo_conf_b}
B = self.fake_gerrit.addFakeChange('org/project', 'master', 'B',
files=file_dict_b,
parent=A.patchsets[0]['ref'])
B.setDependsOn(A, 1)
in_repo_conf_c = textwrap.dedent(
"""
- job:
name: project-test1
run: playbooks/project-test.yaml
- job:
name: project-test2
run: playbooks/project-test.yaml
- job:
name: project-test3
run: playbooks/project-test.yaml
- project:
name: org/project
check:
jobs:
- project-test1
- project-test2
- project-test3
""")
file_dict_c = {'.zuul.yaml': in_repo_conf_c}
C = self.fake_gerrit.addFakeChange('org/project', 'master', 'C',
files=file_dict_c,
parent=B.patchsets[0]['ref'])
C.setDependsOn(B, 1)
B_conflict = self.fake_gerrit.addFakeChange(
'org/project', 'master', 'B_conflict', files=file_dict_a)
B_conflict.setDependsOn(B, 1)
C_conflict = self.fake_gerrit.addFakeChange(
'org/project', 'master', 'C_conflict')
C_conflict.setDependsOn(B_conflict, 1)
in_repo_conf_a_invalid = textwrap.dedent(
"""
- project:
name: org/project
check:
jobs:
- project-test1
""")
file_dict_a_invalid = {'.zuul.yaml': in_repo_conf_a_invalid,
'playbooks/project-test.yaml': in_repo_playbook}
A_invalid = self.fake_gerrit.addFakeChange(
'org/project', 'master', 'A_invalid', files=file_dict_a_invalid)
B_after_invalid = self.fake_gerrit.addFakeChange(
'org/project', 'master', 'B', files=file_dict_b,
parent=A_invalid.patchsets[0]['ref'])
B_after_invalid.setDependsOn(A_invalid, 1)
self.fake_gerrit.addEvent(A.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(B.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(C.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(B_conflict.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(C_conflict.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(A_invalid.getPatchsetCreatedEvent(1))
self.fake_gerrit.addEvent(B_after_invalid.getPatchsetCreatedEvent(1))
self.waitUntilSettled()
self.assertEqual(A.reported, 1, "A should report")
self.assertEqual(B.reported, 1, "B should report")
self.assertEqual(C.reported, 1, "C should report")
self.assertEqual(B_conflict.reported, 1, "B_conflict should report")
self.assertEqual(C_conflict.reported, 1, "C_conflict should report")
self.assertEqual(B_after_invalid.reported, 1,
"B_after_invalid should report")
self.assertIn('Build succeeded', A.messages[0])
self.assertIn('Build succeeded', B.messages[0])
self.assertIn('Build succeeded', C.messages[0])
self.assertIn('Merge Failed', B_conflict.messages[0])
self.assertIn('Merge Failed', C_conflict.messages[0])
self.assertIn('Zuul encountered a syntax error', A_invalid.messages[0])
self.assertIn('Build succeeded', B_after_invalid.messages[0])
self.assertHistory([
# Change A
dict(name='project-test1', result='SUCCESS', changes='1,1'),
# Change B
dict(name='project-test1', result='SUCCESS', changes='1,1 2,1'),
dict(name='project-test2', result='SUCCESS', changes='1,1 2,1'),
# Change C
dict(name='project-test1', result='SUCCESS',
changes='1,1 2,1 3,1'),
dict(name='project-test2', result='SUCCESS',
changes='1,1 2,1 3,1'),
dict(name='project-test3', result='SUCCESS',
changes='1,1 2,1 3,1'),
# Change B_after_invalid
dict(name='project-test1', result='SUCCESS', changes='6,1 7,1'),
dict(name='project-test2', result='SUCCESS', changes='6,1 7,1'),
], ordered=False)
# We expect one merge call per change
self.assertEqual(len(self.merge_client.history['merger:merge']), 7)
class TestJobContamination(AnsibleZuulTestCase):
config_file = 'zuul-connections-gerrit-and-github.conf'

12
zuul/manager/__init__.py
View File

@ -710,7 +710,7 @@ class PipelineManager(object):
change_queue.moveItem(item, nnfi)
changed = True
self.cancelJobs(item)
if actionable:
if actionable and item.live:
ready = self.prepareItem(item) and self.prepareJobs(item)
# Starting jobs reporting should only be done once if there are
# jobs to run for this item.
@ -722,14 +722,8 @@ class PipelineManager(object):
item.reported_start = True
if item.current_build_set.unable_to_merge:
failing_reasons.append("it has a merge conflict")
if (not item.live) and (not dequeued):
self.dequeueItem(item)
changed = dequeued = True
if item.current_build_set.config_errors:
failing_reasons.append("it has an invalid configuration")
if (not item.live) and (not dequeued):
self.dequeueItem(item)
changed = dequeued = True
if ready and self.provisionNodes(item):
changed = True
if ready and self.executeJobs(item):
@ -857,6 +851,10 @@ class PipelineManager(object):
build_set.repo_state = event.repo_state
if event.merged:
build_set.commit = event.commit
items_ahead = item.getNonLiveItemsAhead()
for index, item in enumerate(items_ahead):
files = item.current_build_set.files
files.setFiles(event.files[:index + 1])
build_set.files.setFiles(event.files)
elif event.updated:
build_set.commit = (item.change.newrev or

8
zuul/model.py
View File

@ -2116,6 +2116,14 @@ class QueueItem(object):
return None
return self.job_graph.jobs.get(name)
def getNonLiveItemsAhead(self):
items = []
item_ahead = self.item_ahead
while item_ahead and not item_ahead.live:
items.append(item_ahead)
item_ahead = item_ahead.item_ahead
return reversed(items)
def haveAllJobsStarted(self):
if not self.hasJobGraph():
return False