Log printableStatus for timeout and skip adding vnc screenshot for non booted vms

utilities/data_collector.py

@@ -5,6 +5,7 @@
 from functools import cache
 
 from ocp_resources.namespace import Namespace
+from ocp_resources.virtual_machine import VirtualMachine
 from ocp_utilities.monitoring import Prometheus
 from pytest_testconfig import config as py_config
 
@@ -111,12 +112,21 @@ def collect_alerts_data():
     )
 
 
-def collect_vnc_screenshot_for_vms(vm_name: str, vm_namespace: str) -> None:
-    base_dir = get_data_collector_base_directory()
-    utilities.infra.run_virtctl_command(
-        command=shlex.split(f"vnc screenshot {vm_name} -f {base_dir}/{vm_namespace}-{vm_name}.png"),
-        namespace=vm_namespace,
-    )
+def collect_vnc_screenshot_for_vms(vm: VirtualMachine) -> None:
+    """Collect a VNC screenshot for a VM when its state supports VNC access.
+
+    Args:
+        vm (VirtualMachine): VM object used to read status, name, and namespace.
+    """
+    printable_status = vm.instance.get("status", {}).get("printableStatus")
+    if printable_status in (VirtualMachine.Status.RUNNING, VirtualMachine.Status.MIGRATING):
+        base_dir = get_data_collector_base_directory()
+        utilities.infra.run_virtctl_command(
+            command=shlex.split(f"vnc screenshot {vm.name} -f {base_dir}/{vm.namespace}-{vm.name}.png"),
+            namespace=vm.namespace,
+        )
+    else:
+        LOGGER.warning(f"Skipping VNC screenshot for VM {vm.name}, status is '{printable_status}'.")
 
 
 def collect_ocp_must_gather(since_time):

utilities/unittests/test_data_collector.py

@@ -122,6 +122,8 @@
 
 # Circular dependencies are already mocked in conftest.py
 
+from ocp_resources.virtual_machine import VirtualMachine
+
 # Now import the real data_collector module functions
 from utilities.data_collector import (
     BASE_DIRECTORY_NAME,
@@ -338,12 +340,15 @@ class TestCollectVncScreenshotForVms:
     @patch("utilities.data_collector.get_data_collector_base_directory")
     @patch("utilities.data_collector.utilities.infra.run_virtctl_command")
     @patch("utilities.data_collector.shlex.split")
-    def test_collect_vnc_screenshot_for_vms(self, mock_shlex, mock_run_virtctl, mock_get_base_dir):
-        """Test collect_vnc_screenshot_for_vms runs virtctl command"""
+    def test_collect_vnc_screenshot_for_vms_running(self, mock_shlex, mock_run_virtctl, mock_get_base_dir):
+        """Test collect_vnc_screenshot_for_vms takes screenshot when VM is Running"""
         mock_get_base_dir.return_value = "/base/dir"
         mock_shlex.return_value = ["vnc", "screenshot", "test-vm", "-f", "/base/dir/test-ns-test-vm.png"]
-
-        collect_vnc_screenshot_for_vms("test-vm", "test-ns")
+        mock_vm = MagicMock()
+        mock_vm.name = "test-vm"
+        mock_vm.namespace = "test-ns"
+        mock_vm.instance.get.return_value = {"printableStatus": VirtualMachine.Status.RUNNING}
+        collect_vnc_screenshot_for_vms(vm=mock_vm)
 
         mock_get_base_dir.assert_called_once()
         expected_command = "vnc screenshot test-vm -f /base/dir/test-ns-test-vm.png"
@@ -352,6 +357,16 @@ def test_collect_vnc_screenshot_for_vms(self, mock_shlex, mock_run_virtctl, mock
             command=["vnc", "screenshot", "test-vm", "-f", "/base/dir/test-ns-test-vm.png"], namespace="test-ns"
         )
 
+    @patch("utilities.data_collector.utilities.infra.run_virtctl_command")
+    def test_collect_vnc_screenshot_for_vms_skips_error_status(self, mock_run_virtctl):
+        """Test collect_vnc_screenshot_for_vms skips screenshot when VM is in error state"""
+        mock_vm = MagicMock()
+        mock_vm.name = "test-vm"
+        mock_vm.instance.get.return_value = {"printableStatus": VirtualMachine.Status.ERR_IMAGE_PULL}
+        collect_vnc_screenshot_for_vms(vm=mock_vm)
+
+        mock_run_virtctl.assert_not_called()
+
 
 class TestCollectOcpMustGather:
     """Test cases for collect_ocp_must_gather function"""

utilities/virt.py

@@ -1636,7 +1636,7 @@ def wait_for_ssh_connectivity(
 
     for sample in TimeoutSampler(
         wait_timeout=timeout,
-        sleep=5,
+        sleep=TIMEOUT_5SEC,
         func=vm.ssh_exec.run_command,
         command=["exit"],
         tcp_timeout=tcp_timeout,
@@ -1794,7 +1794,7 @@ def wait_for_running_vm(
         if check_ssh_connectivity:
             wait_for_ssh_connectivity(vm=vm, timeout=ssh_timeout)
     except TimeoutExpiredError:
-        collect_vnc_screenshot_for_vms(vm_name=vm.name, vm_namespace=vm.namespace)  # type: ignore[arg-type]
+        collect_vnc_screenshot_for_vms(vm=vm)
         raise
 
 
@@ -1990,8 +1990,7 @@ def verify_vm_migrated(
         if check_ssh_connectivity:
             wait_for_ssh_connectivity(vm=vm)
     except TimeoutExpiredError:
-        LOGGER.error(f"VM {vm.name} unresponsive after migration; getting VNC screenshot")
-        collect_vnc_screenshot_for_vms(vm_name=vm.name, vm_namespace=vm.namespace)
+        collect_vnc_screenshot_for_vms(vm=vm)
         raise