When using gerrit topics containing white spaces, zuul fails to find the
changes contained in the topic because the query it builds does not
enclose the topic in quotes. So only the first word of the topic is
considered by the gerrit driver. Fixing this by quoting the topic in the
query.
Change-Id: I99d2890d317fb8424740e25d166d17381f1319c8
This adds support for the hashtags-changed trigger event as well
as using hashtags as pipeline and trigger requirements.
Change-Id: I1f6628d7c227d12355f651c3c822b06e2d5c5562
We receive some events from Gerrit (such as the currently-unhandled
hashtags-changed or currently-partially-handled topic-changed) events
without a patchset, only a change number. This makes sense from a
data model perspective, but is not useful in Zuul since we must have
a patchset to actually enqueue something.
Currently we will handle these events by querying for the change and
storing the results in the change cache under the key (change, None).
Note that the change itself will have the current patchset assigned
as a result of the query. But this "upgrade" of information applies
only to the change itself, not the change key, so this up-to-date
change object will never be used for anything productive in Zuul.
In order to actually trigger off of these events, let's "upgrade" the
change key as well after performing the query. To do that, we will
use a new change key with patchset information when storing the change
object in the cache if our initial change key lacked patchset info but
the resulting change has it. But only in the specific case where we
are performing our first query after receiving an event. We will also
update the event with the same patchset information. This should
mean that after receiving an event and performing the initial query,
we should be guaranteed to have patchset information about the change
and therefore Zuul should never see a (change, None) tuple for a
change key any more.
* (Change keys have more information than that tuple, but those are
the relevant parts for this change.)
Change-Id: I6f077376044ffbbd3853e2050c507f449da77962
The fix in I473ba605decb136cd527308a63f16a5e548697fb did not fully solve
the problem with the new Github default merge modes.
In case the branch cache already contains the new merge modes, but the
tenant project config (TPC) still only supports the old subset of modes,
dynamic layout creation will fail saying that the new default merge mode
is not supported.
To fix this we will supply a new parameter with valid merge modes from
the TPC when getting the project default branch instead of getting the
project merge modes directly from the branch cache. Based on the list of
valid modes the driver can then select the best default merge mode.
This change also updates the model API upgrade test v17 -> v18 to cover
this case.
Change-Id: Ibc5645d4725f0ec31cb7ab18d4500452d866166a
This refactors the the config error handling based on nested
context managers that add increasing amounts of information
about error locations. In other words, when we start processing
a Job object, we will:
with ParseContext.errorContext(info about job):
do some stuff
with ParseContext.errorContext(info about job attr):
do some stuff regarding the job attribute
with ParseContext.errorContext(next job attr):
do stuff with a different attr
We store a stack of error contexts on the parse context, and at any
point we can access the accumulator for the most recent one with
ParseContext.accumulator in order to add a warning or error. If we
have an attribute line number, we'll use it, otherwise we'll just
use the object-level information.
We also collapse the exception hnadlers into a single context
manager which catches exceptions and adds them to the accumulator.
This lets us decide when to catch an exception and skip to the next
phase of processing separately from where we narrow our focus to
a new object or attribute. These two actions often happen together,
but not always.
This should serve to simplify the configloader code and make it
easier to have consistent error handling within.
Change-Id: I180f9b271acd4b62039003fa8b38900f9863bad8
When updating topic dependencies we only consider the current patchset.
However, when there are changes that have a git-level dependency to an
outdated patchset and those dependencies are in a different topic we can
run into max. recursion depth issues.
To fix this problem, we will keep a history of topics that we've already
processed and not call `getChangesByTopic()` again when we've already
seen a topic.
Change-Id: I6c15f502cfd593a44d7adda930670692151b6713
This adds a configuration warning (viewable in the web UI) for any
regular expressions found in in-repo configuration that can not
be compiled and executed with RE2.
Change-Id: I092b47e9b43e9548cafdcb65d5d21712fc6cc3af
This extends the previous change to include project default branch
support for the Gerrit driver as well as GitHub.
Change-Id: I2b1f6feed72277f5e61a2789d8af5276ee4c7b05
This updates the Gerrit driver to match the pattern in the GitHub
driver where instead of specifying individual trigger
requirements such as "require-approvals", instead a complete ref
filter (a la "requirements") can be embedded in the trigger
filter.
The "require-approvals" and "reject-approvals" attributes are
deprecated in favor of the new approach.
Additionally, all require filters in Gerrit are now available as
reject filters.
And finally, the Gerrit filters are updated to return
FalseWithReason so that log messages are more useful, and the
Github filters are updated to improve the language, avoid
apostraphes for ease of grepping, and match the new Gerrit
filters.
Change-Id: Ia9c749f1c8e318fe01e84e52831a9d0d2c10b203
The test helper method that handles fake gerrit queries had a bug
which would cause the "topic:" queries to return all open changes.
When we correct that, we can see, by virtue of a newly raised
execption that there was some unexercised code in getChangesByTopic
which is now exercised. This change also corrects the exception
that is raised when mutating a set while iterating over it.
Change-Id: I1874482b2c28fd1082fcd56036afb20333232409
Before getting a change by key we need to convert the reference to a
`ChangeKey` object.
Traceback (most recent call last):
File "/opt/src/zuul/zuul/scheduler.py", line 2423, in process_tenant_trigger_queue
self._forward_trigger_event(event, tenant)
File "/opt/src/zuul/zuul/scheduler.py", line 2538, in _forward_trigger_event
pipeline.manager.updateCommitDependencies(change, event)
File "/opt/src/zuul/zuul/manager/__init__.py", line 924, in updateCommitDependencies
for dep in source.getChangesByTopic(change.topic):
File "/opt/src/zuul/zuul/driver/gerrit/gerritsource.py", line 171, in getChangesByTopic
git_change = self.getChange(git_key)
File "/opt/src/zuul/zuul/driver/gerrit/gerritsource.py", line 86, in getChange
return self.connection.getChange(change_key, refresh=refresh,
File "/opt/src/zuul/zuul/driver/gerrit/gerritconnection.py", line 791, in getChange
if change_key.connection_name != self.connection_name:
AttributeError: 'str' object has no attribute 'connection_name'
Change-Id: I20663b538ccb48bc13191a134caf3326b6f5b76e
A user pointed out that approving a parent change [0] did not enqueue a
child change [1] that had all necessary votes. On inspection of the
logs [2] it isn't clear why this happened as at least one other change
was found.
One thing I noticed is that 'Depends-on' instead of 'Depends-On' was
used but both Gerrit and zuul seem to treat this string case
insensitively so that shouldn't matter.
Update our logging to aid in further debugging should this happen again.
In particular log already known following changes which comes from our
change cache and newly discovered following changes which comes from the
code review system. Hopefully this will help identify where we're not
finding the info we expect. Additionally record the number of possible
depending changes found by Gerrit. This should give us an indication for
whether or not Gerrit returning the information we expect.
[0] https://review.opendev.org/c/openstack/devstack/+/860795
[1] https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/857031
[2] https://paste.opendev.org/show/b6P1Uy2VjAMZOqtBbwyz/
Change-Id: Iad7fbbc3476ef1ddfd366e93b8505ed0fbdc1dc0
The default merge mode is 'merge-resolve' because it has been observed
that it more closely matches the behavior of jgit in Gerrit (or, at
least it did the last time we looked into this). The other drivers
are unlikely to use jgit and more likely to use the default git
merge strategy.
This change allows the default to differ based on the driver, and
changes the default for all non-gerrit drivers to 'merge'.
The implementation anticipates that we may want to add more granularity
in the future, so the API accepts a project as an argument, and in
the future, drivers could provide a per-project default (which they
may obtain from the remote code review system). That is not implemented
yet.
This adds some extra data to the /projects endpoint in the REST api.
It is currently not easy (and perhaps not possible) to determine what a
project's merge mode is through the api. This change adds a metadata
field to the output which will show the resulting value computed from
all of the project stanzas. The project stanzas themselves may have
null values for the merge modes now, so the web app now protects against
that.
Change-Id: I9ddb79988ca08aba4662cd82124bd91e49fd053c
This adds a pipeline queue setting to emulate the Gerrit behavior
of submitWholeTopic without needing to enable it site-wide in Gerrit.
Change-Id: Icb33a1e87d15229e6fb3aa1e4b1ad14a60623a29
The drivers are expected to populate the change cache before
passing trigger events to the scheduler so that all the difficult
work is done outside the main loop. Further, the cache cleanup
is designed to accomodate this so that events in-flight don't have
their change cache entries removed early.
However, at several points since moving the change cache into ZK,
programming errors have caused us to encounter enqueued changes
without entries in the cache. This usually causes Zuul to abort
pipeline processing and is unrecoverable.
We should continue to address all incidences of those since they
represent Zuul not working as designed. However, it would be nice
if Zuul was able to recover from this.
To that end, this change allows missing changes to be added to the
change cache.
That is primarily accomplished by adjusting the Source.getChange
method to accept a ChangeKey instead of an Event. Events are only
available when the triggering event happens, whereas a ChangeKey
is available when loading the pipeline state.
A ChangeKey represents the minimal distinguishing characteristics
of a change, and so can be used in all cases. Some drivers obtain
extra information from events, so we still pass it into the getChange
method if available, but it's entirely optional -- we should still
get a workable Change object whether or not it's supplied.
Ref (and derived: Branch, Tag) objects currently only store their
newrev attribute in the ChangeKey, however we need to be able to
create Ref objects with an oldrev as well. Since the old and new
revs of a Ref are not inherent to the ref but rather the generating
event, we can't get that from the source system. So we need to
extend the ChangeKey object to include that. Adding an extra
attribute is troublesome since the ChangeKey is not a ZKObject and
therefore doesn't have access to the model api version. However,
it's not too much of a stretch to say that the "revision" field
(which like all ChangeKey fileds is driver-dependent) should include
the old and new revs. Therefore, in these cases the field is
upgraded in a backwards compatible way to include old and newrev
in the standard "old..new" git encoding format. We also need to
support "None" since that is a valid value in Zuul.
So that we can continue to identify cache errors, any time we encounter
a change key that is not in the cache and we also don't have an
event object, we log an error.
Almost all of this commit is the refactor to accept change keys
instead of events in getChange. The functional change to populate
the cache if it's missing basically consists of just removing
getChangeByKey and replacing it with getChange. A test which deletes
the cache midway through is added.
Change-Id: I4252bea6430cd434dbfaacd583db584cc796dfaa
In order to save a list of ltimes for each connection we need a source
interface to get the current ltime of a project branch cache.
Change-Id: If01db0698024beeed813d2c9910651c757377865
The Gerrit change cache is no longer nested (previously by change number
and patchset) and will use the flat cache structure provided by the
Zookeeper change cache.
Change-Id: I3fa7e8f2b865765d8f6c8fee35c1ad16b70eb45b
In order to cache changes in Zookeeper we need to make change objects
JSON serializable. This means that we can no longer reference other
change objects directly. Instead we will use a cache key consisting of
the connection name and a connection specific cache key.
Those cache keys can be resolved by getting the source instance using
the connection name and then retrieving the change instance via the new
`getChangeByKey()` interface.
The pipeline manager provides a helper method for resolving a list of
cache keys. Cache keys that where resolved once are also cached by the
manager as long as the reference is needed by any change in the
pipeline. The cache will be cleaned up at the end of a run of the queue
processor.
Until we can run multiple schedulers the change cache in the pipeline
manager will avoid hitting Zookeeper every time we resolve a cache key.
Later on when we have the pipeline state in Zookeeper we probably want
to clear the change cache in the pipeline manager at the end of the
queue processor. This way we make sure the change is recent enough when
we start processing a pipeline.
Change-Id: I09845d65864edc0e54af5f24d3c7be8fe2f7a919
On GitHub we cannot reliably update all information that's needed for
doing a canmerge check using events. Namely completely missing events
on branch protection changes and ambiguous status events that might
match several changes due to its data model to have statuses on the
commit instead the pr. This was no problem in the past since this
information was only used during the enqueue phase which is directly
after the event preprocessing phase.
However with circular dependencies we re-do the can merge check just
before merging again and need to act on recent data. Therefore add an
allow_refresh flag that makes it possible to refresh the volatile
parts of the data we don't get events for. This is only used on GitHub
for now as the other drivers are either correctly updating their
states using events or didn't yet optimize to not do api calls within
the main loop yet (pagure).
Change-Id: I89ff158642fe32c5004ef62c2e25399110564252
Gerrit's refs are left-padded with zeroes if the change's number is
below 10, for example 9,1 -> refs/changes/09/9/1.
Fix an error in computing the change's ref when the change's number is
below 10.
Modify the test framework to emulate Gerrit ref naming convention more
faithfully in tests.
Change-Id: I54a3c3dcaa9a08cff97bfd701e28b6f240fdb77d
We observe a few exceptions in our system while iterating over
getCachedChanges [1]. This is caused by multithreaded appending new
changes to the change cache. In order to prevent this exception we
need to make lists from the values which are safe to iterate even if
the backing dict gets updated. While we're at it fix it for all other
drivers as well.
[1] Trace:
2020-03-06 12:39:51,429 ERROR zuul.GerritEventConnector: Exception moving Gerrit event:
Traceback (most recent call last):
File "/opt/zuul/lib/python3.6/site-packages/zuul/driver/gerrit/gerritconnection.py", line 284, in run
self._handleEvent()
File "/opt/zuul/lib/python3.6/site-packages/zuul/driver/gerrit/gerritconnection.py", line 243, in _handleEvent
self._getChange(event)
File "/opt/zuul/lib/python3.6/site-packages/zuul/driver/gerrit/gerritconnection.py", line 277, in _getChange
refresh=True, event=event)
File "/opt/zuul/lib/python3.6/site-packages/zuul/driver/gerrit/gerritconnection.py", line 742, in _getChange
self._updateChange(change, event, history)
File "/opt/zuul/lib/python3.6/site-packages/zuul/driver/gerrit/gerritconnection.py", line 820, in _updateChange
self.sched.onChangeUpdated(change, event)
File "/opt/zuul/lib/python3.6/site-packages/zuul/scheduler.py", line 1703, in onChangeUpdated
for other_change in source.getCachedChanges():
RuntimeError: dictionary changed size during iteration
Change-Id: I30396997441c7e7756bfb81be708389d6331bf19
Prior to this change we looked for the current change/PR's url in any
other change/PR's message body. This meant any cross referencing of urls
would create further lookups to determine if there was a real dependency
there. Restrict this a bit more to require the Depends-On string too
when searching to limit the number of spidering queries that must be
done.
This is particularly useful for the github driver because queries are
expensive there and may be rate limited.
Change-Id: Ie49fe1a72dc844b14003d942684fd3d2a9478d21
This adds support for performing queries and git clones over HTTP
(reporting over HTTP was already supported). This will happen
automatically for any connection with a password configured.
Otherwise, the SSH connection will be used as before.
Change-Id: I11920a13615de103eb3d8fb305eacbbcb30e5e40
In case the values being queried in 'gerrit query' commands contain
special characters, they should be quoted - see:
https://gerrit-review.googlesource.com/Documentation/user-search.html#_argument_quoting
In particular, this fixes getChangesDependingOn when the Gerrit URL
contains a port specification and therefore a ':'.
This doesn't bother adding quoting for fields like "change:" which
certainly shouldn't have any such special chars.
In order to get this to pass tests deterministically, it was
necessary to fix an off-by-one error in the
FakeGerritConnection.simpleQuery logic that strips off the parens
from a compound query with "OR"s.
Change-Id: Idbd6b03f0d86cdf24290fa5020686f684fc7e4e3
Gerrit 2.16 change URLs look like:
"{baseurl}/c/{project}/+/{change_no}/". Update the regex so it matches those
too.
Suggested by tristanC on #zuul
Change-Id: I5eabe25bc26ce4a2d829b1a77d2023fd19c1068f
This is a follow-up to https://review.openstack.org/433748
where we need to cast all json access to change and patchset number
as str. At least the refresh_deps logic relies on python eq check,
and Zuul doesn't dequeue child changes when parent change get
updated in Gerrit-2.14.
Here is diagnostic DEBUG informations
(2 depends-on 1, patchset 1,13 doesn't dequeue change 2):
Change <Change 1,13> is a new version of <Change 1,12>,
removing <QueueItem for <Change 1,12> in check>
Canceling builds behind change: <Change 1,12> because it is being removed.
Cancel jobs for change <Change 1,12>
Removing change <Change 1,12> from queue
[snip]
Starting queue processor: check
Checking for changes needed by <Change 2,5>:
Change <Change 2,5> needs change <Change 1,12>:
Needed change is already ahead in the queue
Change-Id: Id73a479d155fa6d9b2c562869cae6c82dd065911
In GitHub with many app installations the getChangesDependingOn
currently iterates over all installations within the system and fires
up a search query. In larger deployments this can sum up to hundreds
of queries for a single parent-change-enqueued event. At least for
multi-tenant deployments this can be greatly improved when limiting
the scope just to the installations related to the tenant. With this
improvement in most tenants this can be accomplished with a handful of
requests then.
Change-Id: Ibfad750a685d2ec58f3e452bfe2098bbdc294e37
This patch aims to fix issue where such a depends-on won't match.
Depends-On: https://sftests2.com/r/#/c/1/
Indeed here Gerrit is hosted behind a reverse proxy under /r.
The regexp matching fail. Also the connection source lookup
may fail if Gerrit web interface is not connection.server or
connection.canonical_hostname so this path defines connection.baseurl
as a new lookup condition.
connection.baseurl is then used to build the regexp in order
to set the path prefix of the Gerrit ui.
Story: 2002080
Co-Authored-By: Nicolas Hicher <nhicher@redhat.com>
Change-Id: Ie764bfc814b821636d447f82147a7d76e0511d2f
Add support for "scoped" autohold requests, making it possible
to create requests that match a given ref filter and hold nodes
for build that matches the filter.
Introduce two new arguments to `zuul autohold`: --ref and --change,
with --ref accepting regex that builds are matched against, and
--change being a shortcut that creates a filter for the specific change.
Change-Id: I801ba1af4b1bda46abff07791dd96828f2738621
Some folks may copy the URL out of their brower location field
rather than using the permalink when creating a Gerrit Depends-On,
so support that as well.
Change-Id: Ie35fde41befcb0c5c062fcc0417afeed1cf60a77
When using a branch and pull model on a shared repository there are
usually one or more protected branches which are gated and a dynamic
number of temporary personal/feature branches which are the source for
the pull requests. These temporary branches while ungated can
potentially include broken zuul config and therefore break the global
tenant wide configuration.
In order to deal with this model add support for excluding unprotected
branches. This can be configured on tenant level and overridden per
project.
Change-Id: I8a45fd41539a3c964a84142f04c1644585c0fdcf
Push and ref-updated events do not have an associated change or
pull-request. Thus, requirements that are specific to changes or
pull-requests cannot possibly be met.
To make this work, Ref filters should only be applied to changes that
come from a matching connection. This was not previously enforced when
filters became connection specific.
Change-Id: I5d67bef264db0ad5ba3a7180ce74e3670ba822ce
Story: 2000774
Task: 4624
As we expand the Github driver, we're seeing a need to specify driver-specific
pipeline requirements. To accomplish this, bump the require/reject pipeline
keywords down a level underneath connection names. This lets users specify
per-source pipeline requirements.
This adds new API methods for sources to create the new pipeline filters
(by returning instances or subclasses of RefFilter, which used to be called
ChangeishFilter).
This change also creates and/or moves driver-specific subclasses of EventFilter
and TriggerEvent in(to) their respective drivers.
Change-Id: Ia56c254e3aa591a688103db5b04b3dddae7b2da4
So that we can store the canonical hostname. Also use this to
find and store the connection name to keep the initializer signature
small.
Story: 2000953
Change-Id: Ie10f86ff3412016b411bcc511b4d9ad3af163d61
This is the start of the implementation of:
http://lists.openstack.org/pipermail/openstack-infra/2017-March/005208.html
It lets us associate a canonical hostname with every connection
that we will later use to uniquely identify source code repos.
Story: 2000953
Change-Id: I7f2e64944d46f304e63a54078e682fd5e1682f27
This had a tail in some places and not others. Standardize on the
tail version since it's not part of the connection interface, and
instead is a private method shared between the gerrit connection
and source.
Change-Id: I614fd74177bb0f832f02f44c094109faff39373b
This change, while substantial, is mostly organizational.
Currently, connections, sources, triggers, and reporters are
discrete concepts, and yet are related by virtue of the fact that
the ConnectionRegistry is used to instantiate each of them. The
method used to instantiate them is called "_getDriver", in
recognition that behind each "trigger", etc., which appears in
the config file, there is a class in the zuul.trigger hierarchy
implementing the driver for that trigger. Connections also
specify a "driver" in the config file.
In this change, we redefine a "driver" as a single class that
organizes related connections, sources, triggers and reporters.
The connection, source, trigger, and reporter interfaces still
exist. A driver class is responsible for indicating which of
those interfaces it supports and instantiating them when asked to
do so.
Zuul instantiates a single instance of each driver class it knows
about (currently hardcoded, but in the future, we will be able to
easily ask entrypoints for these). That instance will be
retained for the life of the Zuul server process.
When Zuul is (re-)configured, it asks the driver instances to
create new connection, source, trigger, reporter instances as
necessary. For instance, a user may specify a connection that
uses the "gerrit" driver, and the ConnectionRegistry would call
getConnection() on the Gerrit driver instance.
This is done for two reasons: first, it allows us to organize all
of the code related to interfacing with an external system
together. All of the existing connection, source, trigger, and
reporter classes are moved as follows:
zuul.connection.FOO -> zuul.driver.FOO.FOOconnection
zuul.source.FOO -> zuul.driver.FOO.FOOsource
zuul.trigger.FOO -> zuul.driver.FOO.FOOtrigger
zuul.reporter.FOO -> zuul.driver.FOO.FOOreporter
For instance, all of the code related to interfacing with Gerrit
is now is zuul.driver.gerrit.
Second, the addition of a single, long-lived object associated
with each of these systems allows us to better support some types
of interfaces. For instance, the Zuul trigger maintains a list
of events it is required to emit -- this list relates to a tenant
as a whole rather than individual pipelines or triggers. The
timer trigger maintains a single scheduler instance for all
tenants, but must be able to add or remove cron jobs based on an
individual tenant being reconfigured. The global driver instance
for each of these can be used to accomplish this.
As a result of using the driver interface to create new
connection, source, trigger and reporter instances, the
connection setup in ConnectionRegistry is much simpler, and can
easily be extended with entrypoints in the future.
The existing tests of connections, sources, triggers, and
reporters which only tested that they could be instantiated and
have names have been removed, as there are functional tests which
cover them.
Change-Id: Ib2f7297d81f7a003de48f799dc1b09e82d4894bc
Zuul