git-cl: Use bb to schedule try jobs.

Bug: 976104 Change-Id: I3423667f1ed9edfc5fa17842932de7704951fc62 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1801799 Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org> Reviewed-by: Andrii Shyshkalov <tandrii@google.com>
6 years ago · 5b6ae8bc74
parent 57fe41698a
commit 5b6ae8bc74
2 changed files with 105 additions and 114 deletions
--- a/git_cl.py
+++ b/git_cl.py
@ -416,78 +416,74 @@ 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'
-
-  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
-
-  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
+  _, ret_code = subprocess2.communicate(
+      ['bb', 'auth-info'], stdout=subprocess2.VOID, stderr=subprocess2.VOID)
+  if ret_code:
+    DieWithError('"bb auth-login" must be executed before scheduling try jobs.')
+
+  gerrit_change = changelist.GetGerritChange()
+  shared_properties = {
+      'category': options.category,
+  }
  if options.clobber:
-    shared_parameters_properties['clobber'] = True
+    shared_properties['clobber'] = True
  extra_properties = _get_properties_from_options(options)
  if extra_properties:
-    shared_parameters_properties.update(extra_properties)
+    shared_properties.update(extra_properties)

-  batch_req_body = {'builds': []}
+  batch_request = {'requests': []}
  print_text = []
  print_text.append('Tried jobs on:')
  for bucket, builders_and_tests in sorted(buckets.iteritems()):
    print_text.append('Bucket: %s' % bucket)
+    project, bucket = bucket.split('.', 1)
    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

      tags = [
-          'builder:%s' % builder,
-          'buildset:%s' % buildset,
-          'user_agent:git_cl_try',
+          {
+              'key': 'builder',
+              'value': builder,
+          },
+          {
+              'key': 'user_agent',
+              'value': 'git_cl_try',
+          },
      ]

-      batch_req_body['builds'].append(
+      batch_request['requests'].append(
          {
-              'bucket': bucket,
-              'parameters_json': json.dumps(parameters),
-              'client_operation_id': str(uuid.uuid4()),
-              'tags': tags,
+              'scheduleBuild': {
+                  'builder': {
+                      'project': options.project or project,
+                      'bucket': bucket,
+                      'builder': builder,
+                  },
+                  'properties': properties,
+                  'requestId': str(uuid.uuid4()),
+                  'gerritChanges': [gerrit_change],
+                  'tags': tags,
+              }
          }
      )

-  _buildbucket_retry(
-      'triggering tryjobs',
-      http,
-      buildbucket_put_url,
-      'PUT',
-      body=json.dumps(batch_req_body),
-      headers={'Content-Type': 'application/json'}
-  )
+  (stdout, _), ret_code = subprocess2.communicate(
+      ['bb', 'batch', '-host', options.buildbucket_host],
+      stdin=json.dumps(batch_request, sort_keys=True),
+      stdout=subprocess2.PIPE)
+  if ret_code:
+    print_text.append('Failed to schedule builds for some bots:')
+    if stdout:
+      responses = json.loads(stdout)
+    print_text.extend(
+        '  ' + response['error']['message']
+        for response in responses['responses'] if 'error' in response)
+
  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))
@ -1617,10 +1613,6 @@ class Changelist(object):
    """Returns reason (str) if unable trigger tryjobs on this CL or None."""
    return self._codereview_impl.CannotTriggerTryJobReason()

-  def GetTryJobProperties(self, patchset=None):
-    """Returns dictionary of properties to launch tryjob."""
-    return self._codereview_impl.GetTryJobProperties(patchset=patchset)
-
  def __getattr__(self, attr):
    # This is because lots of untested code accesses Rietveld-specific stuff
    # directly, and it's hard to fix for sure. So, just let it work, and fix
@ -1760,10 +1752,6 @@ class _ChangelistCodereviewBase(object):
  def GetReviewers(self):
    raise NotImplementedError()

-  def GetTryJobProperties(self, patchset=None):
-    raise NotImplementedError()
-
-
 class _GerritChangelistImpl(_ChangelistCodereviewBase):
  def __init__(self, changelist, codereview_host=None):
    super(_GerritChangelistImpl, self).__init__(changelist)
@ -2882,26 +2870,27 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
    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):
--- a/tests/git_cl_test.py
+++ b/tests/git_cl_test.py
@ -2381,6 +2381,42 @@ class TestGitCl(TestCase):
  def test_git_cl_try_buildbucket_with_properties_gerrit(self):
    self.mock(git_cl.Changelist, 'GetMostRecentPatchset', lambda _: 7)
    self.mock(git_cl.uuid, 'uuid4', lambda: 'uuid4')
+    def mocked_call_with_stdin(*a, **kw):
+      args = list(a)
+      if kw.get('stdin'):
+        args.append(kw['stdin'])
+      return ([self._mocked_call(*args), ''], 0)
+
+    self.mock(subprocess2, 'communicate', mocked_call_with_stdin)
+
+    bb_request = json.dumps({
+        "requests": [{
+            "scheduleBuild": {
+                "requestId": "uuid4",
+                "builder": {
+                    "project": "luci",
+                    "builder": "win",
+                    "bucket": "chromium.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"},
+                ],
+            },
+        }],
+    }, sort_keys=True)
+

    self.calls = [
        ((['git', 'symbolic-ref', 'HEAD'],), 'feature'),
@ -2410,46 +2446,12 @@ class TestGitCl(TestCase):
            },
          },
        }),
+        ((['bb', 'auth-info'],), ''),
+        ((['git', 'config', 'branch.feature.gerritpatchset'],), '7'),
+        ((['bb', 'batch', '-host', 'cr-buildbucket.appspot.com'],
+          bb_request,), ''),
    ]

-    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, '_buildbucket_retry', _buildbucket_retry)
-
    self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
    self.assertEqual(0, git_cl.main([
        'try', '-B', 'luci.chromium.try', '-b', 'win',