From d4d1ba4971aa0bb0cf2cf059251a12d31b275587 Mon Sep 17 00:00:00 2001 From: Edward Lemur Date: Fri, 20 Sep 2019 21:46:37 +0000 Subject: [PATCH] git-cl: Use buildbucket v2 to schedule try jobs. Change-Id: I3bad4314973cda7e285b5b9cb823f61cd7fb2dff Bug: 976104 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1809516 Commit-Queue: Edward Lesmes Reviewed-by: Andrii Shyshkalov --- git_cl.py | 213 ++++++++++++++++++++++++++----------------- tests/git_cl_test.py | 133 +++++++++++++++++++-------- 2 files changed, 223 insertions(+), 123 deletions(-) diff --git a/git_cl.py b/git_cl.py index 19b7c952b..42f689bc1 100755 --- a/git_cl.py +++ b/git_cl.py @@ -71,6 +71,8 @@ GIT_HASH_RE = re.compile(r'\b([a-f0-9]{6})[a-f0-9]{34}\b', flags=re.I) # Used to redact the cookies from the gitcookies file. GITCOOKIES_REDACT_RE = re.compile(r'1/.*') +MAX_ATTEMPTS = 3 + # The maximum number of traces we will keep. Multiplied by 3 since we store # 3 files per trace. MAX_TRACES = 3 * 10 @@ -341,6 +343,8 @@ def _get_properties_from_options(options): return properties +# TODO(crbug.com/976104): Remove this function once git-cl try-results has +# migrated to use buildbucket v2 def _buildbucket_retry(operation_name, http, *args, **kwargs): """Retries requests to buildbucket service and returns parsed json content.""" try_count = 0 @@ -379,6 +383,38 @@ def _buildbucket_retry(operation_name, http, *args, **kwargs): assert False, 'unreachable' +def _call_buildbucket(http, buildbucket_host, method, request=None): + """Calls a buildbucket v2 method and returns the parsed json response.""" + headers = { + 'Accept': 'application/json', + 'Content-Type': 'application/json', + } + request = json.dumps(request) + url = 'https://%s/prpc/buildbucket.v2.Builds/%s' % (buildbucket_host, method) + + logging.info('POST %s with %s' % (url, request)) + + attempts = 1 + time_to_sleep = 1 + while True: + response, content = http.request(url, 'POST', body=request, headers=headers) + if response.status == 200: + return json.loads(content[4:]) + if attempts >= MAX_ATTEMPTS or 400 <= response.status < 500: + msg = '%s error when calling POST %s with %s: %s' % ( + response.status, url, request, content) + raise BuildbucketResponseException(msg) + logging.debug( + '%s error when calling POST %s with %s. ' + 'Sleeping for %d seconds and retrying...' % ( + response.status, url, request, time_to_sleep)) + time.sleep(time_to_sleep) + time_to_sleep *= 2 + attempts += 1 + + assert False, 'unreachable' + + def _get_bucket_map(changelist, options, option_parser): """Returns a dict mapping bucket names to builders and tests, for triggering tryjobs. @@ -407,6 +443,21 @@ def _get_bucket_map(changelist, options, option_parser): 'Please specify the bucket, e.g. "-B luci.chromium.try".') +def _parse_bucket(bucket): + if '/' in bucket: + return tuple(bucket.split('/', 1)) + # Legacy buckets. + print('WARNING Please specify buckets as /.') + # Assume luci... + if bucket.startswith('luci.'): + return tuple(bucket[len('luci.'):].split('.', 1)) + # Otherwise, assume prefix is also the project name. + if '.' in bucket: + project = bucket.split('.')[0] + return project, bucket + return None, None + + def _trigger_try_jobs(auth_config, changelist, buckets, options, patchset): """Sends a request to Buildbucket to trigger tryjobs for a changelist. @@ -416,81 +467,76 @@ def _trigger_try_jobs(auth_config, changelist, buckets, options, patchset): buckets: A nested dict mapping bucket names to builders to tests. options: Command-line options. """ - assert changelist.GetIssue(), 'CL must be uploaded first' - codereview_url = changelist.GetCodereviewServer() - assert codereview_url, 'CL must be uploaded first' - patchset = patchset or changelist.GetMostRecentPatchset() - assert patchset, 'CL must be uploaded first' + print('Scheduling jobs on:') + for bucket, builders_and_tests in sorted(buckets.iteritems()): + print('Bucket:', bucket) + print('\n'.join( + ' %s: %s' % (builder, tests) + for builder, tests in sorted(builders_and_tests.iteritems()))) + print('To see results here, run: git cl try-results') + print('To see results in browser, run: git cl web') + + gerrit_changes = [changelist.GetGerritChange()] + shared_properties = { + 'category': options.category, + } + if options.clobber: + shared_properties['clobber'] = True + shared_properties.update(_get_properties_from_options(options) or {}) - codereview_host = urlparse.urlparse(codereview_url).hostname - # Cache the buildbucket credentials under the codereview host key, so that - # users can use different credentials for different buckets. - authenticator = auth.get_authenticator_for_host(codereview_host, auth_config) - http = authenticator.authorize(httplib2.Http()) - http.force_exception_to_status_code = True + requests = [] + for raw_bucket, builders_and_tests in sorted(buckets.iteritems()): + project, bucket = _parse_bucket(raw_bucket) + if not project or not bucket: + print('WARNING Could not parse bucket "%s". Skipping.' % raw_bucket) + continue - buildbucket_put_url = ( - 'https://{hostname}/_ah/api/buildbucket/v1/builds/batch'.format( - hostname=options.buildbucket_host)) - buildset = 'patch/gerrit/{hostname}/{issue}/{patch}'.format( - hostname=codereview_host, - issue=changelist.GetIssue(), - patch=patchset) - - shared_parameters_properties = changelist.GetTryJobProperties(patchset) - shared_parameters_properties['category'] = options.category - if options.clobber: - shared_parameters_properties['clobber'] = True - extra_properties = _get_properties_from_options(options) - if extra_properties: - shared_parameters_properties.update(extra_properties) - - batch_req_body = {'builds': []} - print_text = [] - print_text.append('Tried jobs on:') - for bucket, builders_and_tests in sorted(buckets.iteritems()): - print_text.append('Bucket: %s' % bucket) for builder, tests in sorted(builders_and_tests.iteritems()): - print_text.append(' %s: %s' % (builder, tests)) - parameters = { - 'builder_name': builder, - 'changes': [{ - 'author': {'email': changelist.GetIssueOwner()}, - 'revision': options.revision, - }], - 'properties': shared_parameters_properties.copy(), - } + properties = shared_properties.copy() if 'presubmit' in builder.lower(): - parameters['properties']['dry_run'] = 'true' + properties['dry_run'] = 'true' if tests: - parameters['properties']['testfilter'] = tests + properties['testfilter'] = tests + + requests.append({ + 'scheduleBuild': { + 'requestId': str(uuid.uuid4()), + 'builder': { + 'project': options.project or project, + 'bucket': bucket, + 'builder': builder, + }, + 'gerritChanges': gerrit_changes, + 'properties': properties, + 'tags': [ + {'key': 'builder', 'value': builder}, + {'key': 'user_agent', 'value': 'git_cl_try'}, + ], + } + }) - tags = [ - 'builder:%s' % builder, - 'buildset:%s' % buildset, - 'user_agent:git_cl_try', - ] + if not requests: + return - batch_req_body['builds'].append( - { - 'bucket': bucket, - 'parameters_json': json.dumps(parameters), - 'client_operation_id': str(uuid.uuid4()), - 'tags': tags, - } - ) + codereview_url = changelist.GetCodereviewServer() + codereview_host = urlparse.urlparse(codereview_url).hostname - _buildbucket_retry( - 'triggering tryjobs', - http, - buildbucket_put_url, - 'PUT', - body=json.dumps(batch_req_body), - headers={'Content-Type': 'application/json'} - ) - print_text.append('To see results here, run: git cl try-results') - print_text.append('To see results in browser, run: git cl web') - print('\n'.join(print_text)) + authenticator = auth.get_authenticator_for_host(codereview_host, auth_config) + http = authenticator.authorize(httplib2.Http()) + http.force_exception_to_status_code = True + + batch_request = {'requests': requests} + batch_response = _call_buildbucket( + http, options.buildbucket_host, 'Batch', request=batch_request) + + errors = [ + ' ' + response['error']['message'] + for response in batch_response.get('responses', []) + if 'error' in response + ] + if errors: + raise BuildbucketResponseException( + 'Failed to schedule builds for some bots:\n%s' % '\n'.join(errors)) def fetch_try_jobs(auth_config, changelist, buildbucket_host, @@ -2690,26 +2736,27 @@ class Changelist(object): if data['status'] in ('ABANDONED', 'MERGED'): return 'CL %s is closed' % self.GetIssue() - def GetTryJobProperties(self, patchset=None): - """Returns dictionary of properties to launch a tryjob.""" - data = self._GetChangeDetail(['ALL_REVISIONS']) + def GetGerritChange(self, patchset=None): + """Returns a buildbucket.v2.GerritChange message for the current issue.""" + host = urlparse.urlparse(self.GetCodereviewServer()).hostname + issue = self.GetIssue() patchset = int(patchset or self.GetPatchset()) - assert patchset - revision_data = None # Pylint wants it to be defined. - for revision_data in data['revisions'].itervalues(): - if int(revision_data['_number']) == patchset: - break - else: + data = self._GetChangeDetail(['ALL_REVISIONS']) + + assert host and issue and patchset, 'CL must be uploaded first' + + has_patchset = any( + int(revision_data['_number']) == patchset + for revision_data in data['revisions'].itervalues()) + if not has_patchset: raise Exception('Patchset %d is not known in Gerrit change %d' % (patchset, self.GetIssue())) + return { - 'patch_issue': self.GetIssue(), - 'patch_set': patchset or self.GetPatchset(), - 'patch_project': data['project'], - 'patch_storage': 'gerrit', - 'patch_ref': revision_data['fetch']['http']['ref'], - 'patch_repository_url': revision_data['fetch']['http']['url'], - 'patch_gerrit_url': self.GetCodereviewServer(), + 'host': host, + 'change': issue, + 'project': data['project'], + 'patchset': patchset, } def GetIssueOwner(self): diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index cceb3ebf4..98446a5bd 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -2378,7 +2378,20 @@ class TestGitCl(TestCase): 'Scheduling CQ dry run on: ' 'https://chromium-review.googlesource.com/123456\n') - def test_git_cl_try_buildbucket_with_properties_gerrit(self): + def test_parse_bucket(self): + self.assertEqual(git_cl._parse_bucket('chromium/try'), ('chromium', 'try')) + self.assertEqual( + git_cl._parse_bucket('luci.chromium.try'), ('chromium', 'try')) + self.assertEqual(git_cl._parse_bucket('not-a-bucket'), (None, None)) + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) + self.assertEqual( + git_cl._parse_bucket('skia.primary'), + ('skia', 'skia.primary')) + self.assertIn( + 'WARNING Please specify buckets', + git_cl.sys.stdout.getvalue()) + + def test_git_cl_try_buildbucket_with_wrong_bucket(self): self.mock(git_cl.Changelist, 'GetMostRecentPatchset', lambda _: 7) self.mock(git_cl.uuid, 'uuid4', lambda: 'uuid4') @@ -2410,53 +2423,93 @@ class TestGitCl(TestCase): }, }, }), + ((['git', 'config', 'branch.feature.gerritpatchset'],), '7'), ] - def _buildbucket_retry(*_, **kw): - # self.maxDiff = 10000 - body = json.loads(kw['body']) - self.assertEqual(len(body['builds']), 1) - build = body['builds'][0] - params = json.loads(build.pop('parameters_json')) - self.assertEqual(params, { - u'builder_name': u'win', - u'changes': [{u'author': {u'email': u'owner@e.mail'}, - u'revision': None}], - u'properties': { - u'category': u'git_cl_try', - u'key': u'val', - u'json': [{u'a': 1}, None], - - u'patch_gerrit_url': - u'https://chromium-review.googlesource.com', - u'patch_issue': 123456, - u'patch_project': u'depot_tools', - u'patch_ref': u'refs/changes/56/123456/7', - u'patch_repository_url': - u'https://chromium.googlesource.com/depot_tools', - u'patch_set': 7, - u'patch_storage': u'gerrit', - } - }) - self.assertEqual(build, { - u'bucket': u'luci.chromium.try', - u'client_operation_id': u'uuid4', - u'tags': [ - u'builder:win', - u'buildset:patch/gerrit/chromium-review.googlesource.com/123456/7', - u'user_agent:git_cl_try', - ], - }) + self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) + self.assertEqual(0, git_cl.main([ + 'try', '-B', 'not-a-bucket', '-b', 'win', + '-p', 'key=val', '-p', 'json=[{"a":1}, null]'])) + self.assertIn( + 'Could not parse bucket "not-a-bucket"', + git_cl.sys.stdout.getvalue()) + + def test_git_cl_try_buildbucket_with_properties_gerrit(self): + def _mock_call_buildbucket(_http, buildbucket_host, method, request=None): + bb_request = { + "requests": [{ + "scheduleBuild": { + "requestId": "uuid4", + "builder": { + "project": "chromium", + "builder": "win", + "bucket": "try", + }, + "gerritChanges": [{ + "project": "depot_tools", + "host": "chromium-review.googlesource.com", + "patchset": 7, + "change": 123456, + }], + "properties": { + "category": "git_cl_try", + "json": [{"a": 1}, None], + "key": "val", + }, + "tags": [ + {"value": "win", "key": "builder"}, + {"value": "git_cl_try", "key": "user_agent"}, + ], + }, + }], + } + self.assertEqual(method, 'Batch') + self.assertEqual(buildbucket_host, 'cr-buildbucket.appspot.com') + self.assertEqual(request, bb_request) + return {} + + self.mock(git_cl, '_call_buildbucket', _mock_call_buildbucket) + self.mock(git_cl.Changelist, 'GetMostRecentPatchset', lambda _: 7) + self.mock(git_cl.uuid, 'uuid4', lambda: 'uuid4') - self.mock(git_cl, '_buildbucket_retry', _buildbucket_retry) + self.calls = [ + ((['git', 'symbolic-ref', 'HEAD'],), 'feature'), + ((['git', 'config', 'branch.feature.gerritissue'],), '123456'), + ((['git', 'config', 'branch.feature.gerritserver'],), + 'https://chromium-review.googlesource.com'), + ((['git', 'config', 'branch.feature.merge'],), 'feature'), + ((['git', 'config', 'branch.feature.remote'],), 'origin'), + ((['git', 'config', 'remote.origin.url'],), + 'https://chromium.googlesource.com/depot_tools'), + (('GetChangeDetail', 'chromium-review.googlesource.com', + 'depot_tools~123456', + ['DETAILED_ACCOUNTS', 'ALL_REVISIONS', 'CURRENT_COMMIT']), { + 'project': 'depot_tools', + 'status': 'OPEN', + 'owner': {'email': 'owner@e.mail'}, + 'revisions': { + 'deadbeaf': { + '_number': 6, + }, + 'beeeeeef': { + '_number': 7, + 'fetch': {'http': { + 'url': 'https://chromium.googlesource.com/depot_tools', + 'ref': 'refs/changes/56/123456/7' + }}, + }, + }, + }), + ((['git', 'config', 'branch.feature.gerritpatchset'],), '7'), + ] self.mock(git_cl.sys, 'stdout', StringIO.StringIO()) self.assertEqual(0, git_cl.main([ 'try', '-B', 'luci.chromium.try', '-b', 'win', '-p', 'key=val', '-p', 'json=[{"a":1}, null]'])) - self.assertRegexpMatches( - git_cl.sys.stdout.getvalue(), - 'Tried jobs on:\nBucket: luci.chromium.try') + self.assertIn( + 'Scheduling jobs on:\nBucket: luci.chromium.try', + git_cl.sys.stdout.getvalue()) def _common_GerritCommitMsgHookCheck(self): self.mock(git_cl.sys, 'stdout', StringIO.StringIO())