zuul
/
zuul
Code Issues Proposed changes

Merge "Check Gerrit submit requirements"

changes/23/812523/2
Zuul 2023-03-30 16:58:54 +00:00 committed by Gerrit Code Review
parent 987fba9f28 7b08cb15d4
commit b2dc863b44
5 changed files with 148 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
releasenotes/notes/submit-requirements-1d61f88e54be1fde.yaml Normal file
View File

@ -0,0 +1,16 @@
---
fixes:
- |
Zuul will now attempt to honor Gerrit "submit requirements" when
determining whether to enqueue a change into a dependent (i.e.,
"gate") pipeline. Zuul previously honored only Gerrit's older
"submit records" feature. The new checks will avoid enqueing
changes in "gate" pipelines in the cases where Zuul can
unambiguously determine that there is no possibility of merging,
but some non-mergable changes may still be enqueued if Zuul can
not be certain whether a rule should apply or be disregarded (in
these cases, Gerrit will fail to merge the change and Zuul will
report the buildset as a MERGE_FAILURE).
This requires Gerrit version 3.5.0 or later, and Zuul to be
configured with HTTP access for Gerrit.

8
tests/base.py
View File

@ -403,6 +403,7 @@ class FakeGerritChange(object):
self.comments = []
self.checks = {}
self.checks_history = []
self.submit_requirements = []
self.data = {
'branch': branch,
'comments': self.comments,
@ -788,6 +789,12 @@ class FakeGerritChange(object):
return [{'status': 'NOT_READY',
'labels': labels}]
def getSubmitRequirements(self):
return self.submit_requirements
def setSubmitRequirements(self, reqs):
self.submit_requirements = reqs
def setDependsOn(self, other, patchset):
self.depends_on_change = other
self.depends_on_patchset = patchset
@ -894,6 +901,7 @@ class FakeGerritChange(object):
data['parents'] = self.data['parents']
if 'topic' in self.data:
data['topic'] = self.data['topic']
data['submit_requirements'] = self.getSubmitRequirements()
return json.loads(json.dumps(data))
def queryRevisionHTTP(self, revision):

86
tests/unit/test_gerrit.py
View File

@ -958,6 +958,92 @@ class TestGerritConnection(ZuulTestCase):
self.assertEqual(A.data['status'], 'MERGED')
self.assertEqual(B.data['status'], 'MERGED')
def test_submit_requirements(self):
A = self.fake_gerrit.addFakeChange('org/project', 'master', 'A')
A.addApproval('Code-Review', 2)
# Set an unsatisfied submit requirement
A.setSubmitRequirements([
{
"name": "Code-Review",
"description": "Disallow self-review",
"status": "UNSATISFIED",
"is_legacy": False,
"submittability_expression_result": {
"expression": "label:Code-Review=MAX,user=non_uploader "
"AND -label:Code-Review=MIN",
"fulfilled": False,
"passing_atoms": [],
"failing_atoms": [
"label:Code-Review=MAX,user=non_uploader",
"label:Code-Review=MIN"
]
}
},
{
"name": "Verified",
"status": "UNSATISFIED",
"is_legacy": True,
"submittability_expression_result": {
"expression": "label:Verified=MAX -label:Verified=MIN",
"fulfilled": False,
"passing_atoms": [],
"failing_atoms": [
"label:Verified=MAX",
"-label:Verified=MIN"
]
}
},
])
self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
self.waitUntilSettled()
self.assertHistory([])
self.assertEqual(A.queried, 1)
self.assertEqual(A.data['status'], 'NEW')
# Mark the requirement satisfied
A.setSubmitRequirements([
{
"name": "Code-Review",
"description": "Disallow self-review",
"status": "SATISFIED",
"is_legacy": False,
"submittability_expression_result": {
"expression": "label:Code-Review=MAX,user=non_uploader "
"AND -label:Code-Review=MIN",
"fulfilled": False,
"passing_atoms": [
"label:Code-Review=MAX,user=non_uploader",
],
"failing_atoms": [
"label:Code-Review=MIN"
]
}
},
{
"name": "Verified",
"status": "UNSATISFIED",
"is_legacy": True,
"submittability_expression_result": {
"expression": "label:Verified=MAX -label:Verified=MIN",
"fulfilled": False,
"passing_atoms": [],
"failing_atoms": [
"label:Verified=MAX",
"-label:Verified=MIN"
]
}
},
])
self.fake_gerrit.addEvent(A.addApproval('Approved', 1))
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"),
], ordered=False)
self.assertEqual(A.queried, 3)
self.assertEqual(A.data['status'], 'MERGED')
class TestGerritUnicodeRefs(ZuulTestCase):
config_file = 'zuul-gerrit-web.conf'

