teuthology/suite/run.py: Improve ScheduleFail exception

docs/docker-compose/start.sh

@@ -1,5 +1,5 @@
 #!/bin/bash
-set -e
+set -e -x
 export TEUTHOLOGY_BRANCH=${TEUTHOLOGY_BRANCH:-$(git branch --show-current)}
 export TEUTH_BRANCH=${TEUTHOLOGY_BRANCH}
 if [ -n "$ANSIBLE_INVENTORY_REPO" ]; then

docs/docker-compose/teuthology/teuthology.sh

@@ -4,8 +4,8 @@ set -e
 if [ -n "$SSH_PRIVKEY_FILE" ]; then
     echo "$SSH_PRIVKEY" > $HOME/.ssh/$SSH_PRIVKEY_FILE
 fi
-source /teuthology/virtualenv/bin/activate
 set -x
+source /teuthology/virtualenv/bin/activate
 if [ -n "$TESTNODES" ]; then
     for node in $(echo $TESTNODES | tr , ' '); do
         teuthology-update-inventory -m $MACHINE_TYPE $node

teuthology/repo_utils.py

@@ -68,10 +68,14 @@ def ls_remote(url, ref):
     """
     sha1 = None
     cmd = "git ls-remote {} {}".format(url, ref)
-    result = subprocess.check_output(
-        cmd, shell=True).split()
-    if result:
-        sha1 = result[0].decode()
+    try:
+        result = subprocess.check_output(
+            cmd, stderr=subprocess.STDOUT,
+            shell=True).split()
+        if result:
+            sha1 = result[0].decode()
+    except subprocess.CalledProcessError as e:
+        raise GitError(e.output) from None
     log.debug("{} -> {}".format(cmd, sha1))
     return sha1
 

teuthology/suite/__init__.py

@@ -16,6 +16,7 @@
 from teuthology.suite.run import Run
 from teuthology.suite.util import schedule_fail
 from teuthology.util.strtobool import strtobool
+from teuthology.exceptions import ScheduleFailError
 
 log = logging.getLogger(__name__)
 
@@ -52,36 +53,40 @@ def process_args(args):
         key = key.lstrip('--').replace('-', '_')
         # Rename the key if necessary
         key = rename_args.get(key) or key
-        if key == 'suite_branch':
-            value = value or override_arg_defaults('--suite-branch', None)
-        if key == 'suite' and value is not None:
-            value = normalize_suite_name(value)
-        if key == 'suite_relpath' and value is None:
-            value = ''
-        elif key in ('limit', 'priority', 'num', 'newest', 'seed', 'job_threshold'):
-            value = int(value)
-        elif key == 'subset' and value is not None:
-            # take input string '2/3' and turn into (2, 3)
-            value = tuple(map(int, value.split('/')))
-        elif key == 'expire' and value is None:
-            # Skip empty 'expire' values
-            continue
-        elif key in ('filter_all', 'filter_in', 'filter_out', 'rerun_statuses'):
-            if not value:
-                value = []
-            else:
-                value = [x.strip() for x in value.split(',')]
-        elif key == 'ceph_repo':
-            value = expand_short_repo_name(
-                value,
-                config.get_ceph_git_url())
-        elif key == 'suite_repo':
-            value = expand_short_repo_name(
-                value,
-                config.get_ceph_qa_suite_git_url())
-        elif key in ('validate_sha1', 'filter_fragments', 'kdb'):
-            value = strtobool(value)
-        conf[key] = value
+        try:
+            if key == 'suite_branch':
+                value = value or override_arg_defaults('--suite-branch', None)
+            if key == 'suite' and value is not None:
+                value = normalize_suite_name(value)
+            if key == 'suite_relpath' and value is None:
+                value = ''
+            elif key in ('limit', 'priority', 'num', 'newest', 'seed', 'job_threshold'):
+                value = int(value)
+                if key != 'seed' and value < 0:
+                    raise ScheduleFailError(f"{key} value cannot be < 0", '')
+            elif key == 'subset' and value is not None:
+                # take input string '2/3' and turn into (2, 3)
+                value = tuple(map(int, value.split('/')))
+                if len(value) != 2:
+                    raise ValueError
+            elif key in ('filter_all', 'filter_in', 'filter_out', 'rerun_statuses'):
+                if not value:
+                    value = []
+                else:
+                    value = [x.strip() for x in value.split(',')]
+            elif key == 'ceph_repo':
+                value = expand_short_repo_name(
+                    value,
+                    config.get_ceph_git_url())
+            elif key == 'suite_repo':
+                value = expand_short_repo_name(
+                    value,
+                    config.get_ceph_qa_suite_git_url())
+            elif key in ('validate_sha1', 'filter_fragments'):
+                value = strtobool(value)
+            conf[key] = value
+        except ValueError:
+            raise ScheduleFailError(f"--{key} value has incorrect type/format", '')
     return conf
 
 
@@ -137,10 +142,11 @@ def main(args):
 
     run = Run(conf)
     name = run.name
-    run.prepare_and_schedule()
+    job_count = run.prepare_and_schedule()
     if not conf.dry_run and conf.wait:
         return wait(name, config.max_job_time,
                     conf.archive_upload_url)
+    return job_count
 
 
 def get_rerun_conf_overrides(conf):

teuthology/suite/run.py

@@ -16,6 +16,7 @@
 from teuthology.config import config, JobConfig
 from teuthology.exceptions import (
     BranchMismatchError, BranchNotFoundError, CommitNotFoundError,
+    ScheduleFailError, GitError
 )
 from teuthology.misc import deep_merge, get_results_url, update_key
 from teuthology.orchestra.opsys import OS
@@ -106,13 +107,18 @@ def create_initial_config(self):
                 suite_branch, test_name=self.name, dry_run=self.args.dry_run, commit=suite_hash)
         teuthology_branch, teuthology_sha1 = self.choose_teuthology_branch()
 
-
         if self.args.distro and self.args.distro_version:
-            self.args.distro_version, _ = \
-                OS.version_codename(
-                    self.args.distro,
-                    self.args.distro_version,
+            try:
+                self.args.distro_version, _ = \
+                    OS.version_codename(
+                        self.args.distro,
+                        self.args.distro_version,)
+            except KeyError:
+                raise ScheduleFailError(
+                    " ditro: {} or distro_version: {} doesn't exists".format(
+                        self.args.distro, self.args.distro_version), ''
                 )
+
         self.config_input = dict(
             suite=self.args.suite,
             suite_branch=suite_branch,
@@ -159,8 +165,11 @@ def choose_os(self):
         os_type = self.args.distro
         os_version = self.args.distro_version
         if not (os_type and os_version):
-            os_ = util.get_distro_defaults(
-                self.args.distro, self.args.machine_type)[2]
+            try:
+                os_ = util.get_distro_defaults(
+                    self.args.distro, self.args.machine_type)[2]
+            except KeyError:
+                raise ScheduleFailError(f"distro {self.args.distro} doesn't exist")
         else:
             os_ = OS(os_type, os_version)
         return os_
@@ -208,7 +217,6 @@ def choose_ceph_hash(self):
         tip.
         """
         repo_name = self.ceph_repo_name
