Commit Graph

10151 Commits

Author SHA1 Message Date
Albin Vass a94768c645 Fix unbound variable in call to check_config_path
Change-Id: Ib7e90a5d5b3a2ed108d9ed7672b73974ea13048d
2024-05-08 18:20:27 +02:00
Zuul 9fb90a4348 Merge "Check pre run failure cases with only 2 retry attempts" 2024-05-07 07:21:02 +00:00
Zuul 9ce5c5e471 Merge "Limit bytes when reading Ansible output" 2024-05-06 20:15:39 +00:00
Zuul 1198e3cb32 Merge "gerrit: Add `approval-change` trigger" 2024-05-06 19:32:55 +00:00
Clark Boylan 1b869c8237 Check pre run failure cases with only 2 retry attempts
test_pre_run_failure_retry has been hitting timeouts semi regularly.
This test was checking that after all retry attempts other things
continue on. Rather than increasing the timeout time to avoid timeouts
we reduce the number of retry attempts from 3 to 2 which should make
things run faster and within the timeout. This should be safe from a
testing perspective because we're still doing at least one retry which
ensures that previous code paths are still exercised.

Change-Id: If78c7cdfac63c30f9e52a1a5984a662ab969c2ee
2024-05-06 11:46:23 -07:00
Joshua Watt ffb615e6c7 gerrit: Add `approval-change` trigger
Adds a new type of trigger to the Gerrit driver that only triggers if
the approval value was changed by the user in the comment. This is
useful if Zuul is configured to allow many different scores to trigger a
pipeline (with an additional requirement on all of them), but arbitrary
comments made while the scores are present should _not_ trigger (or
potentially re-trigger) the pipeline. This can happen because Gerrit
sends all approvals by a user on all comments, regardless of if they
were changed by the comment.

The new `approval-change` trigger requirement inspects the `oldValue`
field in the Gerrit event. The pipeline will only trigger if this value
is present and not equal to the new approval value (thus, only when the
user actually changed it).

`oldValue` has been present since at least Gerrit 3.4

Change-Id: I88cf840ae8b4e63c77f10ee68b6901e85f7c5fb1
2024-05-03 15:39:46 -07:00
James E. Blair 3589762367 Speed up merger git resets
The merger starts every operation by resetting the repository.
That means clearing out any failed previous merges and updating
and restoring the branch state to match the upstream origin
repo.

For repos with a very large number of branches (10k), this can take
some time (minutes).  This is mostly due to the inefficiency of
looking up the origin ref one at a time (gitpython reads the
packed-refs file for each lookup, ironically negating the benefit
of packed-refs).  To bypass this, use our previously developed
method for getting all the refs efficiently and do that once at
the start of the reset method.

Change-Id: If21245cd562c6499378c4f3353332d87c4ca4b47
2024-04-30 15:47:17 -07:00
Zuul 25cc922116 Merge "Fix issue with reopened PR dependencies" 2024-04-29 22:12:19 +00:00
Zuul e81f063df2 Merge "Replace status_url with item_url in pipeline reporter templates" 2024-04-29 22:02:22 +00:00
Zuul e12a98b905 Merge "Temporarily pin urllib3 != 2.1.0" 2024-04-29 21:37:31 +00:00
Zuul de6bd67e1f Merge "Add zuul.build_refs variable" 2024-04-29 21:37:27 +00:00
Simon Westphahl 0349628249 Fix issue with reopened PR dependencies
Given two PRs with B depending on A which are enqueued in gate, A is
closed and then immediately reopened.

This sequence of events will currently dequeue A and then immediately
enqueue it behind B. Since the check for whether a dependency is already
in the queue doesn't care if it's ahead or behind the current change,
we'll not dequeue B and the content of builds executed by B will not
include A.

This change updates the check to determine if a change is already in
the queue to only check for changes ahead of it.  This causes B to
be correctly dequeued in the next pipeline pass.

This behavior is correct, but isn't always intuitive or consistent.
If the time between closing and reopening a change is long enough for
a pipeline process, then both changes will be enqueued by the reopening
(because we check for changes needing enqueued changes and enqueue them
behind).  But if both events are processed in a single pipeline run,
then the removal of B happens after the re-enqueue of A which means that
it won't be re-added.

To correct this, whenever we remove abandoned changes, we will also remove
changes behind them that depend on the removed abandoned changes at the
same time.  This means that in our scenario above, the re-enqueue happens
under the same conditions as the original enqueue, and both A and B are
re-enqueued.

Co-Authored-By: James E. Blair <jim@acmegating.com>
Change-Id: Ia1d79bccb9ea39e486483283611601aa23903000
2024-04-26 14:20:07 -07:00
Christian von Schultz 5933704a6a Catch ZeroDivisionError when f_files=0
On BTRFS, f_files and f_ffree are always 0. For now, assume there is
no limit by setting files_percent_avail to 100%.

