Commit Graph

1695 Commits

Author SHA1 Message Date
Xavier Morel
5bce73c97d [IMP] *: optimise loading of home page
Fix outstanding query to make a positive `state` filtering, instead of
negative, matching 3b52b1aace8674259812a76b1566260937dbcacb.

Also manually create a map of stagings (grouped by branch) sharing a
single prefetch set.

For odoo the mergebot home page has 12 branches in the odoo project
and 8 in spreadsheet, 6 stagings each. This means 120 queries to
retrieve all the heads (Odoo stagings have 5 heads and spreadsheet
have 1, but that seems immaterial).

By fixing `_compute_statuses` and creating a single prefetch set for
all stagings of all branches we can fetch all the commits in a single
query instead of 120.
2023-07-10 15:23:31 +02:00
Xavier Morel
81ce4ea02b [IMP] rewrite /forwardport/outstanding
- add support for authorship (not just approval)
- make display counts directly
- fix `state` filter: postgres can't do negative index lookups
- add indexes for author and reviewed_by as we look them up
- ensure we handle the entire source filtering via a single subquery

Closes #778
2023-07-10 15:23:31 +02:00
Xavier Morel
aefbdaf974 [IMP] *: client side sorted implementation
Allow sorting by a callback. Sort remains client-side.
2023-07-10 15:23:31 +02:00
Xavier Morel
d748d4b215 [IMP] *: create a single template db per module to test
Before this, when testing in parallel (using xdist) each worker would
create its own template database (per module, so 2) then would copy
the database for each test.

This is pretty inefficient as the init of a db is quite expensive in
CPU, and when increasing the number of workers (as the test suite is
rather IO bound) this would trigger a stampede as every worker would
try to create a template at the start of the test suite, leading to
extremely high loads and degraded host performances (e.g. 16 workers
would cause a load of 20 on a 4 cores 8 thread machine, which makes
its use difficult).

Instead we can have a lockfile at a known location of the filesystem,
the first worker to need a template for a module install locks it,
creates the templates, then writes the template's name to the
lockfile.

Every other worker can then lock the lockfile and read the name out,
using the db for duplication.

Note: needs to use `os.open` because the modes of `open` apparently
can't express "open at offset 0 for reading or create for writing",
`r+` refuses to create the file, `w+` still truncates, and `a+` is
undocumented and might not allow seeking back to the start on all
systems so better avoid it.

The implementation would be simplified by using `lockfile` but that's
an additional dependency plus it's deprecated. It recommends
`fasteners` but that seems to suck (not clear if storing stuff in the
lockfile is supported, it opens the lockfile in append mode). Here the
lockfiles are sufficient to do the entire thing.

Conveniently, this turns out to improve *both* walltime CPU time
compared to the original version, likely because while workers now
have to wait on whoever is creating the template they're not competing
for resources with it.
2023-07-10 15:23:31 +02:00
Xavier-Do
f22ae357e3 [DEL] runbot: 15.0 eol 2023-06-22 15:59:50 +02:00
Xavier-Do
b72fdff01d [DEL] runbot: 15.0 eol 2023-06-22 15:59:36 +02:00
Xavier Morel
72281b0c63 [IMP] runbot_merge: allow running test suite without an explicit addons path 2023-06-22 14:38:10 +02:00
Xavier-Do
fc41e09f44 [IMP] runbot: add useful indexes remaining from upgrade 2023-06-22 11:23:43 +02:00
Xavier Morel
765281a665 [FIX] runbot_merge: make provisioning more resilient
A few cases of conflict were missing from the provisioning
handler.

They can't really be auto-fixed, so just output a warning and ignore
the entry, that way the rest of the provisioning succeeds.
2023-06-21 14:26:19 +02:00
Xavier Morel
9260384284 [FIX] runbot_merge: concurrency error in freeze wizard (hopefully)
During the 16.3 freeze an issue was noticed with the concurrency
safety of the freeze wizard (because it blew up, which caused a few
issues): it is possible for the cancelling of an active staging to the
master branch to fail, which causes the mergebot side of the freeze to
fail, but the github state is completed, which puts the entire thing
in a less than ideal state.