-
         ceph_hash = None
         if self.args.ceph_sha1:
             ceph_hash = self.args.ceph_sha1
@@ -223,8 +231,11 @@ def choose_ceph_hash(self):
             log.info("ceph sha1 explicitly supplied")
 
         elif self.args.ceph_branch:
-            ceph_hash = util.git_ls_remote(
-                self.args.ceph_repo, self.args.ceph_branch)
+            try:
+                ceph_hash = util.git_ls_remote(
+                    self.args.ceph_repo, self.args.ceph_branch)
+            except GitError as e:
+                raise util.schedule_fail(message=str(e), name=self.name, dry_run=self.args.dry_run) from None
             if not ceph_hash:
                 exc = BranchNotFoundError(
                     self.args.ceph_branch,
@@ -469,6 +480,8 @@ def prepare_and_schedule(self):
         if num_jobs:
             self.write_result()
 
+        return num_jobs
+
     def collect_jobs(self, arch, configs, newest=False, limit=0):
         jobs_to_schedule = []
         jobs_missing_packages = []
@@ -610,8 +623,11 @@ def schedule_suite(self):
             self.suite_repo_path,
             self.args.suite_relpath,
             'suites',
-            self.base_config.suite.replace(':', '/'),
+            suite_name.replace(':', '/'),
         ))
