Update auto_update_boot_source_vm to use instance types #191

RoniKishner · 2025-01-28T03:23:32Z

Short description:

Use instance types instead of templates to create VM from provided data sources

More details:

Since centos-10/rhel-10 don't have templates we can't add them to the testing matrix, as such we should update VM creation method in Data Source tests

Add centos-stream10, rhel-10 the testing matrix auto_update_data_source_matrix
Remove template_os from auto_update_data_source_matrix
Change creation method in auto_update_boot_source_vm to use instance types
Change assertion method in assert_os_version_mismatch_in_vm to assert: vm guest OS == preference name given by the data source label
Change matrix_auto_boot_data_import_cron_prefixes since the matrix no longer has auto_update_data_source_matrix

What this PR does / why we need it:

Add coverage for centos10/rhel10-beta

Special notes for reviewer:

Next release rhel10-beta naming will be needed to change to rhel-10

jira-ticket:

https://issues.redhat.com/browse/CNV-41859

redhat-cnv-qe-bot1 · 2025-01-28T03:23:49Z

Report bugs in Issues

The following are automatically added:

Add reviewers from OWNER file (in the root of the repository) under reviewers section.
Set PR size label.
New issue is created for the PR. (Closed when PR is merged/closed)
Run pre-commit if .pre-commit-config.yaml exists in the repo.

Available user actions:

To mark PR as WIP comment /wip to the PR, To remove it from the PR comment /wip cancel to the PR.
To block merging of PR comment /hold, To un-block merging of PR comment /hold cancel.
To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
verified label removed on each new commit push.
To cherry pick a merged PR comment /cherry-pick <target branch to cherry-pick to> in the PR.
- Multiple target branches can be cherry-picked, separated by spaces. (/cherry-pick branch1 branch2)
- Cherry-pick will be started when PR is merged
To build and push container image command /build-and-push-container in the PR (tag will be the PR number).
- You can add extra args to the Podman build command
  - Example: /build-and-push-container --build-arg OPENSHIFT_PYTHON_WRAPPER_COMMIT=<commit_hash>
To add a label by comment use /<label name>, to remove, use /<label name> cancel
To assign reviewers based on OWNERS file use /assign-reviewers
To check if PR can be merged use /check-can-merge
to assign reviewer to PR use /assign-reviewer @<reviewer>

Supported /retest check runs

/retest tox: Retest tox
/retest build-container: Retest build-container
/retest all: Retest all

Supported labels

hold
verified
wip
lgtm

polarion-jenkins

D/S test tox -e verify-bugs-are-open passed: cnv-tests-tox-executor/6073

for more information, see https://pre-commit.ci

polarion-jenkins

D/S test tox -e verify-bugs-are-open passed: cnv-tests-tox-executor/6152

RoniKishner · 2025-01-29T10:12:23Z

/build-and-push-container

redhat-cnv-qe-bot · 2025-01-29T10:12:58Z

New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-191 published

RoniKishner · 2025-01-29T20:15:59Z

/verified
Verified all uses of auto_update_data_source_matrix

dbasunag · 2025-01-31T22:10:16Z

tests/global_config.py

