Permalink
Browse files

Case sensitive label matching

After upgrading Gerrit to 2.13 our gate stopped working. The reason
for this is that after a successful gate run zuul does something like
'gerrit review --label verified=2 --submit'. The verified label in
Gerrit by default is configured as 'Verified'. The newer version of
gerrit behaves different now. It accepts the +2 vote on verified but
doesn't submit the patch anymore if the casing is not correct. This
forces us to specify the label in the same casing as gerrit
expects. In that case the tolower() in canMerge prevents the patch
from entering the gate.

In order to avoid confusion and be consistent, avoid any case
conversions and use the labels exactly as defined in Gerrit.

Note that this patch requires changes to the pipelines such that the
labels are spelled exactly as defined in Gerrit.

This is a backport of I9713a075e07b268e4f2620c0862c128158283c7c to
master.

Change-Id: I55e6f12969c1c920a5017382523e71e12bc7ac3c
  • Loading branch information...
tobiashenkel authored and cboylan committed Jul 12, 2017
1 parent c2def90 commit 01740bd14778cfc2632804522d4548fac57efff8
View
@@ -12,6 +12,11 @@ Since 2.0.0:
the Zuul server in smaller deployments. Several configuration
options have moved from the ``zuul`` section to ``merger``.
* Gerrit label names must now be listed in your layout.yaml exactly as
they appear in Gerrit. This means case and special characters must
match. This change was made to accomodate Gerrit 2.13 which needs the
strings to match for changes to be successfully submitted.
Since 1.3.0:
* The Jenkins launcher is replaced with Gearman launcher. An internal
View
@@ -103,9 +103,22 @@ class ChangeReference(git.Reference):
class FakeChange(object):
categories = {'APRV': ('Approved', -1, 1),
'CRVW': ('Code-Review', -2, 2),
'VRFY': ('Verified', -2, 2)}
categories = {'Approved': ('Approved', -1, 1),
'Code-Review': ('Code-Review', -2, 2),
'Verified': ('Verified', -2, 2)}
# TODO(tobiash): This is used as a translation layer between the tests
# which use lower case labels. This can be removed if all
# tests are converted to use the correct casing.
categories_translation = {'approved': 'Approved',
'code-review': 'Code-Review',
'verified': 'Verified',
'Approved': 'Approved',
'Code-Review': 'Code-Review',
'Verified': 'Verified',
'CRVW': 'Code-Review',
'APRV': 'Approved',
'VRFY': 'Verified'}
def __init__(self, gerrit, number, project, branch, subject,
status='NEW', upstream_root=None):
@@ -290,8 +303,8 @@ def addApproval(self, category, value, username='reviewer_john',
if not granted_on:
granted_on = time.time()
approval = {
'description': self.categories[category][0],
'type': category,
'description': self.categories_translation[category],
'type': self.categories_translation[category],
'value': str(value),
'by': {
'username': username,
@@ -300,7 +313,8 @@ def addApproval(self, category, value, username='reviewer_john',
'grantedOn': int(granted_on)
}
for i, x in enumerate(self.patchsets[-1]['approvals'][:]):
if x['by']['username'] == username and x['type'] == category:
if x['by']['username'] == username and \
x['type'] == self.categories_translation[category]:
del self.patchsets[-1]['approvals'][i]
self.patchsets[-1]['approvals'].append(approval)
event = {'approvals': [approval],
@@ -6,10 +6,10 @@ pipelines:
- event: patchset-created
success:
gerrit:
verified: 1
Verified: 1
failure:
gerrit:
verified: -1
Verified: -1
- name: gate
manager: DependentPipelineManager
@@ -18,17 +18,17 @@ pipelines:
gerrit:
- event: comment-added
approval:
- approved: 1
- Approved: 1
start:
gerrit:
verified: 0
Verified: 0
success:
gerrit:
verified: 2
Verified: 2
submit: true
failure:
gerrit:
verified: -2
Verified: -2
- name: post
manager: IndependentPipelineManager
@@ -6,10 +6,10 @@ pipelines:
- event: patchset-created
success:
gerrit:
verified: 1
Verified: 1
failure:
gerrit:
verified: -1
Verified: -1
- name: post
manager: IndependentPipelineManager
@@ -25,17 +25,17 @@ pipelines:
gerrit:
- event: comment-added
approval:
- approved: 1
- Approved: 1
success:
gerrit:
verified: 2
Verified: 2
submit: true
failure:
gerrit:
verified: -2
Verified: -2
start:
gerrit:
verified: 0
Verified: 0
precedence: high
projects:
@@ -10,21 +10,21 @@ pipelines:
gerrit:
- event: comment-added
approval:
- approved: 1
- Approved: 1
success:
gerrit:
verified: 2
Verified: 2
submit: true
smtp:
to: success@example.org
failure:
gerrit:
verified: -2
Verified: -2
smtp:
to: failure@example.org
start:
gerrit:
verified: 0
Verified: 0
precedence: high
projects:
@@ -9,17 +9,17 @@ pipelines:
gerrit:
- event: comment-added
approval:
- approved: 1
- Approved: 1
success:
gerrit:
verified: 2
Verified: 2
submit: true
failure:
gerrit:
verified: -2
Verified: -2
start:
gerrit:
verified: 0
Verified: 0
precedence: high
jobs:
@@ -6,10 +6,10 @@ pipelines:
- event: patchset-created
success:
gerrit:
verified: 1
Verified: 1
failure:
gerrit:
verified: -1
Verified: -1
- name: post
manager: IndependentPipelineManager
@@ -26,22 +26,22 @@ pipelines:
gerrit:
- event: comment-added
approval:
- approved: 1
- Approved: 1
success:
gerrit:
verified: 2
Verified: 2
submit: true
failure:
gerrit:
verified: -2
Verified: -2
merge-failure:
gerrit:
verified: -1
Verified: -1
smtp:
to: you@example.com
start:
gerrit:
verified: 0
Verified: 0
precedence: high
projects:
@@ -6,13 +6,13 @@ pipelines:
gerrit:
- event: comment-added
approval:
- approved: 1
- Approved: 1
start:
gerrit:
verified: 0
Verified: 0
success:
gerrit:
verified: 2
Verified: 2
submit: true
failure:
gerrit:
@@ -6,10 +6,10 @@ pipelines:
- event: patchset-created
success:
gerrit:
verified: 1
Verified: 1
failure:
gerrit:
verified: -1
Verified: -1
- name: post
manager: IndependentPipelineManager
@@ -25,17 +25,17 @@ pipelines:
gerrit:
- event: comment-added
approval:
- approved: 1
- Approved: 1
success:
gerrit:
verified: 2
Verified: 2
submit: true
failure:
gerrit:
verified: -2
Verified: -2
start:
gerrit:
verified: 0
Verified: 0
precedence: high
projects:
@@ -6,10 +6,10 @@ pipelines:
- event: patchset-created
success:
gerrit:
verified: 1
Verified: 1
failure:
gerrit:
verified: -1
Verified: -1
- name: post
manager: IndependentPipelineManager
@@ -25,17 +25,17 @@ pipelines:
gerrit:
- event: comment-added
approval:
- approved: 1
- Approved: 1
success:
gerrit:
verified: 2
Verified: 2
submit: true
failure:
gerrit:
verified: -2
Verified: -2
start:
gerrit:
verified: 0
Verified: 0
precedence: high
jobs:
@@ -4,7 +4,7 @@ pipelines:
source: gerrit
require:
approval:
- verified: -1
- Verified: -1
trigger:
gerrit:
- event: patchset-created
@@ -13,36 +13,36 @@ pipelines:
pipeline: gate
success:
gerrit:
verified: 1
Verified: 1
failure:
gerrit:
verified: -1
Verified: -1
- name: gate
manager: DependentPipelineManager
failure-message: Build failed. For information on how to proceed, see http://wiki.example.org/Test_Failures
source: gerrit
require:
approval:
- verified: 1
- Verified: 1
trigger:
gerrit:
- event: comment-added
approval:
- approved: 1
- Approved: 1
zuul:
- event: parent-change-enqueued
pipeline: gate
success:
gerrit:
verified: 2
Verified: 2
submit: true
failure:
gerrit:
verified: -2
Verified: -2
start:
gerrit:
verified: 0
Verified: 0
precedence: high
projects:
Oops, something went wrong.

0 comments on commit 01740bd

Please sign in to comment.