Commit Graph

1117 Commits

Author SHA1 Message Date
Xavier Morel
1b5a05e40c [FIX] mergebot: improve handling of having missed PR updates
1. if we try to stage a PR and realize we'd stored / checked the wrong
   head, cancel the staging and notify the PR
2. provide a command to forcefully update pr heads (or at least check
   that a PR's head is up to date)

Closes #241
2019-11-21 08:10:39 +01:00
Xavier Morel
a45f7260fa [IMP] forwardport: better handle authorship information
Before this:

* the forwardport bot always sets itself as the committer when it
  creates a forward-port branch
* when creating squashed / conflict commits, it becomes both author
  and committer

This is not great as it loses a fair amount of authorship information
and doesn't properly give credit where credit is due. Improve this in
the following ways:

* use the original authorship information as-is when forward-porting
  commits
* the the forward port fails, use the original authorship information
  as-is if there's a single commit to forward port
* otherwise if there's only a single author / committer across the
  branch being forwardported use that, if there are multiple give up
  and use the identity of the 'bot, since the branch will probably
  need to be re-done in full the authorship information of the
  placeholder commit should not matter overly much

Uses git's magic ENV variables as that's pretty much the only way to
properly override the COMMITTER date: conf items only provide for
author and committer *identity* (name and email), and while `commit`
allows overriding the *authorship* date (`--date`) it doesn't provide
any option for the *commit* date.

Fixes #255
2019-11-21 08:10:39 +01:00
Xavier Morel
8e67aed792 [IMP] runbot_merge: limit spamming on PR close
When closing a PR, github completely separates the events "close the
PR" and "comment on the PR" (even when using "comment and close" in
the UI, a feature which isn't even available in the API). It doesn't
aggregate the notifications either, so users following the PR for
one reason or another get 2 notifications / mails every time a PR
gets merged, which is a lot of traffic, even more so with
forward-ported PRs multiplying the amount of PRs users are involved
in.

The comment on top of the closure itself is useful though: it allows
tracking exactly where and how the PR was merged from the PR, this
information should not be lost.

While more involved than a simple comment, *deployments* seem like
a suitable solution: they allow providing links as permanent
information / metadata on the PRs, and apparently don't trigger
notifications to users.

Therefore, modify the "close" method so it doesn't do
"comment-and-close", and provide a way to close PRs with non-comment
feedback: when the feedback's message is structured (parsable as
json) assume it's intended as deployment-bound notifications.

