Commit Graph

626 Commits

Author SHA1 Message Date
Xavier Morel
41400f791b [FIX] runbot_merge: remove commit from message when closing a PR
As noted in the old comment, the provided commit was the head of the
staging branch. This confused many users as they'd click the link
expecting to see their commit and potentially got something completely
unrelated.

Since we already get backlinks from the commit through the "closes
<pr_number>" added either by the committer or by the bot itself, the
information is already available.
2018-10-12 16:56:25 +02:00
Xavier Morel
bab7e25a9a [IMP] runbot_merge: split the MegaCron into sub-methods 2018-10-12 16:24:35 +02:00
Xavier Morel
5ec2c12454 [IMP] runbot_merge: split tagging cron out of main progress cron
They're completely independent (or should be), and there's no reason
for the tagging cron to extend the "lifetime" of the main cron's
transaction (and thus extend the odds of racey behaviour).
2018-10-12 15:54:14 +02:00
Xavier Morel
f238304c44 [IMP] runbot_merge: stagings views
* fix "Active" filter which was not updated when the active field was
  added
* properly enable it by default instead of relying on active_test
* disable active_test on the Stagings action, otherwise the batches
  are not visible in the staging once the staging and batches have been
  disabled
2018-10-12 13:46:22 +02:00
Christophe Monniez
c5e645df9e [IMP] runbot: add rebuild button on build page
When examining a particular build with the build view, one can be
frustrated being forced to navigate the frontend page to ask for a
rebuild.

With this commit, the rebuild menu entry is also visible on the build
page when the build is the last one of the branch.

Also the build host is now visible on the build page for the same
usability reason.
2018-10-10 17:19:59 +02:00
Xavier Morel
af8c62e4ad [FIX] runbot_merge: better handle targets being branch-protected
If a staging covers multiple repositories and there's a fast-forward
issue on any but the first repo/target, runbot_merge attempted to
revert the commits it had fast-forwarded on the previous repos.

This doesn't work when branch-protection is active, unless runbot_merge
is a repository administrator (and branch protection is not configured
to apply to those): reverting is done by push-forcing the original head
back onto the ref, which branch-protection unconditionally precludes.

This commit does not entirely fix the race condition (it does not look
like github provides any way to do that), but it should significantly
reduce the race-condition window as it performs a semi-wet run of the
fast-forward process on the tmp branches before actually updating the
targets. That way the only remaining breakage should be when somebody
pushes on repositories 1.. between the test-FF on tmp branches and the
actual fast forward.

While at it, updated the github API thing to *always* dump the JSON body
on an error response, if the content-type is json.
2018-10-10 10:50:21 +02:00
Christophe Monniez
070dbee204
[IMP] runbot: add priority field on branch
This commit permits to prioritize a branch when scheduling builds.
It's main purpose is for the runbot_merge module. It avoids to have
staging branches as sticky and pollutes the main repo view.
Also, that branch can benefit of the autokill feature when a newer build
is found, freeing ressources for other builds.

Closes #43
2018-10-09 16:42:34 +02:00
Xavier Morel
a1384b3868 [FIX] runbot_merge: iterate commits in topological order
Previously, runbot_merge assumed github would return commits in
topological order (from base to head of PR). However as in the UI
github sorts commits using the author's date field, so depending on
rebasing/cherrypick/... it's possible to have the head of the commit
be "younger" than the rest. In that case robodoo will try to merge
it *first*, then attempt to merge the rest on top of it (-ish, it'd
probably make a hash of it if that worked), at which point github
replies with a 204 (nothing to merge) because the PR head has already
included everything which topologically precedes it.

Fix: re-sort commits topologically when fetching the PR's log. That
way they're rebased in the proper order and correctly linked to one
another.

Example problematic PR: odoo/enterprise#2794, the commits are

773aef03a59d50b33221d7cdcdf54cd0cbe0c914
    author.date: 2018-10-01T14:58:38Z
879547c8dd37e7f413a97393a82f92377785b50b (parent: 773aef03)
    author.date: 2018-10-01T12:02:08Z

Because 879547c8 is "older" than 773aef03, github returns it first,
both in the UI and via the API.