Change-Id: I53455e46101130596ae178a5933fe51ebaee206f
2024-04-26 17:56:34 +02:00
Simon Westphahl 6e163780e3 Temporarily pin urllib3 != 2.1.0
It looks like urllib3 version 2.1.0 causes problems when connecting to
Windows nodes.

Fixed in 2.2.0, but ibm-cos-sdk is preventing that from installing, so
exclude 2.1.0 for now.

https://github.com/urllib3/urllib3/pull/3326

Change-Id: I5d4a33c477d6872389c1d4197e926991b70f06ec
2024-04-24 16:11:20 -07:00
Zuul 2c2a2d61a5 Merge "Gerrit: skip ref-updated /meta events" 2024-04-24 19:50:03 +00:00
Zuul 4d6db3602f Merge "Update docs for job.dependencies and provides/requires" 2024-04-19 12:17:10 +00:00
James E. Blair 239fe205ec Gerrit: skip ref-updated /meta events
Approximately 40% of all Gerrit events that OpenDev processes are
ref-updated events for refs/changes/.../meta refs.  This is likely
due to the increased use of notedb for storing data in Gerrit.  Since
Zuul users are not likely to need to trigger off of ref-updates to
the meta ref, let's avoid enqueing them into Zuul's event queue.
This will reduce ZK traffic.

Change-Id: I724f5b20790d1ad32e72b1ce642355c2257026c1
2024-04-18 15:02:13 -07:00
James E. Blair 4bb7fa3d83 Add zuul.build_refs variable
This variable is mostly intended for retroactive debugging of jobs.
There is currently no way to determine for which specific refs a
deduplicated job is being run.  This records that information in
the inventory file for the job.

This change also corrects the docs to indicate that change_url is
always present.

Change-Id: I990ecfe2a4b455fa800906ed0e542a3581bb8b29
2024-04-16 09:38:14 -07:00
Zuul 01e9472306 Merge "Don't consider type of dependencies to process" 2024-04-15 18:52:11 +00:00
Zuul 57c5c59acb Merge "Allow empty commits with squash merges" 2024-04-15 16:47:47 +00:00
Zuul 53e22bbfa6 Merge "Cancel jobs of abandoned circular dep. change" 2024-04-15 16:28:28 +00:00
Simon Westphahl 8223a336af
Don't consider type of dependencies to process
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
2024-04-15 14:54:54 +02:00
Vitaliy Lotorev 34a86cd05b Update docs for job.dependencies and provides/requires
Specify dependent jobs are made available with artifacts.

Specify job.provides/requires is ignored for non-change items.
This documenting implementation made in [1].

Specify examples of passing artifacts for parent/child jobs in
while using job.provides/requires.

[1] https://review.opendev.org/c/zuul/zuul/+/915188

Change-Id: Ibb36ce2fdfa8b07ae399ae69ebf63de663c71fb3
2024-04-12 20:33:54 +00:00
Simon Westphahl aca9238de9
Allow empty commits with squash merges
Since squash merges will not create a merge commit Zuul is creating one
separately. However, this step can fail when there are no changes to
commit. This might for example happen in change-triggered post
pipelines.

To fix this issue we will restore the behavior used prior to
I3379b19d77badbe2a2ec8347ddacc50a2551e505 and allow creating empty
merge commits.

Change-Id: Ic18cb98f12260338799e963b8dc915c8be1d421b
2024-04-12 20:40:09 +02:00
Zuul 838b7de885 Merge "Fix missing label evaluation in Gerrit" 2024-04-12 07:03:45 +00:00
Joshua Watt fa431cf4f8 Fix missing label evaluation in Gerrit
Fixes the way that missing labels are handled in Gerrit. The intention
is that labels provided by Zuul are removed from the set of missing
labels on the change (and thus ignored). The original code was using the
">" set comparison operator to do this, but this operator is actually
"issuperset()". This means that if there was any disjoint members in the
allow_needs set (that is allow_needs had labels that were not missing),
the comparison would be False, and any actual missing labels would be
ignored.

The fix is to use set difference to calculate the missing labels and
remove the allow_needs set. If any labels are left after this, they are
actually missing and the change cannot be merged

Change-Id: Ibdb5df44e80d75198493f8287443ed19bcf269f1
2024-04-11 11:33:12 -06:00
Simon Westphahl c8ec0b25b5
Cancel jobs of abandoned circular dep. change
When a change that is part of a circular dependency is abandoned we'd
set the item status to dequeued needing change. This will set all builds
as skipped, overwriting exiting builds.

