Skip to content
59 changes: 38 additions & 21 deletions tests/storage/memory_dump/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,42 @@
import pytest
from ocp_resources.datavolume import DataVolume
from ocp_resources.persistent_volume_claim import PersistentVolumeClaim
from pytest_testconfig import config as py_config

from tests.storage.memory_dump.utils import wait_for_memory_dump_status_completed
from utilities.constants import TIMEOUT_2MIN, Images
from utilities.storage import PodWithPVC, get_containers_for_pods_with_pvc, virtctl_memory_dump
from utilities.virt import running_vm, vm_instance_from_template
from tests.utils import create_windows2022_dv_template_from_registry, create_windows2022_vm_with_vtpm
from utilities.constants import (
TIMEOUT_2MIN,
Images,
)
from utilities.storage import (
PodWithPVC,
get_containers_for_pods_with_pvc,
virtctl_memory_dump,
)


@pytest.fixture()
def windows_vm_for_memory_dump(
request,
def windows_vm_with_vtpm_for_memory_dump(
unprivileged_client,
namespace,
golden_image_data_source_scope_function,
modern_cpu_for_migration,
):
with vm_instance_from_template(
request=request,
unprivileged_client=unprivileged_client,
namespace=namespace,
data_source=golden_image_data_source_scope_function,
) as vm:
running_vm(vm=vm)
with (
create_windows2022_dv_template_from_registry(
dv_name="windows-2022-dv",
namespace=namespace.name,
client=unprivileged_client,
storage_class=py_config["default_storage_class"],
) as dv_template,
create_windows2022_vm_with_vtpm(
vm_name="windows-2022-vm",
namespace=namespace.name,
client=unprivileged_client,
dv_template=dv_template,
cpu_model=modern_cpu_for_migration,
) as vm,
Comment on lines +23 to +35

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Import the renamed Windows helpers.

create_windows2022_dv_template_from_registry and create_windows2022_vm_with_vtpm are unresolved at Line 23 and Line 29, so this module will fail during collection before any memory-dump fixture runs. Update the tests.utils import to match the renamed helpers.

Suggested import update
-from tests.utils import create_windows2022_dv_from_registry, create_windows2022_vm_with_vtpm_from_registry
+from tests.utils import create_windows2022_dv_template_from_registry, create_windows2022_vm_with_vtpm
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
create_windows2022_dv_template_from_registry(
dv_name="windows-2022-dv",
namespace=namespace.name,
client=unprivileged_client,
storage_class=py_config["default_storage_class"],
) as dv_template,
create_windows2022_vm_with_vtpm(
vm_name="windows-2022-vm",
namespace=namespace.name,
client=unprivileged_client,
dv_template=dv_template,
cpu_model=modern_cpu_for_migration,
) as vm,
from tests.utils import create_windows2022_dv_template_from_registry, create_windows2022_vm_with_vtpm
🧰 Tools
🪛 Ruff (0.15.20)

[error] 23-23: Undefined name create_windows2022_dv_template_from_registry

(F821)


[error] 29-29: Undefined name create_windows2022_vm_with_vtpm

(F821)

🤖 Prompt for 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.

In `@tests/storage/memory_dump/conftest.py` around lines 23 - 35, The memory-dump
conftest is still importing outdated Windows helper names, so test collection
fails before the fixture can run. Update the `tests.utils` import used by
`conftest.py` to the renamed helpers that match
`create_windows2022_dv_template_from_registry` and
`create_windows2022_vm_with_vtpm`, and make sure the fixture setup continues to
reference those same symbols consistently.

Source: Linters/SAST tools

):
yield vm


Expand All @@ -49,24 +64,26 @@ def pvc_for_windows_memory_dump(unprivileged_client, namespace, storage_class_wi


@pytest.fixture()
def windows_vm_memory_dump(namespace, windows_vm_for_memory_dump, pvc_for_windows_memory_dump):
def windows_vm_memory_dump(namespace, windows_vm_with_vtpm_for_memory_dump, pvc_for_windows_memory_dump):
status, out, err = virtctl_memory_dump(
action="get",
namespace=namespace.name,
vm_name=windows_vm_for_memory_dump.name,
vm_name=windows_vm_with_vtpm_for_memory_dump.name,
claim_name=pvc_for_windows_memory_dump.name,
)
assert status, f"Failed to get memory dump, out: {out}, err: {err}."
yield


@pytest.fixture()
def windows_vm_memory_dump_completed(windows_vm_for_memory_dump):
wait_for_memory_dump_status_completed(vm=windows_vm_for_memory_dump)
def windows_vm_memory_dump_completed(windows_vm_with_vtpm_for_memory_dump):
wait_for_memory_dump_status_completed(vm=windows_vm_with_vtpm_for_memory_dump)


@pytest.fixture()
def consumer_pod_for_verifying_windows_memory_dump(namespace, windows_vm_for_memory_dump, pvc_for_windows_memory_dump):
def consumer_pod_for_verifying_windows_memory_dump(
namespace, windows_vm_with_vtpm_for_memory_dump, pvc_for_windows_memory_dump
):
with PodWithPVC(
namespace=namespace.name,
name="consumer-pod",
Expand All @@ -79,18 +96,18 @@ def consumer_pod_for_verifying_windows_memory_dump(namespace, windows_vm_for_mem
pod.wait_for_status(status=pod.Status.RUNNING, timeout=TIMEOUT_2MIN)

assert re.match(
rf"{windows_vm_for_memory_dump.name}-{pvc_for_windows_memory_dump.name}-\d*-\d*.memory.dump",
rf"{windows_vm_with_vtpm_for_memory_dump.name}-{pvc_for_windows_memory_dump.name}-\d*-\d*.memory.dump",
pod.execute(command=shlex.split("bash -c 'ls -1 /pvc | grep dump'")),
re.IGNORECASE,
), "Memory dump file doesn't exist"


@pytest.fixture()
def windows_vm_memory_dump_deletion(namespace, windows_vm_for_memory_dump):
def windows_vm_memory_dump_deletion(namespace, windows_vm_with_vtpm_for_memory_dump):
status, out, err = virtctl_memory_dump(
action="remove",
namespace=namespace.name,
vm_name=windows_vm_for_memory_dump.name,
vm_name=windows_vm_with_vtpm_for_memory_dump.name,
)
assert status, f"Failed to remove memory dump, out: {out}, err: {err}."
yield
26 changes: 3 additions & 23 deletions tests/storage/memory_dump/test_memory_dump.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,19 @@
"""

import pytest
from pytest_testconfig import config as py_config

from tests.os_params import WINDOWS_LATEST, WINDOWS_LATEST_LABELS
from tests.storage.memory_dump.utils import wait_for_memory_dump_status_removed


@pytest.mark.tier3
@pytest.mark.parametrize(
"golden_image_data_volume_scope_function, windows_vm_for_memory_dump",
[
pytest.param(
{
"dv_name": "dv-windows",
"image": WINDOWS_LATEST.get("image_path"),
"storage_class": py_config["default_storage_class"],
"dv_size": WINDOWS_LATEST.get("dv_size"),
},
{
"vm_name": "windows-vm-mem",
"template_labels": WINDOWS_LATEST_LABELS,
},
marks=pytest.mark.polarion("CNV-8518"),
),
],
indirect=True,
)
@pytest.mark.polarion("CNV-8518")
def test_windows_memory_dump(
namespace,
windows_vm_for_memory_dump,
windows_vm_with_vtpm_for_memory_dump,
pvc_for_windows_memory_dump,
windows_vm_memory_dump,
windows_vm_memory_dump_completed,
consumer_pod_for_verifying_windows_memory_dump,
windows_vm_memory_dump_deletion,
):
wait_for_memory_dump_status_removed(vm=windows_vm_for_memory_dump)
wait_for_memory_dump_status_removed(vm=windows_vm_with_vtpm_for_memory_dump)
8 changes: 4 additions & 4 deletions tests/storage/test_hotplug.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from ocp_resources.storage_profile import StorageProfile

from tests.storage.utils import assert_disk_bus
from tests.utils import create_windows2022_dv_from_registry, create_windows2022_vm_with_vtpm_from_registry
from tests.utils import create_windows2022_dv_template_from_registry, create_windows2022_vm_with_vtpm
from utilities.constants import HOTPLUG_DISK_SCSI_BUS, HOTPLUG_DISK_SERIAL, HOTPLUG_DISK_VIRTIO_BUS, Images
from utilities.hco import ResourceEditorValidateHCOReconcile
from utilities.jira import is_jira_open
Expand Down Expand Up @@ -78,7 +78,7 @@ def windows_dv_from_registry_scope_class(
storage_class_matrix__class__,
):
"""Creates a Windows 2022 DataVolume from registry container disk."""
with create_windows2022_dv_from_registry(
with create_windows2022_dv_template_from_registry(
dv_name="dv-windows-2022-hotplug",
namespace=namespace.name,
client=unprivileged_client,
Expand All @@ -95,8 +95,8 @@ def vm_instance_from_template_multi_storage_scope_class(
windows_dv_from_registry_scope_class,
):
"""Creates a Windows 2022 VM with vTPM from registry container disk."""
with create_windows2022_vm_with_vtpm_from_registry(
dv_dict=windows_dv_from_registry_scope_class,
with create_windows2022_vm_with_vtpm(
dv_template=windows_dv_from_registry_scope_class,
namespace=namespace.name,
client=unprivileged_client,
vm_name="vm-win-2022-hotplug",
Expand Down
12 changes: 6 additions & 6 deletions tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -705,14 +705,14 @@ def verify_rwx_default_storage(client: DynamicClient) -> None:


@contextmanager
def create_windows2022_dv_from_registry(
def create_windows2022_dv_template_from_registry(
dv_name: str,
namespace: str,
client: DynamicClient,
storage_class: str,
) -> Generator[dict]:
"""
Creates a Windows Server 2022 DataVolume from registry container disk.
Creates a Windows Server 2022 DataVolume template from registry container disk.
Comment on lines +708 to +715

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Restore the required PR template sections.

The PR summary says the description still lacks meaningful content for ##### What this PR does / why we need it: and does not preserve the required template sections. Please restore the template and explain why this vTPM/helper rewire is needed before merge. As per path instructions, What this PR does / why we need it: must have meaningful content and all required sections must be present.

🤖 Prompt for 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.

In `@tests/utils.py` around lines 708 - 715, The PR template content is incomplete
and the required sections are missing meaningful text, especially “What this PR
does / why we need it:”. Restore the full template structure and add a
substantive explanation for the vTPM/helper rewire so all required PR sections
are present. Use the surrounding helper context in
create_windows2022_dv_template_from_registry as the place to ensure the
template-driven output preserves the expected sections before merge.

Source: Path instructions


Args:
dv_name: Name for the DataVolume
Expand Down Expand Up @@ -749,8 +749,8 @@ def create_windows2022_dv_from_registry(


@contextmanager
def create_windows2022_vm_with_vtpm_from_registry(
dv_dict: dict,
def create_windows2022_vm_with_vtpm(
dv_template: dict,
namespace: str,
client: DynamicClient,
vm_name: str,
Expand All @@ -760,7 +760,7 @@ def create_windows2022_vm_with_vtpm_from_registry(
Creates a Windows Server 2022 VM with vTPM from registry container disk.

Args:
dv_dict: DataVolume template dictionary with metadata and spec
dv_template: DataVolume template dictionary with metadata and spec
namespace: Kubernetes namespace
client: Kubernetes client
vm_name: Name for the VirtualMachine
Expand All @@ -776,7 +776,7 @@ def create_windows2022_vm_with_vtpm_from_registry(
os_flavor=OS_FLAVOR_WIN_CONTAINER_DISK,
vm_instance_type=VirtualMachineClusterInstancetype(name=U1_LARGE, client=client),
vm_preference=VirtualMachineClusterPreference(name=WINDOWS_2K22_PREFERENCE, client=client),
data_volume_template=dv_dict,
data_volume_template=dv_template,
cpu_model=cpu_model,
) as vm:
running_vm(vm=vm)
Expand Down