TODO: maybe add more keys to the feedback event payload, though in my
      tests (odoo/runbot#222) none of the deployment metadata
      outside of "environment" and "target_url" is listed on the PR
      UI

Fixes #224
2019-11-21 08:10:39 +01:00
Xavier Morel
b853e5ba96 [IMP] runbot_merge: further validation of squash update on retargeting
It should have already been working, added an additional check for
update-then-retarget just in case but that worked out of the box. So
not sure why odoo/odoo#40106 failed.

Closes #256
2019-11-21 08:10:39 +01:00
Xavier Morel
8a47c57c3d [ADD] runbot_merge: logging of every GH request / response
If odoo is configured with a logfile, log to a separate file in the
same directory.

* log request / response when querying github
* log *received* requests for webhooks

Either way log the entire request metadata, though only the first 400
bytes/chars of the entity bodies.

This is intended to help mostly with post-mortem debugging: timestamps
from the main log can be correlated with the timestamps from the
github log in order to have more relevant information, both for
internal use and to send to gh support.

Closes #257
2019-11-21 08:10:39 +01:00
Christophe Monniez
f8614c4abd [FIX] runbot: grep for enhanced test-tag for auto_tags
When using auto_tags, most of the time, the enhanced version are used.
For example, using "-:TestPoSStock" to disable the test class.
If the tested Odoo version does not support this kind of tag, they are
considered as simple tags, thus disabling all tests.

It 's the case for Odoo saas-11.3.

With this commit, the auto_tags are only used on Odoo versions that
support the new test tags.
2019-11-19 14:54:16 +01:00
Xavier Morel
6f68db15d6 [IMP] runbot_merge: sanity check when rebasing
Ensure that the commits we're creating are based on the commit we're
expecting.

This is the second cause (and really the biggest issue) of the "Great
Reset" of master on November 6: a previous commit explains the issue
with non-linear github operations (update a branch, get the branch
head, they don't match).

The second issue is why @awa-odoo's PR was merged with a reversion of
@tivisse's as part of its first commit.

The stage for this issues is based on the incoherence noted above:
having updated a branch, getting that branch's head afterward may
still return the old head. However either delays allow that update to
be visible *or* different operations can have different views of the
system. Regardless it's possible that `repos/merges` "sees" a
different branch head than a `git/refs/heads` which preceded it by a
few milliseconds. This is an issue because github's API does not
provide a generic "rebase" operation, and the operation is thus
implemented by hand:

1. get the head of the branch we're trying to rebase a PR on
2. for each commit of the PR (oldest to newest), *merge* commit on the
   base and associate the merge commit with the original
3. reset the branch to the head we stored previously
4. for each commit of the PR, create a new commit with:

   - the metadata of the original
   - the tree of the merge commit
   - the "current head" as parent

   then update the "current head" to that commit's ref'

If the head fetched at (1) and the one the first merge of (2) sees are
different, the first commit created during (4) will look like it has
not only its own changes but also all the changes between the two
heads, as github records not changes but snapshots of working
copies (that's what a git tree is, a complete snapshot of the entire
state of a working copy).

As a result, we end up not only with commits from a previous staging
but the first commit of the next PR rollbacks the changes of those
commits, making a mess of the entire thing twice over. And because the
commits of the previous staging get reverted, even if there was a good
reason for them to fail (not the case here it was a false positive)
the new staging might just go through.

As noted at the start, mitigate that by asserting that the merge
commits created at (2) have the "base parent" (left parent / parent
from the base branch) we were expecting, and cancel the staging if
that's not the case.

This can probably be removed if / when odoo/runbot#247 happens.
2019-11-12 07:44:01 +01:00
Xavier-Do
1e102b7d76 [FIX] runbot: check support before adding auto-tags 2019-11-08 15:40:36 +01:00
Xavier-Do
0c4d78f497 [FIX] runbot: various small fixes 2019-11-08 15:33:30 +01:00
Xavier-Do
a7992e9f50 [IMP] runbot: add no_build on repo
When runbot is installed to test customs addons, we don't
want to build all odoo commit, but we need to update branches
in order to make _get_closest_branch work.

This commit will allow a user to set odoo in poll mode
with no_build set to True, to create branches only.

(And a small fix for additionnal_env)
2019-11-08 12:08:02 +01:00
Christophe Monniez
631b29fcf8 [FIX] runbot: fix container cli tests
Since 81fefee, the container.py CLI does not work as expected.

With this commit, the CLI is working, a new arg was added to test
flamegraphs and the dump is adapted to mimic the runbot.

Also, a small issue is fixed in the zip file creation. Before the zip
creation, the directory is changed, if the directory change fails, the
zip is created from the current directory which is removed by zip at the
end of the process. That could lead to the deletion of the build dir.
2019-11-08 10:29:51 +01:00
Xavier-Do
685cc282ba [IMP] runbot: add env parameter support for docker in config steps 2019-11-08 10:29:51 +01:00
Xavier Morel
7598d45283 [IMP] runbot_merge: use exponential backoff on head check
It's a waste to lose the entire staging if it's only a short blip /
delay thing, so retry multiple times. Add utility function to make
backoff functions easier (though the UI is not great ATM).

Also log the "left" parent of a merge commit (which should be the
"base") when creating it, for additional post-mortem information.
2019-11-07 10:03:54 +01:00
Xavier-Do
58c683030d [IMP] runbot: add auto test-tags management.
A typical use case when an error is detected is to disable
this test by adding a negated test-tags on config
step 'all' and 'split_all'. This commit will help
to do that by adding a test_tags management on build error.

The user define a test_tag that will only execute failling test.
if a config step has the flag enable_auto_tags, the test tag will
be negated and added to config test-tags.

This commit also add some information for monitoring.
2019-11-07 09:51:05 +01:00
Xavier Morel
10ef6b82f0 [IMP] runbot_merge: sanity check PATCH git/ref/heads
Turns out not only can that operation fail, that operation can succeed
but have its effect delayed. To try and guard against that,
immediately check that we get the correct ref' after having reset it.

This is the cause of the November 6 mess: when preparing a staging,
the mergebot does the following,

1. get the head of <branch>
2. hard-reset tmp.<branch> to that
3. start merging PRs, which requires getting the current state of
   tmp.<branch> back

On the 6ths, these steps looked like this

```text
2019-11-06 10:03:21,588 head(odoo/odoo, master) -> ab6d0c38512e4944458b0b6f80f38d6c26b6b597
2019-11-06 10:03:22,375 set_ref(update, odoo/odoo, tmp.master, ab6d0c38512e4944458b0b6f80f38d6c26b6b597 -> 200 (OK)
2019-11-06 10:03:28,674 head(odoo/odoo, tmp.master) -> de2a852e7cc1f390e50190cfc497bc253687fba8
2019-11-06 10:03:30,292 head(odoo/odoo, tmp.master) -> de2a852e7cc1f390e50190cfc497bc253687fba8
```
So the 'bot fetched the commit at the head of master (ab6d0c), reset
tmp.master to that... and then got a different commit when it fetched
the tmp head to stage a PR on it.

That different head being of course a previous rejected staging. When
the new staging succeeded, it brought the entire thing in and made a
mess.

This was compounded by an issue I still have to investigate: the
staging of the new PR took the wrong base commit *but the right base
tree*, as a result the first thing it did was *reverse the entire
previous commit* (without that we could probably have left it as-is
rather than need to force-push master -- twice).
2019-11-07 07:52:12 +01:00
Christophe Monniez
34d26ad253 [IMP] runbot: add host in build errors debug mode 2019-11-06 14:57:55 +01:00
Xavier-Do
40bc185d28 [FIX] runbot: quite zip 2019-11-06 14:11:15 +01:00
Xavier-Do
d7f6075c8b [FIX] runbot: avoid build failure when test_tags not supported 2019-11-06 12:55:58 +01:00
Xavier-Do
10799ba9d6 [FIX] runbot: allow to manually hook by calling route 2019-11-05 13:42:39 +01:00
Christophe Monniez
daea9018d3 [FIX] runbot: fix database dump
* Add zip utility in Dockerfile
* fix zip command to avoid useless prefix
* write json info file at the right place
* fix zip file link
2019-11-04 17:28:37 +01:00
Xavier-Do
b9550dccca [IMP] runbot: improve db dump
Current dump version doesn't include filestore. This new
version adds the filestore trying to match odoo backup format
in order to ease restore.
manifest.json file is not create since it isn't usefull,
but an info.json is added, with build info.
2019-11-04 14:03:41 +01:00
Christophe Monniez
6e7d8d57f8 [IMP] runbot: add a multibuild wizard
Creating multi builds configs can be tedious. One must create 2 build
configs and 2 build config steps in the right order.

With this commit, a simple wizard is added that creates those 4
configurations by simply filling 4 fields.

Also, a new field, group, is added in order to be able to gather
config's and config steps into  groups. The group is a Many2one on a
config.

While at it, the runbot menu has been a bit rearranged with everything
about config's in a parent menu named Configs.

Config's and config's steps tree views have been enhanced to show the
config group and add some filters in the search views.
2019-10-28 15:26:15 +01:00
Xavier-Do
af4037261f [IMP] runbot: display childrens on duplicate 2019-10-28 10:54:46 +01:00
Xavier-Do
ddb7bec0c2 [FIX] runbot: remove useless divide by 2 2019-10-28 10:54:08 +01:00
Xavier-Do
3bf0550b3a [FIX] runbot: rename duplicate field description 2019-10-25 19:19:27 +02:00
Xavier-Do
a3d908ac03 [FIX] runbot: fix forgotten cleanup 2019-10-22 17:18:30 +02:00
Xavier-Do
18b37d9002 [IMP] runbot: gc database faster if corresponding build has parent_id 2019-10-22 16:55:12 +02:00
Xavier-Do
c874f4a046 [IMP] runbot: execute flamegraph in case of failure and log dl link 2019-10-22 14:33:10 +02:00
Xavier-Do
8284c7270d [IMP] runbot: add message if docker does not match dest format 2019-10-22 11:40:46 +02:00
Xavier-Do
702a52464d [FIX] avoid docker_ps crash if docker is not installed. 2019-10-22 10:35:09 +02:00
Xavier-Do
7f4f82a881 [FIX] runbot: docker cleanup should be run on workers only 2019-10-21 17:41:59 +02:00
Xavier-Do
ecd718ca2a [FIX] runbot: fix default modules and flamegraph 2019-10-21 17:03:43 +02:00
Xavier-Do
ec8a70a410 [IMP] runbot: allow to generate a flamegraph during a build
With this commit, a new boolean field "flamegraph" is added on the
build_config to allow a flamegraph generation.

In order to be able to generate a flamegraph during a runbot build, the
flamegraph package is added to the Docker image as well as the
flamegraph.pl tool.
2019-10-21 16:30:22 +02:00
Christophe Monniez
d9b20d6961 [IMP] runbot: dump database at the end of step
Dump a db at the end of a build, using a new 'finals' cmd part
added in order to execute dump even if build fails.

Add a link in last step log to download dump.
2019-10-21 16:23:05 +02:00
Christophe Monniez
d869d22f7b [FIX] runbot: add forgotten imports
Oversight of previous commit.
2019-10-21 15:17:58 +02:00
Christophe Monniez
5ff9cc2382 [FIX] runbot: clean running dockers with done builds
In different situations, a docker container may stay alive even if the
build global_state is done. This can lead to a build failure when a
build wants to go in running state and tries to expose the same ports as
the left over build.
2019-10-21 10:52:45 +02:00
Xavier-Do
5b12f37c74 [REV] runbot: Avoid infinite loop fetching"
This reverts commit 1207daded1.

A too quick review, setting a default value is a good idea but since field is a float now,
default value should be time.time
2019-10-18 20:20:10 +02:00
Christophe Monniez
3308829e80 [IMP] runbot: uses fnmatch patterns to select modules
Actually some Odoo modules are black_listed from a set hardcoded in the
runbot code. In some cases, one needs to blacklist custom modules,
preferably in a config_step.

With this commit, the repo.modules, branch.modules,
config_step.install_modules fields are concatained in a comma separated
list of fnmatch patterns. The patterns can be prefixed with a dash to
exclude the matching module(s).

Co-authored by @Xavier-Do
2019-10-18 16:30:06 +02:00
Xavier Morel
7c46a2006f [FIX] forwardport: the fix
Of course I forgot the most relevant bit
2019-10-18 12:01:47 +02:00
Xavier Morel
5f8041552b [FIX] forwardport: apparently git/refs/heads can fuzzy-match
If the ref we asked for does not exist, github apparently decides to
fall-back to prefix-matching. So if we're trying to delete
already-deleted branch A and someone called their branch A-x we're
going to get it as a result.

Thankfully they were apparently smart enough to return a list even if
there's only a single fuzzy match. So if we get a list (instead of a
dict) as response to git/refs/heads assume the branch was already
deleted as if we got a 404.
2019-10-18 11:22:13 +02:00
Moises Lopez - https://www.vauxoo.com/
1207daded1 [FIX] runbot: Avoid infinite loop fetching
Using `repo.hook_time = None` and `repo.mode = hook` the cron run fetch command in an infinite-loop.
2019-10-18 11:18:46 +02:00
Christophe Monniez
27b1c2b5f4 [IMP] runbot: add the active_step_id on the ir_logging log
In order to have a more efficient ir_logging filtering, this commit adds
the currrent build step id on the ir_logging line.
2019-10-18 11:03:22 +02:00
Christophe Monniez
db52bff323 [IMP] runbot: number of log lines per build
When a build is running, a cron, an evil query or something else can
start to fill and bloat the runbot ir_logging table.

With this commit, a log_counter field is added on the build, starting at
100. The SQL trigger decrement this counter after a line is inserted.

When the counter drops to 0, a the last log line contains a message
stating that the limit has been reached. Further log lines are dropped
for this build step.

The counter is reset to a default of 100 before each step.
This value is configurable through an optional ir.config_parameter
runbot_maxlogs.

The runbot itself is still able to add logs lines through the build _log
method.

Thanks @Xavier-Do for the smart idea.
2019-10-18 11:03:22 +02:00
Xavier Morel
13d76fdfb9 [FIX] forwardport: the fix
it's a per-call parameter not a per-instance one
2019-10-18 08:11:27 +02:00
Xavier Morel
c1cef0c18b [FIX] forwardport: gh api raises by default, avoid that here 2019-10-18 08:02:04 +02:00
Xavier Morel
ea410ab6d1 [ADD] forwardport: automatic branch deleter
If a PR is *merged*, enqueue it for deletion (with a 2 weeks delay).

Mainly to avoid FW branches staying around long after they've been
merged (possibly eventually closed?), will also clean up regular
merged branches, including historical merges forgotten by their
author.

Fixes #230
2019-10-17 11:55:20 +02:00
Xavier-Do
ac578d430d [IMP] runbot: propagate end_time to parent_build
When a build only create sub-builds, the build_time is verry small (a few seconds),
and this information is not relevant. This commit propagates end_time to parent_build
if parent_build is done or running.
2019-10-16 14:25:43 +02:00
Xavier Morel
401787b7ae [FIX] forwardport: co-dependent FPs where one PR is updated
In the case where we have a co-dependent forward port (co-dependent
PRs got forward-ported, which they should be together) where *one* of
the PRs got explicitly updated, the batch would "fall into a hole"
being handled as neither "this is part of a forward-port sequence" nor
"this is a new merge to forward-port" (the latter being the proper
one).

Modify & remove guards which checked that either no or all PRs in a
batch have parents: should be either all or not all.

Fixes #231
2019-10-15 08:54:25 +02:00
Xavier Morel
8937f379ce [FIX] forwardport: fix the fix
Re-add token field name when creating the tagging object, as it's a
required field....
2019-10-14 12:01:12 +02:00
Xavier Morel
97999318be [FIX] forwardport: don't use token field for tags update
Turns out tagging PRs requires a pretty significant level of ACLs
which we may not want to give to the forwardbot?

Anyway use the mergebot ACLs (which already include tagging) for this.
2019-10-14 10:09:48 +02:00