40
zuul/driver/gerrit/gerritconnection.py
View File

@ -1182,9 +1182,34 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection):
return True
if change.wip:
return False
if change.missing_labels <= set(allow_needs):
return True
return False
if change.missing_labels > set(allow_needs):
self.log.debug("Unable to merge due to "
"missing labels: %s", change.missing_labels)
return False
for sr in change.submit_requirements:
if sr.get('status') == 'UNSATISFIED':
# Otherwise, we don't care and should skip.
# We're going to look at each unsatisfied submit
# requirement, and if one of the involved labels is an
# "allow_needs" label, we will assume that Zuul may be
# able to take an action which can cause the
# requirement to be satisfied, and we will ignore it.
# Otherwise, it is likely a requirement that Zuul can
# not alter in which case the requirement should stand
# and block merging.
result = sr.get("submittability_expression_result", {})
expression = result.get("expression", '')
expr_contains_allow = False
for allow in allow_needs:
if f'label:{allow}' in expression:
expr_contains_allow = True
break
if not expr_contains_allow:
self.log.debug("Unable to merge due to "
"submit requirement: %s", sr)
return False
return True
def getProjectOpenChanges(self, project: Project) -> List[GerritChange]:
# This is a best-effort function in case Gerrit is unable to return
@ -1443,9 +1468,12 @@ class GerritConnection(ZKChangeCacheMixin, ZKBranchCacheMixin, BaseConnection):
return data
def queryChangeHTTP(self, number, event=None):
data = self.get('changes/%s?o=DETAILED_ACCOUNTS&o=CURRENT_REVISION&'
'o=CURRENT_COMMIT&o=CURRENT_FILES&o=LABELS&'
'o=DETAILED_LABELS' % (number,))
query = ('changes/%s?o=DETAILED_ACCOUNTS&o=CURRENT_REVISION&'
'o=CURRENT_COMMIT&o=CURRENT_FILES&o=LABELS&'
'o=DETAILED_LABELS' % (number,))
if self.version >= (3, 5, 0):
query += '&o=SUBMIT_REQUIREMENTS'
data = self.get(query)
related = self.get('changes/%s/revisions/%s/related' % (
number, data['current_revision']))
files = self.get('changes/%s/revisions/%s/files?parent=1' % (

4
zuul/driver/gerrit/gerritmodel.py
View File

@ -34,6 +34,7 @@ class GerritChange(Change):
self.wip = None
self.approvals = []
self.missing_labels = set()
self.submit_requirements = []
self.commit = None
self.zuul_query_ltime = None
@ -52,6 +53,7 @@ class GerritChange(Change):
"wip": self.wip,
"approvals": self.approvals,
"missing_labels": list(self.missing_labels),
"submit_requirements": self.submit_requirements,
"commit": self.commit,
"zuul_query_ltime": self.zuul_query_ltime,
})
@ -64,6 +66,7 @@ class GerritChange(Change):
self.wip = data["wip"]
self.approvals = data["approvals"]
self.missing_labels = set(data["missing_labels"])
self.submit_requirements = data.get("submit_requirements", [])
self.commit = data.get("commit")
self.zuul_query_ltime = data.get("zuul_query_ltime")
@ -189,6 +192,7 @@ class GerritChange(Change):
if 'approved' in label_data:
continue
self.missing_labels.add(label_name)
self.submit_requirements = data.get('submit_requirements', [])
self.open = data['status'] == 'NEW'
self.status = data['status']
self.wip = data.get('work_in_progress', False)