From 1fbc0886ba1c7c9172492b92ed88ba802aa7b1be Mon Sep 17 00:00:00 2001 From: sayuri Date: Wed, 7 Oct 2026 18:30:45 +0530 Subject: [PATCH 1/2] fix(orchestrator,discovery): enforce OS version suffix in PXE functional group names The PXE mapping file used generic functional group names without OS version identifiers (e.g. slurm_control_node_x86_64), creating ambiguity about which OS version is intended for each node. Changes: - Add validation in pxe_mapping_validator.py requiring OS version suffix (e.g. _rhel_10_0) for catalog-managed functional group names - Tighten catalog matching to require exact OS version match instead of treating missing version as wildcard - Update discovery generate_pxe_mapping.py to accept os_type/os_version parameters and auto-inject version suffix into functional group names - Switch discovery from hardcoded SUPPORTED_FUNCTIONAL_GROUPS set to pattern-based prefix matching for forward compatibility - Add discovery_os_type/discovery_os_version defaults sourced from catalog metadata Signed-off-by: sayuri --- .../plugins/modules/generate_pxe_mapping.py | 121 +++++++++++++----- .../roles/ome_discovery/defaults/main.yml | 7 + .../tasks/generate_pxe_mapping.yml | 2 + .../messages/orchestrator_messages.py | 9 ++ .../validators/pxe_mapping_validator.py | 17 ++- 5 files changed, 122 insertions(+), 34 deletions(-) diff --git a/src/discovery/plugins/modules/generate_pxe_mapping.py b/src/discovery/plugins/modules/generate_pxe_mapping.py index 1e3de46a42..9dd9610fe2 100644 --- a/src/discovery/plugins/modules/generate_pxe_mapping.py +++ b/src/discovery/plugins/modules/generate_pxe_mapping.py @@ -18,6 +18,7 @@ import csv import os import re +from typing import Optional from ansible.module_utils.basic import AnsibleModule DOCUMENTATION = r''' @@ -71,6 +72,16 @@ required: false type: str default: "" + os_type: + description: OS type to embed in functional group names (e.g. rhel, rocky, ubuntu) + required: false + type: str + default: "" + os_version: + description: OS version to embed in functional group names (e.g. 10.0, 10.2). Dots are replaced with underscores. + required: false + type: str + default: "" author: - Dell Inc. ''' @@ -101,31 +112,71 @@ DEFAULT_FUNCTIONAL_GROUP = "slurm_node_aarch64" -PARENT_TAG_SOURCE_GROUP = "service_kube_node_x86_64" - -# Omnia-supported functional group names. -# Only servers whose OME static group matches one of these will be -# included in the PXE mapping file. -SUPPORTED_FUNCTIONAL_GROUPS = { - "service_kube_control_plane_x86_64", - "service_kube_node_x86_64", - "login_node_x86_64", - "login_node_aarch64", - "login_compiler_node_x86_64", - "login_compiler_node_aarch64", - "slurm_control_node_x86_64", - "slurm_node_x86_64", - "slurm_node_aarch64", - "os_x86_64", - "os_aarch64", -} - -# Roles that receive PARENT_SERVICE_TAG (set to a service_kube_node_x86_64 +PARENT_TAG_SOURCE_PREFIX = "service_kube_node_" + +# Supported functional group role prefixes. +# OME static group names must match one of these prefixes (followed by an +# optional OS version segment and an architecture suffix) to be included +# in the PXE mapping file. +SUPPORTED_ROLE_PREFIXES = ( + "service_kube_control_plane_", + "service_kube_node_", + "login_node_", + "login_compiler_node_", + "slurm_control_node_", + "slurm_node_", + "os_", +) + +SUPPORTED_ARCHITECTURES = ("x86_64", "aarch64") + +# Pattern to detect an optional OS-version segment before the architecture +# suffix (e.g. _rhel_10_0 in slurm_node_rhel_10_0_x86_64). +_ARCH_PATTERN = re.compile(r"_(?Px86_64|aarch64)$") + +# Roles that receive PARENT_SERVICE_TAG (set to a service_kube_node # service tag from the same Scalable Unit). -CHILD_ROLES_WITH_PARENT_TAG = { - "slurm_node_aarch64", - "slurm_node_x86_64", -} +CHILD_ROLE_PREFIXES = ("slurm_node_",) + + +def _is_supported_functional_group(group_name: str) -> bool: + """Check if an OME group name matches a supported Omnia role pattern.""" + if not group_name: + return False + lower = group_name.lower() + if not lower.startswith(SUPPORTED_ROLE_PREFIXES): + return False + return bool(_ARCH_PATTERN.search(group_name)) + + +def _inject_os_version( + fg_name: str, + os_type: Optional[str], + os_version: Optional[str], +) -> str: + """Insert _{os_type}_{version} before the architecture suffix. + + If the name already contains an OS version segment, leave it unchanged. + If os_type or os_version are empty, return the name unchanged. + Dots in os_version are replaced with underscores (10.0 -> 10_0). + """ + if not os_type or not os_version: + return fg_name + arch_match = _ARCH_PATTERN.search(fg_name) + if arch_match is None: + return fg_name + + # Check if an OS version segment is already present + prefix = fg_name[:arch_match.start()] + os_segment_pattern = re.compile( + r"_(?:rhel|rocky|ubuntu|sles)(?:_[0-9]+)+$" + ) + if os_segment_pattern.search(prefix): + return fg_name + + normalized_version = os_version.replace(".", "_") + arch = arch_match.group("arch") + return f"{prefix}_{os_type}_{normalized_version}_{arch}" def extract_su_from_hostname(bmc_hostname): @@ -196,7 +247,9 @@ def main(): "hostname_start": {"type": "int", "required": False, "default": 1}, "hostname_padding": {"type": "int", "required": False, "default": 3}, "ib_subnet": {"type": "str", "required": False, "default": ""}, - "admin_subnet": {"type": "str", "required": False, "default": ""} + "admin_subnet": {"type": "str", "required": False, "default": ""}, + "os_type": {"type": "str", "required": False, "default": ""}, + "os_version": {"type": "str", "required": False, "default": ""}, } module = AnsibleModule( @@ -213,6 +266,8 @@ def main(): hostname_padding = module.params['hostname_padding'] ib_subnet = module.params['ib_subnet'] admin_subnet = module.params['admin_subnet'] + os_type = module.params['os_type'].strip().lower() + os_version = module.params['os_version'].strip() # CSV headers as specified headers = [ @@ -253,16 +308,19 @@ def main(): server_group = server.get('group_name', '').strip() # Skip servers whose OME group is not a supported Omnia functional group - if server_group and server_group not in SUPPORTED_FUNCTIONAL_GROUPS: + if server_group and not _is_supported_functional_group(server_group): svc_tag = server.get('service_tag', 'unknown') module.warn( f"Skipping device {svc_tag}: OME static group '{server_group}' " f"is not a supported Omnia functional group. " - f"Supported groups: {', '.join(sorted(SUPPORTED_FUNCTIONAL_GROUPS))}" + f"Supported prefixes: {', '.join(SUPPORTED_ROLE_PREFIXES)}" ) continue resolved_functional_group = server_group if server_group else functional_group + resolved_functional_group = _inject_os_version( + resolved_functional_group, os_type, os_version + ) # Derive GROUP_NAME: try SU from BMC hostname first, # then from OME group name, then fall back to module default (grp0) @@ -289,15 +347,18 @@ def main(): # Build SU -> service_kube_node service tag map su_kube_node_map = {} for row in rows: - if row["FUNCTIONAL_GROUP_NAME"] == PARENT_TAG_SOURCE_GROUP: + if row["FUNCTIONAL_GROUP_NAME"].startswith(PARENT_TAG_SOURCE_PREFIX): su = row["GROUP_NAME"] if su and su not in su_kube_node_map: su_kube_node_map[su] = row["SERVICE_TAG"] # Assign PARENT_SERVICE_TAG only to slurm_node roles, - # using a service_kube_node_x86_64 service tag from the same GROUP_NAME + # using a service_kube_node service tag from the same GROUP_NAME for row in rows: - if row["FUNCTIONAL_GROUP_NAME"] not in CHILD_ROLES_WITH_PARENT_TAG: + if not any( + row["FUNCTIONAL_GROUP_NAME"].startswith(p) + for p in CHILD_ROLE_PREFIXES + ): continue su = row["GROUP_NAME"] if su in su_kube_node_map: diff --git a/src/discovery/roles/ome_discovery/defaults/main.yml b/src/discovery/roles/ome_discovery/defaults/main.yml index 2d30076c0f..f3070301d9 100644 --- a/src/discovery/roles/ome_discovery/defaults/main.yml +++ b/src/discovery/roles/ome_discovery/defaults/main.yml @@ -28,6 +28,13 @@ hostname_start_number: 1 # Hostname padding (e.g., 3 digits = nid001, must be 3 for orchestrator NID support: nid000-nid999) hostname_padding: 3 +# OS type and version to embed in functional group names. +# When set, discovery appends _{os_type}_{os_version} before the architecture +# suffix (e.g. slurm_node_x86_64 -> slurm_node_rhel_10_0_x86_64). +# Sourced from the active catalog or set explicitly. +discovery_os_type: "{{ cluster_os_type | default('') }}" +discovery_os_version: "{{ cluster_os_version | default('') }}" + # OME device type for servers ome_server_device_type: 1000 diff --git a/src/discovery/roles/ome_discovery/tasks/generate_pxe_mapping.yml b/src/discovery/roles/ome_discovery/tasks/generate_pxe_mapping.yml index 3b6463fd08..5d4376d5ce 100644 --- a/src/discovery/roles/ome_discovery/tasks/generate_pxe_mapping.yml +++ b/src/discovery/roles/ome_discovery/tasks/generate_pxe_mapping.yml @@ -55,6 +55,8 @@ hostname_padding: "{{ hostname_padding }}" ib_subnet: "{{ ib_subnet }}" admin_subnet: "{{ admin_subnet }}" + os_type: "{{ discovery_os_type | default('') }}" + os_version: "{{ discovery_os_version | default('') }}" register: pxe_mapping_result - name: Create symlink to latest PXE mapping file diff --git a/src/orchestrator/plugins/module_utils/orchestrator_validation/messages/orchestrator_messages.py b/src/orchestrator/plugins/module_utils/orchestrator_validation/messages/orchestrator_messages.py index 0f01d328d4..aa9022e438 100644 --- a/src/orchestrator/plugins/module_utils/orchestrator_validation/messages/orchestrator_messages.py +++ b/src/orchestrator/plugins/module_utils/orchestrator_validation/messages/orchestrator_messages.py @@ -378,6 +378,15 @@ def pxe_mapping_architecture_mismatch_msg( ) +def pxe_mapping_missing_os_version_msg(value: str, row: int) -> str: + """Return a missing OS version suffix error message.""" + return ( + f"orchestrator_config: FUNCTIONAL_GROUP_NAME '{value}' at mapping " + f"row {row} must include an OS version suffix " + "(e.g. _rhel_10_0) before the architecture suffix." + ) + + def pxe_mapping_unknown_catalog_group_msg( value: str, row: int, path: str ) -> str: diff --git a/src/orchestrator/plugins/module_utils/orchestrator_validation/validators/pxe_mapping_validator.py b/src/orchestrator/plugins/module_utils/orchestrator_validation/validators/pxe_mapping_validator.py index 0130574f8e..91938fc6fc 100644 --- a/src/orchestrator/plugins/module_utils/orchestrator_validation/validators/pxe_mapping_validator.py +++ b/src/orchestrator/plugins/module_utils/orchestrator_validation/validators/pxe_mapping_validator.py @@ -294,6 +294,18 @@ def _validate_names( functional_group, row_number ), ) + elif functional_group and functional_group.lower().startswith( + CATALOG_MANAGED_PREFIXES + ): + identity = _functional_group_identity(functional_group) + if identity is not None and identity[1] is None: + record_error( + errors, + logger, + msg.pxe_mapping_missing_os_version_msg( + functional_group, row_number + ), + ) def _validate_ib_pair( @@ -493,10 +505,7 @@ def _catalog_matches( or mapping_architecture != catalog_architecture ): continue - if ( - mapping_os_version is None - or mapping_os_version == catalog_os_version - ): + if mapping_os_version == catalog_os_version: return True return False From a909a71d3ac47f3d94cb6a302df0bea44e06b945 Mon Sep 17 00:00:00 2001 From: sayuri Date: Fri, 9 Oct 2026 09:22:01 +0000 Subject: [PATCH 2/2] fix(discovery): parse catalog to extract OS type and version for PXE mapping The PR #5484 added OS version injection into functional group names, but the implementation was incomplete. The discovery role referenced cluster_os_type and cluster_os_version variables that were only set by image_build_manager when parsing the catalog. Since discovery runs independently, these variables were always empty, causing the OS version to never be injected. This fix adds catalog parsing to the discovery workflow: - Check if catalog file exists (from CATALOG_FILE_PATH env var or default) - Parse catalog JSON to extract os and os_version from baseos_group - Set discovery_os_type and discovery_os_version facts - Pass these to generate_pxe_mapping module for OS version injection Result: Functional group names now include OS version (e.g., slurm_node_rhel_10_0_aarch64 instead of slurm_node_aarch64) Signed-off-by: sayuri --- .../tasks/generate_pxe_mapping.yml | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/discovery/roles/ome_discovery/tasks/generate_pxe_mapping.yml b/src/discovery/roles/ome_discovery/tasks/generate_pxe_mapping.yml index 5d4376d5ce..765bd9f0d0 100644 --- a/src/discovery/roles/ome_discovery/tasks/generate_pxe_mapping.yml +++ b/src/discovery/roles/ome_discovery/tasks/generate_pxe_mapping.yml @@ -18,6 +18,29 @@ file: "{{ discovery_input_dir }}/network_spec.yml" name: network_spec_data +- name: Parse catalog to extract OS type and version + when: catalog_file is defined + vars: + catalog_file: "{{ lookup('env', 'CATALOG_FILE_PATH') | default('/omnia/catalog/catalog_rhel.json') }}" + block: + - name: Check if catalog file exists + ansible.builtin.stat: + path: "{{ catalog_file }}" + register: catalog_stat + + - name: Parse catalog JSON to extract OS metadata + ansible.builtin.set_fact: + _catalog_data: "{{ lookup('file', catalog_file) | from_yaml }}" + when: catalog_stat.stat.exists + + - name: Extract OS type and version from catalog + ansible.builtin.set_fact: + discovery_os_type: "{{ _catalog_data.catalog.groups.baseos_group.os | default('') }}" + discovery_os_version: "{{ _catalog_data.catalog.groups.baseos_group.os_version | default('') }}" + when: + - catalog_stat.stat.exists + - _catalog_data.catalog.groups.baseos_group is defined + - name: Set admin_subnet from network spec ansible.builtin.set_fact: admin_subnet: "{{ (network_spec_data.Networks | selectattr('admin_network', 'defined') | first).admin_network.subnet | default('') }}"