ci/lava: Use python-fire in job submitter
authorGuilherme Gallo <guilherme.gallo@collabora.com>
Tue, 4 Apr 2023 10:48:44 +0000 (07:48 -0300)
committerMarge Bot <emma+marge@anholt.net>
Wed, 19 Apr 2023 14:36:37 +0000 (14:36 +0000)
Cleanup argparse to use dataclasses+python-fire to give easier
maintenance to job submitter.

Signed-off-by: Guilherme Gallo <guilherme.gallo@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/22500>

.gitlab-ci/image-tags.yml
.gitlab-ci/lava/lava-submit.sh
.gitlab-ci/lava/lava_job_submitter.py
.gitlab-ci/lava/requirements.txt
.gitlab-ci/lava/utils/lava_job_definition.py

index fb8d6bf..0403327 100644 (file)
@@ -3,7 +3,7 @@ variables:
    DEBIAN_BASE_TAG: "2023-04-16-kernel-6.3"
 
    DEBIAN_X86_BUILD_IMAGE_PATH: "debian/x86_build"
-   DEBIAN_BUILD_TAG: "2023-04-13-agility-610"
+   DEBIAN_BUILD_TAG: "2023-04-14-pyfire"
 
    DEBIAN_X86_BUILD_MINGW_IMAGE_PATH: "debian/x86_build-mingw"
    DEBIAN_BUILD_MINGW_TAG: "2023-01-03-ci-libva-2.17"
index b359ab5..5e1ec0c 100755 (executable)
@@ -45,22 +45,23 @@ fi
 touch results/lava.log
 tail -f results/lava.log &
 PYTHONPATH=artifacts/ artifacts/lava/lava_job_submitter.py \
+       submit \
        --dump-yaml \
        --pipeline-info "$CI_JOB_NAME: $CI_PIPELINE_URL on $CI_COMMIT_REF_NAME ${CI_NODE_INDEX}/${CI_NODE_TOTAL}" \
        --rootfs-url-prefix "https://${BASE_SYSTEM_HOST_PATH}" \
        --kernel-url-prefix "https://${BASE_SYSTEM_HOST_PATH}" \
        --build-url "${ARTIFACT_URL}" \
        --job-rootfs-overlay-url "${FDO_HTTP_CACHE_URI:-}https://${JOB_ROOTFS_OVERLAY_PATH}" \
-       --job-timeout ${JOB_TIMEOUT:-30} \
+       --job-timeout-min ${JOB_TIMEOUT:-30} \
        --first-stage-init artifacts/ci-common/init-stage1.sh \
-       --ci-project-dir ${CI_PROJECT_DIR} \
-       --device-type ${DEVICE_TYPE} \
-       --dtb ${DTB} \
+       --ci-project-dir "${CI_PROJECT_DIR}" \
+       --device-type "${DEVICE_TYPE}" \
+       --dtb-filename "${DTB}" \
        --jwt-file "${CI_JOB_JWT_FILE}" \
-       --kernel-image-name ${KERNEL_IMAGE_NAME} \
+       --kernel-image-name "${KERNEL_IMAGE_NAME}" \
        --kernel-image-type "${KERNEL_IMAGE_TYPE}" \
-       --boot-method ${BOOT_METHOD} \
-       --visibility-group ${VISIBILITY_GROUP} \
+       --boot-method "${BOOT_METHOD}" \
+       --visibility-group "${VISIBILITY_GROUP}" \
        --lava-tags "${LAVA_TAGS}" \
        --mesa-job-name "$CI_JOB_NAME" \
        >> results/lava.log
index 5aa9ec9..7ffdcd2 100755 (executable)
 """Send a job to LAVA, track it and collect log back"""
 
 
-import argparse
 import contextlib
 import pathlib
 import sys
 import time
+from dataclasses import dataclass, fields
 from datetime import datetime, timedelta
 from io import StringIO
 from os import getenv
 from typing import Any, Optional
 
