Tiaf bucket top level fix (#3085)

* Revert to regular run when invalid commits used. * Cirrect s3 logging of last commit hash storage * Add s3 top level url and build number script params. * Use existing REPOSITORY_NAME env var. Signed-off-by: John <jonawals@amazon.com>
2021-08-12 20:48:23 +01:00
parent 8c21c85972
commit ff659fbbb6
7 changed files with 57 additions and 18 deletions
@@ -89,7 +89,7 @@
      "CONFIGURATION": "profile",
      "SCRIPT_PATH": "scripts/build/TestImpactAnalysis/tiaf_driver.py",
      "SCRIPT_PARAMETERS": 
-      "--config=\"!OUTPUT_DIRECTORY!/bin/TestImpactFramework/profile/Persistent/tiaf.json\" --src-branch=!BRANCH_NAME! --dst-branch=!CHANGE_TARGET! --commit=!CHANGE_ID! --s3-bucket=!TEST_IMPACT_S3_BUCKET! --mars-index-prefix=jonawals --suite=main --test-failure-policy=continue"
+      "--config=\"!OUTPUT_DIRECTORY!/bin/TestImpactFramework/profile/Persistent/tiaf.json\" --src-branch=!BRANCH_NAME! --dst-branch=!CHANGE_TARGET! --commit=!CHANGE_ID! --s3-bucket=!TEST_IMPACT_S3_BUCKET! --mars-index-prefix=jonawals --s3-top-level-dir=!REPOSITORY_NAME! --build-number=!BUILD_NUMBER! --suite=main --test-failure-policy=continue"
    }
  },
  "debug_vs2019": {
@@ -14,6 +14,7 @@ import pathlib
 class Repo:
    def __init__(self, repo_path: str):
        self._repo = git.Repo(repo_path)
+        self._remote_url = self._repo.remotes[0].config_reader.get("url")

    # Returns the current branch
    @property
@@ -21,6 +22,11 @@ class Repo:
        branch = self._repo.active_branch
        return branch.name

+    # Returns the remote URL
+    @property
+    def remote_url(self):
+        return self._remote_url
+
    def create_diff_file(self, src_commit_hash: str, dst_commit_hash: str, output_path: pathlib.Path, multi_branch: bool):
        """
        Attempts to create a diff from the src and dst commits and write to the specified output file.
@@ -14,6 +14,7 @@ from tiaf_logger import get_logger
 logger = get_logger(__file__)

 MARS_JOB_KEY = "job"
+BUILD_NUMBER_KEY = "build_number"
 SRC_COMMIT_KEY = "src_commit"
 DST_COMMIT_KEY = "dst_commit"
 COMMIT_DISTANCE_KEY = "commit_distance"
@@ -175,12 +176,14 @@ def get_duration_in_seconds(duration_in_milliseconds: int):

    return duration_in_milliseconds * 0.001

-def generate_mars_job(tiaf_result, driver_args):
+def generate_mars_job(tiaf_result, driver_args, build_number: int):
    """
    Generates a MARS job document using the job meta-data used to drive the TIAF sequence.

-    @param tiaf_result: The result object generated by the TIAF script.
-    @param driver_args: The arguments specified to the driver script.
+    @param tiaf_result:  The result object generated by the TIAF script.
+    @param driver_args:  The arguments specified to the driver script.
+    @param driver_args:  The arguments specified to the driver script.
+    @param build_number: The build number this job corresponds to.
    @return:            The MARS job document with the job meta-data.
    """

@@ -203,6 +206,7 @@ def generate_mars_job(tiaf_result, driver_args):
    ]}

    mars_job[DRIVER_ARGS_KEY] = driver_args
+    mars_job[BUILD_NUMBER_KEY] = build_number
    return mars_job

 def generate_test_run_list(test_runs):
@@ -418,7 +422,7 @@ def generate_mars_test_targets(sequence_report: dict, mars_job: dict, t0_timesta

    return mars_test_targets

-def transmit_report_to_mars(mars_index_prefix: str, tiaf_result: dict, driver_args: list):
+def transmit_report_to_mars(mars_index_prefix: str, tiaf_result: dict, driver_args: list, build_number: int):
    """
    Transforms the TIAF result into the appropriate MARS documents and transmits them to MARS.

@@ -434,7 +438,7 @@ def transmit_report_to_mars(mars_index_prefix: str, tiaf_result: dict, driver_ar
        t0_timestamp = datetime.datetime.now().timestamp()

        # Generate and transmit the MARS job document
-        mars_job = generate_mars_job(tiaf_result, driver_args)
+        mars_job = generate_mars_job(tiaf_result, driver_args, build_number)
        filebeat.send_event(mars_job, f"{mars_index_prefix}.tiaf.job")

        if tiaf_result[REPORT_KEY]:
@@ -161,7 +161,7 @@ class TestImpact:
        result["change_list"] = self._change_list
        return result

-    def run(self, commit: str, src_branch: str, dst_branch: str, s3_bucket: str, suite: str, test_failure_policy: str, safe_mode: bool, test_timeout: int, global_timeout: int):
+    def run(self, commit: str, src_branch: str, dst_branch: str, s3_bucket: str, s3_top_level_dir: str, suite: str, test_failure_policy: str, safe_mode: bool, test_timeout: int, global_timeout: int):
        """
        Determins the type of sequence to run based on the commit, source branch and test branch before running the
        sequence with the specified values.
@@ -170,6 +170,7 @@ class TestImpact:
        @param src_branch:          If not equal to dst_branch, the branch that is being built.
        @param dst_branch:          If not equal to src_branch, the destination branch for the PR being built.
        @param s3_bucket:           Location of S3 bucket to use for persistent storage, otherwise local disk storage will be used.
+        @param s3_top_level_dir:    Top level directory to use in the S3 bucket.
        @param suite:               Test suite to run.
        @param test_failure_policy: Test failure policy for regular and test impact sequences (ignored when seeding).
        @param safe_mode:           Flag to run impact analysis tests in safe mode (ignored when seeding).
@@ -218,7 +219,7 @@ class TestImpact:
            try:
                # Persistent storage location
                if s3_bucket:
-                    persistent_storage = PersistentStorageS3(self._config, suite, s3_bucket, self._source_of_truth_branch)
+                    persistent_storage = PersistentStorageS3(self._config, suite, s3_bucket, s3_top_level_dir, self._source_of_truth_branch)
                else:
                    persistent_storage = PersistentStorageLocal(self._config, suite)
            except SystemError as e:
@@ -226,14 +227,20 @@ class TestImpact:
                persistent_storage = None

            if persistent_storage:
+                # Flag to signify whether or not this is a re-run (multiple runs of the same commit)
+                # Right now, we don't fully support re-runs but in the future we will have an extra subfolder for each commit hash with the
+                # last run hash that was used for the first run for the commit so we can retreive the same reference point for building the
+                # change list to ensure each subsequent run is using the same data but for the time being, just perform a regular run
+                is_rerun = False
                if persistent_storage.has_historic_data:
                    logger.info("Historic data found.")
                    self._src_commit = persistent_storage.last_commit_hash

-                    # Perform some basic sanity checks on the commit hashes to ensure confidence in the integrity of of the environment
+                    # Perform some basic sanity checks on the commit hashes to ensure confidence in the integrity of the environment
                    if self._src_commit == self._dst_commit:
-                        logger.error(f"Source commit '{self._src_commit}' and destination commit '{self._dst_commit}', implying the integrity of the historic data is compromised.")
+                        logger.info(f"Source commit '{self._src_commit}' and destination commit '{self._dst_commit}', implying this is a re-run. A regular sequence will instead be performed.")
                        persistent_storage = None
+                        is_rerun = True
                    else:
                        self._attempt_to_generate_change_list()
                else:
@@ -261,7 +268,7 @@ class TestImpact:
                    args.append(f"--changelist={self._change_list_path}")
                    logger.info(f"Change list is set to '{self._change_list_path}'.")
                else:
-                    if self._is_source_of_truth_branch:
+                    if self._is_source_of_truth_branch and not is_rerun:
                        # Use seed sequence (instrumented all tests) for coverage updating branches so we can generate the coverage bed for future sequences
                        sequence_type = "seed"
                        # We always continue after test failures when seeding to ensure we capture the coverage for all test targets
@@ -11,6 +11,7 @@ import mars_utils
 import sys
 import pathlib
 import traceback
+import re
 from tiaf import TestImpact
 from tiaf_logger import get_logger

@@ -66,13 +67,20 @@ def parse_args():
        required=True
    )

-    # S3 bucket
+    # S3 bucket name
    parser.add_argument(
        '--s3-bucket', 
        help="Location of S3 bucket to use for persistent storage, otherwise local disk storage will be used", 
        required=False
    )

+    # S3 bucket top level directory
+    parser.add_argument(
+        '--s3-top-level-dir', 
+        help="The top level directory to use in the S3 bucket", 
+        required=False
+    )
+
    # MARS index prefix
    parser.add_argument(
        '--mars-index-prefix', 
@@ -80,6 +88,13 @@ def parse_args():
        required=False
    )

+    # Build number
+    parser.add_argument(
+        '--build-number', 
+        help="The build number this run of TIAF corresponds to", 
+        required=True
+    )
+
    # Test suite
    parser.add_argument(
        '--suite', 
@@ -127,12 +142,19 @@ if __name__ == "__main__":
    
    try:
        args = parse_args()
+
+        s3_top_level_dir = None
+        if args.s3_top_level_dir:
+            s3_top_level_dir = args.s3_top_level_dir
+        else:
+            s3_top_level_dir = "tiaf"
+
        tiaf = TestImpact(args.config)
-        tiaf_result = tiaf.run(args.commit, args.src_branch, args.dst_branch, args.s3_bucket, args.suite, args.test_failure_policy, args.safe_mode, args.test_timeout, args.global_timeout)
+        tiaf_result = tiaf.run(args.commit, args.src_branch, args.dst_branch, args.s3_bucket, s3_top_level_dir, args.suite, args.test_failure_policy, args.safe_mode, args.test_timeout, args.global_timeout)
        
        if args.mars_index_prefix:
            logger.info("Transmitting report to MARS...")
-            mars_utils.transmit_report_to_mars(args.mars_index_prefix, tiaf_result, sys.argv)
+            mars_utils.transmit_report_to_mars(args.mars_index_prefix, tiaf_result, sys.argv, args.build_number)

        logger.info("Complete!")
        # Non-gating will be removed from this script and handled at the job level in SPEC-7413
@@ -106,7 +106,7 @@ class PersistentStorage(ABC):

        historic_data_json = self._pack_historic_data(last_commit_hash)
        if historic_data_json:
-            logger.info(f"Attempting to store historic data with new last commit hash '{self._last_commit_hash}'...")
+            logger.info(f"Attempting to store historic data with new last commit hash '{last_commit_hash}'...")
            self._store_historic_data(historic_data_json)
            logger.info("The historic data was successfully stored.")

@@ -18,7 +18,7 @@ logger = get_logger(__file__)

 # Implementation of s3 bucket persistent storage
 class PersistentStorageS3(PersistentStorage):
-    def __init__(self, config: dict, suite: str, s3_bucket: str, branch: str):
+    def __init__(self, config: dict, suite: str, s3_bucket: str, root_dir: str, branch: str):
        """
        Initializes the persistent storage with the specified s3 bucket.

@@ -36,8 +36,8 @@ class PersistentStorageS3(PersistentStorage):
            # historic_data.json.zip is the file containing the coverage and meta-data of the last TIAF sequence run
            historic_data_file = f"historic_data.{object_extension}"

-            # The location of the data is in the form <branch>/<config> so the build config of each branch gets its own historic data
-            self._dir = f'{branch}/{config["meta"]["build_config"]}'
+            # The location of the data is in the form <root_dir>/<branch>/<config> so the build config of each branch gets its own historic data
+            self._dir = f'{root_dir}/{branch}/{config["meta"]["build_config"]}'
            self._historic_data_key = f'{self._dir}/{historic_data_file}'
            
            logger.info(f"Attempting to retrieve historic data for branch '{branch}' at location '{self._historic_data_key}' on bucket '{s3_bucket}'...")