[FIX] runbot_merge: removal of fw tasks on limit update

There is no check or unique index, so nothing actually prevents having
multiple fw tasks for the same batch.

In that case, if we try to update the limit the handler will crash
because the code assumes a single forwardport task per batch.

Triggered by nle on odoo/odoo#201394 probably while trying to recover
from the incident of March 13/14 (which was not going to work as the
forwardport queue itself was hosed, see two commits above for the
reason).
This commit is contained in:
Xavier Morel 2025-03-14 15:13:15 +01:00
parent 751154514a
commit c4c32957be
2 changed files with 32 additions and 3 deletions

View File

@ -345,6 +345,35 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
] ]
def test_limit_multiple_fw_tasks(env, config, make_repo, users):
"""Bit of a special case here: technically nothing prevents having multiple
forward port tasks in flight (the second will just fail / be cancelled), but
`_maybe_update_limit` assumed only one batch was possible.
"""
# disable cron so we don't risk the entire thing running on us
env.ref('forwardport.port_forward').active = False
prod, _ = make_basic(env, config, make_repo, statuses='default')
with prod:
prod.make_commits('a', Commit('c', tree={'a': '0'}), ref='heads/abranch')
pr = prod.make_pr(target='a', head='abranch')
prod.post_status('abranch', 'success')
pr.post_comment('hansen r+', config['role_reviewer']['token'])
env.run_crons()
with prod:
prod.post_status('staging.a', 'success')
env.run_crons()
fp = env['forwardport.batches'].search([])
assert len(fp) == 1
assert fp.source == 'merge'
fp.copy()
with prod:
pr.post_comment("hansen up to a", config['role_reviewer']['token'])
env.run_crons()
assert to_pr(env, pr).limit_id.name == 'a'
@pytest.mark.parametrize("update_from", [ @pytest.mark.parametrize("update_from", [
pytest.param(lambda source: [('id', '=', source)], id='source'), pytest.param(lambda source: [('id', '=', source)], id='source'),

View File

@ -1089,8 +1089,8 @@ For your own safety I've ignored *everything in your entire comment*.
with self.env.cr.savepoint(): with self.env.cr.savepoint():
self.env.cr.execute( self.env.cr.execute(
"SELECT FROM forwardport_batches " "SELECT FROM forwardport_batches "
"WHERE id = %s FOR UPDATE NOWAIT", "WHERE id = any(%s) FOR UPDATE NOWAIT",
[task.id]) [task.ids])
except psycopg2.errors.LockNotAvailable: except psycopg2.errors.LockNotAvailable:
# row locked = port occurring and probably going to succeed, # row locked = port occurring and probably going to succeed,
# so next(real_limit) likely a done deal already # so next(real_limit) likely a done deal already
@ -1099,7 +1099,7 @@ For your own safety I've ignored *everything in your entire comment*.
f"ongoing, unable to cancel, close next forward port " f"ongoing, unable to cancel, close next forward port "
f"when it completes.") f"when it completes.")
else: else:
self.env.cr.execute("DELETE FROM forwardport_batches WHERE id = %s", [task.id]) self.env.cr.execute("DELETE FROM forwardport_batches WHERE id = any(%s)", [task.ids])
if real_limit != tip.target: if real_limit != tip.target:
# forward porting was previously stopped at tip, and we want it to # forward porting was previously stopped at tip, and we want it to