So far, when getting parent jobs recursively we've kept a set of tuples
with the job UUID and the dependency type (hard/soft) that needed to be
processed.
This worked as long as all relevant jobs used the same type of
dependency to a job. However, when hard/soft dependencies are mixed this
could lead to a job being processed twice. Zuul wrongly detected this as
a job dependency cycle.
This problem also lead to exceptions in pipeline processing as some
schedulers detected a dependency cycle whereas other did not. The reason
here is the non-deterministic odering of the set data type.
Since we don't need the information about the dependency type, we'll
just keep a set of job UUIDs to be processed, which prevents Zuul from
processing a job with a different type of dependency twice.
Note: The provided regression test did not fail consistently due to the
set non-determinism mentioned above.
Change-Id: I6459cbc90bf745ddda2da9dbc25bcee97005d933
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
This is part of the circular dependency refactor. It changes the
job map (a dictionary shared by the BuildSet and JobGraph classes
(BuildSet.jobs is JobGraph._job_map -- this is because JobGraph
is really just a class to encapsulate some logic for BuildSet))
to be indexed by FrozenJob.uuid instead of job name. This helps
prepare for supporting multiple jobs with the same name in a
buildset.
Change-Id: Ie17dcf2dd0d086bd18bb3471592e32dcbb8b8bda
This is part of the circular dependency refactor.
Update the job graph to record job dependencies when it is frozen,
and store these dependencies by uuid. This means our dependency
graph points to actual frozen jobs rather than mere job names.
This is a pre-requisite to being able to disambiguate dependencies
later when a queue item supports multiple jobs with the same name.
The behavior where we would try to unwind an addition to the job
graph if it failed is removed. This was originally written with the
idea that we would try to run as many jobs as possible if there was
a config error. That was pre-zuul-v3 behavior. Long since, in all
cases when we actually encounter an error adding to the job graph,
we bail and report that to the user. No longer handling that
case simplifies the code somewhat and makes it more future-proof
(while complicating one of the tests that relied on that behavior
as a shortcut).
This attempts to handle upgrades by emulating the old behavior
if a job graph was created on an older model version. Since it
relies on frozen job uuids, it also attempts to handle the case
where a frozenjob does not have a uuid (which is a very recent
model change and likely to end up in the same upgrade for some
users) by emulating the old behavior.
Change-Id: I0070a07fcb5af950651404fa8ae66ea18c6ca006
Change the frozen job storage in ZK from being identified by name to
UUID. This allows us to handle multiple frozen jobs in a buildset with
the same name.
The job graph will get a new field that is a sorted list of job UUIDs
with the index being the same as for the job name list. Jobs that are
created with the old name-based path will have their UUID set to None.
This is done in preparation of the circular dependency refactoring as
detailed in I8c754689ef73ae20fd97ac34ffc75c983d4797b0.
Change-Id: Ic4df16e8e1ec6908234ecdf91fe08408182d05bb
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
This method has gotten somewhat complicated, so add a unit test
that verifies that deduplication works and also verify that two
secrets with the same name but different contents are not
deduplicated.
Change-Id: I17ced6eea3c867a1119251631113550581c40bf4
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
The re2 library does not support negative lookahead expressions.
Expressions such as "(?!stable/)", "(?!master)", and "(?!refs/)" are
very useful branch specifiers with likely many instances in the wild.
We need to provide a migration path for these.
This updates the configuration options which currently accepts Python
regular expressions to additionally accept a nested dictionary which
allows specifying that the regex should be negated. In the future,
other options (global, multiline, etc) could be added.
A very few options are currently already compiled with re2. These are
left alone for now, but once the transition to re2 is complete, they
can be upgraded to use this syntax as well.
Change-Id: I509c9821993e1886cef1708ddee6d62d1a160bb0
This further reduces the number of ZK object reads during pipeline
refreshes by tracking when builds and frozen jobs are updated.
During the phases of a build where we know no updates can occur,
we already avoid refreshing the Build and FrozenJob objects.
But, for example, while a build is running we have to continually
refresh it to see if it has completed.
We can avoid this by recording expected version information in ZK
and only refresh those objects if we know our local copy is out
of date.
We can store the latest ZK object version of FrozenJob and Build
objects on the Buildset. On pipeline refresh, we currently
always refresh the buildset object, which means that when we
prepare to refresh the FrozenJob or Build objects underneath a
Buildset, we will have information about the latest versions of
those objects in ZK and can compare to the versions we currently
have in memory to decide if we need to refresh them. This should
reduce the number of reads in a pipeline refresh by about 50%.
But it will cause more writes, in that we will update the
Buildset object each time we modify one of its children. This
may affect pipeline processing times but the impact should be
very small.
We will use version numbers (rather than transaction ids) because they
are predictable, and updating the buildset first with the predicted
next version before updating the child avoids issues caused by a crash
between those two steps.
Since it is typical for many objects to be created at once, we do
optimize the case where the objects are initially created and we
avoid making an update to the BuildSet in that case so that we
don't repeatedly write the buildset object.
Change-Id: I3824af6149bf27c41a8d895fc682236bd0d91f6b
We may be able to speed up pipeline refreshes in cases where there
are large numbers of items or jobs/builds by parallelizing ZK reads.
Quick refresher: the ZK protocol is async, and kazoo uses a queue to
send operations to a single thread which manages IO. We typically
call synchronous kazoo client methods which wait for the async result
before returning. Since this is all thread-safe, we can attempt to
fill the kazoo pipe by having multiple threads call the synchronous
kazoo methods. If kazoo is waiting on IO for an earlier call, it
will be able to start a later request simultaneously.
Quick aside: it would be difficult for us to use the async methods
directly since our overall code structure is still ordered and
effectively single threaded (we need to load a QueueItem before we
can load the BuildSet and the Builds, etc).
Thus it makes the most sense for us to retain our ordering by using
a ThreadPoolExecutor to run some operations in parallel.
This change parallelizes loading QueueItems within a ChangeQueue,
and also Builds/Jobs within a BuildSet. These are the points in
a pipeline refresh tree which potentially have the largest number
of children and could benefit the most from the change, especially
if the ZK server has some measurable latency.
Change-Id: I0871cc05a2d13e4ddc4ac284bd67e5e3003200ad
This adds the ability to specify that the Zuul executor should
acquire a semaphore before running an individual playbook. This
is useful for long running jobs which need exclusive access to
a resources for only a small amount of time.
Change-Id: I90f5e0f570ef6c4b0986b0143318a78ddc27bbde
The model tests exercise some tracing code (by creating buildsets,
etc) but do not have the tracing module initialized because they are
not full Zuul system tests and do not inherit from ZuulTestCase.
Without initialization, we get different behavior from opentelemetry
(we have not provided our global tracer object, etc), which makes
these tests perform differently than they would in production. They
also fail when run in isolation because of these differences. They
likely only succeed when we run the whole test suite because of leakage
of the opentelemetry module initialization.
To correct this, initialize the tracing library (but with no
configuration, so we still are not configuring it to emit traces).
Change-Id: Iea5a8890f545c4e4ddf0a796b3a2fcfc7078a50e
Change I0421c5a446d0b75194096b1d8f0e0866dae3b8f0 began including
BranchMatcher objects instead of strings in the SourceContext class
if an implied-branches pragma was supplied. If a syntax error
occurs in a repo with such a pragma, the source context needs to
be serialized to zk, and that would fail. To correct this, add
serialization methods to the source context and branch matchers.
Also correct an unrelated error where the syntax "- job:" would
fail to report the error to the user because the value None was
added to the list of config objects, and we always assume that
is a dict. To correct this, raise the syntax error earlier in
that case.
Change-Id: Id7c1dff5e9865d21c3215bdc32c8fa2cb6603aaa
This adds (de)serialization to some helper classes in some cases,
but in others we simplify the attributes we store on the FrozenJob
so that we don't have to deal with objects any more.
Notably, the process of serializing playbooks is incorporated into
creating the frozen job (rather than creating the executor params)
since it uses quite a few helper classes each of which would need
to be (de)serialized as well.
We could probably turn the rest of these helper classes into
dictionaries too (which would be more efficient), but as they are
simple and only one level deep, it seemed easier to implement
serialization for them right now.
Change-Id: I924c94ede1b0b6747b1f5cf92e658a45e6956a93
These are serialized as attributes of the BuildSet. Essentially,
the JobGraph is a logic encapsulation class with a small amount of
data. These extra data are stored on the BuildSet object, and
the JobGraph is reconstructed from them as necessary. This saves
extra ZK network traffic for a small amount of data that has the
same lifecycle as the BuildSet.
It might be structurally simpler to just move all of this into the
BuildSet directly, but the logic is fairly complex and I think
benefits from staying in its own class.
The ProjectMetadata object needs to be serializable to complete
this.
Change-Id: Id451d2b24556f62927120ccdd2657772f12c787b
RepoFiles are stored in the following Zookeeper path:
/zuul/<tenant>/pipeline/<pipeline>/item/<item-uuid>/buildset/<buildset-uuid>/files
Since the files can become very large the data will be sharded.
Change-Id: Ic591b53308b8dd0dec5530a8f040c7d487026182
This is a ZKContext that can be used to tell the ZKObjects that they
shouldn't actually save to ZK. This makes dealing with transient
ZKObjects for job freezing (and unit testing) easier.
Change-Id: I66ec9a0417645a72e18311ac207fcd22c0849662
ZuulMark instances are referenced from config errors that are part of a
build set. That means we need to serialize and store it in Zookeeper.
In order to avoid circular imports between the model and configloader
module we move the ZuulMark class to the model.
Change-Id: I36fecddc8ce038209fdc99a0979bf046fa16b32b
The change queue state is store in the following Zookeeper path:
/zuul/<tenant>/pipeline/<pipeline>/queue/<layout-uuid>/<queue-uuid>
Change-Id: I0a64bd9adc7b9f8f7a775280bb7a01ace22baac4
Save the state of a queue item in Zookeeper. The queue item is stored in
the following path:
/zuul/<tenant>/pipeline/<pipeline>/items/<item-uuid>
Since items can move between queues during a reconfiguration, we store
them direcly below the pipeline instead of the queue.
Change-Id: I50e8ae66026a099c0148d910359249653b3ca16d
This stores necessary attributes from a Project directly on the
SourceContext instead of the whole Project.
This will help us to serialize the SourceContext without serialising the
Project as well as the Project also contains references to the
Connection and Source.
This is part of a bigger change to serialize the JobGraph (including all
frozen jobs).
Change-Id: Ib4037da2f7a0f803aca24ce880dbc262375db6a4
We can obtain the same information from the SQL database now, so
do that and remove the filesystem-based time database. This will
help support multiple schedulers (as they will all have access to
the same data).
Nothing in the scheduler uses the state directory anymore, so clean
up the docs around that. The executor still has a state dir where
it may install ansible-related files.
The SQL query was rather slow in practice because it created a
temporary table since it was filtering mostly by buildset fields
then sorting by build.id. We can sort by buildset.id and get nearly
the same results (equally valid from our perspective) much faster.
In some configurations under postgres, we may see a performance
variation in the run-time of the query. In order to keep the time
estimation out of the critical path of job launches, we perform
the SQL query asynchronously. We may be able to remove this added
bit of complexity once the scale-out-scheduler work is finished
(and/or we we further define/restrict our database requirements).
Change-Id: Id3c64be7a05c9edc849e698200411ad436a1334d
We want to restrict layout access to job freezing so that fewer
actions (like creating a build request after a job is frozen) require
access to it.
To prevent components other than the pipeline manager from accessing the
layout, we will remove the layout as an attribute from the queue item
and store it in an internal cache of the manager.
Queue items will reference their (dynamic) layout via the layout's UUID.
An item's layout UUID will change if any of it's input to the layout
creation changed. Those inputs are the tenant layout and list of items
ahead. This means that during a re-enqueue and in case of a gate reset
we will set the layout UUID of an item back to None.
This also prepares us for the scale-out scheduler where we have to
re-calculate a layout on a scheduler if it is not available in the
cache, as we are not storing the layout in Zookeeper due to it's size.
Since the project metadata is needed by the executor client, which
should not access to layout anymore, the metadata is now available via
the job graph.
Change-Id: I93c7a3932fbf9ad8915d1d3d1fff9682778b28f8
The change to move trigger events into ZK had an error. It tries
to determine if it needs to forward each trigger event to each
pipeline in each tenant. To do that, it re-uses the pipeline
management functions to determine if an event might be relevant to
a change in that pipeline (for example, it is an abandon event).
Those comparisons have not been used to compare items of different
types before (changes to tags, for example), and raise exceptions
in those cases.
To correct this, improve the equality comparisons for those types.
Additionally, when this exception was raised, it caused any further
processing of the event to abort, so any pipelines or tenants after
the tenant-pipeline which caused the error would not receive the
event. To make the system more robust so that an error related to
one tenant does not affect others, we add an extra exception handler
to log those errors and proceed to the next tenant.
Change-Id: I9b62b83d51f66bee85cdb968d24bd5367e62e35d
Updates the environment variable processing to only affect variables
prefixed with ZUUL_.
Adds a test showing the os.environ with % in it.
This reverts commit b3929b5633.
Change-Id: Ic6c3dd0327ef70dc1375486827e4503a4cea9bfc
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
Currently the default ansible version is selected by the version of
zuul itself. However we want to make this configurable per deployment
(zuul.conf), tenant and job.
Change-Id: Iccbb124ac7f7a8260c730fbc109ccfc1dec09f8b
A "soft" dependency can be used to indicate that a job must run
after another completes, but only if it runs at all. For example,
a deployment job which depends on a build job with different file
matcher criteria.
Change-Id: I4d7fc2b40942569323da273c4529fdb365a3b11a
When pipelines are defined multiple times zuul currently silently uses
the last one which is defined. This is unsafe. Instead stick with the
first found pipeline and add a configuration error. While at it add
the missing start_mark to the pipeline object as it's needed when
throwing an error.
Change-Id: I98637ecd7d4978a803d6131b7cd61da0a74e669f
To make room for upcoming per-project and per-tenant ssh keys,
clarify that the current "key" is the secrets key.
Change-Id: I4267ee0002af02afb0331bad15488a6c5a293392
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
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
Files and irrelevant-files don't behave in the way people intuitively
expect. If a job or template has an irrelevant-files matcher, it's not
possible to alter its set of irrelevant-files in a project-pipeline
variant.
This changes them to be treated as overwriteable attributes and evaluated
after branch-matching variants are combined.
* Files and irrelevant-files are overwritten, so the last value
encountered when combining all the matching variants (looking only at
branches) wins.
* It's possible to both reduce and expand the scope of jobs, but the
user may need to manually copy values from a parent or other variant
in order to do so.
* It will no longer be possible to alter a job attribute by adding a
variant with only a files matcher -- in all cases files and
irrelevant-files are used solely to determine whether the job is run,
not to determine whether to apply a variant.
See http://lists.openstack.org/pipermail/openstack-dev/2018-May/130074.html
and http://lists.zuul-ci.org/pipermail/zuul-discuss/2018-May/000397.html
for further context.
Change-Id: I7b619df10c0896e696e065cb093d4e11911d3ce1
This makes it more consistent with the ConfigObjects (which this
isn't, because it isn't frozen, but it's like them).
Change-Id: I354c9ef0d52bf6cf1adb8136c167d15eb5376cac
Since some validation is moving from the stage where we parse
YAML-dicts into a stage where we only work with existing parsed
config objects, make sure that they all have source_context and
start_mark attributes so that we can continue to return nice
error messages.
Change-Id: I2cc9dfa61657d7fb95e3140ecf2fa560689ce03e
None of the configuration items need to reference the layout during
their construction (they are now expected to remain independent of
the layout so they may be used in multiple layouts), so clean up
the last few places where that was happening and remove the layout
attribute of the ParseContext to make it impossible to rely on that
in the future.
Change-Id: Idf30f8f41e012cba0b8f56cd1fd8354f84755857
If we want to use configuration objects between layouts, we can't change
them once they are loaded. To validate that the code doesn't do that,
and prevent regressions, make them immutable after loading is complete.
A variable is added to the base job in the check-vars ansible test to make
sure that we exercise Job.applyVariant in the case where the base job
starts with a variable defined.
Change-Id: I42d01962c4b2dd3cdfce3641ea367b4f4f6892c9
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 removes direct references to secret objects, instead resolving
them at run time. They are also resolved and decrypted (but not
stored) at configuration time for validation purposes.
This is part of a change series to re-use configuration objects
between layouts to save memory.
This is a re-proposal of I004f381d9df8c165b04ba540527d42ed74ee8fe1
with better testing.
Change-Id: Ib5d1d4ed20429749785c169565a8fcf94e747569
It seems like this change makes secrets defined in a base job unusable. I think we need to call freezePlaybook for all run, not only the variants'.
This reverts commit f37568cc10.
Change-Id: I1b06b503357d79c5b9e755037d699226bc70e5bd
This removes direct references to secret objects, instead resolving
them at run time. They are also resolved and decrypted (but not
stored) at configuration time for validation purposes.
This is part of a change series to re-use configuration objects
between layouts to save memory.
Change-Id: I004f381d9df8c165b04ba540527d42ed74ee8fe1
Voluptuous schema creation is expensive. A recent change[1] updated
the ProjectParser to create a schema for each project stanza. This
is because we moved the instantiation of the ProjectParser class to
a point before the pipelines are parsed, and the project schema
requires knowing the pipelines.
Correct this by instantiating the ProjectParser after the pipelines
are set. With that in place, we can go back to creating only a single
project schema.
While we're at it, update some other classes to use cached schemas as
well for further performance improvements.
[1] 831ff3ebba3932c671f7e67a4ad4ed741b2cd08f
Change-Id: Ied5a1ff4a8717f4fb4fa214af57dbeba21c52afb
If an author doesn't want a job to be run except on certain repos,
then they almost certainly didn't want those repos to be able
to inherit from it changing only the override-projects setting so
that the child job could be run.
Instead, perform a set intersection any time that more than one
allowed-projects comes into play in an inheritance path, causing
it to become only more restrictive.
Remove the test from test_model which is covered by the new test.
Change-Id: I7695620ac52f2ae076e9a66818c00de1e8cf7066
We now know, before attempting to run a job, whether the inheritance
hierarchy has produced a main playbook for the job. If there is none,
error early.
Also, in the executor, assume that any specified playbooks are
required to exist, and use the more specific version of the error
message if they don't.
Change-Id: Id7dc5934c665cf939820b12b5ded53adeb60c0a8
Simon Westphahl