Also fixed up support for committer & author metadata in fake_github
so the local tests would both expose the issue properly and allow
fixing it.
2018-10-09 15:08:13 +02:00
Xavier Morel
16c492ef0a [FIX] runbot_merge: merge() can be non-json in some cases (???)
Rather than blow up with a json error and take down the cron, convert
json decode error to a mergeerror in order to put the PR in error and
try to dump however much data we can.
2018-10-08 16:52:33 +02:00
Xavier Morel
dd29e6b8a8 [IMP] runbot_merge: attempt to fix race condition
Because mergebot cron can run on any runbot, it's apparently possible
that a staging gets merged and the "closed" feedback from github
overwrites the merged status which the mergebot is supposed to set
despite the supposed protection.
2018-10-08 16:52:33 +02:00
Xavier Morel
e590d565c1 [FIX] runbot_merge: cancel reason on PR close
Was not properly formatted, so the message would just be
"PR %s:%s closed by %s".
2018-10-08 15:31:07 +02:00
xmo-odoo
d042bc541f Better reporting of staging state
* [ADD] runbot_merge: more informative states to stagings on error

Currently, when a staging fails for other reasons than a CI failure:

* the staging having been cancelled is known implicitly, because the
  staging will be deactivated but will never get a status beyond
  pending (because it's not found when looking for it since it's not
  `active`)
* the fast-forward having failed is completely silent (logging aside),
  it looks for all the world like the staging succeeded

Timeout fails the PR already, but split-on-timeout was not so fix that
one bit.

* [FIX] odoo/odoo#cb2862ad2a60ff4ce66c14e7af2548fdf6fc5961

Closes #41
2018-10-01 10:21:32 +02:00
Xavier Morel
5ebb53cdc7 [IMP] runbot_merge: error logging on 422 responses from GH 2018-09-28 13:05:41 +02:00
Xavier Morel
c687e9ae8b [IMP] runbot_merge: support @ and # prefixes to delegate= logins 2018-09-25 16:42:56 +02:00
Xavier Morel
359a3cf872 [ADD] runbot_merge: support for r- on approved & ready PRs
Can be used by the PR author or a reviewer. Removes reviewed state of
PR, and cancels the staging if the PR is already staged.
2018-09-25 15:04:31 +02:00
Xavier Morel
6df9a68af2 [CHG] runbot_merge: treat github reviews as regular comments
Treating them specially turns out to be inconvenient: it becomes
harder/riskier to ask for informal reviews (for which GH reviews are
useful).
2018-09-25 14:05:41 +02:00
Xavier Morel
7fc7b78a04 [FIX] runbot_merge: security concern
The webhook used the "sender" of the event as comment author, however
if the comment is edited by a maintainer github sends a
"issue_comment" event with that maintainer as sender.

This means a random user could create a comment with a robodoo
command, and if a registered reviewer happened to edit the comment the
command would suddenly be taken in account. This was not the intention.
2018-09-24 10:06:58 +02:00
Xavier Morel
ac2adfbdea [IMP] runbot_merge: only show mergebot configuration to mergebot admins 2018-09-22 16:10:54 +02:00
Xavier Morel
20bb058f06 [FIX] runbot_merge: make staging cancels more greppable
I just spent 10mn trying to find out why staging 28 was cancelled
(a p=0 comment). Add a common prefix to all staging cancels to make
them easier to find.
2018-09-21 15:58:30 +02:00
Xavier Morel
e6a1a0634a [FIX] runbot_merge: re-enable staging delay
staging delay was mistakenly commented in
bb664455ec

Also modified testing fixtures so the staging delay is not enabled when
running tests locally: on my box it increases the local runtime from
~70s to ~1500s (20s/staging, ~1 staging/test, 73 tests)
2018-09-21 15:57:16 +02:00
Xavier Morel
f3383daf60 [IMP] runbot_merge: remove ref update while squashing/rebasing
It should be unnecessary: creating commits directly does not update
the ref (hence 2b1cd83b07) and we're
forcefully setting the ref afterwards, either resetting it to the
original head (for rebase) or updating it to the commits we've just
created (for squash).
2018-09-21 13:53:09 +02:00
Xavier Morel
7a31099d72 [IMP] runbot_merge: looks of dashboard page 2018-09-21 12:19:53 +02:00
Xavier Morel
bb664455ec [IMP] runbot_merge: allow invoking bot as @bot and #bot
Before this, the bot would only acknowledge commands of the form

    <botname>: <commands>

but since the bot is an actual user, people regularly use `@<botname>`
as it seems like it should work *and* provides for autocompletion.

Support that, as well as the octothorpe in case users want to pound
robodoo.

Related to odoo/runbot#38
2018-09-21 12:19:53 +02:00
Christophe Monniez
b427cc675d
[IMP] runbot: add a frontend template for builds by branch
The frontend view shows only the four last builds by branch, which is a
little bit small to explore builds and search for failed builds.