This means that when the item was removed, we did not cancel any of the
builds. For normal builds this mainly waste resources, but if there are
paused builds, those will be leaked and continue running until the
executor is force-restarted.

The fix here is to cancel the jobs before setting it as dequeued needing
change.

Change-Id: If111fe1a21a1c944abcf460a6601293c255376d6
2024-04-11 12:26:54 +02:00
Zuul 59ca809a9e Merge "Handle artifacts created by non change refs" 2024-04-10 22:48:09 +00:00
Zuul afcbe6559d Merge "Split build/buildset list queries into two" 2024-04-09 17:54:42 +00:00
James E. Blair 95bd778fcc Split build/buildset list queries into two
The only way to get reasonable performance out of all three databases
(mariadb, mysql, postgres) appears to be to make the following changes:

* Use group-by instead of distinct.
  Some versions of mysql produce the wrong output with distinct;
  this is more descriptive of what we want anyway.
* Split the query into two statements instead of joining on a subquery.
  Mariadb seems to be unable to produce a good query plan for the
  buildset list query.  Neither mariadb nor mysql support using
  "IN" with a subquery (so the idea of using IN instead of JOIN for
  the subquery is out).

These methods now perform a query to get the ids of the builds or
buildsets that match the criteria, then perform a second query to load
the ORM objects that match those ids.  This appears to be quite
fast for all three queries with the latest versions of all three database
systems.

