The following problem was observed:
Change A depends-on change B, which is in turn the tip of a
patch series of several changes.
Drivers warm the change cache on events by querying information
about changes related to those events. But they don't process
depends-on headers, which means most drivers only warm one change,
and while the gerrit driver will follow other types of dependency
links which are unique to it, it stops at depends-on boundaries.
So in the example above, the only change in the cach which was warm
was change A.
The triggering event was enqueued, forwarded, and processed by
two responding pipelines simultaneously on two executors.
Each of them noticed the depends-on link and started querying gerrit
for change B and its dependencies. One of the schedulers was about
1 second ahead of the other in this process.
In the gerrit driver, there is a two phase process for updating
changes. First the change itself is updated in the normal way
common to all drivers, and then gerrit-specific dependency links
are updated. That means the change is added to the change cache
with no dependencies, then mutated to add dependencies later.
The first scheduler added change B to the cache with no dependencies.
The second scheduler saw the update and refreshed its copy of B.
The second scheduler begin updating B, saw that the ltime of its
copy of B was sufficiently new it didn't need to update the cache
and stopped updating.
The second scheduler enqueued changes A and B, but no others in its
pipeline.
The first scheduler finished querying the stack of changes ending at
B, added them to the change cache, and mutated the entry for B in the
cache.
The first scheduler enqueued A, B, and the rest of the stack in its
pipeline.
The second scheduler updated its copy of B to include the new
dependencies.
The second scheduler ran a pipeline processor, noticed that B lacked
dependencies, and dequeued A and B, and reported an error to the user.
The good news is that Zuul noticed the mistake and dequeued the
changes.
To correct this, we will now collect all of the information about a
change and its gerrit-specific dependencies before writing any of
that information to the change cache. This means that in our example
above, the second scheduler would not have aborted its queries.
Eventually, both schedulers would end up with the same information
before enqueing anything.
This process is still not quite optimal, in that we will have multiple
schedulers racing to update the same changes at the same time, but
they are designed to deal with collisions like that, so it should
at least be correct.
A future area of work might be to investigate whether we can optimize
this case further.
Change-Id: I647c2b54a55789e521fca71c8c3814907df65da6
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
To protect Zuul servers from accidental DoS attacks in case someone,
say, uploads a 1k change tree to gerrit, add an option to limit the
dependency processing in the Gerrit driver and in Zuul itself (since
those are the two places where we recursively process deps).
Change-Id: I568bd80bbc75284a8e63c2e414c5ac940fc1429a
This allows users to set a maximum value for the active window
in the event they have a project that has long stretches of
passing tests but they still don't want to commit too many resources
in case of a failure.
We should all be so lucky.
Change-Id: I52b5f3a9e7262b88fb16afc4520b35854e8df184
So far, when the scheduler re-enqueued a change that was missing
dependencies, it also reported the Github check-run as cancelled but did
not report start as the re-enqueued item was added as a "quiet" item.
The check-run on Github was still marked as success after the item
finished. But until then it appeared as cancelled even if the change was
successfully re-enqueued.
To fix this we'll not call the dequeue reporters when the change could
be re-enqueued. The dequeued item will still be reported to the
database though.
Change-Id: Iea465ca1d9132322b912f7723e3ae41a8c6d3002
This change fixes a bug related to circular dependency resolution where
non-cycle changes could be enqueued between changes of the same cycle.
This violated the invariant assumption that changes of the same
dependency cycle are enqueued in sequence. This could cause the pipeline
processor to loop indefinitely under certain conditions.
The idea behind this fix is to treat all unprocessed dependencies of
other changes in the same cycle as if they were direct dependencies of
the current change. By that we will try to enqueue dependencies of any
change in the cycle ahead of the whole cycle.
Change-Id: I3eeb9fc9f6fca73982ce01d180dca9f58868bff3
Most of a change's attributes are tenant-independent. This however is
different for topic dependencies, which should only be considered in
tenants where the dependencies-by-topic feature is enabled.
This is mainly a problem when a project is part of multiple tenants as
the dependencies-by-topic setting might be different for each tenant. To
fix this we will only return the topic dependencies for a change in
tenants where the feature has been activated.
Since the `needs_changes` property is now a method called
`getNeedsChanges()`, we also changed `needed_by_changes` to
`getNeededByChanges()` so they match.
Change-Id: I343306db0abbe2fbf98ddb3f81b6d509eaf4a2bf
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 report message "This change depends on a change that failed to
merge" (and a similar change for circular dependency bundles) is
famously vague. To help users identify the actual problem, include
URLs for which change(s) caused the problem so that users may more
easily resolve the issue.
Change-Id: Id8b9f8cf2c108703e9209e30bdc9a3933f074652
If a cycle of dependencies is attempted to be enqueued into a pipeline
and at least one of the participating projects has a per-branch change queue
and the changes in the cycle are in different branches, it can be confusing
for users why the changes were not enqueued. This is even more likely to
happen with implicit cyclic dependencies such as those from Gerrit's
submitted-together feature (but can happen with any driver).
To aid users in this situation, report this situation back to the code
review system.
Change-Id: I26174849deab627b2cf91d75029c5a2674cc37d6
In case a cycle and another item ahead failed in the same pipeline run,
the circular dependency wasn't correctly reset in some cases.
Instead of resetting the items in the cycle and moving it behind the
nearest non-failing item (nnfi), it was moved behind the failing item
ahead. Only when the failing item reached the head of the queue it lead
to a gate reset which corrected the pipeline state. Since the gate reset
was delayed this can lead to wasted build resources.
To fix this we will only look at failing bundle items if we are behind
the nnfi.
Change-Id: I3f11a917028b2ad58403a81d6542c6166bb3a663
Provide separate methods for resolving change key and references to
change objects as suggested in
https://review.opendev.org/c/zuul/zuul/+/816460
Change-Id: I433b1cba3b0eec362a686682ce508fb82c3813ae
The /api/tenant/{tenant}/status includes information about
pipelines' type (ie manager) and triggers drivers.
Change-Id: I6abe4bdcf175426908c321a2a298b9743f5ea80b
The change queue state is store in the following Zookeeper path:
/zuul/<tenant>/pipeline/<pipeline>/queue/<layout-uuid>/<queue-uuid>
Change-Id: I0a64bd9adc7b9f8f7a775280bb7a01ace22baac4
The onChangeUpdated method is called every time a change is updated.
Its purpose is to iterate over every change in the cache and, if the
updated change is a commit dependency (Depends-On) of the other change,
to set a flag telling the queue processor that changes' dependencies
are out of date and need to be refreshed. The idea is that if this
happens, the queue processor should notice the dependencies no longer
match and dequeue it.
This became quite slow with the change cache in ZK.
Instead, we can do the following:
* Make sure that every event regarding a change is forwarded to every
pipeline with that change in it (including non-live changes).
* When processing pipeline trigger events, refresh the dependencies
of any changes that need to be updated.
There are a few cases that need to be covered in that last point
(but remember, all of this only applies to Depends-on footers which
are in the commit_needs variable):
* Changes to depends-on in gerrit necessarily mean a new patchset
of the change with the depends-on header, so in the B->A situation,
if B is updated, it will be re-enqueued.
* Note that normally the orignal B->A series is dequeued, unless
dequeue-on-new-patchset is false. In that case it will remain
and we will have B->A and B1->A, which is what the user asked for.
To handle this case, we only need to refresh the deps of the change
which is updated.
* In a C->B->A situation in gerrit, if B is updated then regardless
of whether dequeue-on-new-patchset is true or false, the series should
be dequeued because the dependency changed (and so the test setup
is incorrect).
To handle this case, we need to refresh the deps of any change whose
commit_needs point to the updated change.
* Changes to depends-on in github don't create new patchsets, so we
only need to refresh the deps of the change which is updated. Because
the changes it points to are still valid changes, they will have
their own deps refreshed in place.
Change-Id: Ifc40ff5583a14c7f015356b963e1ba354974e1c8
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
In order to reduce the chance of a partially merged cycle we make sure
all items in the cycle can still be merged prior to starting reporting.
Change-Id: I22a9e8b76325e09b6a0f6bc563840f264c12dfae
Allow Zuul to process circular dependencies between changes. Gating of
circular dependencies must be explicitly enabled on a per tenant or
project basis.
In case Zuul detects a dependency cycle it will make sure that every
change also include all other changes that are part of the cycle. However
each change will still be a normal item in the queue with its own jobs.
When it comes to reporting, all items in the cycle are treated as one
unit that determines the success/failure of those changes.
Changes with cross-repo circular dependencies are required to share the
same change queue.
Depends-On: https://review.opendev.org/#/c/643309/
Change-Id: Ic121b2d8d057a7dc4448ae70045853347f265c6c
We have several large projects with most of the time long gate
queues. Those projects typically work on master and few release
branches where the changes in the release branches are more important
to the changes for master. Currently all of those changes are queued
up in a shared gate queue which makes the process of getting changes
into the release branches very slow especially if occasional gate
resets are involved. In order to improve this allow specifying the
change queues per branch so we can queue up the changes for each
release branch in a separate queue.
This is done by adding a new config element 'queue' which can be
configured to work on a per branch level.
Change-Id: Ie5c1a2b8f413fd595dbaaeba67251da14c6b4b36
This is designed to handle the case where we want:
* The pipeline to be triggered by changes so results report back to
the change.
* Triggered on change merged.
* Jobs with file matchers so that if a subsystem is changed, only the
deployment job for that subsystem is run.
* Every change is processed in strict sequence.
This is designed to accomodate a deployment pipeline with the above
constraints.
The pipeline manager hierarchy is getting complicated; mark the base
class as abstract, and also move the shared-queue methods into an
intermediate abstract class. These are shared by both serial and
dependent managers.
Change-Id: I3c5f3b2f6298292c5c25665923e3a10b07be5419
When the queue for a project is changed during reconfiguration the
dependent pipeline manager was using the queue of the item
ahead in the old queue.
This lead to items ending up in the wrong queues and caused the
following exception in the run handler:
2019-11-28 13:33:47,313 zuul.Scheduler ERROR Exception in run handler:
Traceback (most recent call last):
File "/tmp/zuul/zuul/scheduler.py", line 1145, in run
while (pipeline.manager.processQueue() and
File "/tmp/zuul/zuul/manager/__init__.py", line 914, in processQueue
item, nnfi)
File "/tmp/zuul/zuul/manager/__init__.py", line 898, in _processOneItem
priority = item.getNodePriority()
File "/tmp/zuul/zuul/model.py", line 2684, in getNodePriority
return self.pipeline.manager.getNodePriority(self)
File "/tmp/zuul/zuul/manager/dependent.py", line 101, in getNodePriority
return items.index(item)
ValueError: <QueueItem 0x7f02583af4e0 for <Change 0x7f02583a9cf8 org/project2 2,1> in gate> is not in list
The issue is fixed, by ignoring the given existing queue in the
dependent pipeline manager, since it's always possible to get the
correct queue from the pipeline itself.
Change-Id: Ia5b1b58377e4420b9ab1440c0b9f67cb15967263
This change changes the dependency cycle error handling in order
to report the error to the user via the standard report mechanics.
Change-Id: I7cdbd9b7b48fd612b2887007b70acedd7a8e3113
The pipeline processing is very complex and it's helpful to be able to
track events through the pipeline processing.
Change-Id: I455cdcf34128a2c067c10425736bf1d195df6fa8
As a preparation to further attach the event id to the logs we need to
add the trigger event to the queue items so we can use it later.
Change-Id: I9cc87e5be9a391a53a0dcfd92a990ca848616ca0
When calculating relative_priority for independent pipelines,
use shared change queues just as is done for dependent pipelines.
To implement this, we now calculate shared change queues for all
pipelines, not just dependent ones, though we don't use those
queues for any purpose other than this.
Change-Id: I59b1090ca1f4fcc72276445e6ff4c5cf4f2f5030
Add a relative_priority field to node requests and continuously
adjust it for each queue item based on the contents of queues.
This allows for a more fair distribution of build resources between
different projects. The first item in a pipeline from a given
project (or, in the case of a dependent pipeline, group of projects)
has equal priority to all other first-items of other projcets in
the same pipeline. Second items have a lower priority, etc.
Depends-On: https://review.openstack.org/620954
Change-Id: Id3799aeb2cec6d96a662bfa394a538050f7ea947
We had cases where zuul used unmerged job descriptions to a trusted
parent job (change A) in non related downstream jobs (change B) not
having zuul.yaml changes. This happened if the trusted parent job is
not defined in the same config repo as the pipeline. E.g. if change A
adds a new post playbook an unrelated change B fails with 'post
playbook not found'. This is caused by the scheduler using the wrong
unmerged job definition of change A but the final workspace contains
the correct state without change A.
In case of change B there is no dynamic layout and the current active
layout should be taken. However it is taken directly from the pipeline
object in getLayout (item.queue.pipeline.layout) which doesn't have
the correct layout referenced at any time while the layout referenced
by the tenant object is correct.
Because the pipeline definition is in a different repository than the
proposed config repo change, when the dynamic layout is created for
the config repo change, the previously cached Pipeline objects are used
to build the layout. These objects are the actual live pipelines, and
when they are added to the layout, they have their Pipeline.layout
attributes set to the dynamic layout. This dynamic layout is then not
used further (it is only created for syntax validation), but the pipelines
remain altered.
We could go ahead and just change that to
item.queue.pipeline.layout.tenant.layout but this feels awkward and
would leave the possibility of similar bugs that are hard to find and
debug. Further pipeline.layout is almost everywhere just used to get
the tenant and not the layout. So this attempt to fix this bug goes
further and completely rips out the layout from the Pipeline object
and replaces it by the tenant. Because the tenant object is never
expected to change during the lifetime of the pipeline object, holding
the reference to the tenant, rather than the layout, is safe.
Change-Id: I1e663f624db5e30a8f51b56134c37cc6e8217029
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
A recent change to late-bind project-templates omitted updating
the dependent pipeline manager to look at both project and
project-template objects when building shared queues.
Add a test for this and a helper method to collect all of the
project and project-template objects for a project.
Change-Id: Id43089dac79a3f523939c59b71c98879390597ad
Store each independent project stanza as its own config object,
instead of collecting them all together and synthesizing a single
object from all of them.
This will later allow us to re-use unchanged project stanzas
across dynamic configuration changes.
This requires changes to the way project-pipelines are created.
Rather than creating them during configuration loading, they are
now created from the project and project-template config objects
when the job graph is frozen.
We used to create a fixed job list for every project-pipeline,
and in that job list, we would add implied branch matchers to
the project-pipeline job variants based on where the project
stanza was located. This means that if a 'project:' stanza
within a 'stable' branch listed a certain job, that project-pipeline
job variant would match the 'stable' branch because of the
implied branch matcher added to the project-pipeline job variant.
The same applies to template *invocations* -- that is, where a
project stanza has a 'templates:' attribute. The templates
themselves didn't have implied branch matchers.
All of that together means that now that we are keeping all of the
individual project stanzas around (instead of combining them at
configuration time) we can move the implied branch matchers from
the project-pipeline job variants to the project stanzas themselves.
Then, when we later dynamically construct the project config based
on the enqueued item, we can just use or ignore entire project
stanzas based on their implied branch matchers. We no longer need
to modify the project-pipeline job variants.
Change-Id: Ia984ae5c1a4825528a9fa3ea6eb254b75af9b4dd
This corrects a false-positive in the dependency cycle detection,
but only for the new URL-style depends-on headers. It does not
do so for the legacy gerrit headers.
We used a single history list to store all the changes we enqueued
ahead of a given change, but this meant that if there was more
than one path to a change, we would see it in the history on the
second traversal. Instead, when traversing the tree, use copies
of the history list at each stage so that it can be rewound when
going back up the tree. The second path to a change will not
trip the cycle detection, and will proceed on to the point where
it notices the change is already in the queue and return harmlessly.
Also, check whether the exact change is in the history, not just
the number, since numbers are no longer unique with multiple sources.
Also, fix a bug in the test_crd_cycle test which was causing the
test to always pass since the changes were never enqueued due to
missing approval requirements.
Change-Id: I3241f90a1d7469d433cfa176e719322203d4d089
Story: 2001427
Task: 6133
* The "Adding .. event" log messages are redundant -- whatever is adding the
event logs it just as well, and it makes filtering for Scheduler messages
difficult. Remove them.
* The gerrit/github connections log both the driver and connection name.
the driver can be inferred from the connection, and very frequently
this just ends up saying "Scheduling gerrit event from gerrit" which
is weird. Remove the driver name.
* Make the pipeline logs say which pipeline they are for. This is way more
useful than the manager name (which is implied by the pipeline name anyway).
Change-Id: I8fcaef87ddcd8428776ee76a4519e4764f2d9c5b
Previously one could not speculatively add a project to a dependent
pipeline if the desired queue did not exist. This change handles this
special case by creating a dynamic queue for the use of the speculative
change. If the change merges, then the queue will exist for any future
change.
Introduces a new ChangeQueue attribute, 'dynamic', to help
DependentPipelineManager objects determine whether a ChangeQueue was
created dynamically or not, during a clean up phase. Only used in
DependentPipelineManager pipelines.
Note that this doesn't necessarily support a newly added named queue, or
adding a project to an existing named queue. That will be follow up
work.
Change-Id: I51ab7fb113f1bcbcef5f4f6c96ca046d0c76fdd9
Story: 2000898
Task: 3528
This changes the gerrit connection to tolerate cyclic patch
dependencies. This is a first step towards support of cyclic patch
dependencies where this will need to be handled in the pipelines
themselves. For now the pipelines are changed to detect the cycles by
themselves allowing to enhance them for cyclic dependencies later on.
Change-Id: I26e37b3fe66f46250a1baea1b803fd82d07145d7
Now that sources are associated with projects, remove the source
attribute from pipelines altogether.
For ease of review, the test fixtures are updated in a subsequent
commit.
Story: 2000953
Change-Id: Ie6b3b6be1b11b0d4462188902121d9cd29c91284
The layout stores the configuration of a project in the ProjectConfig
class (not the Project class -- that represents the abstract idea
of a project independent of Zuul, the ProjectConfig represents a
particular Zuul configuration operating on that project). Therefore,
in the continuing effort to fully qualify project names, index
ProjectConfig objects by their canonical project name. Use that
name when looking for a ProjectConfig to find the jobs to run for
a given change.
Story: 2000953
Change-Id: I733a66369c969770e57c2fa8b30822bd15e1aca7
This makes everything a child of Ref. As a result of this rearrangement,
NullChanges are awkward to use, and thus, have been replaced by enqueued
Refs where the 'ref' attribute is still unknown. As a result,
status.json will show timer-created jobs with an ID, where they used to
show a null, which is why that change to test_timer is necessary.
Change-Id: Ief0d3dde089b5529b9df7a804f6fea72b8b7dc48
Story: 2000781
Task: 3300
This was mostly already done when we created 'ProjectConfig' and
'ProjectPipelineConfig' classes. Clean up the remaining leftover
code and comments.
Story: 2000786
Task: 3310
Change-Id: I8a0bfb2cd148c174bc7e0b5121f8c39ed1d4645b
In Zuul v2, shared queues were determined automatically by combining
queues which shared a job name. In v3, they are explicitly created
by configuration by naming them. Update the queue construction
to behave this way.
Change-Id: I7e328b2170351bc51232c1c6ea48946a36d34e27
It feels more natrual to talk about DependentPipelineManager being a
PipelineManager in the internals doc. It's not used many places, so
just go ahead and change it.
Change-Id: Ie6aa4a27c2fec9469e266a0113447e3102985f61
Sometimes having narrative text describing how objects hang together it
handy for knowing what to do.
Change-Id: Ib12af2d993740d82b4f43de74b11ecefd1cd363a
This begins a lot of related changes refactoring config loading,
the data model, etc., which will continue in subsequent changes.
Change-Id: I2ca52a079a837555c1f668e29d5a2fe0a80c1af5
James E. Blair