+import fire
 from lava.exceptions import (
     MesaCIException,
     MesaCIParseException,
@@ -252,8 +253,6 @@ def print_job_final_status(job):
 
 def execute_job_with_retries(proxy, job_definition, retry_count) -> Optional[LAVAJob]:
     for attempt_no in range(1, retry_count + 2):
-        # Need to get the logger value from its object to enable autosave
-        # features, if AutoSaveDict is enabled from StructuredLogging module
         job = LAVAJob(proxy, job_definition)
 
         try:
@@ -294,71 +293,84 @@ def retriable_follow_job(proxy, job_definition) -> LAVAJob:
     )
 
 
-def treat_mesa_job_name(args):
-    # Remove mesa job names with spaces, which breaks the lava-test-case command
-    args.mesa_job_name = args.mesa_job_name.split(" ")[0]
-
-
-def main(args):
-    proxy = setup_lava_proxy()
-
-    # Overwrite the timeout for the testcases with the value offered by the
-    # user. The testcase running time should be at least 4 times greater than
-    # the other sections (boot and setup), so we can safely ignore them.
-    # If LAVA fails to stop the job at this stage, it will fall back to the
-    # script section timeout with a reasonable delay.
-    GL_SECTION_TIMEOUTS[LogSectionType.TEST_CASE] = timedelta(minutes=args.job_timeout)
+@dataclass
+class PathResolver:
+    def __post_init__(self):
+        for field in fields(self):
+            value = getattr(self, field.name)
+            if not value:
+                continue
+            if field.type == pathlib.Path:
+                value = pathlib.Path(value)
+                setattr(self, field.name, value.resolve())
+
+
+@dataclass
+class LAVAJobSubmitter(PathResolver):
+    boot_method: str
+    ci_project_dir: str
+    device_type: str
+    job_timeout_min: int  # The job timeout in minutes
+    build_url: str = None
+    dtb_filename: str = None
+    dump_yaml: bool = False  # Whether to dump the YAML payload to stdout
+    first_stage_init: str = None
+    jwt_file: pathlib.Path = None
+    kernel_image_name: str = None
+    kernel_image_type: str = ""
+    kernel_url_prefix: str = None
+    lava_tags: str = ""  # Comma-separated LAVA tags for the job
+    mesa_job_name: str = "mesa_ci_job"
+    pipeline_info: str = ""
+    rootfs_url_prefix: str = None
+    validate_only: bool = False  # Whether to only validate the job, not execute it
+    visibility_group: str = None  # Only affects LAVA farm maintainers
+    job_rootfs_overlay_url: str = None
+
+    def __post_init__(self) -> None:
+        super().__post_init__()
+        # Remove mesa job names with spaces, which breaks the lava-test-case command
+        self.mesa_job_name = self.mesa_job_name.split(" ")[0]
+
+    def dump(self, job_definition):
+        if self.dump_yaml:
+            with GitlabSection(
+                "yaml_dump",
+                "LAVA job definition (YAML)",
+                type=LogSectionType.LAVA_BOOT,
+                start_collapsed=True,
+            ):
+                print(hide_sensitive_data(job_definition))
+
+    def submit(self):
+        proxy = setup_lava_proxy()
+
+        # Overwrite the timeout for the testcases with the value offered by the
+        # user. The testcase running time should be at least 4 times greater than
+        # the other sections (boot and setup), so we can safely ignore them.
+        # If LAVA fails to stop the job at this stage, it will fall back to the
+        # script section timeout with a reasonable delay.
+        GL_SECTION_TIMEOUTS[LogSectionType.TEST_CASE] = timedelta(
+            minutes=self.job_timeout_min
+        )
 
-    job_definition_stream = StringIO()
-    lava_yaml.dump(generate_lava_yaml_payload(args), job_definition_stream)
-    job_definition = job_definition_stream.getvalue()
+        job_definition_stream = StringIO()
+        lava_yaml.dump(generate_lava_yaml_payload(self), job_definition_stream)
+        job_definition = job_definition_stream.getvalue()
 
