Skip to content

fix(orchestrator,discovery): enforce OS version suffix in PXE functional group names - #5484

Merged
abhishek-sa1 merged 2 commits into
dell:stagingfrom
SAYUK09:fix/enforce-os-version-in-pxe-functional-groups
Oct 9, 2026
Merged

abhishek-sa1 merged 2 commits into
dell:stagingfrom
SAYUK09:fix/enforce-os-version-in-pxe-functional-groups

Conversation

@SAYUK09

@SAYUK09 SAYUK09 commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Description of the Solution

The PXE mapping file (pxe_mapping_file.csv) used generic functional group names without OS version identifiers (e.g., slurm_control_node_x86_64 instead of slurm_control_node_rhel_10_0_x86_64), creating ambiguity about which OS version is intended for each node even though nodes boot correctly. This PR adds validation to reject functional group names missing an OS version suffix and updates the discovery module to auto-inject OS version from catalog metadata when generating the mapping file.

Related Issue

Changes

Orchestrator Validation (src/orchestrator/plugins/module_utils/orchestrator_validation/)

  • Add OS version suffix validation for catalog-managed functional group names (slurm_*, service_kube_*, login_node_*, os_*) in _validate_names()
  • Tighten _catalog_matches() to require exact OS version match instead of treating missing version as a wildcard
  • Add pxe_mapping_missing_os_version_msg() error message

Discovery Module (src/discovery/plugins/modules/)

  • Add os_type and os_version module parameters to generate_pxe_mapping.py
  • Add _inject_os_version() helper that inserts _{os_type}_{version} before the architecture suffix, with idempotency (skips names that already contain a version segment)
  • Replace hardcoded SUPPORTED_FUNCTIONAL_GROUPS exact-match set with _is_supported_functional_group() pattern-based prefix matching for forward compatibility with versioned names
  • Update parent-tag and child-role matching from exact strings to prefix-based checks

Discovery Role (src/discovery/roles/ome_discovery/)

  • Add discovery_os_type and discovery_os_version defaults sourced from cluster_os_type / cluster_os_version catalog metadata
  • Pass OS type/version parameters to generate_pxe_mapping module in playbook

Files Changed

File Change Type Description
src/orchestrator/plugins/module_utils/orchestrator_validation/validators/pxe_mapping_validator.py Modified Add OS version suffix validation in _validate_names(); tighten _catalog_matches() to exact OS version match
src/orchestrator/plugins/module_utils/orchestrator_validation/messages/orchestrator_messages.py Modified Add pxe_mapping_missing_os_version_msg() error message function
src/discovery/plugins/modules/generate_pxe_mapping.py Modified Add os_type/os_version params, _inject_os_version() helper, replace exact-match set with prefix-based _is_supported_functional_group()
src/discovery/roles/ome_discovery/defaults/main.yml Modified Add discovery_os_type and discovery_os_version defaults
src/discovery/roles/ome_discovery/tasks/generate_pxe_mapping.yml Modified Pass os_type/os_version to generate_pxe_mapping module

Testing

  • Validator logic verified: Tested that version-agnostic names (slurm_node_x86_64) are rejected and versioned names (slurm_node_rhel_10_0_x86_64) pass validation
  • Discovery helpers verified: Unit tested _is_supported_functional_group() with both versioned and unversioned names, custom groups, and unsupported architectures; unit tested _inject_os_version() for injection, idempotency (already-versioned names unchanged), and no-op when os_type/os_version are empty
  • End-to-end orchestrator validation: Ran orchestrator validation with version-agnostic PXE mapping file and confirmed all rows rejected with clear error messages; updated mapping file to versioned names and confirmed validation passes

Backward Compatibility

  • Breaking change for PXE mapping file: Existing mapping files with version-agnostic functional group names (e.g., slurm_node_x86_64) will fail orchestrator validation. Users must update names to include OS version suffix (e.g., slurm_node_rhel_10_0_x86_64) matching their active catalog's FunctionalLayer names.
  • Breaking change for additional_cloud_init.yml: Group keys in additional_cloud_init.yml must be updated to match the versioned functional group names in the mapping file.
  • Discovery module backward compatible: os_type and os_version parameters default to empty strings; when empty, no version injection occurs and behavior is unchanged from before.
  • Catalog matching tightened: _catalog_matches() no longer treats missing OS version as a wildcard match. This is intentional — the new validation ensures all catalog-managed names include a version, so the wildcard path is no longer reachable.

…nal 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 <sayuri.kamble@dell.com>
@SAYUK09
SAYUK09 force-pushed the fix/enforce-os-version-in-pxe-functional-groups branch from 45d268e to 1fbc088 Compare October 8, 2026 05:58
@SAYUK09
SAYUK09 marked this pull request as ready for review October 9, 2026 07:25
balajikumaran-c-s pushed a commit to SAYUK09/omnia that referenced this pull request Oct 9, 2026
…mapping

The PR dell#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 <sayuri.kamble@dell.com>
balajikumaran-c-s pushed a commit to SAYUK09/omnia that referenced this pull request Oct 9, 2026
…mapping

The PR dell#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 <sayuri.kamble@dell.com>
@balajikumaran-c-s
balajikumaran-c-s force-pushed the fix/enforce-os-version-in-pxe-functional-groups branch from 630f768 to c40c869 Compare October 9, 2026 09:32
balajikumaran-c-s pushed a commit to SAYUK09/omnia that referenced this pull request Oct 9, 2026
…mapping

The PR dell#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 <sayuri.kamble@dell.com>
@balajikumaran-c-s
balajikumaran-c-s force-pushed the fix/enforce-os-version-in-pxe-functional-groups branch from c40c869 to 658001f Compare October 9, 2026 09:43
…mapping

The PR dell#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 <sayuri.kamble@dell.com>
@SAYUK09
SAYUK09 force-pushed the fix/enforce-os-version-in-pxe-functional-groups branch from 658001f to a909a71 Compare October 9, 2026 09:50
@abhishek-sa1
abhishek-sa1 merged commit 01a9cd6 into dell:staging Oct 9, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants