jobs.py: read the config like pytest and tools.py

Make jobs.py use the same configuration as pytest and tools.py: -c/--config,
XCPNG_CONFIG/XCPNG_CONFIG_DIR and --config-value are now accepted, and
jobs.py run falls back to the hosts defined in the config's [hosts] section
when no hosts are given explicitly. Documented in the README.

Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>

Author: glehmann

README.md

@@ -418,15 +418,18 @@ pytest tests/uefi_sb -m "multi_vms and unix_vm" --hosts=ip_of_poolmaster --vm=ht
 
 #### Run a job
 ```
-usage: jobs.py run [-h] [--print-only] job hosts ...
+usage: jobs.py run [-h] [-c PATH] [--config-value KEY=VALUE] [--print-only] job [hosts] ...
 
 positional arguments:
   job               name of the job to run.
   hosts             master host(s) of pools to run the tests on, comma-separated.
+                    When omitted, the hosts from the config's [hosts] section are used.
   pytest_args       all additional arguments after the last positional argument will be passed to pytest and replace default job params if needed.
 
 optional arguments:
   -h, --help        show this help message and exit
+  -c PATH, --config PATH  config overlay: a .toml file path or profile name
+  --config-value KEY=VALUE  override a config value (repeatable; highest priority)
   --print-only, -p  print the command, but don't run it. Must be specified before positional arguments.
 ```
 
@@ -440,6 +443,13 @@ pytest tests/uefi_sb -m "multi_vms and unix_vm" --hosts=ip_of_poolmaster --vm=ht
 
 Any parameter added at the end of the command will be passed to `pytest`. Any parameter added that is already defined in the job's "params" (see output of `./jobs.py show`) will replace it, and `--vm` also replaces `--vm[]` in the case of jobs designed to run tests on multiple VMs.
 