Especially with the additional issue that the branch inserter has its
own concurrency issue (which maybe I should fix): if there are
branches *being* forward-ported across the new branch, it's unable to
see them, and thus can not create the now-missing PRs.

Try to make the freeze wizard more resilient:

1. Take a lock on the master staging (if any) early on, this means if
   we can acquire it we should be able to cancel it, and it won't
   suffer a concurrency error.
2. Add the `process_updated_commits` cron to the set of locked crons,
   trying to read the log timeline it looks like the issue was commits
   being impacted on that staging while the action had started:
   REPEATABLE READ meant the freeze's transaction was unable to see
   the update from the commit statuses, therefore creating a diverging
   update when it cancelled the staging, which postgres then reported
   as a serialization error.

I'd like to relax the locking of the cron (to just FOR SHARE), but I
think it would work, per postgres:

> SELECT FOR UPDATE, and SELECT FOR SHARE commands behave the same as
> SELECT in terms of searching for target rows: they will only find
> target rows that were committed as of the transaction start
> time. However, such a target row might have already been updated (or
> deleted or locked) by another concurrent transaction by the time it
> is found. In this case, the repeatable read transaction will wait
> for the first updating transaction to commit or roll back (if it is
> still in progress). If the first updater rolls back, then its
> effects are negated and the repeatable read transaction can proceed
> with updating the originally found row. But if the first updater
> commits (and actually updated or deleted the row, not just locked
> it) then the repeatable read transaction will be rolled back with
> the message

This means it would be possible to lock the cron, and then get a
transaction error because the cron modified one of the records we're
going to hit while it was running: as far as the above is concerned
the cron's worker had "just locked" the row so it's fine to
continue. However this makes it more and more likely an error will be
hit when trying to freeze (to no issue, but still). We'll have to see
how that ends up.

Fixes #766 maybe
2023-06-21 14:26:19 +02:00
Xavier Morel
ed0fd88854 [ADD] runbot_merge: sentry instrumentation
Currently sentry is only hooked from the outside, which doesn't
necessarily provide sufficiently actionable information.

Add some a few hooks to (try and) report odoo / mergebot metadata:

- add the user to WSGI transactions
- add a transaction (with users) around crons
- add the webhook event info to webhook requests
- add a few spans to the long-running crons, when they cover multiple
  units per iteration (e.g. a span per branch being staged)

Closes #544
2023-06-21 14:26:19 +02:00
Xavier Morel
06a3a1bab5 [IMP] runbot_merge: add sentry filtering, rework some error messages
- move sentry configuration and add exception-based filtering
- clarify and reclassify (e.g. from warning to info) a few messages
- convert assertions in rebase to MergeError so they can be correctly
  logged & reported, and ignored by sentry, also clarify them
  (especially the consistency one)

Related to #544
2023-06-15 08:21:20 +02:00
Xavier Morel
cd4ded899b [IMP] runbot_merge: error reporting
Largely informed by sentry,

- Fix an inconsistency in staging ref verification, `set_ref`
  internally waits for the observed head to match the requested head,
  but then `try_staging` would re-check that and immediately fail if
  it didn't.

  Because github is *eventually* consistent (hopefully) this second
  check can fail (and is also an extra API call), breaking staging
  unnecessarily, especially as we're still going to wait for the
  update to be visible to git.

  Remove this redundant check entirely, as github provides no way to
  ensure we have a consistent view of anything, it doesn't have much
  value and can do much harm.
- Add github request id to one of the sanity check warnings as that
  could be a useful thing to send upstream, missing github request ids
  in the future should be noted and added.
- Reworked the GH object's calls to be clearer and more coherent:
  consistently log the same thing on all GH errors (if `check`),
  rather than just on the one without a `check` entry.

  Also remove `raise_for_status` and raise `HTTPError` by hand every
  time we hit a status >= 400, so we always forward the response body
  no matter what its type is.