Change-Id: I30bb3214807dfa8b26a848f85bb7a7bc660c6c1d
2024-04-09 06:39:08 -07:00
Zuul d20b8539d0 Merge "Handle annotated and signed tags when packing refs" 2024-04-09 09:45:15 +00:00
Clark Boylan 505253bed2 Handle artifacts created by non change refs
Artifacts processing assumed it was always operating on Change objects
as speculative artifact state only makes sense for speculative Changes.
However, this was not enforced and we would attempt to process artifacts
for refs/branches/tag/etc leading to this exception:

  Traceback (most recent call last):
    File "/usr/local/lib/python3.11/site-packages/zuul/manager/__init__.py", line 1065, in _executeJobs
      self.sched.executor.execute(
    File "/usr/local/lib/python3.11/site-packages/zuul/executor/client.py", line 63, in execute
      params = zuul.executor.common.construct_build_params(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/zuul/executor/common.py", line 97, in construct_build_params
      ) = item.getJobParentData(job)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/zuul/model.py", line 5431, in getJobParentData
      artifact_data = job.artifact_data or self.getArtifactData(job)
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/zuul/model.py", line 5414, in getArtifactData
      self.providesRequirements(job, data)
    File "/usr/local/lib/python3.11/site-packages/zuul/model.py", line 5377, in providesRequirements
      'change': change.number,
                ^^^^^^^^^^^^^
  AttributeError: 'Tag' object has no attribute 'number'

Address this by checking that the object type is Change before we handle
artifacts for it. If it isn't a Change we report that there are no
satisfied provides/requires for this dependency by the currently
processed queue item.

Change-Id: I5ba7f30e56fc890847b86ee776e74272ec04046d
2024-04-08 09:00:44 -07:00
Simon Westphahl 5b96a94bab
Limit bytes when reading Ansible output
When an Ansible task outputs too much data w/o a newline character, we
may run out of memory.

Change-Id: Idb89246ea643dcd2aa7594cc53e89d2c8b5aa57f
2024-04-08 10:43:02 +02:00
James E. Blair 014717bde6 Restore mysql hint conditional
This restores the conditional around adding the mysql index hint
for the builds query.  It was thought (incorrectly) that the hint
is now safe in all cases, but it is still the case that it can
slow down the query if some columns appear in the where clause.

Change-Id: Ieafab7e529e305f084ebf8c7844c8fa78da86902
2024-04-07 09:06:48 -07:00
Zuul 4a7a534191 Merge "Zuul-web: Fix literal filter queries being esacped" 2024-04-06 15:52:09 +00:00
Clark Boylan 3984e11020 Handle annotated and signed tags when packing refs
Zuul packs refs directly rather than rely on git to do so. The reason
for this is it greatly speeds up repo resetting. Typically there are two
pieces of information for each packged ref (sha and refname). Git
annotated and signed tags are special because they have the sha of the
tag object proper, the tag refname, and finally the sha of the object
the tag refers to.

Update Zuul's ref packing to handle this extra piece of information for
git tags.

Co-Authored-By: James E. Blair <jim@acmegating.com>
Change-Id: I828ab924a918e3ded2cd64deadf8ad0b4726eb1e
2024-04-05 13:12:59 -07:00
James E. Blair eab0be8519 Restore mysql index hint
We previously included this mysql index hint under some circumstances
for both build and buildset queries.  It initially appeared to no
longer be necessary after recent table changes, however that does not
seem to be the case, and in fact, it is necessary for some simple
build table queries.

It does no longer appear to be necessary for the buildset table, so
it is not added back here.

When applied to MySQL 5.7, these hints result in the build and buildset
queries returning the first N rows rather than the last N rows as
expected.

We should be able to resolve that issue as well, but it will require
further changes.  We're going to merge this as a short-term fix while
we work on a better solution.  The Zuul community has been notified
that master is currently a work in progress and to temporarily avoid
continuous deployment under mysql/mariadb.

Change-Id: Ia6d962957af3cac87d174145e69571a405ed505b
2024-04-05 09:35:37 -07:00
Benjamin Schanzel 800171a205
Zuul-web: Fix literal filter queries being esacped
https://review.opendev.org/c/908420 introduced a bug where literal
filters on builds and buildsets with sql wildcard characters (_, %) get
escaped, altering the search string. That leads to missing filter
results, e.g. when the user searches for a job_name with an underscore
in the name, because zuul escapes the underscore, searching for
`foo$_bar` instead of `foo_bar` (where `$` is the escape char). Fix this
by only applying the sanitation if we're going to filter with the sql
`LIKE` operator.

Change-Id: Iccef71fdf8a5a1150ff957be68358882e16e9da8
2024-04-03 12:12:57 +02:00
Simon Westphahl 9aea549305
Log Zookeeper session on connection state change
In order to identify e.g. the ephemeral owner of a node in Zookeeper it
is helpful to log the Zookeeper session ID when the connection is
established or changes.

Change-Id: I8a3645497ff73541b15d18ba32c20a7629792418
2024-03-27 12:17:49 +01:00
Zuul e1ad2443a3 Merge "Correctly limit buildsets with multiple refs" 2024-03-27 10:48:20 +00:00
Zuul 1561fe775e Merge "Web UI: substring search for builds, buildsets" 2024-03-27 10:27:49 +00:00
Zuul 15e41212c9 Merge "Fix Gantt chart overlap on dup build names" 2024-03-26 09:04:59 +00:00
Simon Westphahl 8bcfb8bc4a Correctly limit buildsets with multiple refs
The current SQL query will not correctly limit the number of buildsets
when some of the buildsets are related to multiple refs (circular
dependencies). The problem is that LIMTI works on number of rows, but we
want to limit only on the number of buildsets.

This corrects the problem by using a subquery to identify the distinct
buildsets, limiting that, and then querying for all of the information
about those buildsets.

This also updates the methods which perform the same function for builds,
even though we are not yet seeing an issue in practice.  It is
theoretically possible to call the getBuilds method with 'provides' and
'limit' arguments, which would produce the same problem as the buildsets
query.  That is not possible in practice, as the REST API doesn't support
provides, and the scheduler which does pass the provides argument doesn't
pass limit.  However, that could easily change in the future.  Additionally,
this future-proofs us if we add more queryable one-to-many relationships
to builds in the future (such as if we linked builds to multiple refs).
Also, it's easier to maintain these methods if they follow the same pattern.

There does not appear to be a performance loss in either mysql or postgres
with local testing on large data sets.  There may actually be an improvement
(but it's within the margin of error, so hard to say).

The index hints previously needed for mysql appear to no longer be
necessary and are removed.

Change-Id: Ib19e4cb8171f5d4d2873fb6b9c0301eb5d4ee43d
Co-Authored-By: James E. Blair <jim@acmegating.com>
2024-03-25 13:31:19 -07:00
Zuul 3a6208f342 Merge "Make ansible package check more robust" 2024-03-25 19:53:20 +00:00
Zuul 56a73f86eb Merge "Fix github docs for pull_request_review.state" 2024-03-25 19:53:18 +00:00
Zuul a3abea408b Merge "Emit per-branch queue stats separately" 2024-03-25 19:22:37 +00:00
Zuul 3b19ca9cb3 Merge "Add zuul_unreachable ansible host group" 2024-03-25 18:26:14 +00:00
James E. Blair 8821dbaa13 Make ansible package check more robust
The JSON parser will fail if given the empty string, so make sure
that if we don't get any output back, we don't try to parse it.

Additionally, if no extra packages are required, then don't bother
running the command in the first place.

Finally, include the missing package list in the error log message
rather than in a separate debug log entry, for easier correlation.

Change-Id: I0c39c74fdf05611439b35cd72b8ab70b836f4c1a
2024-03-25 11:25:35 -07:00
Zuul 0496c249be Merge "Reset jobs behind non-mergeable cycle" 2024-03-25 18:21:43 +00:00
Zuul ad1ef8e862 Merge "Add python 3.12 testing to Zuul" 2024-03-25 17:37:19 +00:00