The validate-tenants scheduler subcommand is supposed to perform
complete tenant validation, and in doing so, it interacts with zk.
It is supposed to isolate itself from the production data, but
it appears to accidentally use the same unparsed config cache
as the production system. This is mostly okay, but if the loading
paths are different, it could lead to writing cache errors into
the production file cache.
The error is caused because the ConfigLoader creates an internal
reference to the unparsed config cache and therefore ignores the
temporary/isolated unparsed config cache created by the scheduler.
To correct this, we will always pass the unparsed config cache
into the configloader.
Change-Id: I40bdbef4b767e19e99f58cbb3aa690bcb840fcd7
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
This change adds a warning (viewable in the web UI) when multiple
configurations are found for a project-branch combination.
Change-Id: I3defb2b1e6d7b59c450361c11d709802fe193373
The current "layout_lock" in the scheduler is really an "abide" lock.
We lock it every time we change something in the abide (including
tenant layouts). The name is inherited from pre-multi-tenant Zuul.
This can cause some less-than-optimal behavior when we need to wait to
acquire the "layout_lock" for a tenant reconfiguration event in one
thread while another thread holds the same lock because it is
reloading the configuration for a different tenant. Ideally we should
be able to have finer-grained tenant-level locking instead, allowing
for less time waiting to reconfigure.
The following sections describe the layout lock use prior to this
commit and how this commit adjusts the code to make it safe for
finer-grained locking.
1) Tenant iteration
The layout lock is used in some places (notably some cleanup methods)
to avoid having the tenant list change during the method. However,
the configloader already performs an atomic replacement of the tenant
list making it safe for iteration. This change adds a lock around
updates to the tenant list to prevent corruption if two threads update
it at the same time.
The semaphore cleanup method indirectly references the abide and
layout for use in global and local semaphores. This is just for path
construction, and the semaphores exist apart from the abide and layout
configurations and so should not be affected by either changing while
the cleanup method is running.
The node request cleanup method could end up running with an outdated
layout objects, including pipelines, however it should not be a
problem if these orphaned objects end up refreshing data from ZK right
before they are removed.
In these cases, we can simply remove the layout lock.
2) Protecting the unparsed project branch cache
The config cache cleanup method uses the unparsed project branch cache
(that is, the in-memory cache of the contents of zuul config files) to
determine what the active projects are.
Within the configloader, the cache is updated and then also used while
loading tenant configuration. The layout lock would have made sure
all of these actions were mutually exclusive. In order to remove the
layout lock here, we need to make sure the Abide's
unparsed_project_branch_cache is safe for concurrent updates.
The unparsed_project_branch_cache attribute is a dictionary that
conains references to UnparsedBranchCache objects. Previously, the
configloader would delete each UnparsedBranchCache object from the
dictionary, reinitialize it, then incrementially add to it.
This current process has a benign flaw. The branch cache is cleared,
and then loaded with data based on the tenant project config (TPC)
currently being processed. Because the cache is loaded based on data
from the TPC, it is really only valid for one tenant at a time despite
our intention that it be valid for the entire abide. However, since
we do check whether it is valid for a given TPC, and then clear and
reload it if it is not, there is no error in data, merely an
incomplete utilization of the cache.
In order to make the cache safe for use by different tenants at the
same time, we address this problem (and effectively make it so that it
is also *effective* for different tenants, even at different times).
The cache is updated to store the ltime for each entry in the cache,
and also to store null entries (with ltimes) for files and paths that
have been checked but are not present in the project-cache. This
means that at any given time we can determine whether the cache is
valid for a given TPC, and support multiple TPCs (i.e., multiple
tenants).
It's okay for the cache to be updated simultaneously by two tenants
since we don't allow the cache contents to go backwards in ltime. The
cache will either have the data with at least the ltime required, or
if not, that particular tenant load will spawn cat jobs and update it.
3) Protecting Tenant Project Configs (TPCs)
The loadTPC method on the ConfigLoader would similarly clear the TPCs
for a tenant, then add them back. This could be problematic for any
other thread which might be referencing or iterating over TPCs. To
correct this, we take a similar approach of atomic replacement.
Because there are two flavors of TPCs (config and untrusted) and they
are stored in two separate dictionaries, in order to atomically update
a complete tenant at once, the storage hierarchy is restructured as
"tenant -> {config/untrusted} -> project" rather than
"{config/untrusted} -> tenant -> project". A new class named
TenantTPCRegistry holds both flavors of TPCs for a given tenant, and
it is this object that is atomically replaced.
Now that these issues are dealt with, we can implement a tenant-level
thread lock that is used simply to ensure that two threads don't
update the configuration for the same tenant at the same time.
The scheduler's unparsed abide is updated in two places: upon full
reconfiguration, or when another scheduler has performed a full
reconfiguration and updated the copy in ZK. To prevent these two
methods from performing the same update simultaneously, we add an
"unparsed_abide_lock" and mutually exclude them.
Change-Id: Ifba261b206db85611c16bab6157f8d1f4349535d
This supplies a per-project default value for Zuul's default-branch
based on what the default branch is set to in GitHub. This means
that if users omit the default-branch setting on a Zuul project
stanza, Zuul will automatically use the correct value.
If the value in GitHub is changed, an event is emitted which allows
us to automatically reconfigure the tenant.
This could be expanded to other drivers that support an indication
of which branch is default.
Change-Id: I660376ecb3f382785d3bf96459384cfafef200c9
So far we've used "master" as default branch when we encounter a project
configuration that doesn't have a default branch set. The problem here
is, that later project stanzas that do define a default branch won't
have any effect.
To fix this, we will only fall back on using "master" in the project
metadata if none of the project definitions declares a default branch.
Change-Id: Iddd5ef906c2bdbcc472615ffe25033c4ca961df7
Per the documentation, include-branches should be able to override
exclude-branches, but this was not the case in the way the code was
written. Rework the code to correct this, and also add a test to ensure
it works as documented
Change-Id: I2e23b1533c67ccf84b4d6a36f5a003adc7b3e45a
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 is a preparatory step to add access-control for read-level
access to the API and web UI. Because we will likely end up with
tenant config that looks like:
- tenant:
name: example
admin-rules: ['my-admin-rule']
access-rules: ['my-read-only-rule']
It does not make sense for 'my-read-only-rule' to be defined as:
- admin-rule:
name: read-only-rule
In other words, the current nomenclature conflates (new word:
nomenconflature) the idea of an abstract authorization rule and
what it authorizes. The new name makes it more clear than an
authorization-rule can be used to authorize more than just admin
access.
Change-Id: I44da8060a804bc789720bd207c34d802a52b6975
If the list of branches for a project includes items that are not (yet)
in the min. ltimes for a layout state we can end up in a situation where
a scheduler is unable to start.
2022-05-16 17:34:50,895 ERROR zuul.Scheduler: Error starting Zuul:
Traceback (most recent call last):
File "/opt/zuul/lib/python3.8/site-packages/zuul/cmd/scheduler.py", line 98, in run
self.sched.prime(self.config)
File "/opt/zuul/lib/python3.8/site-packages/zuul/scheduler.py", line 931, in prime
tenant = loader.loadTenant(
File "/opt/zuul/lib/python3.8/site-packages/zuul/configloader.py", line 2530, in loadTenant
new_tenant = self.tenant_parser.fromYaml(
File "/opt/zuul/lib/python3.8/site-packages/zuul/configloader.py", line 1631, in fromYaml
self._cacheTenantYAML(abide, tenant, loading_errors, min_ltimes,
File "/opt/zuul/lib/python3.8/site-packages/zuul/configloader.py", line 1896, in _cacheTenantYAML
pb_ltime = min_ltimes[project.canonical_name][branch]
KeyError: 'new_branch'
The difference could be due to a missed branch creation event or a
simple race condition. The later case might fix itself after the
reconfig triggered by the branch creation event was processed.
Change-Id: I1838e66bc5296f153aa4c7a83ac0addb6c4db1aa
This allows operators to filter the set of branches from which
Zuul loads configuration. They are similar to exclude-unprotected-branches
but apply to all drivers.
Change-Id: I8201b3a19efb266298decb4851430b7205e855a1
When performing a tenant reconfiguration, we check the zk-based
file cache for every project-branch to see if we need to refresh
our unparsed config cache from the contents in ZK. However, we
know which project-branches have changed. We can assume that if
the unparsed branch cache is valid for for a project-branch ltime
(it won't be for projects that trigger tenant reconfig), then we
can use its contents without checking the ltime of the file cache
in ZK. This will typically save a few minutes worth of ZK traffic
during tenant reconfigurations on large tenants.
In order to make the background layout update similarly quick, we
need to prove the same min_ltimes to that process which were used
by the last reconfig. To do this, we store the min_ltimes in ZK
for each reconfig (as sharded data).
Change-Id: Ia0a524053a110e8e48f709d219651e2ad9c8513d
This change adds support for configuring non-top-level directories
(e.g. `foobar/zuul.d/`) as an extra config path which did not work so
far.
It's not clear if this was a bug or intended behavior that was just not
documented.
Change-Id: I1bc468130c9324a2e1b5d7f50b42fdc045eaa741
Currently, there are two "history approaches" for merge jobs in the unit
tests. Both, the RecordingMergeClient and the HoldableMergerApi store
each submitted job in a local dictionary. Those dictionaries can be used
in tests to assert that a specific number (or type) of merge job was
issued during the test.
The majority of the tests uses the history from the HoldableMergerApi.
This change adapts the two remaining test cases that use the history
from the merge client to use the history from the merger api instead.
Additionally, this change adapts the internal structure of the merger
api's history dictionary to reflect the one from the merge client as
this makes the assertions in the unit tests easier.
Change-Id: I459cbe5001d068a10e9851104e0d7a0e213a07cc
These tests raise a config error during the startup of the test case
which makes the first scheduler fail during its startup. The second (or
any additional) scheduler won't even run as the startup is serialized in
tests/base.py.
Thus it doesn't make sense to execute these tests with multiple
schedulers.
Change-Id: I9c6c29854c55edcac32d2ec922fe7e09939df5dd
The number of schedulers can be defined via the ZUUL_SCHEDULER_COUNT
environment variable (also forwarded by tox).
Disclaimer: running the tests with multiple schedulers will still fail
as not all test support multiple schedulers yet.
Change-Id: Ie11467dcf0fc36a5e167adcbd29ac4f256e0b3c5
It is currently possible to list default zuul config file paths in the
extra-config-paths config directive. Doing so will duplicate the configs
in Zuul which can cause problems. Prevent this entirely via
configuration validation.
Note: There has been a bit of refactoring to ensure that the voluptuous
schema is validated when reading the config. This ensures that an
invalid config doesn't produce hard to understand error messages because
loadTPCs() has attempted to process configuration that isn't valid.
Instead we can catch schema errors early and report them with human
friendly messages.
Change-Id: I07e9d4d3614cbc6cdee06b2866f7ae41d7779135
This is an early preparation step for removing the RPC calls between
zuul-web and the scheduler.
We want to format the status JSON and do the job freezing (job freezing
API) directly in zuul-web without utilising the scheduler via RPC. In
order to make this work, zuul-web must instantiate a ConfigLoader.
Currently this would require a scheduler instance which is not available
in zuul-web, thus we have to make this parameter optional.
Change-Id: I41214086aaa9d822ab888baf001972d2846528be
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
This is the second part of I767c0b4c5473b2948487c3ae5bbc612c25a2a24a.
It uses the MergerAPI.
Note: since we no longer have a central gearman server where we can
record all of the merge jobs, some tests now consult the merger api
to get the list of merge jobs which were submitted by that scheduler.
This should generally be equivalent, but something to keep in mind
as we add multiple schedulers.
Change-Id: I1c694bcdc967283f1b1a4821df7700d93240690a
We have a local unparsed branch cache on each scheduler and the global
config cache in Zookeeper. In order to check if the local unparsed
branch cache is up-to-date, we need to also store the logical timestamp
similar to the Zookeeper config cache.
Depending on the event that triggers (re-)loading of a tenant we must
make sure that those caches are considered valid or invalid correctly.
Instead of clearing the cache for a project(-branch) we will use the
event ltime to detected an outdated project-branch cache. This way
parallel tenant reconfigurations will not all clear out the cache in
Zookeeper even though it might be valid again because of a previously
processed reconfiguration.
There are four scenarios to consider when loading a tenant.
1. Processing a tenant reconfig event:
- The min. ltime for the changed project(-branches) will be set to
the event's ``zuul_event_ltime`` (to establish a happened-before
relation in respect to the config change). The min. ltime for all
other project-branches will be -1.
- Config for needed project-branch(es) is updated via cat job if the
cache is not valid (cache ltime < min. ltime).
- Cache in Zookeeper and local unparsed branch cache is updated. The
ltime of the cache will be the timestamp created shortly before
requesting the config via the mergers (only for outdated items).
2. Processing a full reconfiguration event:
- The min. ltime for all project-branches is given as the
``zuul_event_ltime`` of the reconfiguration event.
- Config for needed project-branch(es) is updated via cat job if the
cache is not valid (cache ltime < min. ltime). Otherwise the local
unparsed branch cache or the global config cache in Zookeper is
used.
- Cache in Zookeeper and local unparsed branch cache is updated, with
the ltime shortly before requesting the config via the mergers
(only for outdated items).
3. Processing a smart reconfiguration event:
- The min. ltime for all project-branches is given as -1 in order to
use cached data wherever possible.
- Config for new project-branch(es) is updated via cat job if the
project is not yet cached. Otherwise the local unparsed branch
cache or the global config cache in Zookeper is used.
- Cache in Zookeeper and local unparsed branch cache is updated, with
the ltime shortly before requesting the config via the mergers
(only for new items).
4. (Re-)loading a tenant due to a changed layout (happens after an event
according to one of the other scenarios was processed on another
scheduler):
- The min. ltime for all project-branches is given as -1 in order to
only use cached config.
- Local unparsed branch cache is updated if needed.
Change-Id: I45ec6dd794413870f85e4f9e9de8122eddce222e
In order to establish a proper happens-before relationship we need to
create the update ltimestamp before requesting the files from the
mergers. For that we will touch the /zuul/ltime node and use the
returned modified transaction id (mzxid).
Before we were using the mzxid of the files cache Znode. But since this
happend AFTER the cat job there is a small chance that the cache would
be considered valid later on even though the content of the repo has
changed in the meantime.
It would also be possible to use the zuul_event_ltime e.g. of the tenant
reconfiguration event for the update ltime. However, this is inefficient
for reconfigurations when a project is part of multiple tenants. In that
case we would trigger ~N cat jobs (where N is the number of tenants the
project is part of) since the cache would most likely be considered
outdated.
Change-Id: I9403bca6710c1c50460fcad3fd26ae48dc558ef9
Clearing of connection caches doesn't seem to be needed in practice and
is removed as it can't be easily coordinated between multiple
schedulers.
Change-Id: I663fa1f1752d0dd599af06a7275a14235faaff2f
With the way the global management and trigger event queues are
currently implemented we can't continue processing other events during a
reconfiguration and end up serializing reconfigurations.
The implementation is diverging from the spec here, as we did not
anticipate the need for tenant event queues back then.
We also add the following gauge metrics:
* zuul.tenant.<tenant>.trigger_events
* zuul.tenant.<tenant>.management_events
The `zuul.scheduler.eventqueues.trigger` gauge was removed together with
the global trigger event queue.
Change-Id: I1e0d872daf0fc4dd90e22b70c334e3e8152aa5b2
The scheduler is a better home for the build request cleanup because
the cleanup process should run across all zones, and the executor
api object in the executors are tied to specific zones, so they
have an incomplete cache.
It also means that if all the executors go offline, the schedulers
can still cancel and reset the builds, so it doesn't look like
dead builds are still running.
Since we now have two cleanup actions, instead of making a thread
for each, we use APScheduler to run the cleanup functions on an
interval. We use ZK locks to make sure that they aren't running
on more than one scheduler.
Change-Id: I0245b71f31aae9616d8e65d27c63b25b2c27f815
Cache raw config files in Zookeeper so we can load the config on
multiple schedulers with only one round trip to the mergers.
In case no cache ltime is give, the configloader will get the content of
the config files via a cat job and store the returned config in
Zookeeper.
For tenant reconfigurations the 'zuul_event_ltime' of the corresponding
event will be passed as the cache ltime to 'reloadTenant()'. This has
the advantage that in case a project is part of multiple tenants, only
the first tenant will request the config from the mergers. All following
tenants can get the config from Zookeeper.
In a later step we can save the logical timestamp of a tenant
reconfiguration in the 'global layout state'. This timestamp can then be
used as the cache ltime by other schedulers to update their local
layout.
Change-Id: Ia3d2a124eaf6ee9d82c94a0375ee7ecba61caadf
If an admin specifies two tenant blocks in main.yaml with the same
name, raise an exception.
Zuul assumes tenant names are unique, and there is no "overlay"
functionality (like we have with jobs, etc) in the main tenant config
file that would warrant two entries with the same name.
Change-Id: I0e44ef644e17f8d412e82ce4ca51febc7c65a00c
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
The << operator in yaml is currently handled as a key and not like an
operator. This leads to a YAMLDuplicateKeyError when parsing yaml that
merges multiple mappings into the same dict.
This can be solved by not saving this operator into our key list and
let pyyaml handle the rest.
Change-Id: Ic444747b9dc6fd64396fdb9944faa4c41c2bcd31
When loading dynamic project data out of a trusted
repository, we are defaulting to automatically selecting
master as a branch to load from instead of the load-branch
option that can be specified.
This leads to a blind spot when verifying changes to zuul config on
that repo.
Change-Id: I766f455220b15f56480304bd5b72560733ece8be
Co-Authored-By: Tobias Henkel <tobias.henkel@bmw.de>
Zuul loads configuration files in a distinct order and currently only
warns to the zuul log if some files got ignored. This can be confusing
to the user so plumb this warning through to the buildset which then
gets reported to a change.
Change-Id: I68f729c83cf6c16fd1f490a60b269401236ef6d9
Some projects would like to keep anything Zuul related, including
playbooks/roles, in the .zuul.d directory. Currently Zuul considers
anything in these directories ending with .yaml as configuration,
leading to parsing errors if you put playbook or role YAML files in
subdirectories there.
Allow this by adding a check for .zuul.ignore stamp files in
subdirectories of .zuul.d/zuul.d. If found, that directory prefix
will be pruned and not considered for config file read.
Change-Id: I7dbd3bb23648b7d17be0e2a0ea24e51c160e7940
To improve consistency and remove `self.sched` and
use `self.scheds.first.sched` in all tests.
This change only touches tests.
Change-Id: I0a8c3f8ad634d1d88f3c68a11e4d5587993f4a0d
Story: 2007192
Setup config will not set the base test config object rather return a new one.
This allows to setup multiple config objects which is needed in order to
instantiate mutliple schedulers enabling them to have different configs, e.g.
command socket.
Change-Id: Icc7ccc82f7ca766b0b56c38e706e6e3215342efa
Story: 2007192
Operators can use the "{tenant.name}" special word when setting conditions'
values. This special word will be replaced at evaluation time by the
name of the tenant for which the authorization check is being done.
Change-Id: I6f1cf14ad29e775d9090e54b4a633384eef61085
Currently it is hard to test changes on trusted repositories. The
currently most common way is to duplicate jobs and roles and have base
jobs that use the test variants of the roles under test. This however
still imposes a risk of breaking things when trying to move the tested
changes to the productive roles and jobs. Further as a downstream
deployer it can be hard to follow e.g. zuul-jobs because those are not
tested in every downstream deployment. There is no easy way of
updating upstream repos like zuul-jobs. Thus most deployers either
just follow zuul-jobs and deal with eventual fall out from
incompatibilities or don't update it on a regularly basis.
This change aims to support an easier and risk free method of updating
repos like zuul-jobs. This can be done by defining branch filters for
a repo in the tenant config. This way one can have a test tenant using
the latest is greatest master branch while keeping a stable branch for
productuve tenants. This way updates can be tested in the test tenant
and if everything works one can merge this to the stable branch to
take the updates into production.
Change-Id: Id4b5e80c0b59e4075774e6ed0049b229173e8426
Allow operators to define complex rules either using the flattened JSONPath syntax,
or the more YAML-friendly nested dicts.
Change-Id: Iabf65313e114dcb15788844a1f0095ae52567275
Add an "authorize_user" RPC call allowing to test a set of claims
against the rules of a given tenant. Make zuul-web use this call
to authorize access to tenant-scoped privileged actions.
Change-Id: I50575f25b6db06f56b231bb47f8ad675febb9d82
Allow an operator to define authorization rules.
Allow an operator to add authorization rules to a tenant.
Add a rule parser and a rule registry.
The authZ engine is not plugged in yet.
Change-Id: I3a86c6c7d62ad2bce68a98dbd2fff18549b94fb9
James E. Blair