- Try again to log the request body (in full as it should be pretty
  small), also remove stripping since we specifically wanted to add a
  newline at the start, I've no idea what I was thinking.

Fixes #735, #764, #544
2023-06-14 16:01:45 +02:00
Xavier Morel
270dfdd495 [REF] *: move most feedback messages to pseudo-templates
Current system makes it hard to iterate feedback messages and make
them clearer, this should improve things a touch.

Use a bespoke model to avoid concerns with qweb rendering
complexity (we just want GFM output and should not need logic).

Also update fwbot test setup to always configure an fwbot name, in
order to avoid ping messages closing the PRs they're talking
about, that took a while to debug, and given the old message I assume
I'd already hit it and just been too lazy to fix. This requires
updating a bunch of tests as fwbot ping are sent *to*
`fp_github_name`, but sent *from* the reference user (because that's
the key we set).

Note: noupdate on CSV files doesn't seem to work anymore, which isn't
great. But instead set tracking on the template's templates, it's not
quite as good but should be sufficient.

Fixes #769
2023-06-14 16:01:45 +02:00
Xavier Morel
e14616b2fb [IMP] runbot_merge: add support for draft check
1cea247e6c missed the update of the
`draft` flag, add support for it.

Fixes #753
2023-06-14 16:01:45 +02:00
Xavier Morel
048ae0c5ff [FIX] forwardport: flag statuses as recursive
I'd been convinced this was an ORM error because the field is not
recursive... in runbot_merge, in forwardbot it is and thus does indeed
need to be flagged to avoid the warning.
2023-06-14 16:01:45 +02:00
Xavier Morel
2009177ada [IMP] *: allow disabling staging on branch, remove fp target flag
- currently disabling staging only works globally, allow disabling on
  a single branch

  - use a toggle
  - remove a pair of tests which work specifically with `fp_target`,
    can't work with `active` (probably)
  - cleanup search of possible and active stagings, add relevant
    indexes and use direct search of relevant branches instead of
    looking up from the project

- also use toggle button for `active` on branches
- shitty workaround for upgrading DB: apparently mail really wants to
  have a `user_id` to do some weird thing, so need to re-add it after
  resetting everything

Fixes #727
2023-06-14 16:01:42 +02:00
Xavier Morel
4a4252b4b9 [FIX] runbot_merge: holes in provisioning
- github logins are case-insensitive while the db field is CI the dict
  in which partners are stored for matching is not, And the caller may
  not preserve casing.

  Thus it's necessary to check the casefolded database values against
  casefolded parameters, rather than exactly.
- users may get disabled by mistake or when one leaves the project,
  they may also get switched from internal to portal, therefore it is
  necessary to re-enable and re-enroll them if they come back.
- while at it remove the user's email when they depart, as they likely
  use an organisational email which they don't have access to anymore

Side-note, but remove the limit on the number of users / partners
being created at once: because there are almost no modules in the
mergebot's instance, creating partner goes quite fast (compared to a
full instance), thus the limitation is almost certainly unnecessary
(creating ~300 users seems to take ~450ms).

Fixes ##776
2023-06-14 16:01:42 +02:00
Xavier Morel
611f9150ff [IMP] runbot_merge: add signed kw support to from_role, use it
Closes #774
2023-06-14 16:01:42 +02:00
Xavier Morel
485d2d7b55 [IMP] runbot_merge: add sitemap params to http controllers
When it's missing, website complains because it's dumb.

Fixes #763
2023-06-14 16:01:42 +02:00
Xavier Morel
4f237d15b0 [FIX] runbot_merge: correctly check request in handle_pr
652b1ff9ae wanted to check if a request
was available, however it deref'd the `request` object without
checking it which is not correct: a `request` normally has an
`httprequest`, but the `request` itself might be missing if the
handler is called from e.g. a cron.

