diff options
author | James E. Blair <jim@acmegating.com> | 2022-06-25 15:17:18 -0700 |
---|---|---|
committer | James E. Blair <jim@acmegating.com> | 2022-06-29 15:33:13 -0700 |
commit | 39aded45178520fba6190f81660025573951a6e4 (patch) | |
tree | e2ac835a281df884e7caa394105efc8b5c689b71 /zuul/reporter | |
parent | f2d4ff276b2e22be982cf261864dd938f707288a (diff) | |
download | zuul-39aded45178520fba6190f81660025573951a6e4.tar.gz |
Fix merging with submitWholeTopic
Previously support for Gerrit's submitWholeTopic feature was added
so that when it is enabled, changes that are submitted together are
treated as circular dependencies in Zuul. However, this feature did
not work in a gating pipeline because when that setting is enabled,
Gerrit requires all changes to be mergable at once so that it can
attempt to atomically merge all of them. That means that every
change would need a Verified+2 vote before any change can be
submitted. Zuul leaves the vote and submits each change one at a
time.
(Note, this does not affect the emulated submitWholeTopic feature
in Zuul, since in that case, Gerrit will merge each change alone.)
To correct this, a two-phase option is added to reporters. In phase1,
reporters will report all information about the change but not submit.
In phase2, they will only submit. In the cases where we are about
to submit a successful bundle, we enable the two-phase option and
report the entire bundle without submitting first, then proceed to
submit each change in the bundle in sequence as normal. In Gerrit,
if submitWholeTopic is enabled, this will cause all changes to be
submitted as soon as the first one is, but that's okay because we
handle the case where we try to submit a change and it is already
submitted.
The fake Gerrit used in the Zuul unit tests is updated to match
the real Gerrit in these cases. If submitWholeTopic is enabled,
it will return a 409 to submit requests if the whole topic is not
able to be submitted.
One unit test of failed bundle reporting is adjusted since we will
now report the buildset result to all changes before potentially
reporting a second time if the bundle later fails to merge.
While this does mean that some changes will have extra report
messages, it actually makes the behavior consistent (before, some
changes would have 2 reports and some would have only 1; now all
changes will have 2 reports: the expected result and then a second
report of the unexpected merge failure).
All reporters are updated to handle the two-phase reporting. Since
all reporters have different API methods to leave comments and merge
changes, this won't actually cause any extra API calls even for
reporters which don't need two-phase reporting.
Non-merging reporters (MQTT, SMTP, etc) simply ignore phase2.
Change-Id: Ibf377ab5b7141fe60ecfd5cbbb296bb4f9c24265
Diffstat (limited to 'zuul/reporter')
-rw-r--r-- | zuul/reporter/__init__.py | 15 |
1 files changed, 13 insertions, 2 deletions
diff --git a/zuul/reporter/__init__.py b/zuul/reporter/__init__.py index 93b218519..513220f4b 100644 --- a/zuul/reporter/__init__.py +++ b/zuul/reporter/__init__.py @@ -35,8 +35,19 @@ class BaseReporter(object, metaclass=abc.ABCMeta): self._action = action @abc.abstractmethod - def report(self, item): - """Send the compiled report message.""" + def report(self, item, phase1=True, phase2=True): + """Send the compiled report message + + Two-phase reporting may be enabled if one or the other of the + `phase1` or `phase2` arguments is False. + + Phase1 should report everything except the actual merge action. + Phase2 should report only the merge action. + + :arg phase1 bool: Whether to enable phase1 reporting + :arg phase2 bool: Whether to enable phase2 reporting + + """ def getSubmitAllowNeeds(self): """Get a list of code review labels that are allowed to be |