[changes] Add ability to specify diff cluster

Summary: Add a way to specify a different diff cluster

Test Plan: unit tests

Reviewers: haoyi, cf, kylec, vishal

Reviewed By: vishal

Subscribers: wwu, alexallain

Maniphest Tasks: T27671

Differential Revision: https://tails.corp.dropbox.com/D84094

changes/backends/jenkins/builder.py

@@ -54,10 +54,11 @@ class NotFound(Exception):
 class JenkinsBuilder(BaseBackend):
     provider = 'jenkins'
 
-    def __init__(self, master_urls=None, job_name=None, token=None, auth=None,
-                 sync_phase_artifacts=True, *args, **kwargs):
+    def __init__(self, master_urls=None, diff_urls=None, job_name=None, token=None,
+                 auth=None, sync_phase_artifacts=True, *args, **kwargs):
         super(JenkinsBuilder, self).__init__(*args, **kwargs)
         self.master_urls = master_urls
+        self.diff_urls = diff_urls
 
         if not self.master_urls and self.app.config['JENKINS_URL']:
             self.master_urls = [self.app.config['JENKINS_URL']]
@@ -337,17 +338,21 @@ def _process_test_report(self, step, test_report):
                 test_list.append(test_result)
         return test_list
 
-    def _pick_master(self, job_name):
+    def _pick_master(self, job_name, is_diff=False):
         """
         Identify a master to run the given job on.
 
         The master with the lowest queue for the given job is chosen. By random
         sorting the first empty queue will be prioritized.
         """
-        if len(self.master_urls) == 1:
-            return self.master_urls[0]
+        candidate_urls = self.master_urls
+        if is_diff and self.diff_urls:
+            candidate_urls = self.diff_urls
 
-        master_urls = self.master_urls[:]
+        if len(candidate_urls) == 1:
+            return candidate_urls[0]
+
+        master_urls = candidate_urls[:]
         random.shuffle(master_urls)
 
         best_match = (sys.maxint, None)
@@ -626,6 +631,7 @@ def _sync_phased_results(self, step, artifacts):
         # insight into the actual steps a build process takes and unfortunately
         # the best way to do this is to rewrite history within Changes
         job = step.job
+        is_diff = not job.source.is_commit()
         project = step.project
 
         artifacts_by_name = dict(
@@ -639,7 +645,7 @@ def _sync_phased_results(self, step, artifacts):
             'job_name': step.data['job_name'],
             'build_no': step.data['build_no'],
             'generated': True,
-            'master': self._pick_master(step.data['job_name']),
+            'master': self._pick_master(step.data['job_name'], is_diff),
         }
 
         phases = set()
@@ -823,7 +829,7 @@ def get_job_parameters(self, job, target_id=None):
             )
         return params
 
-    def create_job_from_params(self, target_id, params, job_name=None):
+    def create_job_from_params(self, target_id, params, job_name=None, is_diff=False):
         if job_name is None:
             job_name = self.job_name
 
@@ -834,7 +840,7 @@ def create_job_from_params(self, target_id, params, job_name=None):
             'parameter': params
         }
 
-        master = self._pick_master(job_name)
+        master = self._pick_master(job_name, is_diff)
 
         # TODO: Jenkins will return a 302 if it cannot queue the job which I
         # believe implies that there is already a job with the same parameters
@@ -879,9 +885,11 @@ def create_job(self, job):
           or a finalized build number.
         """
         params = self.get_job_parameters(job)
+        is_diff = not job.source.is_commit()
         job_data = self.create_job_from_params(
             target_id=job.id.hex,
             params=params,
+            is_diff=is_diff
         )
 
         if job_data['queued']:

changes/backends/jenkins/buildstep.py

@@ -19,16 +19,23 @@ class JenkinsBuildStep(BuildStep):
     builder_cls = JenkinsBuilder
     logger = logging.getLogger('jenkins')
 
-    def __init__(self, job_name=None, jenkins_url=None, token=None, auth=None):
+    def __init__(self, job_name=None, jenkins_url=None, jenkins_diff_url=None, token=None, auth=None):
         # we support a string or a list of strings for master server urls
         if not isinstance(jenkins_url, (list, tuple)):
             if jenkins_url:
                 jenkins_url = [jenkins_url]
             else:
                 jenkins_url = []
 
+        if not isinstance(jenkins_diff_url, (list, tuple)):
+            if jenkins_diff_url:
+                jenkins_diff_url = [jenkins_diff_url]
+            else:
+                jenkins_diff_url = []
+
         self.job_name = job_name
         self.jenkins_urls = jenkins_url
+        self.jenkins_diff_urls = jenkins_diff_url
         self.token = token
         self.auth = auth
 
@@ -38,6 +45,7 @@ def get_builder(self, app=current_app):
     def get_builder_options(self):
         return {
             'master_urls': self.jenkins_urls,
+            'diff_urls': self.jenkins_diff_urls,
             'token': self.token,
             'auth': self.auth,
             'job_name': self.job_name,

tests/changes/backends/jenkins/test_builder.py

@@ -28,6 +28,7 @@ class BaseTestCase(BackendTestCase):
     builder_cls = JenkinsBuilder
     builder_options = {
         'master_urls': ['http://jenkins.example.com'],
+        'diff_urls': ['http://jenkins-diff.example.com'],
         'job_name': 'server',
     }
 
@@ -129,7 +130,7 @@ def test_active_creation(self):
     @mock.patch.object(JenkinsBuilder, '_find_job')
     def test_patch(self, find_job):
         responses.add(
-            responses.POST, 'http://jenkins.example.com/job/server/build/api/json/',
+            responses.POST, 'http://jenkins-diff.example.com/job/server/build/api/json/',
             body='',
             status=201)
 
@@ -138,7 +139,7 @@ def test_patch(self, find_job):
             'item_id': None,
             'job_name': 'server',
             'queued': False,
-            'master': 'http://jenkins.example.com',
+            'master': 'http://jenkins-diff.example.com',
         }
 
         patch = Patch(