Be smarter about choosing branches

Part of this is also about failing sooner, and not allowing invalid
configurations to enter the queue. This commit also fixes an obscure bug
in substitute_placeholders(). Finally, it adds unit tests for all bugs
fixed.

Signed-off-by: Zack Cerza <zack.cerza@inktank.com>

diff --git a/teuthology/repo_utils.py b/teuthology/repo_utils.py
index 6538f0d371cdd25014f7d6cd1a39ef12519be712..a392899d1820237d3e987753518a7eeca5486c90 100644 (file)
--- a/teuthology/repo_utils.py
+++ b/teuthology/repo_utils.py
@@ -123,7 +123,7 @@ class BranchNotFoundError(ValueError):
             repo_str = " in repo: %s" % self.repo
         else:
             repo_str = ""
-        return "Branch {branch} not found{repo_str}!".format(
+        return "Branch '{branch}' not found{repo_str}!".format(
             branch=self.branch, repo_str=repo_str)
 
 

diff --git a/teuthology/suite.py b/teuthology/suite.py
index 815de49d516dc3488c3b7360c4c0827900b6111c..c2acf9d00e42aaab731ddf5b3d6c411c1936edf4 100644 (file)
--- a/teuthology/suite.py
+++ b/teuthology/suite.py
@@ -38,7 +38,7 @@ def main(args):
     teuthology_branch = args['--teuthology-branch']
     machine_type = args['--machine-type']
     distro = args['--distro']
-    suite_branch = args['--suite-branch'] or ceph_branch
+    suite_branch = args['--suite-branch']
     suite_dir = args['--suite-dir']
 
     limit = int(args['--limit'])
@@ -53,14 +53,15 @@ def main(args):
     name = make_run_name(suite, ceph_branch, kernel_branch, kernel_flavor,
                          machine_type)
 
+    job_config = create_initial_config(suite, suite_branch, ceph_branch,
+                                       teuthology_branch, kernel_branch,
+                                       kernel_flavor, distro, machine_type)
+
     if suite_dir:
         suite_repo_path = suite_dir
     else:
-        suite_repo_path = fetch_suite_repo(suite_branch, test_name=name)
-
-    job_config = create_initial_config(suite, ceph_branch,
-                                       teuthology_branch, kernel_branch,
-                                       kernel_flavor, distro, machine_type)
+        suite_repo_path = fetch_suite_repo(job_config.suite_branch,
+                                           test_name=name)
 
     job_config.name = name
     job_config.priority = priority
@@ -68,8 +69,6 @@ def main(args):
         job_config.email = email
     if owner:
         job_config.owner = owner
-    if suite_branch:
-        job_config.suite_branch = suite_branch
 
     with NamedTemporaryFile(prefix='schedule_suite_',
                             delete=False) as base_yaml:
@@ -147,7 +146,7 @@ def fetch_suite_repo(branch, test_name):
     return suite_repo_path
 
 
-def create_initial_config(suite, ceph_branch, teuthology_branch,
+def create_initial_config(suite, suite_branch, ceph_branch, teuthology_branch,
                           kernel_branch, kernel_flavor, distro, machine_type):
     """
     Put together the config file used as the basis for each job in the run.
@@ -155,7 +154,7 @@ def create_initial_config(suite, ceph_branch, teuthology_branch,
     branches specified and specifies them so we know exactly what we're
     testing.
 
-    :returns: A yaml-formatted string
+    :returns: A JobConfig object
     """
     # Put together a stanza specifying the kernel hash
     if kernel_branch == 'distro':
@@ -178,8 +177,8 @@ def create_initial_config(suite, ceph_branch, teuthology_branch,
     # Get the ceph hash
     ceph_hash = get_hash('ceph', ceph_branch, kernel_flavor, machine_type)
     if not ceph_hash:
-        schedule_fail("Ceph branch '{branch}' not found".format(
-            branch=ceph_branch))
+        exc = BranchNotFoundError(ceph_branch, 'ceph.git')
+        schedule_fail(message=str(exc))
     log.info("ceph sha1: {hash}".format(hash=ceph_hash))
 
     # Get the ceph package version
@@ -195,22 +194,37 @@ def create_initial_config(suite, ceph_branch, teuthology_branch,
         s3_branch = ceph_branch
     else:
         log.info("branch {0} not in s3-tests.git; will use master for"
-                 "s3-tests".format(ceph_branch))
+                 " s3-tests".format(ceph_branch))
         s3_branch = 'master'
     log.info("s3-tests branch: %s", s3_branch)
 
-    if not teuthology_branch:
+    if teuthology_branch:
+        if not get_branch_info('teuthology', teuthology_branch):
+            exc = BranchNotFoundError(teuthology_branch, 'teuthology.git')
+            raise schedule_fail(message=str(exc))
+    else:
         # Decide what branch of teuthology to use
         if get_branch_info('teuthology', ceph_branch):
             teuthology_branch = ceph_branch
         else:
             log.info("branch {0} not in teuthology.git; will use master for"
-                     "teuthology".format(ceph_branch))
+                     " teuthology".format(ceph_branch))
             teuthology_branch = 'master'
     log.info("teuthology branch: %s", teuthology_branch)
 
+    if not suite_branch:
+        # Decide what branch of ceph-qa-suite to use
+        if get_branch_info('ceph-qa-suite', ceph_branch):
+            suite_branch = ceph_branch
+        else:
+            log.info("branch {0} not in ceph-qa-suite.git; will use master for"
+                     " ceph-qa-suite".format(ceph_branch))
+            suite_branch = 'master'
+    log.info("ceph-qa-suite branch: %s", suite_branch)
+
     config_input = dict(
         suite=suite,
+        suite_branch=suite_branch,
         ceph_branch=ceph_branch,
         ceph_hash=ceph_hash,
         teuthology_branch=teuthology_branch,
@@ -298,7 +312,7 @@ class ScheduleFailError(RuntimeError):
         self.name = name
 
     def __str__(self):
-        return "Job scheduling {name} failed: '{msg}'".format(
+        return "Job scheduling {name} failed: {msg}".format(
             name=self.name,
             msg=self.message,
         ).replace('  ', ' ')
@@ -617,6 +631,7 @@ def substitute_placeholders(input_dict, values_dict):
                         values to be substituted.
     :returns:           The modified input_dict
     """
+    input_dict = dict(input_dict)
     for (key, value) in input_dict.iteritems():
         if isinstance(value, dict):
             substitute_placeholders(value, values_dict)
@@ -683,6 +698,7 @@ dict_templ = {
         }
     },
     'suite': Placeholder('suite'),
+    'suite_branch': Placeholder('suite_branch'),
     'tasks': [
         {'chef': None},
         {'clock.check': None}

diff --git a/teuthology/test/test_suite.py b/teuthology/test/test_suite.py
index 9dc44a30a283bb1f4d528eb3f4b8b4b7c62166ab..14869a372512cc43f39baef234124f71081d55c8 100644 (file)
--- a/teuthology/test/test_suite.py
+++ b/teuthology/test/test_suite.py
@@ -39,6 +39,25 @@ class TestSuiteOffline(object):
         assert suite.get_gitbuilder_url('ceph', 'squeeze', 'deb', 'x86_64',
                                         'basic') == ref_url
 
+    def test_substitute_placeholders(self):
+        input_dict = dict(
+            suite='suite',
+            suite_branch='suite_branch',
+            ceph_branch='ceph_branch',
+            ceph_hash='ceph_hash',
+            teuthology_branch='teuthology_branch',
+            machine_type='machine_type',
+            distro='distro',
+            s3_branch='s3_branch',
+        )
+        output_dict = suite.substitute_placeholders(suite.dict_templ,
+                                                    input_dict)
+        assert output_dict['suite'] == 'suite'
+        assert isinstance(suite.dict_templ['suite'], suite.Placeholder)
+        assert isinstance(
+            suite.dict_templ['overrides']['admin_socket']['branch'],
+            suite.Placeholder)
+
 
 class TestSuiteOnline(object):
     def setup(self):
@@ -51,41 +70,65 @@ class TestSuiteOnline(object):
         ref_hash = resp.json()['object']['sha']
         assert suite.get_hash('ceph') == ref_hash
 
+    def test_all_master_branches(self):
+        # Don't attempt to send email
+        config.results_email = None
+        job_config = suite.create_initial_config('suite', 'master',
+                                                 'master', 'master', 'testing',
+                                                 'basic', 'centos', 'plana')
+        assert ((job_config.branch, job_config.teuthology_branch,
+                 job_config.suite_branch) == ('master', 'master', 'master'))
+
     def test_config_bogus_kernel_branch(self):
         # Don't attempt to send email
         config.results_email = None
         with raises(suite.ScheduleFailError):
-            suite.create_initial_config('s', 'c', 't', 'bogus_kernel_branch',
-                                        'f', 'd', 'm')
+            suite.create_initial_config('s', None, 'master', 't',
+                                        'bogus_kernel_branch', 'f', 'd', 'm')
 
     def test_config_bogus_kernel_flavor(self):
         # Don't attempt to send email
         config.results_email = None
         with raises(suite.ScheduleFailError):
-            suite.create_initial_config('s', 'c', 't', 'k',
+            suite.create_initial_config('s', None, 'master', 't', 'k',
                                         'bogus_kernel_flavor', 'd', 'm')
 
     def test_config_bogus_ceph_branch(self):
         # Don't attempt to send email
         config.results_email = None
         with raises(suite.ScheduleFailError):
-            suite.create_initial_config('s', 'bogus_ceph_branch', 't', 'k',
-                                        'f', 'd', 'm')
+            suite.create_initial_config('s', None, 'bogus_ceph_branch', 't',
+                                        'k', 'f', 'd', 'm')
+
+    def test_config_bogus_suite_branch(self):
+        # Don't attempt to send email
+        config.results_email = None
+        with raises(suite.ScheduleFailError):
+            suite.create_initial_config('s', 'bogus_suite_branch', 'master',
+                                        't', 'k', 'f', 'd', 'm')
+
+    def test_config_bogus_teuthology_branch(self):
+        # Don't attempt to send email
+        config.results_email = None
+        with raises(suite.ScheduleFailError):
+            suite.create_initial_config('s', None, 'master',
+                                        'bogus_teuth_branch', 'k', 'f', 'd',
+                                        'm')
 
     def test_config_substitution(self):
         # Don't attempt to send email
         config.results_email = None
         job_config = suite.create_initial_config('MY_SUITE', 'master',
-                                                 'master', 'testing', 'basic',
-                                                 'centos', 'plana')
+                                                 'master', 'master', 'testing',
+                                                 'basic', 'centos', 'plana')
         assert job_config['suite'] == 'MY_SUITE'
 
     def test_config_kernel_section(self):
         # Don't attempt to send email
         config.results_email = None
         job_config = suite.create_initial_config('MY_SUITE', 'master',
-                                                 'master', 'testing', 'basic',
-                                                 'centos', 'plana')
+                                                 'master', 'master', 'testing',
+                                                 'basic', 'centos', 'plana')
         assert job_config['kernel']['kdb'] is True