From fe987cd0f332dd8f0b8468ed22665f6a2cce903e Mon Sep 17 00:00:00 2001 From: Xavier-Do Date: Fri, 13 Aug 2021 12:33:00 +0200 Subject: [PATCH] [IMP] runbot: some fixes for ps runbot - searching on number will search for both pr and branche name - hooks are now using payload to define repo when not given in url - fixes .git cleaning in repo (remove rstrip since it can fail for repo starting with g, i, t) - recompute base on prepare if base was not found - remove local_result form write values if there is a single record (instead of raising, makes python step easier to write). - avoid stucked build/loop after removing a step from a config. - avoid to send ci for linked base_commit - add a fallback mechanism for base if no master branch is found - add option on project to avoid to keep sticky running, usefull when using a lots of projects WARNING: this is a change of default behaviour, need to update existing projects. - always discover new commits for branch matching base paterns. This is especially usefull to discover old versions on project with low merge frequency. - always create a batch, event if there is now trigger. This helps to notice that commits are discovered - add line-through on death branches/pr - manual trigger are now displayed on main page --- runbot/controllers/frontend.py | 3 +-- runbot/controllers/hook.py | 16 ++++++++++------ runbot/models/batch.py | 3 +++ runbot/models/build.py | 16 +++++++++++++--- runbot/models/bundle.py | 24 +++++++++++++++--------- runbot/models/project.py | 2 +- runbot/models/repo.py | 5 +---- runbot/models/runbot.py | 2 +- runbot/templates/bundle.xml | 2 +- runbot/templates/frontend.xml | 6 +----- runbot/templates/utils.xml | 6 ++++-- runbot/tests/test_build.py | 7 +++++-- runbot/views/bundle_views.xml | 13 +++++++++++++ 13 files changed, 69 insertions(+), 36 deletions(-) diff --git a/runbot/controllers/frontend.py b/runbot/controllers/frontend.py index 18eb5285..97434cad 100644 --- a/runbot/controllers/frontend.py +++ b/runbot/controllers/frontend.py @@ -133,8 +133,7 @@ class Runbot(Controller): for search_elem in search.split("|"): if search_elem.isnumeric(): pr_numbers.append(int(search_elem)) - else: - search_domains.append([('name', 'like', search_elem)]) + search_domains.append([('name', 'like', search_elem)]) if pr_numbers: res = request.env['runbot.branch'].search([('name', 'in', pr_numbers)]) if res: diff --git a/runbot/controllers/hook.py b/runbot/controllers/hook.py index d9c91d67..fb82c7ff 100644 --- a/runbot/controllers/hook.py +++ b/runbot/controllers/hook.py @@ -12,23 +12,27 @@ _logger = logging.getLogger(__name__) class Hook(http.Controller): - @http.route(['/runbot/hook/'], type='http', auth="public", website=True, csrf=False) + @http.route(['/runbot/hook', '/runbot/hook/'], type='http', auth="public", website=True, csrf=False) def hook(self, remote_id=None, **_post): event = request.httprequest.headers.get("X-Github-Event") payload = json.loads(request.params.get('payload', '{}')) if remote_id is None: repo_data = payload.get('repository') - if repo_data and event in ['push', 'pull_request']: + if repo_data: remote_domain = [ - '|', '|', ('name', '=', repo_data['ssh_url']), + '|', '|', '|', + ('name', '=', repo_data['ssh_url']), + ('name', '=', repo_data['ssh_url'].replace('.git', '')), ('name', '=', repo_data['clone_url']), - ('name', '=', repo_data['clone_url'].rstrip('.git')), + ('name', '=', repo_data['clone_url'].replace('.git', '')), ] remote = request.env['runbot.remote'].sudo().search( remote_domain, limit=1) remote_id = remote.id - - remote = request.env['runbot.remote'].sudo().browse([remote_id]) + if not remote_id: + _logger.error("Remote %s not found", repo_data['ssh_url']) + remote = request.env['runbot.remote'].sudo().browse(remote_id) + _logger.info('Remote found %s', remote) # force update of dependencies too in case a hook is lost if not payload or event == 'push': diff --git a/runbot/models/batch.py b/runbot/models/batch.py index 95186bd8..8df69ee9 100644 --- a/runbot/models/batch.py +++ b/runbot/models/batch.py @@ -140,6 +140,9 @@ class Batch(models.Model): def _prepare(self, auto_rebase=False): _logger.info('Preparing batch %s', self.id) + if not self.bundle_id.base_id: + # in some case the base can be detected lately. If a bundle has no base, recompute the base before preparing + self.bundle_id._compute_base_id() for level, message in self.bundle_id.consistency_warning(): if level == "warning": self.warning("Bundle warning: %s" % message) diff --git a/runbot/models/build.py b/runbot/models/build.py index 1c7e3a87..f3262197 100644 --- a/runbot/models/build.py +++ b/runbot/models/build.py @@ -316,7 +316,11 @@ class BuildResult(models.Model): build_by_old_values[record.local_state] += record local_result = values.get('local_result') for build in self: - assert not local_result or local_result == self._get_worst_result([build.local_result, local_result]) # dont write ok on a warn/error build + if local_result and local_result != self._get_worst_result([build.local_result, local_result]): # dont write ok on a warn/error build + if len(self) == 1: + values.pop('local_result') + else: + raise ValidationError('Local result cannot be set to a less critical level') res = super(BuildResult, self).write(values) if 'log_counter' in values: # not 100% usefull but more correct ( see test_ir_logging) self.flush() @@ -1009,7 +1013,13 @@ class BuildResult(models.Model): # means that a step has been run manually without using config return {'active_step': False, 'local_state': 'done'} - next_index = step_ids.index(self.active_step) + 1 if self.active_step else 0 + if not self.active_step: + next_index = 0 + else: + if self.active_step not in step_ids: + self._log('run', 'Config was modified and current step does not exists anymore, skipping.', level='ERROR') + return {'active_step': False, 'local_state': 'done', 'local_result': self._get_worst_result([self.local_result, 'ko'])} + next_index = step_ids.index(self.active_step) + 1 while True: if next_index >= len(step_ids): # final job, build is done @@ -1132,5 +1142,5 @@ class BuildResult(models.Model): if trigger.ci_context: for build_commit in self.params_id.commit_link_ids: commit = build_commit.commit_id - if build_commit.match_type != 'default' and commit.repo_id in trigger.repo_ids: + if 'base_' not in build_commit.match_type and commit.repo_id in trigger.repo_ids: commit._github_status(build, trigger.ci_context, state, target_url, desc, post_commit) diff --git a/runbot/models/bundle.py b/runbot/models/bundle.py index aa598a33..575919ba 100644 --- a/runbot/models/bundle.py +++ b/runbot/models/bundle.py @@ -93,14 +93,17 @@ class Bundle(models.Model): continue project_id = bundle.project_id.id master_base = False + fallback = False for bid, bname in self._get_base_ids(project_id): if bundle.name.startswith('%s-' % bname): bundle.base_id = self.browse(bid) break elif bname == 'master': master_base = self.browse(bid) + elif not fallback or fallback.id < bid: + fallback = self.browse(bid) else: - bundle.base_id = master_base + bundle.base_id = master_base or fallback @tools.ormcache('project_id') def _get_base_ids(self, project_id): @@ -213,14 +216,17 @@ class Bundle(models.Model): if self.defined_base_id: return [('info', 'This bundle has a forced base: %s' % self.defined_base_id.name)] warnings = [] - for branch in self.branch_ids: - if branch.is_pr and branch.target_branch_name != self.base_id.name: - if branch.target_branch_name.startswith(self.base_id.name): - warnings.append(('info', 'PR %s targeting a non base branch: %s' % (branch.dname, branch.target_branch_name))) - else: - warnings.append(('warning' if branch.alive else 'info', 'PR %s targeting wrong version: %s (expecting %s)' % (branch.dname, branch.target_branch_name, self.base_id.name))) - elif not branch.is_pr and not branch.name.startswith(self.base_id.name) and not self.defined_base_id: - warnings.append(('warning', 'Branch %s not starting with version name (%s)' % (branch.dname, self.base_id.name))) + if not self.base_id: + warnings.append(('warning', 'No base defined on this bundle')) + else: + for branch in self.branch_ids: + if branch.is_pr and branch.target_branch_name != self.base_id.name: + if branch.target_branch_name.startswith(self.base_id.name): + warnings.append(('info', 'PR %s targeting a non base branch: %s' % (branch.dname, branch.target_branch_name))) + else: + warnings.append(('warning' if branch.alive else 'info', 'PR %s targeting wrong version: %s (expecting %s)' % (branch.dname, branch.target_branch_name, self.base_id.name))) + elif not branch.is_pr and not branch.name.startswith(self.base_id.name) and not self.defined_base_id: + warnings.append(('warning', 'Branch %s not starting with version name (%s)' % (branch.dname, self.base_id.name))) return warnings def branch_groups(self): diff --git a/runbot/models/project.py b/runbot/models/project.py index 537b26f0..5e93f683 100644 --- a/runbot/models/project.py +++ b/runbot/models/project.py @@ -7,7 +7,7 @@ class Project(models.Model): name = fields.Char('Project name', required=True, unique=True) group_ids = fields.Many2many('res.groups', string='Required groups') - + keep_sticky_running = fields.Boolean('Keep last sticky builds running') trigger_ids = fields.One2many('runbot.trigger', 'project_id', string='Triggers') dockerfile_id = fields.Many2one('runbot.dockerfile', index=True, help="Project Default Dockerfile") diff --git a/runbot/models/repo.py b/runbot/models/repo.py index cc4b74a4..73e617cd 100644 --- a/runbot/models/repo.py +++ b/runbot/models/repo.py @@ -365,7 +365,7 @@ class Repo(models.Model): if not git_refs: return [] refs = [tuple(field for field in line.split('\x00')) for line in git_refs.split('\n')] - refs = [r for r in refs if dateutil.parser.parse(r[2][:19]) + datetime.timedelta(days=max_age) > datetime.datetime.now()] + refs = [r for r in refs if dateutil.parser.parse(r[2][:19]) + datetime.timedelta(days=max_age) > datetime.datetime.now() or self.env['runbot.branch'].match_is_base(r[0])] if ignore: refs = [r for r in refs if r[0].split('/')[-1] not in ignore] return refs @@ -441,9 +441,6 @@ class Repo(models.Model): message = branch.remote_id.repo_id.invalid_branch_message or message branch.head._github_status(False, "Branch naming", 'failure', False, message) - if not self.trigger_ids: - continue - bundle = branch.bundle_id if bundle.no_build: continue diff --git a/runbot/models/runbot.py b/runbot/models/runbot.py index 1dcdc5e3..833ee22a 100644 --- a/runbot/models/runbot.py +++ b/runbot/models/runbot.py @@ -90,7 +90,7 @@ class Runbot(models.AbstractModel): domain_host = self.build_domain_host(host) Build = self.env['runbot.build'] cannot_be_killed_ids = Build.search(domain_host + [('keep_running', '=', True)]).ids - sticky_bundles = self.env['runbot.bundle'].search([('sticky', '=', True)]) + sticky_bundles = self.env['runbot.bundle'].search([('sticky', '=', True), ('project_id.keep_sticky_running', '=', True)]) cannot_be_killed_ids += [ build.id for build in sticky_bundles.mapped('last_batchs.slot_ids.build_id') diff --git a/runbot/templates/bundle.xml b/runbot/templates/bundle.xml index f6c52925..403b7a9b 100644 --- a/runbot/templates/bundle.xml +++ b/runbot/templates/bundle.xml @@ -47,7 +47,7 @@ - + Create pr diff --git a/runbot/templates/frontend.xml b/runbot/templates/frontend.xml index 376d0fd2..a8389120 100644 --- a/runbot/templates/frontend.xml +++ b/runbot/templates/frontend.xml @@ -94,7 +94,7 @@
-
@@ -120,7 +120,3 @@ - - - - diff --git a/runbot/templates/utils.xml b/runbot/templates/utils.xml index 9b04c373..41bfbdc1 100644 --- a/runbot/templates/utils.xml +++ b/runbot/templates/utils.xml @@ -67,7 +67,7 @@ -
+
@@ -305,7 +305,9 @@ diff --git a/runbot/tests/test_build.py b/runbot/tests/test_build.py index 5de8778d..9a3c1c6c 100644 --- a/runbot/tests/test_build.py +++ b/runbot/tests/test_build.py @@ -4,7 +4,7 @@ import datetime from unittest.mock import patch from odoo import fields -from odoo.exceptions import UserError +from odoo.exceptions import UserError, ValidationError from .common import RunbotCase, RunbotCaseMinimalSetup @@ -183,9 +183,12 @@ class TestBuildResult(RunbotCase): 'local_result': 'ko' }) + other.write({'local_result': 'ok'}) + self.assertEqual(other.local_result, 'ko') + # test a bulk write, that one cannot change from 'ko' to 'ok' builds = self.Build.browse([build.id, other.id]) - with self.assertRaises(AssertionError): + with self.assertRaises(ValidationError): builds.write({'local_result': 'ok'}) def test_markdown_description(self): diff --git a/runbot/views/bundle_views.xml b/runbot/views/bundle_views.xml index 0bef48a3..ce0d7a50 100644 --- a/runbot/views/bundle_views.xml +++ b/runbot/views/bundle_views.xml @@ -7,6 +7,7 @@
+ @@ -15,6 +16,18 @@ + + runbot.project + + + + + + + + + + runbot.bundle