With this commit, a new frontend template displays a paged list of
builds for a specified branch.

Closes #39
2018-09-21 08:45:30 +02:00
Xavier Morel
0d0140fad5 [IMP] runbot_merge: staging delay
Continuation of fa94b269de which is
apparently not sufficient:

1. log the staging event so we can check that we're staging in the
   correct order
2. add a delay after each staging in case there's some sort of race
   in the updating of codependent repositories
2018-09-20 16:42:35 +02:00
Xavier Morel
300f55d864 [FIX] runbot_merge: screwed up associativity of if/else 2018-09-20 16:23:27 +02:00
Xavier Morel
8a55407f87 [FIX] runbot_merge: like PRs, review bodies are null if left empty 2018-09-20 16:11:43 +02:00
Xavier Morel
fa94b269de [FIX] runbot_merge: order of staging branch updates
When creating staging branches from tmp, use the iteration order of
the repos in the project (that way it's easy to see and eventually
configure if we add sequences or whatever, in the short term it's the
order in which the repos were added which is the one we want).

This ensures we stage odoo/odoo before we stage odoo/enterprise
without relying on dict order of iteration, or needing meta to be an
OrderedDict.

The issue is if stagings are created/updated the other way around, the
runbot may pick up staging on odoo/enterprise before odoo/odoo has
been updated, and thus build odoo/enterprise with the wrong odoo/odoo
commit, and defeat the entire point of it.

Example: http://runbot.odoo.com/runbot/build/376112 was triggered by
the same staging as http://runbot.odoo.com/runbot/build/376113, but
used the previous staging head.

The creation order of tmp branches should not matter so ignore it.
2018-09-20 15:50:53 +02:00
Xavier Morel
3885ca244c [FIX] runbot_merge: handling of PRs of >50 commits
A limitation to 50 commits PRs was put in place to avoid rebasing
huge PRs (as a rebase means 1 merge + 1 commit *per source commit*),
however the way it was done would also limit regular merges, and the
way the limitation was implemented was not clear.

* explicitly check that limit in the rebase case
* make error message on PR sizes (rebase 50 or merge 250) clearer
* remove limit from commits-fetching (check it beforehand)
* add a test to merge >50 commits PRs
* fix the local implementation of pulls/:number/commits to properly
  paginate
2018-09-20 15:28:55 +02:00
Xavier Morel
b62afb7673 [ADD] runbot_merge: test for previous commit
2b1cd83b07 fixed a bug in PR
squashing (introduced when it was mis-rebuilt on top of rebase) which
was immediately committed & pushed so we could fix the running
mergebot.

This adds a test for that issue, it was checked to fail for
2b1cd83b075a99da7ed905b9e62d7e5acb48b253~1 and work as of the current
head.

Turns out the previous tests checked all the new/complex features to
see if they worked correctly, but I completely forgot that the
previously working squash had been rebuild.
2018-09-20 10:52:58 +02:00
Xavier Morel
2b1cd83b07 [FIX] runbot_merge: forcefully update dest after non-reset rebase 2018-09-20 10:08:08 +02:00
Xavier Morel
3d8add9c11 [IMP] runbot_merge: add change of target branch between merges 2018-09-20 10:04:13 +02:00
Xavier Morel
58f1c34e3f [ADD] runbot_merge: logging of github operations 2018-09-20 09:25:13 +02:00
Xavier Morel
3db55a849f [ADD] runbot_merge: one more test, still doesn't repro staging 13 2018-09-19 18:49:52 +02:00
Xavier Morel
d3952de3d5 [IMP] runbot_merge: add logging to staging
I can't repro staging 13, so adding some logging.
2018-09-19 18:09:45 +02:00
Xavier Morel
6dbfde0389 [ADD] runbot_merge: add tests to try and expose the mystery of staging 13
Staging 13 tried merging 3 PRs (27085, 27083 and 27071) and supposedly
succeeded *but* only merged one of the 3 PRs despite marking all three
as merged. I tried building a few tests constructing multi-PR graphs
and checking them, but the only thing they exposed was the local
github implementation not correctly updating merge targets.

So fixed that, which is good.

Doesn't tell me why the staging didn't work right though.
2018-09-19 17:33:25 +02:00
Xavier Morel
c2db5659d8 [FIX] runbot_merge: handle PRs with no body (not just empty) 2018-09-19 14:40:36 +02:00
Xavier Morel
7310cd1f1d [IMP] runbot_merge: link to failed runbot builds
a0063f9df0 slightly improved the error
message on non-PR ci failure (e.g. a community PR makes enterprise
break) by adding the failed commit, but that's still not exactly clear,
even for technical users (plus it requires having access to all the
repos which is not the case for everyone).