Fixes #739
2023-06-14 16:01:42 +02:00
Xavier-Do
a80dc25699 [FIX] runbot: only top parent is marked killable 2023-06-14 10:30:02 +02:00
Xavier-Do
9cf750119d [IMP] runbot: make search case insensitive 2023-06-02 17:11:54 +02:00
Xavier-Do
03667b703c [IMP] runbot: fallback on python step for upgrade step 2023-06-02 14:44:48 +02:00
Xavier-Do
ad4131789c [FIX] runbot: fix false positive message 2023-06-02 11:10:51 +02:00
Xavier-Do
7845a718b7 [IMP] runbot: add use_ssl to settings 2023-06-02 10:49:05 +02:00
Martin Trigaux
8153fdc4b2 [FIX] runbot: download over https
Firefox blocks downloads from http link if you are on an https page
Allow to deactivate via an ICP in case the runbot is configured over
HTTP (you shouldn't really)
2023-06-02 10:49:05 +02:00
Christophe Monniez
236554b588 [FIX] runbot: catch exceptions during db drop
When the runbot tries to drop a local database, if the that raises an
exception, it goes in a loop failure. It mays happen for example if
someone forgot to close a psql during an investigation :-)

With this commit, the exceptions are catched and at least the database
name is logged.
2023-06-01 16:16:06 +02:00
Xavier-Do
7523dc8000 [FIX] runbot: fix dockerfile choice order
Since all versions will have a defined dockerfile, the project one
will alway be ignored. The idea here is that for a project, we may
definea default dockerfile_id so that we don't have to set it on all
bundle to make it work.
2023-06-01 16:13:40 +02:00
Xavier-Do
db38794f9a [FIX] runbot: avoid sending sattus on running kill
The _kill method was called in multiple case, usually when something
wrong happen:
- exception initiating pending
- kill requested manually
- testing time exceeded
- exception running a job
- ...

But it will also be called when killing a running build.

It was usually not an issue since the status remains the same, but it is
not true if the same commit is used in two build, the new one is green,
the old one is red (enterprise commit remaining the same but community
commit changed as an example)

In this situation, the enterprise commit may receive the red ci from the
old build while the last one is green.

Since with the last version, the github status responsibility is left to
write method, this github status is not useful anymore, updating the
state and result is enough.

This commit also removes the commit since it is not always a god idea.
Most of the time the transaction will be comited quite fast after that
with the new scheduler.

Note that checking in github status if no status has a more recent build
may be a good idea. Only the most recent build using a commit could
sending a status? This would not alway be helpful Imagine a commit used
in 2 branches by mistake, the last build is not always the one we want
(usually fixed by rebuilding a subbuild of the good build)
2023-06-01 15:40:16 +02:00
Xavier-Do
64d3c59ed9 [FIX] runbot: limit max log size
In some case, a build can add a lot of info in a log, there
is already a limit to the number of entry but not to the size of an
entry. This will limit the database usage in case of mistake/abuse.
2023-06-01 15:33:47 +02:00
Xavier-Do
5a5e7693d4 [IMP] runbot: add an option on step to disable logdb 2023-06-01 15:13:01 +02:00
Christophe Monniez
f6eb23f896 [IMP] runbot: improve frontend search
When filtering bundles in the frontend, the user is not able to search
for its final trigram because of the `like`search.

With this commit, if the search contains a `%` symbol, the `=like`
operator is used permitting more accurate searches.
2023-06-01 15:03:13 +02:00
Christophe Monniez
2421a24f78 [IMP] runbot: add show builds the host form 2023-06-01 15:03:13 +02:00
Christophe Monniez
45fb4f8319 [IMP] runbot: add activities on runbot build errors 2023-06-01 15:03:13 +02:00
Christophe Monniez
86616ba88e [IMP] runbot: add a widget to go to runbot frontend
With this commit, a custom widget is added to go to the reunbot frontend
from a Char field. This allows to go from the bundle backend page to the
bundle frontend page wich is more useful in some situations.