+        if not os.path.exists(suite_path):
+            log.error("Suite path doesn't exists")
+            raise ScheduleFailError("Suite path doesn't exists", suite_name)
         log.debug('Suite %s in %s' % (suite_name, suite_path))
         log.debug(f"subset = {self.args.subset}")
         log.debug(f"no_nested_subset = {self.args.no_nested_subset}")
@@ -686,7 +702,9 @@ def schedule_suite(self):
                         self.args.newest
                     )
                 if not sha1s:
-                    util.schedule_fail('Backtrack for --newest failed', name, dry_run=self.args.dry_run)
+                    util.schedule_fail('Backtrack for --newest failed, could not find a git parent with the packages,' \
+                        ' (optionally) use --sha1 to directly point to' \
+                        ' your build.', name, dry_run=self.args.dry_run)
                 cur_sha1 = sha1s.pop(0)
                 self.config_input['ceph_hash'] = cur_sha1
                 # If ceph_branch and suite_branch are the same and

teuthology/suite/test/test_run_.py

@@ -281,7 +281,15 @@ def test_successful_schedule(
         m_get_install_task_flavor,
         m_write_rerun_memo,
         m_schedule_jobs,
+        tmp_path,
     ):
+        # Create the expected suite directory structure
+        suite_path = tmp_path / 'suites' / 'suite'
+        suite_path.mkdir(parents=True, exist_ok=True)
+
+        # Update args to use the temporary directory
+        self.args.suite_dir = str(tmp_path)
+
         m_get_arch.return_value = 'x86_64'
         m_git_validate_sha1.return_value = self.args.ceph_sha1
         m_package_version_for_hash.return_value = 'ceph_version'
@@ -392,7 +400,15 @@ def test_newest_failure(
         m_get_install_task_flavor,
         m_schedule_jobs,
         m_find_git_parents,
+        tmp_path,
     ):
+        # Create the expected suite directory structure
+        suite_path = tmp_path / 'suites' / 'suite'
+        suite_path.mkdir(parents=True, exist_ok=True)
+
+        # Update args to use the temporary directory
+        self.args.suite_dir = str(tmp_path)
+
         m_get_arch.return_value = 'x86_64'
         m_git_validate_sha1.return_value = self.args.ceph_sha1
         m_package_version_for_hash.return_value = None
@@ -440,13 +456,21 @@ def test_newest_success_same_branch_same_repo(
         m_write_rerun_memo,
         m_schedule_jobs,
         m_find_git_parents,
+        tmp_path,
     ):
         """
         Test that we can successfully schedule a job with newest
         backtracking when the ceph and suite branches are the same
         and the ceph_sha1 is not supplied. We should expect that the
         ceph_hash and suite_hash will be updated to the working sha1
         """
+        # Create the expected suite directory structure
+        suite_path = tmp_path / 'suites' / 'suite'
+        suite_path.mkdir(parents=True, exist_ok=True)
+
+        # Update args to use the temporary directory
+        self.args.suite_dir = str(tmp_path)
+
         m_get_arch.return_value = 'x86_64'
         # rig has_packages_for_distro to fail this many times, so
         # everything will run NUM_FAILS+1 times
@@ -548,6 +572,7 @@ def test_newest_success_diff_branch_diff_repo(
         m_write_rerun_memo,
         m_schedule_jobs,
         m_find_git_parents,
+        tmp_path,
     ):
         """
         Test that we can successfully schedule a job with newest
@@ -556,6 +581,13 @@ def test_newest_success_diff_branch_diff_repo(
         ceph_hash will be updated to the working sha1,
         but the suite_hash will remain the original suite_sha1.
         """
+        # Create the expected suite directory structure
+        suite_path = tmp_path / 'suites' / 'suite'
+        suite_path.mkdir(parents=True, exist_ok=True)
+
+        # Update args to use the temporary directory
+        self.args.suite_dir = str(tmp_path)
+
         m_get_arch.return_value = 'x86_64'
         # Set different branches
         self.args.ceph_branch = 'ceph_different_branch'