This commit further improves the situation by storing the target_url
and description fields of the commit statuses, and printing out the
target_url on failure if it's present.

That way the PR comment denoting build failure should now have a link to
the relevant failed build on runbot, as that's the target_url it
provides.

The change is nontrivial as it tries to be compatible with both old and
new statuses storage format, such that there is no migration to perform.
2018-09-17 11:04:31 +02:00
Martin Trigaux
8f7a5e55ef [FIX] runbot_merge: avoid double closes message
If the message already contains the pr number, no need to add it again
2018-09-13 10:30:47 +02:00
Xavier Morel
2a7a3c6167 [FIX] runbot_merge: don't close PRs pointing to dummy commit
e98a8caffb added dummy commits to the
heads of stagings and fixed most places to make a difference between
the staging head (including dummy commit) and the actual merge head,
but the difference was missed in the comment closing a PR, which was
still using the staging head and thus pointing to the dummy commit
e.g. (https://github.com/odoo/odoo/pull/26821#issuecomment-420244592)
2018-09-11 13:53:41 +02:00
Xavier Morel
a0063f9df0 [IMP] runbot_merge: provide more useful message on some staging failures
If CI fails on a non-PR'd branch of a staging (e.g. given repos A and B
and a PR to A, CI fails on the staging branch to B), the error message
(log and comment on the PR) is unhelpful as it states that the staging
failed for "unknown reason".

Improve this by providing the failed CI context and the commit, which
should allow finding out the branch & CI logs, and understanding the
why of the failure.

Fixes #36
2018-09-11 10:21:24 +02:00
Xavier Morel
e98a8caffb [FIX] runbot_merge: ensure all staging branches are built/tested
Before this change, when staging batches only affecting one repo (of n)
the unaffected repositories would get a staging branch exactly matching
the target.

As a result, either runbot_merge or runbot would simply return the
result of an unrelated build, potentially providing incorrect
information and either failing a staging which should have succeeded
(e.g. change in repo A broke B, PR is making a change in repo A which
fixes B, but B's state is reported as the previous broken build) or
succeeding a staging which should have failed (change in repo A breaking
B except a previous build of the exact same B succeeded with a different
A and is returned).

To fix this issue, create a dummy commit at the head of each staging
branch. Because commit dates are included in the hash and have a second
precision it's pretty unlikely that we can get built duplicates, but
just to be completely sure some random bits are added to the commit
message as well.

Various tests fixed to correctly handle the extra dummy commit on
staging branches.

fixes #35
2018-09-11 10:21:24 +02:00
Xavier Morel
2a17bbec82 [FIX] runbot_merge: wrong model name in a view 2018-09-10 12:32:33 +02:00
Christophe Monniez
f9b057840a
[IMP] readme: add the Odoo workflow
Closes #34
2018-09-10 09:50:17 +02:00
Andreas Perhab
3915f3d7ae [IMP] runbot: perform fetch with one git command
Closes: #33
2018-09-06 11:56:58 +02:00
JKE-be
0542b68b92 [IMP] runbot: allow to search multi terms with |
With this feature you can easily search your branch and branch from your team; or your features...

Eg: jke|-website or -jke|-rde|-qsm|...

Closes: #32
2018-09-06 10:03:57 +02:00
Xavier Morel
47c3e752e9 [IMP] runbot_merge: add action for planned PR fetches 2018-09-03 17:53:43 +02:00
Xavier Morel
fd705d241a [IMP] runbot_merge: secret field & titles for o2m tables 2018-09-03 17:50:18 +02:00
Xavier Morel
ec5d60d027 [CHG] runbot_merge: unapprove on PR update
After discussion with mat, rco and moc, if a PR is updated it should
be unapproved for safety reasons: if a reviewer approves a PR, that's
what should be merged, if there are things to fix/change a reviewer
should at least rubberstamp the changes to avoid mistakes.

This is a bit more noisy/constraining, but can be changed or tuned
afterwards if it's considered too constraining.
2018-09-03 13:55:39 +02:00
Xavier Morel
5c4018b91e [ADD] runbot_merge
Does not change anything in runbot itself, so if there's things which
don't work or we need to change we can probably fix them in master
directly.
2018-09-03 13:23:27 +02:00