fix: decouple datasources arch suffix from preference arch suffix#5105
fix: decouple datasources arch suffix from preference arch suffix#5105geetikakay wants to merge 1 commit into
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
/wip |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughSplit the single arch-suffix flag into two booleans— ChangesArchitecture suffix handling refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
openshift-virtualization-qe-bot-3
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5105 +/- ##
==========================================
- Coverage 98.67% 98.65% -0.02%
==========================================
Files 25 25
Lines 2487 2462 -25
==========================================
- Hits 2454 2429 -25
Misses 33 33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
/build-and-push-container |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@utilities/pytest_utils.py`:
- Line 512: Replace the defensive dict access using
py_config.get("cluster_type") with direct key access py_config["cluster_type"]
so the code for add_data_source_arch_suffix fails fast on a missing
configuration; update the expression that sets add_data_source_arch_suffix
(which compares to MULTIARCH) to use py_config["cluster_type"] and ensure any
related logic in this function continues to rely on the guaranteed presence of
the key.
In `@utilities/unittests/test_os_utils.py`:
- Around line 336-385: Add a unit test that covers the regression where
preference must remain bare while DataSource is suffixed for amd64: create a
test (e.g., test_multiarch_amd64_bare_preference_suffixed_data_source) that
calls generate_linux_instance_type_os_matrix with os_name="rhel",
preferences=[RHEL10_PREFERENCE], arch_suffix=AMD_64,
add_preference_arch_suffix=False, add_data_source_arch_suffix=True (import
AMD_64 from utilities.constants), then assert the result uses the bare
preference key RHEL10_PREFERENCE and that config["preference"] ==
RHEL10_PREFERENCE while config["DATA_SOURCE_NAME"] == f"rhel10-{AMD_64}" to pin
the bug fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7c719c57-ceea-42ae-94ff-99b8bb42fa99
📒 Files selected for processing (5)
tests/global_config_arm64.pyutilities/os_utils.pyutilities/pytest_utils.pyutilities/unittests/test_os_utils.pyutilities/unittests/test_pytest_utils.py
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-5105 published |
|
/wip cancel |
openshift-virtualization-qe-bot-5
changed the title
|
/verified |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== Direct callers of generate_linux_instance_type_os_matrix ==="
rg -n 'generate_linux_instance_type_os_matrix' --type py -C2
echo ""
echo "=== Direct callers of generate_instance_type_matrix_dicts ==="
rg -n 'generate_instance_type_matrix_dicts' --type py -C2
echo ""
echo "=== Smoke test files ==="
rg -l '`@pytest.mark.smoke`' tests/
echo ""
echo "=== Gating test files ==="
rg -l '`@pytest.mark.gating`' tests/
echo ""
echo "=== Check if smoke files import generate_linux_instance_type_os_matrix or generate_instance_type_matrix_dicts ==="
SMOKE_FILES=$(rg -l '`@pytest.mark.smoke`' tests/)
for f in $SMOKE_FILES; do
if grep -q 'generate_linux_instance_type_os_matrix\|generate_instance_type_matrix_dicts' "$f" 2>/dev/null; then
echo "SMOKE MATCH: $f"
fi
done
echo ""
echo "=== Check conftests in smoke file directories for these symbols ==="
SMOKE_FILES=$(rg -l '`@pytest.mark.smoke`' tests/)
for f in $SMOKE_FILES; do
dir=$(dirname "$f")
while [ "$dir" != "." ] && [ "$dir" != "/" ]; do
conf="$dir/conftest.py"
if [ -f "$conf" ]; then
if grep -q 'generate_linux_instance_type_os_matrix\|generate_instance_type_matrix_dicts' "$conf" 2>/dev/null; then
echo "CONFTEST MATCH: $conf for smoke test $f"
fi
fi
dir=$(dirname "$dir")
done
done
echo ""
echo "=== Tests that import or call generate_instance_type_matrix_dicts ==="
rg -n 'generate_instance_type_matrix_dicts\|generate_linux_instance_type_os_matrix' tests/ --type py -l
echo ""
echo "=== Check gating files for these symbols ==="
GATING_FILES=$(rg -l '`@pytest.mark.gating`' tests/)
for f in $GATING_FILES; do
if grep -q 'generate_linux_instance_type_os_matrix\|generate_instance_type_matrix_dicts' "$f" 2>/dev/null; then
echo "GATING MATCH: $f"
fi
done
echo ""
echo "=== global_config_arm64.py content ==="
cat tests/global_config_arm64.py
echo ""
echo "=== Check callers of generate_instance_type_matrix_dicts in conftest files ==="
rg -n 'generate_instance_type_matrix_dicts' --type pyLength of output: 30221 🏁 Script executed: #!/bin/bash
echo "=== Verify test_rhel_os.py uses instance_type_rhel_os_matrix ==="
rg -n 'instance_type_rhel_os_matrix\|instance_type_centos_os_matrix\|instance_type_fedora_os_matrix' tests/infrastructure/instance_types/ --type py -C2
echo ""
echo "=== Check update_cpu_arch_related_config callers ==="
rg -n 'update_cpu_arch_related_config' --type py -l
echo ""
echo "=== Get PR diff to find first changed line position ==="
gh pr diff 5105 --patch | head -80
echo ""
echo "=== Get PR commit SHA ==="
gh pr view 5105 --json commits --jq '.commits[-1].oid'Length of output: 4947 🏁 Script executed: #!/bin/bash
echo "=== Tests that use instance_type_rhel_os_matrix / centos / fedora ==="
rg -rn 'instance_type_rhel_os_matrix\|instance_type_centos_os_matrix\|instance_type_fedora_os_matrix' tests/ --type py -l
echo ""
echo "=== Check conftest.py for instance_type matrices and update_cpu_arch_related_config ==="
grep -n 'instance_type_rhel_os_matrix\|instance_type_centos_os_matrix\|update_cpu_arch_related_config' conftest.py | head -30
echo ""
echo "=== Find all files using these matrices ==="
rg -rn 'instance_type_rhel_os_matrix\|instance_type_centos_os_matrix\|instance_type_fedora_os_matrix' --type py -l
echo ""
echo "=== Post inline review comment via GitHub API ==="
# Get the diff to find position
DIFF_POSITION=$(gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5105/files --jq '
.[] | select(.filename == "tests/global_config_arm64.py") | .patch' | head -5)
echo "Patch snippet: $DIFF_POSITION"
# Get all PR files to confirm positions
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5105/files --jq '.[] | {filename: .filename, sha: .sha}'Length of output: 2162 🏁 Script executed: #!/bin/bash
BODY='**Test Execution Plan**
- **Run smoke tests: False** — No verified dependency path. Smoke-marked files (`tests/virt/node/general/test_container_disk_vm.py`, `tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py`, `tests/storage/cdi_clone/test_clone.py`, `tests/storage/cdi_upload/test_upload_virtctl.py`, `tests/install_upgrade_operators/csv/test_csv.py`, `tests/infrastructure/instance_types/supported_os/test_rhel_os.py`) do not reference `instance_type_rhel_os_matrix`, `instance_type_centos_os_matrix`, or `instance_type_fedora_os_matrix`.
- **Run gating tests: False** — No verified dependency path. Gating-marked test files do not reference the affected matrices populated by the modified `generate_linux_instance_type_os_matrix` / `generate_instance_type_matrix_dicts`.
- **Affected tests to run:**
- `utilities/unittests/test_os_utils.py::TestGenerateInstanceTypeRhelOsMatrix` — directly tests the modified `generate_linux_instance_type_os_matrix` (new `add_preference_arch_suffix` / `add_data_source_arch_suffix` params)
- `utilities/unittests/test_pytest_utils.py::TestGenerateInstanceTypeMatrixDicts` — directly tests the updated `generate_instance_type_matrix_dicts` logic
- **ARM64 / multiarch cluster integration:** Run on an ARM64 or multiarch cluster to validate that the new `instance_type_centos_os_list` in `tests/global_config_arm64.py` is picked up correctly and that DataSource names for RHEL/centos are constructed without an arch suffix on homogeneous clusters and with it on multiarch clusters.
'
gh api \
repos/RedHatQE/openshift-virtualization-tests/pulls/5105/reviews \
-X POST \
-F event=COMMENT \
-F commit_id=59f4d0e29af93f1bc9f54af042a86390f08e63df \
-f "comments[][path]=tests/global_config_arm64.py" \
-F "comments[][position]=4" \
-f "comments[][body]=$BODY" \
-f body=''Length of output: 3648 🧠 Learnings used |
geetikakay
force-pushed
the
fix_arm64_smoke
branch
from
59f4d0e to
f0e171b
Compare
geetikakay
force-pushed
the
fix_arm64_smoke
branch
from
f0e171b to
2a05365
Compare
openshift-virtualization-qe-bot
added
the
tox:verify-tc-requirement-polarion:running
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:passed
|
/lgtm |
|
/test-plan |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get latest commit SHA
COMMIT=$(gh pr view 5105 --repo RedHatQE/openshift-virtualization-tests --json commits --jq '.commits[-1].oid')
echo "Latest commit: $COMMIT"
# Get PR files to find position for tests/global_config_arm64.py
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5105/files \
--jq '.[] | select(.filename == "tests/global_config_arm64.py") | {filename: .filename, patch: .patch}'
BODY='**Test Execution Plan**
- **Run smoke tests: True** — Verified path: `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` (smoke) is parametrized from `py_config["instance_type_rhel_os_matrix"]` / `py_config["instance_type_centos_os_matrix"]`, which are populated by `generate_instance_type_matrix_dicts` → modified `generate_linux_instance_type_os_matrix` (new `add_preference_arch_suffix` / `add_data_source_arch_suffix` params).
- **Run gating tests: False** — No gating-marked file references the modified symbols.
**Affected tests to run:**
- `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` *(smoke)* — parametrized via `instance_type_rhel_os_matrix` / `instance_type_centos_os_matrix`; validate correct preference and DataSource name construction.
- `utilities/unittests/test_os_utils.py::TestGenerateInstanceTypeRhelOsMatrix` — directly tests modified `generate_linux_instance_type_os_matrix`.
- `utilities/unittests/test_pytest_utils.py::TestGenerateInstanceTypeMatrixDicts` — directly tests updated `generate_instance_type_matrix_dicts`.
**Cluster coverage required:**
- Homogeneous ARM64: preference names carry `.arm64` suffix; DataSource names are bare (no suffix).
- Multiarch: DataSource names carry arch suffix for all architectures (AMD64, ARM64, s390x); ARM64 preferences carry `.arm64` suffix, CentOS Stream preferences remain bare.
- Homogeneous amd64: no arch suffix on either preference or DataSource.'
# Post inline review
gh api \
repos/RedHatQE/openshift-virtualization-tests/pulls/5105/reviews \
-X POST \
-F event=COMMENT \
-F commit_id="$COMMIT" \
-f "comments[][path]=tests/global_config_arm64.py" \
-F "comments[][position]=4" \
-f "comments[][body]=$BODY" \
-f body=''Length of output: 4525 🧠 Learnings used |
| add_data_source_arch_suffix: When True, append arch_suffix to the DataSource name. Only True on | ||
| multiarch clusters where SSP creates per-arch DataSources (e.g. "rhel10-arm64"). | ||
| On homogeneous clusters DataSources are bare (e.g. "rhel10"). |
There was a problem hiding this comment.
the decision here is based on internal implementation; why should a user pass this flag? why not just check inside this function if the cluster is multiarch?
There was a problem hiding this comment.
I kept the multiarch check in pytest_utils because that is where the other instance type matrix setup already reads py_config and add_preference_arch_suffix has to be passed anyway (for centos as exception).
There was a problem hiding this comment.
also they look consistent, decision making for preference and datasource regarding suffix is taken in similar kind of arrangement.
There was a problem hiding this comment.
function is os independent now and all os related logic is done in the caller function (generate_instance_type_matrix_dicts).
i believe it's cleaner this way.
just that maybe reduce amount of args here? instead of making new args bool, pass arch name str and drop arch_suffix?
There was a problem hiding this comment.
Thanks @vsibirsk , arch_suffix is what to append and the two flags are where to append it. They are independant preference suffixing follows OS and DataSource suffixing follows cluster type. example : centos on multiarch needs arch_suffix for DataSource but add_preference_arch_suffix=False because ClusterPreferences have no arch variant. A single arch str would either need to be passed twice (preference + data_source) or forces os_utils to know about amd64, multiarch and centos again.
so what we do right now i.e One arch_suffix plus two flags avoids both: arch once, policy stays in the caller.
There was a problem hiding this comment.
Basically this approach handles exceptional cases(centos-stream*) well
rhel.10, rhel.10.arm64, rhel.10.s390x ← arch variants exist
fedora, fedora.arm64, fedora.s390x ← arch variants exist
centos.stream10 ← no arch variants (no centos.stream10.arm64)
centos.stream9 ← no arch variants
There was a problem hiding this comment.
My understanding is add_preference_arch_suffix and add_data_source_arch_suffix flags are because centos-stream-10 preference does not have any suffix for any architecture, keeping as neutral preference.
Unlike other OSes like rhel9 that has its own preference(e.g. rhel.9.arm, rhel9.arm.s390x, ..) and DataSource (e.g. rhel9-arm, rhel9-s390x, ..), centos-stream-10 does not follow same thumb rule having preference(e.g. centos.stream10 ) and DataSource(e.g. centos-stream9-arm64, centos-stream9-s390x )
So, in someway there is a need to pass add_preference_arch_suffix and add_data_source_arch_suffix to decouple the generation of a customized instance_type_os_matrix for centos-stream-10.
For now it's just centos which has neutral preference, maybe we have more such os in future then we do not have to change it
In future if we decide to have suffix for preference as we see need to add extra spec for centos based on cpu we could take off this parameter
|
/verified |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
|
|
||
| from utilities.constants import ( | ||
| ARM_64, | ||
| CENTOS_STREAM10_PREFERENCE, |
There was a problem hiding this comment.
Test Execution Plan
-
Run smoke tests: True — Verified path:
tests/infrastructure/instance_types/supported_os/test_rhel_os.py(@pytest.mark.smoke) is parametrized frompy_config["instance_type_rhel_os_matrix"]/py_config["instance_type_centos_os_matrix"], populated bygenerate_instance_type_matrix_dicts→ modifiedgenerate_linux_instance_type_os_matrix(signature changed:add_arch_suffixreplaced byadd_preference_arch_suffix+add_data_source_arch_suffix). -
Run gating tests: False — No gating-marked file references the modified symbols.
Affected tests to run:
tests/infrastructure/instance_types/supported_os/test_rhel_os.py(smoke) — parametrized viainstance_type_rhel_os_matrix/instance_type_centos_os_matrix; validates correct preference and DataSource name construction per architecture.utilities/unittests/test_os_utils.py::TestGenerateInstanceTypeRhelOsMatrix— unit tests for modifiedgenerate_linux_instance_type_os_matrix.utilities/unittests/test_pytest_utils.py::TestGenerateInstanceTypeMatrixDicts— unit tests for updatedgenerate_instance_type_matrix_dictsincluding new multiarch parametrized cases.
Real tests (cluster required)
Error path (the fix — amd64 multiarch DataSource suffix):
pytest tests/infrastructure/instance_types/supported_os/test_rhel_os.py --collect-only
Expected: matrix entries for multiarch+amd64 show DataSource names like rhel10-amd64, not bare rhel10.
Happy path (regression — homogeneous ARM64 preference suffix):
pytest tests/infrastructure/instance_types/supported_os/test_rhel_os.py -k rhel
Expected: on ARM64 cluster, preferences carry .arm64 suffix; DataSource names are bare (no arch suffix).
Cluster coverage required:
- Homogeneous ARM64: preference names carry
.arm64suffix; DataSource names are bare. - Multiarch: DataSource names carry arch suffix for all architectures; CentOS Stream preferences remain bare.
- Homogeneous amd64: no arch suffix on preference or DataSource.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
COMMIT=$(gh pr view 5105 --repo RedHatQE/openshift-virtualization-tests --json commits --jq '.commits[-1].oid')
echo "Latest commit: $COMMIT"
# Confirm patch position for tests/global_config_arm64.py
gh api repos/RedHatQE/openshift-virtualization-tests/pulls/5105/files \
--jq '.[] | select(.filename == "tests/global_config_arm64.py") | {filename: .filename, patch: .patch}'
BODY='**Test Execution Plan**
- **Run smoke tests: True** — Verified path: `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` (`@pytest.mark.smoke`) is parametrized from `py_config["instance_type_rhel_os_matrix"]` / `py_config["instance_type_centos_os_matrix"]`, populated by `generate_instance_type_matrix_dicts` → modified `generate_linux_instance_type_os_matrix` (signature changed: `add_arch_suffix` replaced by `add_preference_arch_suffix` + `add_data_source_arch_suffix`).
- **Run gating tests: False** — No gating-marked file references the modified symbols.
**Affected tests to run:**
- `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` *(smoke)* — parametrized via `instance_type_rhel_os_matrix` / `instance_type_centos_os_matrix`; validates correct preference and DataSource name construction per architecture.
- `utilities/unittests/test_os_utils.py::TestGenerateInstanceTypeRhelOsMatrix` — unit tests for modified `generate_linux_instance_type_os_matrix`.
- `utilities/unittests/test_pytest_utils.py::TestGenerateInstanceTypeMatrixDicts` — unit tests for updated `generate_instance_type_matrix_dicts` including new multiarch parametrized cases.
**Real tests (cluster required)**
Error path (the fix — amd64 multiarch DataSource suffix):Length of output: 2548 🏁 Script executed: #!/bin/bash
python3 << 'PYEOF'
import subprocess, json
body = """**Test Execution Plan**
- **Run smoke tests: True** — Verified path: `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` (`@pytest.mark.smoke`) is parametrized from `py_config["instance_type_rhel_os_matrix"]` / `py_config["instance_type_centos_os_matrix"]`, populated by `generate_instance_type_matrix_dicts` → modified `generate_linux_instance_type_os_matrix` (signature changed: `add_arch_suffix` replaced by `add_preference_arch_suffix` + `add_data_source_arch_suffix`).
- **Run gating tests: False** — No gating-marked file references the modified symbols.
**Affected tests to run:**
- `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` *(smoke)* — parametrized via `instance_type_rhel_os_matrix` / `instance_type_centos_os_matrix`; validates correct preference and DataSource name construction per architecture.
- `utilities/unittests/test_os_utils.py::TestGenerateInstanceTypeRhelOsMatrix` — unit tests for modified `generate_linux_instance_type_os_matrix`.
- `utilities/unittests/test_pytest_utils.py::TestGenerateInstanceTypeMatrixDicts` — unit tests for updated `generate_instance_type_matrix_dicts` including new multiarch parametrized cases.
**Real tests (cluster required)**
Error path (the fix — amd64 multiarch DataSource suffix):Length of output: 1837 🏁 Script executed: #!/bin/bash
cat > /tmp/review_body.txt << 'ENDBODY'
**Test Execution Plan**
- **Run smoke tests: True** — Verified path: `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` (`@pytest.mark.smoke`) is parametrized from `py_config["instance_type_rhel_os_matrix"]` / `py_config["instance_type_centos_os_matrix"]`, populated by `generate_instance_type_matrix_dicts` → modified `generate_linux_instance_type_os_matrix` (signature changed: `add_arch_suffix` replaced by `add_preference_arch_suffix` + `add_data_source_arch_suffix`).
- **Run gating tests: False** — No gating-marked file references the modified symbols.
**Affected tests to run:**
- `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` *(smoke)* — parametrized via `instance_type_rhel_os_matrix` / `instance_type_centos_os_matrix`; validates correct preference and DataSource name construction per architecture.
- `utilities/unittests/test_os_utils.py::TestGenerateInstanceTypeRhelOsMatrix` — unit tests for modified `generate_linux_instance_type_os_matrix`.
- `utilities/unittests/test_pytest_utils.py::TestGenerateInstanceTypeMatrixDicts` — unit tests for updated `generate_instance_type_matrix_dicts` including new multiarch parametrized cases.
**Real tests (cluster required)**
Error path (the fix — amd64 multiarch DataSource suffix):
pytest tests/infrastructure/instance_types/supported_os/test_rhel_os.py --collect-only
Expected: matrix entries for multiarch+amd64 show DataSource names like `rhel10-amd64`, not bare `rhel10`.
Happy path (regression — homogeneous ARM64 preference suffix):
pytest tests/infrastructure/instance_types/supported_os/test_rhel_os.py -k rhel
Expected: on ARM64 cluster, preferences carry `.arm64` suffix; DataSource names are bare (no arch suffix).
**Cluster coverage required:**
- Homogeneous ARM64: preference names carry `.arm64` suffix; DataSource names are bare.
- Multiarch: DataSource names carry arch suffix for all architectures; CentOS Stream preferences remain bare.
- Homogeneous amd64: no arch suffix on preference or DataSource.
ENDBODY
python3 - << 'PYEOF'
import subprocess, json
with open("/tmp/review_body.txt") as f:
body = f.read()
payload = {
"commit_id": "2a0536598c920068426eaa47ef9311b20f050345",
"event": "COMMENT",
"body": "",
"comments": [
{
"path": "tests/global_config_arm64.py",
"position": 4,
"body": body
}
]
}
result = subprocess.run(
["gh", "api", "repos/RedHatQE/openshift-virtualization-tests/pulls/5105/reviews",
"-X", "POST",
"--input", "-"],
input=json.dumps(payload).encode(),
capture_output=True
)
print("STDOUT:", result.stdout.decode()[:500])
print("STDERR:", result.stderr.decode()[:300])
print("Return code:", result.returncode)
PYEOFLength of output: 3401 |
11 participants
What this PR does / why we need it:
Split the single add_arch_suffix flag into two independent controls:
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
Summary by CodeRabbit
Refactor
Tests
Chores