Skip to content

Commit 4a3345c

Browse files
geetikakaygeetikakay
authored
fix: decouple datasources arch suffix from preference arch suffix (#5105)
##### What this PR does / why we need it: Split the single add_arch_suffix flag into two independent controls: - add_preference_arch_suffix: appends arch to clusterprefernces name - add_data_source_arch_suffix: appends arch to datasources name only when cluster_type is multiarch these should be set independently because clusterpreferences (deployed by virt-operator) and datasources(deployed by ssp) have no direct dependency ##### Which issue(s) this PR fixes: The previous logic tied the DataSource suffix to the preference suffix which is False for amd64, causing multiarch+amd64 to resolve to "rhel10" instead of "rhel10-amd64" ##### Special notes for reviewer: I missed checking with standalone arm cluster and this PR should take care of it. ##### jira-ticket: https://redhat.atlassian.net/browse/CNV-89104 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Architecture suffixing behavior split: naming for preferences and data-source identifiers are now controlled independently to avoid unexpected per-architecture name changes. * **Tests** * Expanded and updated test coverage to validate the new suffixing rules across operating systems, CPU architectures, and cluster types. * **Chores** * ARM64 test configuration now includes a CentOS Stream 10 instance-type preference. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Geetika Kapoor <gkapoor@redhat.com>
1 parent 6cdf39b commit 4a3345c

5 files changed

Lines changed: 124 additions & 28 deletions

File tree

‎tests/global_config_arm64.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from utilities.constants import (
66
ARM_64,
7+
CENTOS_STREAM10_PREFERENCE,
78
EXPECTED_CLUSTER_INSTANCE_TYPE_LABELS,
89
HPP_CAPABILITIES,
910
OS_FLAVOR_FEDORA,
@@ -51,6 +52,7 @@
5152
centos_os_list = ["centos-stream-9"]
5253

5354
instance_type_rhel_os_list = [RHEL10_PREFERENCE]
55+
instance_type_centos_os_list = [CENTOS_STREAM10_PREFERENCE]
5456
instance_type_fedora_os_list = [OS_FLAVOR_FEDORA]
5557

5658
for _dir in dir():

‎utilities/os_utils.py‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,8 @@ def generate_linux_instance_type_os_matrix(
258258
os_name: str,
259259
preferences: list[str],
260260
arch_suffix: str | None = None,
261-
add_arch_suffix: bool = True,
261+
add_preference_arch_suffix: bool = True,
262+
add_data_source_arch_suffix: bool = False,
262263
) -> list[dict[str, dict[str, Any]]]:
263264
"""
264265
Generate a list of dictionaries representing the instance type matrix for a Linux OS type.
@@ -268,9 +269,12 @@ def generate_linux_instance_type_os_matrix(
268269
os_name (str): The name of the OS.
269270
preferences (list[str]): A list of preferences for the instance types. Preference format is "<os>.<version>".
270271
arch_suffix: Optional architecture suffix. Example: "s390x", "arm64" . Omit to keep original preference.
271-
add_arch_suffix: When True, append arch_suffix to the preference name. Set to False for OSes whose
272+
add_preference_arch_suffix: When True, append arch_suffix to the preference name. Set to False for OSes whose
272273
ClusterPreferences have no arch suffix (e.g. centos — "centos.stream10" exists,
273274
"centos.stream10.arm64" does not).
275+
add_data_source_arch_suffix: When True, append arch_suffix to the DataSource name. Only True on
276+
multiarch clusters where SSP creates per-arch DataSources (e.g. "rhel10-arm64").
277+
On homogeneous clusters DataSources are bare (e.g. "rhel10").
274278
275279
Returns:
276280
list[dict[str, dict[str, Any]]]: A list of dictionaries representing the instance type matrix.
@@ -291,11 +295,13 @@ def _format_data_source_name(preference_name: str) -> str:
291295
instance_types: list[dict[str, dict[str, Any]]] = []
292296

293297
for preference in preferences:
294-
arch_preference = f"{preference}.{arch_suffix}" if arch_suffix and add_arch_suffix else preference
298+
arch_preference = f"{preference}.{arch_suffix}" if arch_suffix and add_preference_arch_suffix else preference
295299
data_source_name = _format_data_source_name(preference_name=preference)
296300
preference_config: dict[str, Any] = {
297301
PREFERENCE_STR: arch_preference,
298-
DATA_SOURCE_NAME: f"{data_source_name}-{arch_suffix}" if arch_suffix else data_source_name,
302+
DATA_SOURCE_NAME: f"{data_source_name}-{arch_suffix}"
303+
if add_data_source_arch_suffix and arch_suffix
304+
else data_source_name,
299305
}
300306

301307
if preference == latest_os:

‎utilities/pytest_utils.py‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -508,14 +508,16 @@ def generate_instance_type_matrix_dicts(os_dict: dict[str, Any], cpu_arch: str |
508508
"instance_type_rhel_os_list").
509509
cpu_arch: Optional architecture suffix.
510510
"""
511-
add_arch_suffix = cpu_arch != AMD_64
511+
add_preference_arch_suffix = cpu_arch != AMD_64
512+
add_data_source_arch_suffix = py_config["cluster_type"] == MULTIARCH
512513

513514
if instance_type_rhel_os_list := os_dict.get("instance_type_rhel_os_list"):
514515
py_config["instance_type_rhel_os_matrix"] = generate_linux_instance_type_os_matrix(
515516
os_name="rhel",
516517
preferences=instance_type_rhel_os_list,
517518
arch_suffix=cpu_arch,
518-
add_arch_suffix=add_arch_suffix,
519+
add_preference_arch_suffix=add_preference_arch_suffix,
520+
add_data_source_arch_suffix=add_data_source_arch_suffix,
519521
)
520522
py_config["latest_instance_type_rhel_os_dict"] = generate_latest_os_dict(
521523
os_matrix=py_config["instance_type_rhel_os_matrix"]
@@ -525,14 +527,16 @@ def generate_instance_type_matrix_dicts(os_dict: dict[str, Any], cpu_arch: str |
525527
os_name="fedora",
526528
preferences=instance_type_fedora_os_list,
527529
arch_suffix=cpu_arch,
528-
add_arch_suffix=add_arch_suffix,
530+
add_preference_arch_suffix=add_preference_arch_suffix,
531+
add_data_source_arch_suffix=add_data_source_arch_suffix,
529532
)
530533
if instance_type_centos_os_list := os_dict.get("instance_type_centos_os_list"):
531534
py_config["instance_type_centos_os_matrix"] = generate_linux_instance_type_os_matrix(
532535
os_name="centos.stream",
533536
preferences=instance_type_centos_os_list,
534537
arch_suffix=cpu_arch,
535-
add_arch_suffix=False,
538+
add_preference_arch_suffix=False,
539+
add_data_source_arch_suffix=add_data_source_arch_suffix,
536540
)
537541

538542

‎utilities/unittests/test_os_utils.py‎

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import pytest
88

9-
from utilities.constants import ARM_64, CENTOS_STREAM10_PREFERENCE, RHEL10_PREFERENCE
9+
from utilities.constants import AMD_64, ARM_64, CENTOS_STREAM10_PREFERENCE, RHEL10_PREFERENCE
1010
from utilities.exceptions import OsDictNotFoundError
1111
from utilities.os_utils import (
1212
CENTOS_OS_MAPPING,
@@ -333,38 +333,67 @@ def test_generate_instance_type_single_digit_versions(self):
333333
rhel9_item = next(item for item in result if "rhel-9" in item)
334334
assert rhel9_item["rhel-9"]["latest_released"] is True
335335

336-
def test_arch_suffix_applied_to_preference_and_data_source(self):
337-
"""arch_suffix is appended to both the preference name and the DataSource."""
336+
def test_arch_suffix_applied_to_preference_not_data_source(self):
337+
"""On homogeneous clusters arch_suffix is appended to preference but not DataSource."""
338338
result = generate_linux_instance_type_os_matrix(
339339
os_name="rhel",
340340
preferences=[RHEL10_PREFERENCE],
341341
arch_suffix=ARM_64,
342342
)
343343

344+
config = result[0][f"{RHEL10_PREFERENCE}.{ARM_64}"]
345+
assert config["preference"] == f"{RHEL10_PREFERENCE}.{ARM_64}"
346+
assert config["DATA_SOURCE_NAME"] == "rhel10"
347+
348+
def test_multiarch_data_source_gets_arch_suffix(self):
349+
"""On multiarch clusters add_data_source_arch_suffix appends arch to DataSource."""
350+
result = generate_linux_instance_type_os_matrix(
351+
os_name="rhel",
352+
preferences=[RHEL10_PREFERENCE],
353+
arch_suffix=ARM_64,
354+
add_data_source_arch_suffix=True,
355+
)
356+
344357
config = result[0][f"{RHEL10_PREFERENCE}.{ARM_64}"]
345358
assert config["preference"] == f"{RHEL10_PREFERENCE}.{ARM_64}"
346359
assert config["DATA_SOURCE_NAME"] == f"rhel10-{ARM_64}"
347360

348-
def test_arch_suffix_omitted_from_preference_when_add_arch_suffix_false(self):
349-
"""add_arch_suffix=False keeps the preference plain while the DataSource still gets the arch suffix."""
361+
def test_arch_suffix_omitted_from_preference_when_add_preference_arch_suffix_false(self):
362+
"""add_preference_arch_suffix=False keeps the preference plain. DataSource stays bare without add_data_source_arch_suffix."""
350363
result = generate_linux_instance_type_os_matrix(
351364
os_name="centos.stream",
352365
preferences=[CENTOS_STREAM10_PREFERENCE],
353366
arch_suffix=ARM_64,
354-
add_arch_suffix=False,
367+
add_preference_arch_suffix=False,
355368
)
356369

357370
config = result[0][CENTOS_STREAM10_PREFERENCE]
358371
assert config["preference"] == CENTOS_STREAM10_PREFERENCE, "preference must not include arch suffix"
359-
assert config["DATA_SOURCE_NAME"] == f"centos-stream10-{ARM_64}", "DataSource must include arch suffix"
372+
assert config["DATA_SOURCE_NAME"] == "centos-stream10", (
373+
"DataSource must be bare without add_data_source_arch_suffix"
374+
)
375+
376+
def test_multiarch_amd64_bare_preference_suffixed_data_source(self):
377+
"""Multiarch + amd64: preference stays bare, DataSource gets the arch suffix (the fixed regression)."""
378+
result = generate_linux_instance_type_os_matrix(
379+
os_name="rhel",
380+
preferences=[RHEL10_PREFERENCE],
381+
arch_suffix=AMD_64,
382+
add_preference_arch_suffix=False,
383+
add_data_source_arch_suffix=True,
384+
)
385+
386+
config = result[0][RHEL10_PREFERENCE]
387+
assert config["preference"] == RHEL10_PREFERENCE, "preference must stay bare on amd64"
388+
assert config["DATA_SOURCE_NAME"] == f"rhel10-{AMD_64}", "DataSource must carry arch suffix on multiarch"
360389

361-
def test_add_arch_suffix_false_no_arch_suffix(self):
362-
"""add_arch_suffix=False with no arch_suffix is a no-op."""
390+
def test_add_preference_arch_suffix_false_no_arch_suffix(self):
391+
"""add_preference_arch_suffix=False with no arch_suffix is a no-op."""
363392
result_default = generate_linux_instance_type_os_matrix(
364393
os_name="centos.stream", preferences=[CENTOS_STREAM10_PREFERENCE]
365394
)
366395
result_no_arch = generate_linux_instance_type_os_matrix(
367-
os_name="centos.stream", preferences=[CENTOS_STREAM10_PREFERENCE], add_arch_suffix=False
396+
os_name="centos.stream", preferences=[CENTOS_STREAM10_PREFERENCE], add_preference_arch_suffix=False
368397
)
369398

370399
assert result_default == result_no_arch

‎utilities/unittests/test_pytest_utils.py‎

Lines changed: 65 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,15 @@
77
import pytest
88

99
import utilities.constants
10-
from utilities.constants import AMD_64, ARM_64, CENTOS_STREAM9_PREFERENCE, OS_FLAVOR_FEDORA, RHEL9_PREFERENCE, S390X
10+
from utilities.constants import (
11+
AMD_64,
12+
ARM_64,
13+
CENTOS_STREAM9_PREFERENCE,
14+
MULTIARCH,
15+
OS_FLAVOR_FEDORA,
16+
RHEL9_PREFERENCE,
17+
S390X,
18+
)
1119
from utilities.exceptions import MissingEnvironmentVariableError, UnsupportedCPUArchitectureError
1220

1321
# Circular dependencies are already mocked in conftest.py
@@ -1558,7 +1566,7 @@ def sample_instance_type_matrix(self):
15581566
]
15591567

15601568
@pytest.mark.parametrize(
1561-
("cpu_arch", "expected_add_arch_suffix"),
1569+
("cpu_arch", "expected_add_preference_arch_suffix"),
15621570
[
15631571
(None, True),
15641572
(ARM_64, True),
@@ -1577,9 +1585,10 @@ def test_generate_instance_type_rhel_matrix(
15771585
mock_generate_instance_type,
15781586
sample_instance_type_matrix,
15791587
cpu_arch,
1580-
expected_add_arch_suffix,
1588+
expected_add_preference_arch_suffix,
15811589
):
15821590
"""Test RHEL matrix generation across architecture variants."""
1591+
mock_py_config["cluster_type"] = AMD_64
15831592
mock_generate_instance_type.return_value = sample_instance_type_matrix
15841593
mock_generate_latest.return_value = sample_instance_type_matrix[0][RHEL9_PREFERENCE]
15851594

@@ -1590,7 +1599,8 @@ def test_generate_instance_type_rhel_matrix(
15901599
os_name="rhel",
15911600
preferences=[RHEL9_PREFERENCE],
15921601
arch_suffix=cpu_arch,
1593-
add_arch_suffix=expected_add_arch_suffix,
1602+
add_preference_arch_suffix=expected_add_preference_arch_suffix,
1603+
add_data_source_arch_suffix=False,
15941604
)
15951605
assert mock_py_config["instance_type_rhel_os_matrix"] == sample_instance_type_matrix
15961606
assert mock_py_config["latest_instance_type_rhel_os_dict"] == sample_instance_type_matrix[0][RHEL9_PREFERENCE]
@@ -1605,7 +1615,8 @@ def test_generate_instance_type_rhel_matrix(
16051615
"os_name": OS_FLAVOR_FEDORA,
16061616
"preferences": [OS_FLAVOR_FEDORA],
16071617
"arch_suffix": None,
1608-
"add_arch_suffix": True,
1618+
"add_preference_arch_suffix": True,
1619+
"add_data_source_arch_suffix": False,
16091620
},
16101621
"instance_type_fedora_os_matrix",
16111622
[{OS_FLAVOR_FEDORA: {"preference": OS_FLAVOR_FEDORA}}],
@@ -1617,7 +1628,8 @@ def test_generate_instance_type_rhel_matrix(
16171628
"os_name": OS_FLAVOR_FEDORA,
16181629
"preferences": [OS_FLAVOR_FEDORA],
16191630
"arch_suffix": AMD_64,
1620-
"add_arch_suffix": False,
1631+
"add_preference_arch_suffix": False,
1632+
"add_data_source_arch_suffix": False,
16211633
},
16221634
"instance_type_fedora_os_matrix",
16231635
[{OS_FLAVOR_FEDORA: {"preference": OS_FLAVOR_FEDORA}}],
@@ -1629,7 +1641,8 @@ def test_generate_instance_type_rhel_matrix(
16291641
"os_name": "centos.stream",
16301642
"preferences": [CENTOS_STREAM9_PREFERENCE],
16311643
"arch_suffix": None,
1632-
"add_arch_suffix": False,
1644+
"add_preference_arch_suffix": False,
1645+
"add_data_source_arch_suffix": False,
16331646
},
16341647
"instance_type_centos_os_matrix",
16351648
[{CENTOS_STREAM9_PREFERENCE: {"preference": CENTOS_STREAM9_PREFERENCE}}],
@@ -1641,7 +1654,8 @@ def test_generate_instance_type_rhel_matrix(
16411654
"os_name": "centos.stream",
16421655
"preferences": [CENTOS_STREAM9_PREFERENCE],
16431656
"arch_suffix": S390X,
1644-
"add_arch_suffix": False,
1657+
"add_preference_arch_suffix": False,
1658+
"add_data_source_arch_suffix": False,
16451659
},
16461660
"instance_type_centos_os_matrix",
16471661
[{CENTOS_STREAM9_PREFERENCE: {"preference": CENTOS_STREAM9_PREFERENCE}}],
@@ -1653,7 +1667,8 @@ def test_generate_instance_type_rhel_matrix(
16531667
"os_name": "centos.stream",
16541668
"preferences": [CENTOS_STREAM9_PREFERENCE],
16551669
"arch_suffix": ARM_64,
1656-
"add_arch_suffix": False,
1670+
"add_preference_arch_suffix": False,
1671+
"add_data_source_arch_suffix": False,
16571672
},
16581673
"instance_type_centos_os_matrix",
16591674
[{CENTOS_STREAM9_PREFERENCE: {"preference": CENTOS_STREAM9_PREFERENCE}}],
@@ -1674,6 +1689,7 @@ def test_generate_instance_type_non_rhel_matrix(
16741689
matrix_value,
16751690
):
16761691
"""Test Fedora and CentOS matrix generation call signatures."""
1692+
mock_py_config["cluster_type"] = AMD_64
16771693
mock_generate_instance_type.return_value = matrix_value
16781694

16791695
generate_instance_type_matrix_dicts(os_dict=os_dict, cpu_arch=cpu_arch)
@@ -1692,6 +1708,7 @@ def test_sets_latest_instance_type_rhel_os_dict(
16921708
sample_instance_type_matrix,
16931709
):
16941710
"""Test that latest_instance_type_rhel_os_dict is populated correctly"""
1711+
mock_py_config["cluster_type"] = AMD_64
16951712
mock_generate_instance_type.return_value = sample_instance_type_matrix
16961713
expected_latest = {"preference": RHEL9_PREFERENCE, "latest_released": True}
16971714
mock_generate_latest.return_value = expected_latest
@@ -1709,11 +1726,49 @@ def test_empty_os_dict_does_nothing(
17091726
mock_generate_instance_type,
17101727
):
17111728
"""Test that empty os_dict doesn't call any generation functions"""
1729+
mock_py_config["cluster_type"] = AMD_64
17121730
os_dict = {}
17131731
generate_instance_type_matrix_dicts(os_dict=os_dict)
17141732

17151733
mock_generate_instance_type.assert_not_called()
1716-
assert mock_py_config == {}
1734+
assert mock_py_config == {"cluster_type": AMD_64}
1735+
1736+
@pytest.mark.parametrize(
1737+
("cpu_arch", "expected_add_preference_arch_suffix"),
1738+
[
1739+
(AMD_64, False),
1740+
(ARM_64, True),
1741+
(S390X, True),
1742+
],
1743+
ids=["multiarch_amd64", "multiarch_arm64", "multiarch_s390x"],
1744+
)
1745+
@patch("utilities.pytest_utils.generate_linux_instance_type_os_matrix")
1746+
@patch("utilities.pytest_utils.generate_latest_os_dict")
1747+
@patch("utilities.pytest_utils.py_config", new_callable=dict)
1748+
def test_multiarch_sets_data_source_arch_suffix_for_all_arches(
1749+
self,
1750+
mock_py_config,
1751+
mock_generate_latest,
1752+
mock_generate_instance_type,
1753+
sample_instance_type_matrix,
1754+
cpu_arch,
1755+
expected_add_preference_arch_suffix,
1756+
):
1757+
"""On multiarch clusters add_data_source_arch_suffix is True for every architecture."""
1758+
mock_py_config["cluster_type"] = MULTIARCH
1759+
mock_generate_instance_type.return_value = sample_instance_type_matrix
1760+
mock_generate_latest.return_value = sample_instance_type_matrix[0][RHEL9_PREFERENCE]
1761+
1762+
os_dict = {"instance_type_rhel_os_list": [RHEL9_PREFERENCE]}
1763+
generate_instance_type_matrix_dicts(os_dict=os_dict, cpu_arch=cpu_arch)
1764+
1765+
mock_generate_instance_type.assert_called_once_with(
1766+
os_name="rhel",
1767+
preferences=[RHEL9_PREFERENCE],
1768+
arch_suffix=cpu_arch,
1769+
add_preference_arch_suffix=expected_add_preference_arch_suffix,
1770+
add_data_source_arch_suffix=True,
1771+
)
17171772

17181773

17191774
class TestUpdateLatestOsConfig:

0 commit comments

Comments
 (0)