json_generator: SLFO PR client-tools URLs; static Salt repos for 4.3/5.0; default 51-sles#1984
json_generator: SLFO PR client-tools URLs; static Salt repos for 4.3/5.0; default 51-sles#1984ktsamis wants to merge 1 commit intoSUSE:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the maintenance JSON generator to support SLFO PullRequest-driven client-tools repositories for SLES 16 / SL Micro 6.2, and ensures static Salt image repositories are always included for SUMA 4.3 / 5.0 SL Micro minions. It also changes the CLI default version to 51-sles and updates documentation/tests accordingly.
Changes:
- Add
--slfo-pull-requestto inject SLES 16 and SL Micro 6.2 client-tools repo URLs for 5.1/5.2 (including beta variants) and log the PR id. - Add static SL Micro 6.0/6.1 Salt/image repositories for versions
43,50-micro, and50-sles, and expand static-repo merging logic beyond 51/52. - Update CLI default version to
51-slesand adjust README/tests to reflect the new behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| jenkins_pipelines/scripts/json_generator/maintenance_json_generator.py | Adds --slfo-pull-request, expands static repo merging to 43/50/51/52, changes default version to 51-sles. |
| jenkins_pipelines/scripts/json_generator/repository_versions/init.py | Wires 4.3/5.0 static Salt repos into nodes_by_version. |
| jenkins_pipelines/scripts/json_generator/repository_versions/v43_nodes.py | Introduces v43_static_slmicro_salt_repositories with fixed Salt/image URLs for SL Micro 6.0/6.1. |
| jenkins_pipelines/scripts/json_generator/repository_versions/v51_nodes.py | Removes static definitions for sles160_minion / slmicro62_minion client-tools repos. |
| jenkins_pipelines/scripts/json_generator/repository_versions/v52_nodes.py | Removes static definitions for sles160_minion / slmicro62_minion client-tools repos (beta). |
| jenkins_pipelines/scripts/json_generator/maintenance_json_generator_README.md | Documents new default version, beta options, and --slfo-pull-request plus static Salt repos behavior for 43/50. |
| jenkins_pipelines/scripts/tests/test_maintenance_json_generator.py | Updates default-version expectation and adds assertions around static Salt repo merging for 43/50. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (2)
jenkins_pipelines/scripts/json_generator/maintenance_json_generator.py:52
read_mi_ids_from_fileis annotated to takefile_path: str, but callers now pass aPath(andopen()supports path-like objects). To keep typing accurate (and avoid type-checker noise), widen the type to acceptPath/os.PathLike[str](or convert tostrat the call site consistently).
def read_mi_ids_from_file(file_path: str) -> list[str]:
with open(file_path, 'r') as file:
return file.read().strip().split()
jenkins_pipelines/scripts/tests/test_maintenance_json_generator.py:71
- The assertions inside this
assertRaises(SystemExit)block are unreachable becauseparse_cli_args()raises and exits thewithbody immediately. Move theassertEqual/assertInchecks after thewithblock (using the captured exception incm).
sys.argv = ['maintenance_json_generator.py', '-x']
with self.assertRaises(SystemExit) as cm:
parse_cli_args()
self.assertEqual(cm.exception.code, 2)
self.assertIn("error: unrecognized arguments: -x", cm.msg)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
jenkins_pipelines/scripts/json_generator/repository_versions/init.py:42
DynamicReposis defined asdict[str, list[str]], but thedynamic_*values coming fromget_v51_static_and_client_tools()/get_v52_static_and_client_tools()are currentlydict[str, set[str]]. This is both a typing mismatch and contributes to non-deterministic iteration order downstream. Consider converting those returned sets to sortedlist[str]here (e.g., when assigningdynamic_51_*/dynamic_52_*) or updating the type alias and downstream logic to consistently use ordered sequences.
StaticRepos: TypeAlias = dict[str, dict[str, str]]
DynamicRepos: TypeAlias = dict[str, list[str]]
class VersionNodes(TypedDict):
static: StaticRepos
dynamic: DynamicRepos
static_51_micro, dynamic_51_micro = get_v51_static_and_client_tools("micro")
static_51_sles, dynamic_51_sles = get_v51_static_and_client_tools("sles")
# 5.2 (Non-Beta clients - shared from 5.1)
static_52_micro, dynamic_52_micro = get_v52_static_and_client_tools("micro", beta=False)
static_52_sles, dynamic_52_sles = get_v52_static_and_client_tools("sles", beta=False)
# 5.2 (Beta clients - exclusive -Beta tools)
static_52_micro_beta, dynamic_52_micro_beta = get_v52_static_and_client_tools("micro", beta=True)
static_52_sles_beta, dynamic_52_sles_beta = get_v52_static_and_client_tools("sles", beta=True)
nodes_by_version: dict[str, VersionNodes] = {
"43": {"static": v43_static_slmicro_salt_repositories, "dynamic": get_v43_nodes_sorted()},
"50-micro": {
"static": v43_static_slmicro_salt_repositories,
"dynamic": get_v50_nodes_sorted(get_v43_nodes_sorted(), "micro"),
},
"50-sles": {
"static": v43_static_slmicro_salt_repositories,
"dynamic": get_v50_nodes_sorted(get_v43_nodes_sorted(), "sles"),
},
"51-sles": {"static": static_51_sles, "dynamic": dynamic_51_sles},
"51-micro": {"static": static_51_micro, "dynamic": dynamic_51_micro},
"52-sles": {"static": static_52_sles, "dynamic": dynamic_52_sles},
"52-micro": {"static": static_52_micro, "dynamic": dynamic_52_micro},
"52-sles-beta": {"static": static_52_sles_beta, "dynamic": dynamic_52_sles_beta},
"52-micro-beta": {"static": static_52_micro_beta, "dynamic": dynamic_52_micro_beta}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…s/tests Support SLFO PullRequest IDs for SLES 16 and SL Micro 6.2 client-tools repos on 5.1/5.2, add static SL Micro 6.0/6.1 salt repos for 4.3/5.0, and change the default target version to 51-sles. Also tighten CLI validation and typing, update the README, and refresh tests to cover the new version/PR behavior and stable test-path handling.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -18,12 +30,22 @@ def parse_cli_args() -> argparse.Namespace: | |||
| description="This script reads the open qam-manager requests and creates a json file that can be fed to the BV testsuite pipeline" | |||
| ) | |||
| parser.add_argument("-v", "--version", dest="version", | |||
| help="Version of SUMA/MLM you want to run this script for, options include 43, 50, 51, and 52 variations (including beta)", | |||
| choices=["43", "50-micro", "50-sles", "51-micro","51-sles", "52-micro", "52-sles", "52-micro-beta", "52-sles-beta"], default="43", action='store') | |||
| help="Version of SUMA/MLM to run this script for. Accepted values: 43, 50-micro, 50-sles, 51-micro, 51-sles, 52-micro, 52-sles, 52-micro-beta, 52-sles-beta. Default: 51-sles.", | |||
| choices=SUPPORTED_VERSIONS, default="51-sles", action='store') | |||
There was a problem hiding this comment.
SUPPORTED_VERSIONS duplicates the authoritative version list already present in repository_versions.nodes_by_version. This can drift (e.g., adding a new entry to nodes_by_version but forgetting to update SUPPORTED_VERSIONS, or vice‑versa) and will either reject a valid version or accept one with no mapping. Consider deriving choices (and optionally the help text) directly from nodes_by_version.keys() instead of maintaining a separate constant.
| help="SLFO PullRequest id for sles160_minion and slmicro62_minion (5.1 / 5.2 only; independent of MI ids)", | ||
| ) | ||
| args = parser.parse_args() | ||
| if args.slfo_pull_request and not supports_slfo_pull_request(args.version): |
There was a problem hiding this comment.
--slfo-pull-request is accepted as an arbitrary string and interpolated directly into the generated IBS URLs. This allows values containing /, :, whitespace, etc. to produce malformed URLs (and potentially confusing JSON keys). Consider validating that the PR id is numeric (e.g., type=int in argparse, or an explicit .isdigit() check) and emitting a clear argparse error when invalid.
| help="SLFO PullRequest id for sles160_minion and slmicro62_minion (5.1 / 5.2 only; independent of MI ids)", | |
| ) | |
| args = parser.parse_args() | |
| if args.slfo_pull_request and not supports_slfo_pull_request(args.version): | |
| type=int, | |
| help="SLFO PullRequest id for sles160_minion and slmicro62_minion (5.1 / 5.2 only; independent of MI ids)", | |
| ) | |
| args = parser.parse_args() | |
| if args.slfo_pull_request is not None and not supports_slfo_pull_request(args.version): |
Summary
--slfo-pull-requestsupport forsles160_minionandslmicro62_minionon 5.1/5.2, including beta URL handling--versionto51-slesand update CLI help/README accordingly--slfo-pull-requestis used with unsupported versionsnodes_by_versiontyping to reflect separatestaticanddynamicshapesTesting
PYTHONPATH="susemanager-ci/jenkins_pipelines/scripts/json_generator" python3 -m unittest tests.test_maintenance_json_generatorfixes https://github.com/SUSE/spacewalk/issues/29968