-    if args.dump_yaml:
-        with GitlabSection(
-            "yaml_dump",
-            "LAVA job definition (YAML)",
-            type=LogSectionType.LAVA_BOOT,
-            start_collapsed=True,
-        ):
-            print(hide_sensitive_data(job_definition))
-    job = LAVAJob(proxy, job_definition)
+        self.dump(job_definition)
 
-    if errors := job.validate():
-        fatal_err(f"Error in LAVA job definition: {errors}")
-    print_log("LAVA job definition validated successfully")
+        job = LAVAJob(proxy, job_definition)
+        if errors := job.validate():
+            fatal_err(f"Error in LAVA job definition: {errors}")
+        print_log("LAVA job definition validated successfully")
 
-    if args.validate_only:
-        return
+        if self.validate_only:
+            return
 
-    finished_job = retriable_follow_job(proxy, job_definition)
-    exit_code = 0 if finished_job.status == "pass" else 1
-    sys.exit(exit_code)
-
-
-def create_parser():
-    parser = argparse.ArgumentParser("LAVA job submitter")
-
-    parser.add_argument("--pipeline-info")
-    parser.add_argument("--rootfs-url-prefix")
-    parser.add_argument("--kernel-url-prefix")
-    parser.add_argument("--build-url")
-    parser.add_argument("--job-rootfs-overlay-url")
-    parser.add_argument("--job-timeout", type=int)
-    parser.add_argument("--first-stage-init")
-    parser.add_argument("--ci-project-dir")
-    parser.add_argument("--device-type")
-    parser.add_argument("--dtb", nargs='?', default="")
-    parser.add_argument("--kernel-image-name")
-    parser.add_argument("--kernel-image-type", nargs='?', default="")
-    parser.add_argument("--boot-method")
-    parser.add_argument("--lava-tags", nargs='?', default="")
-    parser.add_argument("--jwt-file", type=pathlib.Path)
-    parser.add_argument("--validate-only", action='store_true')
-    parser.add_argument("--dump-yaml", action='store_true')
-    parser.add_argument("--visibility-group")
-    parser.add_argument("--mesa-job-name")
-
-    return parser
+        finished_job = retriable_follow_job(proxy, job_definition)
+        exit_code = 0 if finished_job.status == "pass" else 1
+        sys.exit(exit_code)
 
 
 if __name__ == "__main__":
@@ -368,9 +380,4 @@ if __name__ == "__main__":
     sys.stdout.reconfigure(line_buffering=True)
     sys.stderr.reconfigure(line_buffering=True)
 
-    parser = create_parser()
-
-    parser.set_defaults(func=main)
-    args = parser.parse_args()
-    treat_mesa_job_name(args)
-    args.func(args)
+    fire.Fire(LAVAJobSubmitter)
index 554cc68..df6fc99 100644 (file)
@@ -21,7 +21,7 @@ def generate_lava_yaml_payload(args) -> dict[str, Any]:
             "extra_nfsroot_args": " init=/init rootwait usbcore.quirks=0bda:8153:k"
         },
         "timeouts": {
-            "job": {"minutes": args.job_timeout},
+            "job": {"minutes": args.job_timeout_min},
             "actions": {
                 "depthcharge-retry": {
                     # Could take between 1 and 1.5 min in slower boots
@@ -60,8 +60,10 @@ def generate_lava_yaml_payload(args) -> dict[str, Any]:
     }
     if args.kernel_image_type:
         deploy["kernel"]["type"] = args.kernel_image_type
-    if args.dtb:
-        deploy["dtb"] = {"url": "{}/{}.dtb".format(args.kernel_url_prefix, args.dtb)}
+    if args.dtb_filename:
+        deploy["dtb"] = {
+            "url": "{}/{}.dtb".format(args.kernel_url_prefix, args.dtb_filename)
+        }
 
     # always boot over NFS
     boot = {
@@ -75,7 +77,7 @@ def generate_lava_yaml_payload(args) -> dict[str, Any]:
     # since LAVA's test parsing is not useful to us
     run_steps = []
     test = {
-        "timeout": {"minutes": args.job_timeout},
+        "timeout": {"minutes": args.job_timeout_min},
         "failure_retry": 1,
         "definitions": [
             {