e.g.: when creating a custom trigger with the wizard, this allows to
test the trigger with 2 clicks.
2023-06-01 15:03:13 +02:00
Christophe Monniez
e445ed27db [IMP] runbot: change fixing commit widget to url 2023-06-01 15:03:13 +02:00
Christophe Monniez
bdd98b07ec [IMP] runbot: rename active to active
Error is not fixed was too disturbing.
2023-06-01 15:03:13 +02:00
Christophe Monniez
6e145ff362 [IMP] runbot: improve build errors tabs
* show only the all builds tab
* hide linked errors tab when there is no linked errors
* hide error history tab when there is no history
* add some readonly
2023-06-01 15:03:13 +02:00
Christophe Monniez
ffe12182ab [IMP] runbot: open frontend_url in a new tab 2023-06-01 15:03:13 +02:00
Christophe Monniez
430a526b5c [FIX] runbot: limit triggers in additionnal_setup
When testing with a populated db, this assertion fails because of
additional repositories.
2023-06-01 15:03:13 +02:00
Christophe Monniez
82c6b22e77 [FIX] runbot: typo in test tag 2023-06-01 15:03:13 +02:00
Christophe Monniez
2e002c2dd7 [IMP] runbot: improve the build errors wizard
With this commit, the wizard now allows to set a fixing PR and/or a
commit text to multiple errors.
2023-06-01 15:03:13 +02:00
Christophe Monniez
388eeb377a [IMP] runbot: improve build errors views
* add a link to the fixing PR on github
* add a warning ribbon on test-tagged errors
* show different colors in tree view to spot fixed PR's
* add some search filters
2023-06-01 15:03:13 +02:00
Xavier-Do
231febab96 [FIX] runbot: fix stats.js 2023-05-08 13:26:07 +02:00
Xavier-Do
04760491a2 [FIX] runbot: hide wakeup action from public users
The initial idea to have a wakeup for public users stopped being viable
due to some abuse of the system, maybe unintentional crawling of
some build page but still, this feature will now be limited to internal
users only.
2023-05-02 13:32:43 +02:00
Xavier Morel
6bc6dd77ab [FIX] runbot_merge: mismatch can contain non-str values
The mismatch diff attribute contains values from the in-db object and
the github PR structure, some of which are explicitly *not*
strings (e.g. the squash flag, possibly the commits # in the future).

As a result, when the squash-flag of a PR differs from the actual the
formatting for diffing blows up, because difflib can't handle
non-strings.

Stringify values between passing them to `format_items`, this way the
string operations on names and values should work correctly.
2023-04-17 08:27:57 +02:00
Xavier-Do
e3d87b5b5d [IMP] runbot: improve test-tags support
The current post_install build mecanism is using extra params to give
test-tags. Unfortunately this disables the support for auto tags
and this have to be done manually. This means that auto tags are in the
build extra-params and not dynamic at rebuild of a post_install.

Also, using extraparams in the post install creation was removing
extra_params comming from custom trigger.

With this commit, the test-tags can be given inside config_data
and will be combined with config step test-tags and auto-tags.

This was an opportunity to simplify the logic.

This commit also fixes the test_install_tags that was broken.
2023-04-06 11:33:01 +02:00
Xavier-Do
2ad188201b [IMP] runbot_merge: speedup frontend page
The mergebot page become a bit slow with the years, it is time to make
small optimisation to speed up thinks a little.

Note: all changes where applied modifying the views or adding index by
hand. There is still room for improvement but it would need more in
depth refactoring, mainly adding specialized computed fields to
enable a better batching.

The first issue was using branch.staging_ids

    branch.staging_ids.sorted(lambda s: s.staged_at, reverse=True)[:6]

The number of staging_ids is increasing and prefetching + sorting all
of them is slow.

The proposed solution is to replace it by a search, not ideal, a
specialized compute field may be a good idea, but this is a quick fix
that can be done editing a view.

    branch.env['runbot_merge.stagings'].search([('target', '=', branch.id)],order='staged_at desc', limit=6)

Other changes are just index on critical columns.

Before changes, /runbot_merge page takes ~5s to load
After changes,  /runbot_merge page takes ~1s to load

Small note: note 100% sure that runbot_merge.batch.target was useful
2023-04-05 09:05:50 +02:00
Xavier-Do
43d5cc9d7e [IMP] runbot: add some anchors in nginx 2023-04-04 10:43:58 +02:00