Merge "Dequeue items that can no longer merge"
This commit is contained in:
commit
c0f9a606cc
|
@ -46,3 +46,11 @@ Version 3
|
|||
:Description: Add a new `SupercedeEvent` and use that for dequeuing of
|
||||
superceded items from other pipelines. This only affects the
|
||||
schedulers.
|
||||
|
||||
Version 4
|
||||
---------
|
||||
|
||||
:Prior Zuul version: 5.1.0
|
||||
:Description: Adds QueueItem.dequeued_missing_requirements and sets it to True
|
||||
if a change no longer meets merge requirements in dependent
|
||||
pipelines. This only affects schedulers.
|
||||
|
|
|
@ -0,0 +1,6 @@
|
|||
---
|
||||
features:
|
||||
- |
|
||||
Changes in dependent queues are now automatically dequeued if they
|
||||
no longer meet the mergability requirements set by the upstream
|
||||
code review system.
|
|
@ -1120,6 +1120,55 @@ class TestGithubDriver(ZuulTestCase):
|
|||
# the change should have entered the gate
|
||||
self.assertEqual(2, len(self.history))
|
||||
|
||||
@simple_layout('layouts/gate-github.yaml', driver='github')
|
||||
def test_status_checks_removal(self):
|
||||
github = self.fake_github.getGithubClient()
|
||||
repo = github.repo_from_project('org/project')
|
||||
repo._set_branch_protection(
|
||||
'master', contexts=['something/check', 'tenant-one/gate'])
|
||||
|
||||
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
|
||||
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.executor_server.hold_jobs_in_build = True
|
||||
# Since the required status 'something/check' is not fulfilled,
|
||||
# no job is expected
|
||||
self.assertEqual(0, len(self.builds))
|
||||
self.assertEqual(0, len(self.history))
|
||||
|
||||
# Set the required status 'something/check'
|
||||
repo.create_status(A.head_sha, 'success', 'example.com', 'description',
|
||||
'something/check')
|
||||
|
||||
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
|
||||
self.waitUntilSettled()
|
||||
self.assertEqual(2, len(self.builds))
|
||||
self.assertEqual(0, len(self.history))
|
||||
|
||||
# Remove it and verify the change is dequeued.
|
||||
repo.create_status(A.head_sha, 'failed', 'example.com', 'description',
|
||||
'something/check')
|
||||
self.fake_github.emitEvent(A.getCommitStatusEvent('something/check',
|
||||
state='failed',
|
||||
user='foo'))
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.executor_server.hold_jobs_in_build = False
|
||||
self.executor_server.release()
|
||||
self.waitUntilSettled()
|
||||
|
||||
# The change should be dequeued.
|
||||
self.assertHistory([
|
||||
dict(name='project-test1', result='ABORTED'),
|
||||
dict(name='project-test2', result='ABORTED'),
|
||||
], ordered=False)
|
||||
self.assertEqual(1, len(A.comments))
|
||||
self.assertFalse(A.is_merged)
|
||||
self.assertIn('This change is unable to merge '
|
||||
'due to a missing requirement.',
|
||||
A.comments[0])
|
||||
|
||||
# This test case verifies that no reconfiguration happens if a branch was
|
||||
# deleted that didn't contain configuration.
|
||||
@simple_layout('layouts/basic-github.yaml', driver='github')
|
||||
|
|
|
@ -193,3 +193,55 @@ class TestSemaphoreModelUpgrade(ZuulTestCase):
|
|||
self.assertEqual(
|
||||
len(tenant.semaphore_handler.semaphoreHolders("test-semaphore")),
|
||||
0)
|
||||
|
||||
|
||||
class TestGithubModelUpgrade(ZuulTestCase):
|
||||
config_file = 'zuul-github-driver.conf'
|
||||
scheduler_count = 1
|
||||
|
||||
@model_version(3)
|
||||
@simple_layout('layouts/gate-github.yaml', driver='github')
|
||||
def test_status_checks_removal(self):
|
||||
# This tests the old behavior -- that changes are not dequeued
|
||||
# once their required status checks are removed -- since the
|
||||
# new behavior requires a flag in ZK.
|
||||
# Contrast with test_status_checks_removal.
|
||||
github = self.fake_github.getGithubClient()
|
||||
repo = github.repo_from_project('org/project')
|
||||
repo._set_branch_protection(
|
||||
'master', contexts=['something/check', 'tenant-one/gate'])
|
||||
|
||||
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
|
||||
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.executor_server.hold_jobs_in_build = True
|
||||
# Since the required status 'something/check' is not fulfilled,
|
||||
# no job is expected
|
||||
self.assertEqual(0, len(self.history))
|
||||
|
||||
# Set the required status 'something/check'
|
||||
repo.create_status(A.head_sha, 'success', 'example.com', 'description',
|
||||
'something/check')
|
||||
|
||||
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
|
||||
self.waitUntilSettled()
|
||||
|
||||
# Remove it and verify the change is not dequeued (old behavior).
|
||||
repo.create_status(A.head_sha, 'failed', 'example.com', 'description',
|
||||
'something/check')
|
||||
self.fake_github.emitEvent(A.getCommitStatusEvent('something/check',
|
||||
state='failed',
|
||||
user='foo'))
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.executor_server.hold_jobs_in_build = False
|
||||
self.executor_server.release()
|
||||
self.waitUntilSettled()
|
||||
|
||||
# the change should have entered the gate
|
||||
self.assertHistory([
|
||||
dict(name='project-test1', result='SUCCESS'),
|
||||
dict(name='project-test2', result='SUCCESS'),
|
||||
], ordered=False)
|
||||
self.assertTrue(A.is_merged)
|
||||
|
|
|
@ -1895,6 +1895,35 @@ class TestScheduler(ZuulTestCase):
|
|||
self.assertEqual(B.reported, 2)
|
||||
self.assertEqual(C.reported, 2)
|
||||
|
||||
def test_approval_removal(self):
|
||||
# Test that we dequeue a change when it can not merge
|
||||
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))
|
||||
|
||||
self.waitUntilSettled()
|
||||
self.assertEqual(1, len(self.builds))
|
||||
self.assertEqual(0, len(self.history))
|
||||
|
||||
# Remove the approval
|
||||
self.fake_gerrit.addEvent(A.addApproval('Approved', 0))
|
||||
self.waitUntilSettled()
|
||||
|
||||
self.executor_server.hold_jobs_in_build = False
|
||||
self.executor_server.release()
|
||||
self.waitUntilSettled()
|
||||
|
||||
# The change should be dequeued.
|
||||
self.assertHistory([
|
||||
dict(name='project-merge', result='ABORTED'),
|
||||
], ordered=False)
|
||||
self.assertEqual(2, len(A.messages))
|
||||
self.assertEqual(A.data['status'], 'NEW')
|
||||
self.assertIn('This change is unable to merge '
|
||||
'due to a missing requirement.',
|
||||
A.messages[1])
|
||||
|
||||
@simple_layout('layouts/nonvoting-job-approval.yaml')
|
||||
def test_nonvoting_job_approval(self):
|
||||
"Test that non-voting jobs don't vote but leave approval"
|
||||
|
|
|
@ -460,6 +460,8 @@ class PipelineManager(metaclass=ABCMeta):
|
|||
item.setConfigErrors(item.getConfigErrors())
|
||||
if item.dequeued_needing_change:
|
||||
item.setDequeuedNeedingChange()
|
||||
if item.dequeued_missing_requirements:
|
||||
item.setDequeuedMissingRequirements()
|
||||
|
||||
# It can happen that all in-flight builds have been removed
|
||||
# which would lead to paused parent jobs not being resumed.
|
||||
|
@ -1421,14 +1423,24 @@ class PipelineManager(metaclass=ABCMeta):
|
|||
item_ahead = None
|
||||
change_queue = item.queue
|
||||
|
||||
if self.checkForChangesNeededBy(item.change, change_queue,
|
||||
item.event) is not True:
|
||||
if COMPONENT_REGISTRY.model_api > 3:
|
||||
# This sets a QueueItem flag which is only understood by
|
||||
# api 4.
|
||||
meets_reqs = self.isChangeReadyToBeEnqueued(
|
||||
item.change, item.event)
|
||||
else:
|
||||
meets_reqs = True
|
||||
needs_met = self.checkForChangesNeededBy(item.change, change_queue,
|
||||
item.event) is True
|
||||
if not (meets_reqs and needs_met):
|
||||
# It's not okay to enqueue this change, we should remove it.
|
||||
log.info("Dequeuing change %s because "
|
||||
"it can no longer merge" % item.change)
|
||||
self.cancelJobs(item)
|
||||
if item.isBundleFailing():
|
||||
item.setDequeuedBundleFailing()
|
||||
elif not meets_reqs:
|
||||
item.setDequeuedMissingRequirements()
|
||||
else:
|
||||
item.setDequeuedNeedingChange()
|
||||
if item.live:
|
||||
|
@ -1916,6 +1928,11 @@ class PipelineManager(metaclass=ABCMeta):
|
|||
action = 'failure'
|
||||
actions = self.pipeline.failure_actions
|
||||
item.setReportedResult('FAILURE')
|
||||
elif item.wasDequeuedMissingRequirements():
|
||||
log.debug("Dequeued missing requirements")
|
||||
action = 'failure'
|
||||
actions = self.pipeline.failure_actions
|
||||
item.setReportedResult('FAILURE')
|
||||
elif not item.getJobs():
|
||||
# We don't send empty reports with +1
|
||||
log.debug("No jobs for change %s", item.change)
|
||||
|
|
|
@ -50,7 +50,7 @@ from zuul.zk.change_cache import ChangeKey
|
|||
|
||||
# When making ZK schema changes, increment this and add a record to
|
||||
# docs/developer/model-changelog.rst
|
||||
MODEL_API = 3
|
||||
MODEL_API = 4
|
||||
|
||||
MERGER_MERGE = 1 # "git merge"
|
||||
MERGER_MERGE_RESOLVE = 2 # "git merge -s resolve"
|
||||
|
@ -4025,6 +4025,7 @@ class QueueItem(zkobject.ZKObject):
|
|||
queue=None,
|
||||
change=None, # a ref
|
||||
dequeued_needing_change=False,
|
||||
dequeued_missing_requirements=False,
|
||||
current_build_set=None,
|
||||
item_ahead=None,
|
||||
items_behind=[],
|
||||
|
@ -4084,6 +4085,8 @@ class QueueItem(zkobject.ZKObject):
|
|||
# This needs change cache and the API to resolve change by key.
|
||||
"change": self.change.cache_key,
|
||||
"dequeued_needing_change": self.dequeued_needing_change,
|
||||
"dequeued_missing_requirements":
|
||||
self.dequeued_missing_requirements,
|
||||
"current_build_set": (self.current_build_set and
|
||||
self.current_build_set.getPath()),
|
||||
"item_ahead": self.item_ahead and self.item_ahead.getPath(),
|
||||
|
@ -4381,6 +4384,9 @@ class QueueItem(zkobject.ZKObject):
|
|||
def wasDequeuedNeedingChange(self):
|
||||
return self.dequeued_needing_change
|
||||
|
||||
def wasDequeuedMissingRequirements(self):
|
||||
return self.dequeued_missing_requirements
|
||||
|
||||
def includesConfigUpdates(self):
|
||||
includes_trusted = False
|
||||
includes_untrusted = False
|
||||
|
@ -4810,6 +4816,12 @@ class QueueItem(zkobject.ZKObject):
|
|||
dequeued_needing_change=True)
|
||||
self._setAllJobsSkipped()
|
||||
|
||||
def setDequeuedMissingRequirements(self):
|
||||
self.updateAttributes(
|
||||
self.pipeline.manager.current_context,
|
||||
dequeued_missing_requirements=True)
|
||||
self._setAllJobsSkipped()
|
||||
|
||||
def setDequeuedBundleFailing(self):
|
||||
self.updateAttributes(
|
||||
self.pipeline.manager.current_context,
|
||||
|
|
|
@ -180,6 +180,9 @@ class BaseReporter(object, metaclass=abc.ABCMeta):
|
|||
msg = 'This change is part of a bundle that failed to merge.\n'
|
||||
elif item.dequeued_needing_change:
|
||||
msg = 'This change depends on a change that failed to merge.\n'
|
||||
elif item.dequeued_missing_requirements:
|
||||
msg = ('This change is unable to merge '
|
||||
'due to a missing requirement.\n')
|
||||
elif item.isBundleFailing():
|
||||
msg = 'This change is part of a bundle that failed.\n'
|
||||
if with_jobs:
|
||||
|
|
Loading…
Reference in New Issue