From 7b08cb15d4a8baf50678fb99f442c397d863b1b7 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Mon, 27 Mar 2023 15:34:46 -0700 Subject: Check Gerrit submit requirements With newer versions of Gerrit, we are increasingly likely to encounter systems where the traditional label requirements are minimized in favor of the new submit requirements rules. If Gerrit is configured to use a submit requirement instead of a traditional label blocking rule, that is typically done by switching the label function to "NoBlock", which, like the "NoOp" function, will still cause the label to appear in the "submit_record" field, but with a value of "MAY" instead of "OK", "NEED", or "REJECT". Instead, the interesting information will be in the "submit_requirements" field. In this field we can see the individual submit requirement rules and whether they are satisfied or not. Since submit requirements do not have a 1:1 mapping with labels, determining whether an "UNSATISFIED" submit requirement should be ignored (because it pertains to a label that Zuul will alter, like "Verified") is not as straightforward is it is for submit records. To be conservative, this change looks for any of the "allow needs" labels (typically "Verified") in each unsatisfied submit record and if it finds one, it ignores that record. With this change in place, we can avoid enqueing changes which we are certain can not be merged into gate pipelines, and will continue to enqueue changes about which we are uncertain. Change-Id: I667181565684d97c1d036e2db6193dc606c76c57 --- zuul/driver/gerrit/gerritconnection.py | 40 +++++++++++++++++++++++++++++----- zuul/driver/gerrit/gerritmodel.py | 4 ++++ 2 files changed, 38 insertions(+), 6 deletions(-) (limited to 'zuul') diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py index f871671aa..6efca17c5 100644 --- a/zuul/driver/gerrit/gerritconnection.py +++ b/zuul/driver/gerrit/gerritconnection.py @@ -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' % ( diff --git a/zuul/driver/gerrit/gerritmodel.py b/zuul/driver/gerrit/gerritmodel.py index 0ac3e7f9d..4ac291f2b 100644 --- a/zuul/driver/gerrit/gerritmodel.py +++ b/zuul/driver/gerrit/gerritmodel.py @@ -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) -- cgit v1.2.1