@@ -187,10 +187,12 @@ def _get_default_storage_class(sc_list):
 # If the DataImportCron uses a different prefix than the DataSource name
 # use data_import_cron_prefix in matrix dict to specify new prefix.
 auto_update_data_source_matrix = [


templates are still supported. this drops coverage for templates. No?

can we extend this for cluster_prefeence

Yes, this is a good idea, but lets try and merge this first before we add an assertion on the used preferences, I don't want to make this PR too big and over extend it's purpose

dbasunag · 2025-01-31T22:11:08Z

tests/infrastructure/golden_images/update_boot_source/test_ssp_common_templates_boot_sources.py


 LOGGER = logging.getLogger(__name__)
 RHEL9_NAME = "rhel9"


-def assert_os_version_mismatch_in_vm(vm, expected_os):
-    expected_os_params = re.match(r"(?P<os_name>[a-z]+)(-stream)?(?P<os_ver>[0-9]+)", expected_os).groupdict()
+def assert_os_version_mismatch_in_vm(vm, latest_fedora_release_version):


If you want to cover instance types, that is great. But let's not remove coverage for templates till they are removed.

should we cover both?

I see your point, but I respectfully disagree that this test is meant to cover templates. Its primary focus is on the Data Import Cron, and while it does use a template, that’s more of a tool rather than the core intent of the test.

In the past, creating a VM from templates was a common practice, and as a result, VirtualMachineForTemplates became the default approach. However, changing the method here does not necessarily imply a loss of coverage. Let me know what you think!

The test file is called "test_ssp_common_templates_boot_sources.py". I am not in favor of removing template coverage with instance type as of now. When templates are removed, please feel free to remove associated tests. Till then if you want to add coverage for instance type, please add new tests in a separate file.

dbasunag · 2025-02-02T23:23:34Z

tests/infrastructure/golden_images/update_boot_source/test_ssp_common_templates_boot_sources.py


 LOGGER = logging.getLogger(__name__)
 RHEL9_NAME = "rhel9"


-def assert_os_version_mismatch_in_vm(vm, expected_os):
-    expected_os_params = re.match(r"(?P<os_name>[a-z]+)(-stream)?(?P<os_ver>[0-9]+)", expected_os).groupdict()
+def assert_os_version_mismatch_in_vm(vm, latest_fedora_release_version):


The test file is called "test_ssp_common_templates_boot_sources.py". I am not in favor of removing template coverage with instance type as of now. When templates are removed, please feel free to remove associated tests. Till then if you want to add coverage for instance type, please add new tests in a separate file.

redhat-cnv-qe-bot1 added the size/M label Jan 28, 2025

redhat-cnv-qe-bot1 requested review from vsibirsk and dbasunag January 28, 2025 03:23

redhat-cnv-qe-bot1 added the branch-main label Jan 28, 2025

redhat-cnv-qe-bot1 mentioned this pull request Jan 28, 2025

Update auto_update_boot_source_vm to use instance types - 191 #192

Open

redhat-cnv-qe-bot1 requested review from dshchedr and geetikakay January 28, 2025 03:23

polarion-jenkins previously approved these changes Jan 28, 2025

View reviewed changes

cnv-qe-bot1 added the lgtm-polarion-jenkins label Jan 28, 2025

RoniKishner dismissed polarion-jenkins’s stale review via 91d593f January 28, 2025 13:58

RoniKishner force-pushed the boot-source-vm-instance-type branch from 1b94678 to 91d593f Compare January 28, 2025 13:58

cnv-qe-bot1 removed the lgtm-polarion-jenkins label Jan 28, 2025

cnv-qe-bot1 removed the lgtm-polarion-jenkins label Jan 29, 2025

[pre-commit.ci] auto fixes from pre-commit.com hooks

7dad37d

for more information, see https://pre-commit.ci

polarion-jenkins approved these changes Jan 29, 2025

View reviewed changes

redhat-cnv-qe-bot1 added the lgtm-polarion-jenkins label Jan 29, 2025

cnv-qe-bot1 added the verified label Jan 29, 2025

dbasunag requested changes Jan 31, 2025

View reviewed changes

cnv-qe-bot1 added the changes-requested-dbasunag label Jan 31, 2025

redhat-cnv-qe-bot1 added the commented-dshchedr label Jan 31, 2025

redhat-cnv-qe-bot added commented-geetikakay commented-RoniKishner labels Feb 1, 2025

dbasunag requested changes Feb 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update auto_update_boot_source_vm to use instance types #191

Update auto_update_boot_source_vm to use instance types #191

RoniKishner commented Jan 28, 2025 •

edited

Loading

redhat-cnv-qe-bot1 commented Jan 28, 2025

polarion-jenkins left a comment

polarion-jenkins left a comment

RoniKishner commented Jan 29, 2025

redhat-cnv-qe-bot commented Jan 29, 2025

RoniKishner commented Jan 29, 2025

dbasunag Jan 31, 2025

geetikakay Feb 1, 2025

RoniKishner Feb 2, 2025

dbasunag Jan 31, 2025

dshchedr Jan 31, 2025

geetikakay Feb 1, 2025

RoniKishner Feb 2, 2025

dbasunag Feb 2, 2025

dbasunag Feb 2, 2025

Update auto_update_boot_source_vm to use instance types #191

Are you sure you want to change the base?

Update auto_update_boot_source_vm to use instance types #191

Conversation

RoniKishner commented Jan 28, 2025 • edited Loading

Short description:

More details:

What this PR does / why we need it:

Special notes for reviewer:

jira-ticket:

redhat-cnv-qe-bot1 commented Jan 28, 2025

polarion-jenkins left a comment

Choose a reason for hiding this comment

polarion-jenkins left a comment

Choose a reason for hiding this comment

RoniKishner commented Jan 29, 2025

redhat-cnv-qe-bot commented Jan 29, 2025

RoniKishner commented Jan 29, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoniKishner commented Jan 28, 2025 •

edited

Loading