+`jobs.py` reads the same configuration as `pytest` and `tools.py`:
+`-c`/`--config`, the `XCPNG_CONFIG`/`XCPNG_CONFIG_DIR` env vars and
+`--config-value` are supported, and when no hosts are given, `jobs.py run`
+falls back to the hosts defined in the config's `[hosts]` section. The
+config options must be specified before the positional `job`/`hosts`
+arguments (like `--print-only`), otherwise they are forwarded to pytest.
+
 ```
 # same, but we override the list of VMs
 $ ./jobs.py run --print-only sb-unix-multi ip_of_poolmaster --vm=http://path/to/vm4.xva

jobs.py

@@ -7,6 +7,7 @@
 import sys
 
 from lib.commands import ssh
+from lib.config_loader import add_config_options, load_config
 
 from typing import NotRequired, TypedDict, cast
 
@@ -815,14 +816,30 @@ def extract_tests(cmd: list[str]) -> set[str]:
     if error:
         sys.exit(1)
 
+def _config_hosts(args: argparse.Namespace) -> str | None:
+    """Return the pool masters listed in the config's [hosts], as a comma-separated string, or None."""
+    cfg = load_config(override=args.config, config_values=args.config_value)
+    hosts = list(cfg.hosts.keys())
+    return ",".join(hosts) if hosts else None
+
+
 def action_run(args: argparse.Namespace) -> None:
-    cmd = build_pytest_cmd(JOBS[args.job], args.hosts, None, args.pytest_args)
+    hosts = args.hosts or _config_hosts(args)
+    if hosts is None:
+        print("Error: no hosts provided. Pass a comma-separated list of pool masters as the positional "
+              "hosts argument, or define them in the [hosts] section of the config.", file=sys.stderr)
+        sys.exit(1)
+    cmd = build_pytest_cmd(JOBS[args.job], hosts, None, args.pytest_args)
+    if args.config is not None:
+        cmd += ["--config", str(args.config)]
+    for config_value in args.config_value:
+        cmd += ["--config-value", config_value]
     print(subprocess.list2cmdline(cmd))
     if args.print_only:
         return
 
     # check that enough pool masters have been provided
-    nb_pools = len(args.hosts.split(","))
+    nb_pools = len(hosts.split(","))
     job_nb_pools = JOBS[args.job]["nb_pools"]
     assert isinstance(job_nb_pools, int)
     if nb_pools < job_nb_pools:
@@ -841,35 +858,43 @@ def action_run(args: argparse.Namespace) -> None:
 
 def main() -> None:
     parser = argparse.ArgumentParser(description="Manage test jobs")
+    common_parser = add_config_options(parser)
+
     subparsers = parser.add_subparsers(dest="action", metavar="action")
     subparsers.required = True
 
-    list_parser = subparsers.add_parser("list", help="list available jobs.")
+    list_parser = subparsers.add_parser("list", help="list available jobs.", parents=[common_parser])
     list_parser.set_defaults(func=action_list)
 
-    run_parser = subparsers.add_parser("show", help="show details about a job definition.")
-    run_parser.add_argument("job", help="name of the job.", choices=JOBS.keys(), metavar="job")
-    run_parser.set_defaults(func=action_show)
+    show_parser = subparsers.add_parser("show", help="show details about a job definition.", parents=[common_parser])
+    show_parser.add_argument("job", help="name of the job.", choices=JOBS.keys(), metavar="job")
+    show_parser.set_defaults(func=action_show)
 
-    run_parser = subparsers.add_parser("collect", help="show test collection based on the job definition.")
-    run_parser.add_argument("job", help="name of the job.", choices=JOBS.keys(), metavar="job")
-    run_parser.add_argument("-v", "--host-version", help="host version to match VM filters.")
-    run_parser.add_argument("pytest_args", nargs=argparse.REMAINDER,
-                            help="all additional arguments after the last positional argument will "
-                                 "be passed to pytest and replace default job params if needed.")
-    run_parser.set_defaults(func=action_collect)
+    collect_parser = subparsers.add_parser("collect", help="show test collection based on the job definition.",
+                                           parents=[common_parser])
+    collect_parser.add_argument("job", help="name of the job.", choices=JOBS.keys(), metavar="job")
+    collect_parser.add_argument("-v", "--host-version", help="host version to match VM filters.")
+    collect_parser.add_argument("pytest_args", nargs=argparse.REMAINDER,
+                                help="all additional arguments after the last positional argument will "
+                                     "be passed to pytest and replace default job params if needed.")
+    collect_parser.set_defaults(func=action_collect)
 
-    run_parser = subparsers.add_parser("check", help="run sanity checks on the tests and jobs.")
-    run_parser.set_defaults(func=action_check)
+    check_parser = subparsers.add_parser(
+        "check", help="run sanity checks on the tests and jobs.", parents=[common_parser])
+    check_parser.set_defaults(func=action_check)
 
-    run_parser = subparsers.add_parser("run", help="run a job.")
+    run_parser = subparsers.add_parser("run", help="run a job.", parents=[common_parser])
     run_parser.add_argument("--print-only", "-p", action="store_true",
                             help="print the command, but don't run it. Must be specified before positional arguments.")
     run_parser.add_argument("job", help="name of the job to run.", choices=JOBS.keys(), metavar="job")
-    run_parser.add_argument("hosts", help="master host(s) of pools to run the tests on, comma-separated.")
+    run_parser.add_argument("hosts", nargs="?", default=None,
+                            help="master host(s) of pools to run the tests on, comma-separated. When omitted, the "
+                                 "hosts from the config's [hosts] section are used.")
     run_parser.add_argument("pytest_args", nargs=argparse.REMAINDER,
                             help="all additional arguments after the last positional argument will "
-                                 "be passed to pytest and replace default job params if needed.")
+                                 "be passed to pytest and replace default job params if needed. "
+                                 "Note: -c/--config and --config-value must be specified before the "
+                                 "positional arguments.")
     run_parser.set_defaults(func=action_run)
 
     args = parser.parse_args()

lib/config_loader.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import argparse
 import os
 import tomllib
 import warnings
@@ -540,6 +541,32 @@ def apply_override(config_name: str | None = None, config_values: list[str] | No
         setattr(config, field, getattr(new, field))
 
 
+def add_config_options(parser: argparse.ArgumentParser) -> argparse.ArgumentParser:
+    """Add -c/--config and --config-value to `parser`, and return a parent parser for subcommands.
+
+    Both options are attached to `parser` (so they can be given before the
+    subcommand) and to a shared ``add_help=False`` parser that subcommands use
+    as a parent.
+    """
+    common_parser = argparse.ArgumentParser(add_help=False)
+    for target in (parser, common_parser):
+        target.add_argument(
+            "-c", "--config",
+            type=Path,
+            default=None,
+            metavar="PATH",
+            help="Config overlay: a .toml file path or profile name (default: config.default.toml or XCPNG_CONFIG)",
+        )
+        target.add_argument(
+            "--config-value",
+            action="append",
+            default=[],
+            metavar="KEY=VALUE",
+            help="Override a config value, e.g. host.default_password=foo (repeatable; highest priority)",
+        )
+    return common_parser
+
+
 def sr_device_config(
     config_key: str, *, required: list[str] | None = None
 ) -> dict[str, str]:

lib/tools/cli.py

@@ -8,10 +8,9 @@
 import logging
 import os
 import sys
-from pathlib import Path
 
 from lib.common import HostAddress
-from lib.config_loader import base_config_dict, load_config
+from lib.config_loader import add_config_options, base_config_dict, load_config
 from lib.tools import logger
 from lib.tools.inventory import into_inventory, inventory_from_config
 from lib.tools.tasks.clean import clean_pools
@@ -101,33 +100,7 @@ def cli() -> None:
         description="Tools that help developers for running recurrent tasks on their XCP-ng sandbox."
     )
     parser.add_argument("-d", "--debug", action="store_true", default=False, help="Enable debug level")
-    parser.add_argument(
-        "-c", "--config",
-        type=Path,
-        default=None,
-        metavar="PATH",
-        help="Config overlay: a .toml file path or profile name (default: config.default.toml or XCPNG_CONFIG)",
-    )
-    common_parser = argparse.ArgumentParser(add_help=False)
-    common_parser.add_argument(
-        "-c", "--config",
-        type=Path,
-        default=None,
-        metavar="PATH",
-        help="Config overlay: a .toml file path or profile name (default: config.default.toml or XCPNG_CONFIG)",
-    )
-
-    def _add_config_value_option(target: argparse.ArgumentParser) -> None:
-        target.add_argument(
-            "--config-value",
-            action="append",
-            default=[],
-            metavar="KEY=VALUE",
-            help="Override a config value, e.g. host.default_password=foo (repeatable; highest priority)",
-        )
-
-    _add_config_value_option(parser)
-    _add_config_value_option(common_parser)
+    common_parser = add_config_options(parser)
 
     subparsers = parser.add_subparsers(required=True, metavar="COMMAND")