We have some replacement fields designed to allow users to construct
deep links to the status page, but that is more difficult to do now
that items may have more than one change. Moreover, users shouldn't
need to do it at all since Zuul already knows how to do that.
Make a new field, "item_url" which is the correct status page url,
and encourage users to use that instead. Deprecate status_url,
change, and changes which are now difficult to use correctly.
Change-Id: I40e98ed220a13b8d2edddf510bab133b05845751
This change completes the circular dependency refactor.
The principal change is that queue items may now include
more than one change simultaneously in the case of circular
dependencies.
In dependent pipelines, the two-phase reporting process is
simplified because it happens during processing of a single
item.
In independent pipelines, non-live items are still used for
linear depnedencies, but multi-change items are used for
circular dependencies.
Previously changes were enqueued recursively and then
bundles were made out of the resulting items. Since we now
need to enqueue entire cycles in one queue item, the
dependency graph generation is performed at the start of
enqueing the first change in a cycle.
Some tests exercise situations where Zuul is processing
events for old patchsets of changes. The new change query
sequence mentioned in the previous paragraph necessitates
more accurate information about out-of-date patchsets than
the previous sequence, therefore the Gerrit driver has been
updated to query and return more data about non-current
patchsets.
This change is not backwards compatible with the existing
ZK schema, and will require Zuul systems delete all pipeline
states during the upgrade. A later change will implement
a helper command for this.
All backwards compatability handling for the last several
model_api versions which were added to prepare for this
upgrade have been removed. In general, all model data
structures involving frozen jobs are now indexed by the
frozen job's uuid and no longer include the job name since
a job name no longer uniquely identifies a job in a buildset
(either the uuid or the (job name, change) tuple must be
used to identify it).
Job deduplication is simplified and now only needs to
consider jobs within the same buildset.
The fake github driver had a bug (fakegithub.py line 694) where
it did not correctly increment the check run counter, so our
tests that verified that we closed out obsolete check runs
when re-enqueing were not valid. This has been corrected, and
in doing so, has necessitated some changes around quiet dequeing
when we re-enqueue a change.
The reporting in several drivers has been updated to support
reporting information about multiple changes in a queue item.
Change-Id: I0b9e4d3f9936b1e66a08142fc36866269dc287f1
Depends-On: https://review.opendev.org/907627
Starting with Github Enterprise 3.8[0] and github.com from September
2022 on[1], the merge strategy changed from using merge-recursive to
merge-ort[0].
The merge-ort strategy is available in the Git client since version
2.33.0 and became the default in 2.34.0[2].
If not configured otherwise, we've so far used the default merge
strategy of the Git client (which varies depending on the client
version). With this change, we are now explicitly choosing the default
merge strategy based on the Github version. This way, we can reduce
errors resulting from the use of different merge strategies in Zuul and
Github.
Since the newly added merge strategies must be understood by the mergers
we also need to bump the model API version.
[0] https://docs.github.com/en/enterprise-server@3.8/admin/release-notes
[1] https://github.blog/changelog/2022-09-12-merge-commits-now-created-using-the-merge-ort-strategy/
[2] https://git-scm.com/docs/merge-strategies#Documentation/merge-strategies.txt-recursive
Change-Id: I354a76fa8985426312344818320980c67171d774
Github will use the PR title as the commit subject for squash merges, so
we don't need include the title once again in the commit description.
Change-Id: Id5a00701c236235f5a49abd025bcfad1b2da916c
GitHub supports a "rebase" merge mode where it will rebase the PR
onto the target branch and fast-forward the target branch to the
result of the rebase.
Add support for this process to the merger so that it can prepare
an effective simulated repo, and map the merge-mode to the merge
operation in the reporter so that gating behavior matches.
This change also makes a few tweaks to the merger to improve
consistency (including renaming a variable ref->base), and corrects
some typos in the similar squash merge test methods.
Change-Id: I9db1d163bafda38204360648bb6781800d2a09b4
For heavy users of "dispatch jobs" (where many jobs are declared as
dependencies of a single job which then mutates the child_jobs
return value to indicate which few of those should be run), there
may be large numbers of "SKIPPED" jobs in the status page and in the
final job report, which reduces the usability of both of those.
Yet it is important for users to be able to see skipped jobs since
they may represent an error (they may be inadvertently skipped).
To address this, we remove "SKIPPED" jobs from the status page by
default, but add a button at the bottom of the change box which
can toggle their display.
We remove "SKIPPED" jobs from the report, but add a note at the
bottom which says "Skipped X jobs". Users can follow the buildset
link to see which ones were skipped.
The buildset page will continue to show all the jobs for the buildset.
Change-Id: Ie297168cdf5b39d1d6f219e9b2efc44c01e87f35
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
Some github users may choose not to publish their real name and/or email
address in their profile. Instead of creating an ugly message like[0]
Reviewed-by: None <None>
try to work up something from the data we have available.
[0] ed2b2ffa8a
Signed-off-by: Dr. Jens Harbott <harbott@osism.tech>
Change-Id: Ife4c325c278860c47b70b37462ebd7b1d6b97755
User feedback: Showing CANCELED jobs with "x" emoji does not help to identify
the real failed jobs. So instead show the CANCELED jobs with a warning emoji.
Change-Id: I05e107e6f30ed5d69f74ad0ee85b1a8cc3bb61ce
When there is a long list of build results, it is difficult quickly
to see which jobs are failed.
This change add emoji of check mark and cross in front of each job
to solve this problem.
Change-Id: I38177824767d475b0c4881ef0ab24ca1164d3187
Github also supported these 2 settings in check runs. In the case of a
PR being dequeued it maybe be nicer in the UI to use 'skipped' as Github
will render this as 'grey'.
https://docs.github.com/en/rest/reference/checks#update-a-check-run
Change-Id: I9313a4f96655054fd2ba1b4f899c1959283f159e
Signed-off-by: Paul Belanger <pabelanger@redhat.com>
When reporting the check run result zuul is currently doing four
github api requests:
1. Get repository
2. Get head commit of pr
3. Get check runs of head commit
4. (optional) Create initial version of check run in case start
reporting is disabled
5. Update check run that has been created in start reporting
The first three requests are basically unneeded if we craft a direct
PATCH request and remember the check run id that has been created in
the start reporting.
This is needed since those run in the critical path of the event
processing and can cause large event processing delays in a very busy
zuul system.
Change-Id: I55df1cc28279bb6923e51686dde8809421486c6a
Re-introduce change https://review.opendev.org/#/c/738590/
There was a bug in fetching a GitHub user while loading pull requests.
`getUser` was given a project object instead of a project name string,
so no installation id was found for GitHub app authentication. That lead
to unauthenticated requests to the user API which resulted in HTTP 401
errors.
This change fixes this by passing the correct `project.name` to
`getUser` and also refactors the parameter names of the involved
methods from `project` to `project_name` to be more precise there.
This reverts commit 56f355ab72.
Change-Id: Ib25f0ef5d27115626792890d0ec282ed0d562485
We do a nice thing with github checks api where, when we report the
url for the check, we report the buildset URL if it's available,
otherwise the deep-link to the status page. However, for the
non-checks-api commit status, we only send the URL of the overall
status page. We didn't have a nice buildset page (or even a deep
link status page) at the time we wrote that, and we never upgraded.
Now that we've got all the details worked out for how to handle that
and produce various fallbacks, use the same method for both cases.
Change-Id: If5c5fa3c8b33b349d95eb52175e4faf17f5bbb36
The Github checks API allows us to define custom actions on a check run.
With that, we can define a custom "abort" action, that enables users to
directly abort a running change by clicking on a respective button in
Github's checks tab.
A click on this button will emit a Github webhook event that is now
handleded by Zuul to dequeue the respective change.
To uniquely identify a change represented by a check run, each check run
now comes with an external ID which is a combination of tenant,
pipeline, queue and PR number.
Change-Id: I574479cd06abd1ea4f3daa7850cf35ea6715a23d
So far, github use the subject of the first change in the commit
message body. This is most of the time redundant with the PR title.
Ths idea is to use instead the change body.
Change-Id: I9a2bc8aa3ff60a28b2ed62a4ee5fa9f7a2c9dcda
Currently, the user stated in the `Reviewed-by` clause of a merge commit
message is the last user that commented on a pull request. This is only
by chance a user that reviewed a pull request.
This change makes sure we append the correct list of GitHub users to the
`Reviewed-by` clause, i.e., those who are in the list of reviews of the
current change/pull request.
Change-Id: Ic1c8187fce2206607789549f02a1d9576818b23e
This patch provides the general functionality to allow reporting of
dequeued items and makes use of that in the Github checks API.
This reporting will only apply if the item wasn't a success or failure.
Change-Id: I1297da4d1708908c6b179110479fe0450e5550fe
When merging a pull request in GitHub fails we currently report the
standard message when a merge confict happened. This is actually
rarely the case because when using proper gating a merge should almost
never fail because of a merge conflict. Instead report the error we
get from GitHub so the user knows why the merge failed. In most cases
this is because branch protection refuses to merge because of a
condition zuul doesn't know about. This will then be reported to the
user so he can do something about it without asking a zuul admin for
the internal logs.
Change-Id: I1c3e82a2ac20e2177352b1a4d078839d3e114467
Zuul is already providing these file comments via the zuul_return value.
So far, the Github reporter wasn't able to use those, but with the help
of the checks API we can add so called "annotations" to each check run.
Change-Id: Iff10172f95dc0430bec8e4dafb9a6c09bbe06077
Utilizing the checks API to report the build state to Github provides
some additional functionality that is not supported by the status API.
Those are:
- Defining custom actions to e.g. cancel a running build
- Report line-based file annotations
This change implements the basic checks API workflow. Once this is in
place, the additional features could simply be added on top.
Change-Id: I7e790783ee35971085863b5487ff094fa0b23d65
Story: 2007268
Task: 38672
Many projects running on Github want to use the squash merge strategy
of github to keep their history clean. Now that the github reporter
forwards the merge-mode that can be configured on a project it is easy
to also support squash merge.
Change-Id: I1f4131002dd380f2860557a9aebcc0bcf4dc2af8
Instead of creating a merge commit, Github also allows for two other
options, squash and rebase. This change makes the github reporter
aware of the merge-mode which already can be set. For now only merge
and merge-resolve are supported which both map to the Github merge
mode 'merge'. The other two merge modes that are supported by Github
are not yet supported by the merger and will be implemented in later
changes.
Change-Id: I3d51261f248072d2e3ee7e3212377f81b9a41010
Co-Authored-By: Tobias Henkel <tobias.henkel@bmw.de>
GitHub has added a lot of controls around the review object, so it is
useful to be able to run tests and then submit a review rather than
simply merge. One use-case is also to be able to self-approve with a
comment, such as is done in the test code added.
Change-Id: I16872062e627b385f78023878bea348555ec5348
There is no need to dynamically generate the github users url with zuul,
we can rely on html_url from github. This fixes an issue where we
constructed the wrong url for the merge commit when using a github app.
Change-Id: I65028be07636aeaad48ccb6de80b3e2a0d29b436
Signed-off-by: Paul Belanger <pabelanger@redhat.com>
Pipelines are closely associated with their tenants, so that
pipeline managers may find the currently active layout. To ensure
that a Pipeline is always associated with exactly one tenant,
require the tenant when constructing the pipeline, and raise an
exception if we try to add the pipeline to a layout for a different
tenant object.
Change-Id: I9fafa0133d57c77ee172245d0897e375dcd0de15
The github provider was doing a very naive check for whether a PR was
able to be merged, simply checking if there was a current merge
conflict. We also want to make sure that if there are any branch
protection requirements these are also checked because if zuul tries to
merge a patch via github that doesn't meet the branch protection it will
be rejected and cause zuul to report a failure to users.
Change-Id: I66d54c2603c462cb029510dd4e37fc89afeb200d
Signed-off-by: Jamie Lennox <jamielennox@gmail.com>
Co-authored-by: Tobias Henkel <tobias.henkel@bmw.de>
This change make the job name in the job report comments of github
driver as a hotlink to instead of using a log url directly. This can
make the report comments more brief.
Change-Id: I19fb8ffbc153230b7f8eedfcd5ac15ec81a66c72
Using the pipeline description as the status description seemed like a
good idea at first, however some sites will make use of lengthy
descriptions which can overflow GitHub's 1024 byte limit on the status
description. To keep the description smaller, simplify by using a simple
template of "{pipeline_name} status: {status}". This could still
overflow if a site uses an unusually large name for a pipeline, so do
some effort to prevent such an overflow. In testing, it seems anything
over 140 characters seems to trip the size limit in GitHub, which
doesn't make a lot of sense, but it's what we have to deal with.
Change-Id: I5255f6cf307f54cd1366f6cb325ed66c1bc4bc27
Story: 2001138
Task: 4854
Also limit it to the specific github host that generated the event. This
corrects an issue seen live where the github driver attempts to report
on gerrit events causing a failure, which will eject the change without
reporting it on the gerrit side.
Testing this required adding start reporters, so that a traceback in
reporting a start to the wrong driver would cause the change to get
dequeued before the jobs run, and before the success is reported.
Also a new test needed to be added to make sure multiple connections
using the github driver are able to correctly report to the right github
server.
Change-Id: Ic4630dd46d72fbeb45f691615a99ef1299a8cd05
This is the last entry standing in the 'zuul' section, it seems
like it may be more appropriate here.
Change-Id: I48d2c4d69025a93a61c4d4fc64574545e3215e2c
This adds the capability for one github project pr to depend on another
project's PR, by marking it so in the pull request body. This means we
need to be able to react to editing of the pull request body to see if a
dependency was created or removed.
Reverse searching for a change that needs the current change is
implemented via a github search of (open) pull request bodies.
Various bits of how change objects are created and cached needed to be
fixed to support a change being added while adding another change.
Also put a try around the webhook event handling, so that a traceback,
such as when a dependency loop is detected, can be gracefully logged
instead of the whole thing shutting down.
Change-Id: Ia32e6cc70b163f7e32e74d88f0a1f9b4e3255320
Story: 2000774
Task: 4686
Also fix reporting on push along the way, as it turns out this wasn't
really working to begin with.
Within the fake github connection class, keep track of all the
reportings that have happened. These are just a simple list of tuples
about the reports, and we're only checking them in one test case for
now. This may be more useful down the road in other tests.
Change-Id: I85bb7a580c21f0762187f2176b74983baf6e8509
For a push event that has no associated pull request, we cannot report
by way of a comment, label, or merge. We can only set the status.
Clarify this in the docs as well.
Change-Id: Ibd903fe0496958d869eafd2165f381305ca07404
Story: 2000774
Task: 4658
Make it possible to set the github status link to something other than the
zuul server status_url. This allows the github status to link to the log output
for the jobs. If set, this url will be run through the same formatter
as the url_pattern used for comment links. If unset, it will fall back to the
previous behaviour of using the zuul server status_url.
Handle status-url from the reporter config and the zuul config the same
way and remove the hard coded change url format from zuul.conf. If a
user wants to specify the change id in the status url they should use
a template string in the same way as configuring a reporter.
Co-Authored-By: Jamie Lennox <jamielennox@gmail.com>
Change-Id: I5f2ff35bb38426ddcd7f65798b4f114256a54847
The pipeline can be obtained by way of the item which was also being
passed in.
Change-Id: Ibcafa7daef3567c840a23defcab6fd40a9c6b206
Signed-off-by: Jesse Keating <omgjlk@us.ibm.com>
James E. Blair