From d4c867355a8bc3b62fa53d886d229632435c3172 Mon Sep 17 00:00:00 2001 From: Andrii Shyshkalov Date: Tue, 25 Sep 2018 04:29:31 +0000 Subject: [PATCH] git cl: use explicit Gerrit mirrors on 404s during upload. This will cycle through known gerrit mirrors for chromium-review host if Gerrit HTTP RPC results in 404 after successful initial git push refs/for/refs/... Tested locally by intentionally using wrong change number (2000000): $ PATH=`pwd`:$PATH git cl upload --bypass-hooks ... remote: Processing changes: refs: 1, new: 1, done remote: SUCCESS remote: New Changes: remote: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1227440 * [new branch] ea9aea5faa85b4b289b7add4bc6f4d5dd6a01caf -> refs/for/refs/heads/master%wip,m=Initial_upload,hashtag=git-cl WARNING:root:404 NotFound error occurred while querying POST https://chromium-review.googlesource.com/a/changes/chromium%2Ftools%2Fdepot_tools~2000000/revisions/current/review: Not Found WARNING:root:404 NotFound error occurred while querying POST https://ap1-mirror-chromium-review.googlesource.com/a/changes/chromium%2Ftools%2Fdepot_tools~2000000/revisions/current/review: Not Found WARNING:root:404 NotFound error occurred while querying POST https://us1-mirror-chromium-review.googlesource.com/a/changes/chromium%2Ftools%2Fdepot_tools~2000000/revisions/current/review: Not Found ^C Bug: 881860 Change-Id: Iac7dbe4e35052007650a7a2646a394caed6bd400 Reviewed-on: https://chromium-review.googlesource.com/1227441 Commit-Queue: Andrii Shyshkalov Reviewed-by: Edward Lesmes --- gerrit_util.py | 40 ++++++++++++++++++++++++++++++++++++++++ tests/git_cl_test.py | 18 ++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/gerrit_util.py b/gerrit_util.py index 6d45ab781..b81f7b5cc 100644 --- a/gerrit_util.py +++ b/gerrit_util.py @@ -16,6 +16,7 @@ import json import logging import netrc import os +import random import re import socket import stat @@ -417,6 +418,17 @@ def ReadHttpResponse(conn, accept_statuses=frozenset([200])): conn.req_host, conn.req_params['method'], conn.req_params['uri'], http_version, http_version, response.status, response.reason) + if response.status == 404: + # TODO(crbug/881860): remove this hack. + # HACK: try different Gerrit mirror as a workaround for potentially + # out-of-date mirror hit through default routing. + if conn.req_host == 'chromium-review.googlesource.com': + conn.req_params['uri'] = _UseGerritMirror( + conn.req_params['uri'], 'chromium-review.googlesource.com') + # And don't increase sleep_time in this case, since we suspect we've + # just asked wrong git mirror before. + sleep_time /= 2.0 + if TRY_LIMIT - idx > 1: LOGGER.info('Will retry in %d seconds (%d more times)...', sleep_time, TRY_LIMIT - idx - 1) @@ -914,3 +926,31 @@ def ChangeIdentifier(project, change_number): """ assert int(change_number) return '%s~%s' % (urllib.quote(project, safe=''), change_number) + + +# TODO(crbug/881860): remove this hack. +_GERRIT_MIRROR_PREFIXES = ['us1', 'us2', 'us3'] +assert all(3 == len(p) for p in _GERRIT_MIRROR_PREFIXES) + + +def _UseGerritMirror(url, host): + """Returns new url which uses randomly selected mirror for a gerrit host. + + url's host should be for a given host or a result of prior call to this + function. + + Assumes url has a single occurence of the host substring. + """ + assert host in url + suffix = '-mirror-' + host + prefixes = set(_GERRIT_MIRROR_PREFIXES) + prefix_len = len(_GERRIT_MIRROR_PREFIXES[0]) + st = url.find(suffix) + if st == -1: + actual_host = host + else: + # Already uses some mirror. + assert st >= prefix_len, (uri, host, st, prefix_len) + prefixes.remove(url[st-prefix_len:st]) + actual_host = url[st-prefix_len:st+len(suffix)] + return url.replace(actual_host, random.choice(list(prefixes)) + suffix) diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 1e4b03c20..25beba318 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -439,6 +439,24 @@ class TestGitClBasic(unittest.TestCase): 'Cr-Commit-Position: refs/heads/branch@{#2}\n' 'Cr-Branched-From: somehash-refs/heads/master@{#12}') + def test_gerrit_mirror_hack(self): + cr = 'chromium-review.googlesource.com' + url0 = 'https://%s/a/changes/x?a=b' % cr + origMirrors = git_cl.gerrit_util._GERRIT_MIRROR_PREFIXES + try: + git_cl.gerrit_util._GERRIT_MIRROR_PREFIXES = ['us1', 'us2'] + url1 = git_cl.gerrit_util._UseGerritMirror(url0, cr) + url2 = git_cl.gerrit_util._UseGerritMirror(url1, cr) + url3 = git_cl.gerrit_util._UseGerritMirror(url2, cr) + + self.assertNotEqual(url1, url2) + self.assertEqual(sorted((url1, url2)), [ + 'https://us1-mirror-chromium-review.googlesource.com/a/changes/x?a=b', + 'https://us2-mirror-chromium-review.googlesource.com/a/changes/x?a=b']) + self.assertEqual(url1, url3) + finally: + git_cl.gerrit_util._GERRIT_MIRROR_PREFIXES = origMirrors + class TestParseIssueURL(unittest.TestCase): def _validate(self, parsed, issue=None, patchset=None, hostname=None,