diff --git a/runbot/controllers/frontend.py b/runbot/controllers/frontend.py index 1e2802060..cab8c6189 100644 --- a/runbot/controllers/frontend.py +++ b/runbot/controllers/frontend.py @@ -329,6 +329,7 @@ def build(self, build_id, search=None, from_batch=None, **post): if not build.exists(): return request.not_found() siblings = (build.parent_id.children_ids if build.parent_id else from_batch.slot_ids.build_id if from_batch else build).sorted('id') + # TODO FIXME for linked builds. Sibling may depends on the context of the batch context = { 'build': build, 'from_batch': from_batch, diff --git a/runbot/models/build.py b/runbot/models/build.py index f9b611d22..825719614 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -350,10 +350,15 @@ class BuildResult(models.Model): string='Build type') parent_id = fields.Many2one('runbot.build', 'Parent Build', index=True) + parent_link_ids = fields.One2many('runbot.build.link', 'child_id', string='Used in links') + linked_parent_build_ids = fields.Many2many('runbot.build', compute='_compute_linked_parent_build_ids', string='All parent builds') + + child_link_ids = fields.One2many('runbot.build.link', 'parent_id', string='Child links') + linked_children_build_ids = fields.Many2many('runbot.build', compute='_compute_linked_children_build_id', string='All child builds') + parent_path = fields.Char('Parent path', index=True) top_parent = fields.Many2one('runbot.build', compute='_compute_top_parent') ancestors = fields.Many2many('runbot.build', compute='_compute_ancestors') - # should we add a has children stored boolean? children_ids = fields.One2many('runbot.build', 'parent_id') all_children_ids = fields.One2many('runbot.build', compute='_compute_all_children_ids') @@ -390,12 +395,22 @@ def _compute_host_id(self): for record in self: record.host_id = get_host(record.host) + def _compute_linked_parent_build_ids(self): + for build in self: + build.linked_parent_build_ids = build.parent_link_ids.parent_id + + def _compute_linked_children_build_ids(self): + for build in self: + build.linked_children_build_ids = build.child_link_ids.child_id + def _update_globals(self): for record in self: waiting_score = record._get_state_score('waiting') - children_ids = [child for child in record.children_ids if not child.orphan_result] - if record._get_state_score(record.local_state) > waiting_score and children_ids: # if finish, check children - children_state = record._get_youngest_state([child.global_state for child in children_ids]) + children_ids = [child.id for child in record.children_ids if not child.orphan_result] + children_ids += [link.child_id.id for link in record.child_link_ids if not link.orphan_result] + children = self.browse(children_ids) + if record._get_state_score(record.local_state) > waiting_score and children: # if finish, check children + children_state = record._get_youngest_state([child.global_state for child in children]) if record._get_state_score(children_state) > waiting_score: record.global_state = record.local_state else: @@ -406,9 +421,11 @@ def _update_globals(self): if record.local_result and record._get_result_score(record.local_result) >= record._get_result_score('ko'): record.global_result = record.local_result else: - children_ids = [child for child in record.children_ids if not child.orphan_result] - if children_ids: - children_result = record._get_worst_result([child.global_result for child in children_ids], max_res='ko') + children_ids = [child.id for child in record.children_ids if not child.orphan_result] + children_ids += [link.child_id.id for link in record.child_link_ids if not link.orphan_result] + children = self.browse(children_ids) + if children: + children_result = record._get_worst_result([child.global_result for child in children], max_res='ko') if record.local_result: record.global_result = record._get_worst_result([record.local_result, children_result]) else: @@ -555,7 +572,7 @@ def write(self, values): return res - def _add_child(self, param_values, orphan=False, description=False, additionnal_commit_links=False, reference_parent=...): + def _add_child(self, param_values, orphan=False, description=False, additionnal_commit_links=False, reference_parent=..., link=False): build_values = {key: value for key, value in param_values.items() if key not in self.params_id._fields} param_values = {key: value for key, value in param_values.items() if key in self.params_id._fields} @@ -588,9 +605,10 @@ def _add_child(self, param_values, orphan=False, description=False, additionnal_ } param_values['config_data'] = new_config_data - return self.create({ - 'params_id': self.params_id.copy(param_values).id, - 'parent_id': self.id, + params = self.params_id.copy(param_values) + + build_values = { + 'params_id': params.id, 'build_type': self.build_type, 'priority_level': self.priority_level, 'description': description, @@ -598,7 +616,29 @@ def _add_child(self, param_values, orphan=False, description=False, additionnal_ 'keep_host': self.keep_host, 'host': self.host if self.keep_host else False, **build_values, - }) + } + + if link: + existing_builds = params.build_ids.filtered(lambda b: not b.parent_id) + if self.keep_host: + existing_builds = existing_builds.filtered(lambda b: b.host == self.host) + if existing_builds: + build = existing_builds.sorted('id')[-1] + # build.killable = False + + if not existing_builds: + build = self.create(build_values) + + link = self.env['runbot.build.link'].create({ + 'parent_id': self.id, + 'child_id': build.id, + }) + return link, build + else: + return self.create({ + 'parent_id': self.id, + **build_values, + }) @api.depends('params_id.version_id.name') def _compute_dest(self): @@ -661,6 +701,7 @@ def _compute_last_update(self): def _compute_load_time(self): for build in self: build.load_time = sum([build.build_time] + [child.load_time for child in build.children_ids]) + # TODO check, we don't count linked childrent in load time to avoid duplicate, but we need to ensure we take them into account @api.depends('job_start') def _compute_build_age(self): @@ -687,8 +728,17 @@ def _rebuild(self, message=None): 'parent_id': self.parent_id.id, 'description': self.description, }) - + new_build = self.create(values) + + if self.parent_link_ids: # TODO check, should we forbid rebuild in some cases if we have multiple parents? Or just link to an active parent? + for link in self.parent_link_ids: + link.orphan_result = True + self.env['runbot.build.link'].create({ + 'parent_id': link.parent_id.id, + 'child_id': new_build.id, + }) + if self.parent_id: self.orphan_result = True @@ -1797,3 +1847,26 @@ def action_view_build_errors(self): "name": "Build errors", "view_mode": "list,form" } + + +class BuildLink(models.Model): + _name = 'runbot.build.link' + _description = 'Runbot Build Link' + _order = 'id desc' + + parent_id = fields.Many2one('runbot.build', string='Parent Build', required=True, ondelete='cascade') + child_id = fields.Many2one('runbot.build', string='Child Build', required=True, ondelete='cascade') + params_id = fields.Many2one('runbot.build.params', string='Params', related='child_id.params_id', store=True) + orphan_result = fields.Boolean(string='Orphan Result', help='If set, the result of the child build will not be taken into account for the parent build result') + + @api.model_create_multi + def create(self, vals_list): + links = super().create(vals_list) + links.mapped('parent_id')._update_globals() + return links + + def write(self, values): + res = super().write(values) + if 'orphan_result' in values or 'child_id' in values: + self.parent_id._update_globals() + return res diff --git a/runbot/models/build_config.py b/runbot/models/build_config.py index e5aa7ab2c..7f394c29c 100644 --- a/runbot/models/build_config.py +++ b/runbot/models/build_config.py @@ -1052,7 +1052,7 @@ def get_reference_builds_for_versions(versions): build._log('_run_configure_upgrade', 'No database found for pattern %s' % (upgrade_db.db_pattern), level='ERROR') for db in valid_databases: - child = build._add_child({ + _link, child = build._add_child({ 'upgrade_to_build_id': None, 'upgrade_from_build_id': source.id, 'dump_db': db.id, @@ -1062,7 +1062,7 @@ def get_reference_builds_for_versions(versions): 'version_id': target.params_id.version_id.id, 'trigger_id': None, 'dockerfile_id': target.params_id.dockerfile_id.id, - }) + }, link=True) source_description = source.params_id.version_id.name target_description = target.params_id.version_id.name if source in build.create_batch_id.slot_ids.build_id: @@ -1075,14 +1075,14 @@ def get_reference_builds_for_versions(versions): db.name, ) - if self.allow_similar_build_quick_result: - existing_done_build = next((build for build in child.params_id.build_ids.sorted('id') if build.global_state == 'done' and build.global_result == 'ok'), None) - if not existing_done_build and not build.create_batch_id.bundle_id.is_staging: - existing_done_build = next((build for build in child.params_id.build_ids.sorted('id') if build.global_state == 'done' and build.local_result not in ('skipped', 'killed') and not build.orphan_result), None) - if existing_done_build: - child._log('', 'A similar [build](%s) has been found, marking as done directly', existing_done_build.build_url, log_type='markdown') - child.local_state = 'done' - child.local_result = existing_done_build.local_result + # if self.allow_similar_build_quick_result: + # existing_done_build = next((build for build in child.params_id.build_ids.sorted('id') if build.global_state == 'done' and build.global_result == 'ok'), None) + # if not existing_done_build and not build.create_batch_id.bundle_id.is_staging: + # existing_done_build = next((build for build in child.params_id.build_ids.sorted('id') if build.global_state == 'done' and build.local_result not in ('skipped', 'killed') and not build.orphan_result), None) + # if existing_done_build: + # child._log('', 'A similar [build](%s) has been found, marking as done directly', existing_done_build.build_url, log_type='markdown') + # child.local_state = 'done' + # child.local_result = existing_done_build.local_result def _filter_upgrade_database(self, dbs, pattern): pat_list = pattern.split(',') if pattern else [] diff --git a/runbot/models/runbot.py b/runbot/models/runbot.py index b24cf5cd9..823b33a40 100644 --- a/runbot/models/runbot.py +++ b/runbot/models/runbot.py @@ -161,9 +161,30 @@ def _gc_testing(self, host): if available_slots > 0 or nb_pending == 0: return + killable_build = [] + for build in testing_builds: if build.top_parent.killable: - build.top_parent._ask_kill(message='Build automatically killed, new build found.') + killable_build.append(build.top_parent) + continue + # a build can have multiple parents but no direct parent id + # in this cas we can kill under two conditions: + # - the build is not linked to any parent non killable build (or the link are orphan_result) + # - the build won't be linked again to any + if not build.parent_id and build.parent_link_ids: + has_alive_parent = not all(l.orphan_result or l.parent_id.top_parent.killable for l in build.parent_link_ids) + if not has_alive_parent: + top_parents = build.linked_parent_build_ids.top_parent + candidate_batches = top_parents.slot_ids.batch_id.bundle_id.last_batch.filtered(lambda b: b.state in ['preparing', 'ready']) + if any(candidate_batche.state == 'preparing' for candidate_batche in candidate_batches): + continue + if any(batch.slot_ids.filtered(lambda s: s.trigger_id in top_parents.trigger_id and (not s.build_id or s.build_id.local_state != 'done')) for batch in candidate_batches): + continue + killable_build.append(build.top_parent) + continue + + for build in killable_build: + build._ask_kill(message='Build automatically killed, new build found.') def _reload_nginx(self): env = self.env diff --git a/runbot/security/ir.model.access.csv b/runbot/security/ir.model.access.csv index f0ca84182..bb77ca61d 100644 --- a/runbot/security/ir.model.access.csv +++ b/runbot/security/ir.model.access.csv @@ -160,6 +160,9 @@ access_runbot_docker_layer_admin,access_runbot_docker_layer_admin,runbot.model_r access_runbot_docker_build_result_public,access_runbot_docker_build_result_public,runbot.model_runbot_docker_build_result,runbot.base_runbot_model_access,1,0,0,0 access_runbot_docker_build_result_admin,access_runbot_docker_build_result_admin,runbot.model_runbot_docker_build_result,runbot.group_runbot_admin,1,1,1,1 +runbot.access_runbot_build_link,access_runbot_build_link,runbot.model_runbot_build_link,base.group_user,1,0,0,0 +runbot.access_runbot_build_link_admin,access_runbot_build_link_admin,runbot.model_runbot_build_link,base.group_runbot_admin,1,1,1,1 + access_runbot_codeowner_admin,runbot_codeowner_admin,runbot.model_runbot_codeowner,runbot.group_runbot_admin,1,1,1,1 access_runbot_codeownermanager,runbot_codeowner_manager,runbot.model_runbot_codeowner,runbot.group_runbot_codeowner_manager,1,1,1,1 access_runbot_codeowner_user,runbot_codeowner_user,runbot.model_runbot_codeowner,group_user,1,0,0,0 diff --git a/runbot/tests/__init__.py b/runbot/tests/__init__.py index 3db76a690..0ded99ace 100644 --- a/runbot/tests/__init__.py +++ b/runbot/tests/__init__.py @@ -4,6 +4,7 @@ from . import test_build_error from . import test_branch from . import test_build +from . import test_build_link from . import test_schedule from . import test_build_config_step from . import test_event diff --git a/runbot/tests/test_build.py b/runbot/tests/test_build.py index a696ee3ee..b30bf5801 100644 --- a/runbot/tests/test_build.py +++ b/runbot/tests/test_build.py @@ -792,6 +792,7 @@ def get_log(message): 'File "/data/build/odoo/addons/web/tests/test_web.py", in test_web' ) + class TestGc(RunbotCaseMinimalSetup): def test_repo_gc_testing(self): diff --git a/runbot/tests/test_build_link.py b/runbot/tests/test_build_link.py new file mode 100644 index 000000000..241589639 --- /dev/null +++ b/runbot/tests/test_build_link.py @@ -0,0 +1,238 @@ +from .common import RunbotCase + + +class TestLinkedBuildGc(RunbotCase): + + def setUp(self): + super().setUp() + self.BuildLink = self.env['runbot.build.link'] + self.dev_batch.state = 'done' + self.host = self.env['runbot.host'].create({'name': 'runbot_link', 'nb_worker': 1}) + + # a pending build waiting for a slot, otherwise the gc returns early + self.Build.create({'params_id': self.base_params.id, 'local_state': 'pending'}) + + self.parent = self.create_new_parent(self.dev_batch) + self.child_params = self.BuildParameters.create({ + 'version_id': self.version_13.id, + 'project_id': self.project.id, + 'config_id': self.default_config.id, + 'create_batch_id': self.dev_batch.id, + 'extra_params': 'child', + }) + + self.child = self.Build.create({ + 'params_id': self.child_params.id, + 'local_state': 'testing', + 'host': self.host.name, + }) + self.link = self.BuildLink.create({'parent_id': self.parent.id, 'child_id': self.child.id}) + self.trigger_server.batch_dependent = True # avoid linking of top_level parent as upgrade trigger is configured + + def create_new_parent(self, batch=None, local_state='done', killable=False): + if not batch: + batch = self.dev_bundle._force() + batch.state = 'ready' + params = self.BuildParameters.create({ + 'version_id': self.version_13.id, + 'project_id': self.project.id, + 'config_id': self.default_config.id, + 'trigger_id': self.trigger_server.id, + 'create_batch_id': batch.id, + }) + build = self.Build.create({ + 'params_id': params.id, + 'local_state': local_state, + 'killable': killable, + 'create_batch_id': batch.id, + }) + self.env['runbot.batch.slot'].create({ + 'batch_id': batch.id, + 'trigger_id': self.trigger_server.id, + 'build_id': build.id, + 'params_id': params.id, + 'link_type': 'created', + }) + return build + + def test_living_parent(self): + self.env['runbot.runbot']._gc_testing(self.host) + self.assertFalse(self.child.to_kill) + + def test_killable_parent(self): + self.parent.killable = True + self.env['runbot.runbot']._gc_testing(self.host) + self.assertTrue(self.child.to_kill) + + def test_orphan_link(self): + self.link.orphan_result = True + self.env['runbot.runbot']._gc_testing(self.host) + self.assertTrue(self.child.to_kill) + + def test_second_living_parent(self): + self.parent.killable = True + other_parent = self.create_new_parent() + self.BuildLink.create({'parent_id': other_parent.id, 'child_id': self.child.id}) + self.env['runbot.runbot']._gc_testing(self.host) + self.assertFalse(self.child.to_kill) + + def test_new_batch_window(self): + self.parent.killable = True + batch = self.dev_bundle._force() + self.assertEqual(batch.state, 'preparing') + self.env['runbot.runbot']._gc_testing(self.host) + self.assertFalse(self.child.to_kill, 'a preparing batch may create a new parent') + + batch.state = 'ready' + new_parent = self.create_new_parent(batch, local_state='testing') + self.env['runbot.runbot']._gc_testing(self.host) + self.assertFalse(self.child.to_kill, 'the new parent may still link this child since the build is testing') + + new_parent.local_state = 'done' + self.env['runbot.runbot']._gc_testing(self.host) + self.assertTrue(self.child.to_kill) + + +class TestLinkedBuildGlobals(RunbotCase): + """global_state and global_result of a parent using build links""" + + def setUp(self): + super().setUp() + self.BuildLink = self.env['runbot.build.link'] + self.parent = self.create_build('parent') + self.parent.local_state = 'done' + + def create_build(self, name, **values): + params = self.BuildParameters.create({ + 'version_id': self.version_13.id, + 'project_id': self.project.id, + 'config_id': self.default_config.id, + 'create_batch_id': self.dev_batch.id, + 'extra_params': name, + }) + return self.Build.create(dict(values, params_id=params.id)) + + def link_child(self, name, parent=None, orphan_result=False): + child = self.create_build(name) + self.BuildLink.create({ + 'parent_id': (parent or self.parent).id, + 'child_id': child.id, + 'orphan_result': orphan_result, + }) + return child + + def test_no_child(self): + self.assertEqual(self.parent.global_state, 'done') + self.assertEqual(self.parent.global_result, 'ok') + + def test_pending_child(self): + self.link_child('child') + self.parent._update_globals() + self.assertEqual(self.parent.global_state, 'waiting') + self.assertEqual(self.parent.global_result, 'ok') + + def test_done_child(self): + child = self.link_child('child') + child.local_state = 'done' + self.parent._update_globals() + self.assertEqual(self.parent.global_state, 'done') + self.assertEqual(self.parent.global_result, 'ok') + + def test_ko_child(self): + child = self.link_child('child') + child.write({'local_state': 'done', 'local_result': 'ko'}) + self.parent._update_globals() + self.assertEqual(self.parent.global_state, 'done') + self.assertEqual(self.parent.global_result, 'ko') + + def test_killed_child(self): + child = self.link_child('child') + child.write({'local_state': 'done', 'local_result': 'killed'}) + self.parent._update_globals() + self.assertEqual(self.parent.global_state, 'done') + self.assertEqual(self.parent.global_result, 'ko', 'a killed child is capped to ko') + + def test_orphan_child(self): + child = self.link_child('child', orphan_result=True) + child.write({'local_state': 'done', 'local_result': 'ko'}) + self.parent._update_globals() + self.assertEqual(self.parent.global_state, 'done') + self.assertEqual(self.parent.global_result, 'ok') + + def test_orphan_child_does_not_hold_the_parent(self): + self.link_child('child', orphan_result=True) + self.parent._update_globals() + self.assertEqual(self.parent.global_state, 'done') + + def test_worst_result_of_children(self): + warn_child = self.link_child('warn_child') + ko_child = self.link_child('ko_child') + warn_child.write({'local_state': 'done', 'local_result': 'warn'}) + ko_child.write({'local_state': 'done', 'local_result': 'ko'}) + self.parent._update_globals() + self.assertEqual(self.parent.global_result, 'ko') + + def test_one_pending_child_keeps_waiting(self): + done_child = self.link_child('done_child') + self.link_child('pending_child') + done_child.write({'local_state': 'done', 'local_result': 'ko'}) + self.parent._update_globals() + self.assertEqual(self.parent.global_state, 'waiting') + self.assertEqual(self.parent.global_result, 'ko') + + def test_local_result_of_the_parent(self): + child = self.link_child('child') + child.local_state = 'done' + self.parent.local_result = 'ko' + self.parent._update_globals() + self.assertEqual(self.parent.global_result, 'ko') + + def test_shared_child(self): + other_parent = self.create_build('other_parent') + other_parent.local_state = 'done' + child = self.link_child('child') + self.BuildLink.create({'parent_id': other_parent.id, 'child_id': child.id}) + child.write({'local_state': 'done', 'local_result': 'ko'}) + (self.parent + other_parent)._update_globals() + self.assertEqual(self.parent.global_result, 'ko') + self.assertEqual(other_parent.global_result, 'ko') + + def test_shared_child_orphan_on_one_parent_only(self): + other_parent = self.create_build('other_parent') + other_parent.local_state = 'done' + child = self.link_child('child') + self.BuildLink.create({ + 'parent_id': other_parent.id, + 'child_id': child.id, + 'orphan_result': True, + }) + child.write({'local_state': 'done', 'local_result': 'ko'}) + (self.parent + other_parent)._update_globals() + self.assertEqual(self.parent.global_result, 'ko') + self.assertEqual(other_parent.global_result, 'ok') + + def test_parent_id_child(self): + child = self.create_build('child', parent_id=self.parent.id) + self.assertEqual(self.parent.global_state, 'waiting') + child.write({'local_state': 'done', 'local_result': 'ko'}) + self.parent._update_globals() + self.assertEqual(self.parent.global_state, 'done') + self.assertEqual(self.parent.global_result, 'ko') + + def test_new_link_updates_the_parent(self): + child = self.link_child('child') + child.local_state = 'done' + self.parent._update_globals() + self.assertEqual(self.parent.global_state, 'done') + + self.link_child('new_child') + self.assertEqual(self.parent.global_state, 'waiting', 'creating a link must update the parent') + + def test_orphaning_a_link_updates_the_parent(self): + child = self.link_child('child') + child.write({'local_state': 'done', 'local_result': 'ko'}) + self.parent._update_globals() + self.assertEqual(self.parent.global_result, 'ko') + + self.parent.child_link_ids.orphan_result = True + self.assertEqual(self.parent.global_result, 'ok', 'orphaning a link must update the parent')