Fix MemoryDeltaFromRequestedBytes tests flakiness (#762) (#1140)

OhadRevah · dshchedr · web-flow · commit 1b0ce98c6141 · 2025-06-05T09:08:54.000+03:00
* Fix MemoryDeltaFromRequestedBytes tests flakiness

Fixed MemoryDeltaFromRequestedBytes tests by coverting
the fixtures used into utils that used in sampler so the expected
value will be updated, before the expected value pulled one time
and sometime it is higher than the metric value and it needs to be
updated.

* Update tests/observability/metrics/utils.py



* Refactored functions and vars names

Renamed functions and vars according to comments.

* Combined expected kubvievirt memory delta
functions

* Combine MemoryDeltaFromRequestedBytes tests under
the same test with parametrize.

* changed get_highest_memory_usage_virt_api_pod_dict to return tuple

* Fixed get_metrics_value

---------

Co-authored-by: Den Shchedrivyi &lt;dshchedr@redhat.com&gt;
diff --git a/tests/observability/metrics/conftest.py b/tests/observability/metrics/conftest.py
@@ -30,6 +30,7 @@
     KUBEVIRT_VMI_STATUS_ADDRESSES,
     KUBEVIRT_VMSNAPSHOT_PERSISTENTVOLUMECLAIM_LABELS,
     KUBEVIRT_VNC_ACTIVE_CONNECTIONS_BY_VMI,
+    RSS_MEMORY_COMMAND,
 )
 from tests.observability.metrics.utils import (
     SINGLE_VM,
@@ -106,7 +107,6 @@
 CDI_UPLOAD_PRIME = "cdi-upload-prime"
 IP_RE_PATTERN_FROM_INTERFACE = r"eth0.*?inet (\d+\.\d+\.\d+\.\d+)/\d+"
 IP_ADDR_SHOW_COMMAND = shlex.split("ip addr show")
-RSS_MEMORY_COMMAND = shlex.split("bash -c \"cat /sys/fs/cgroup/memory.stat | grep '^anon ' | awk '{print $2}'\"")
 LOGGER = logging.getLogger(__name__)
 
 
@@ -824,53 +824,6 @@ def vm_for_test_with_resource_limits(namespace):
         yield vm
 
 
-@pytest.fixture(scope="class")
-def highest_memory_usage_virt_api_pod(hco_namespace, admin_client):
-    oc_adm_top_pod_output = (
-        run_command(command=shlex.split(f"oc adm top pod -n {hco_namespace.name} -l kubevirt.io=virt-api"))[1]
-        .strip()
-        .split("\n")[1:]
-    )
-    virt_api_with_highest_memory_usage = max(
-        {pod.split()[0]: int(bitmath.parse_string_unsafe(pod.split()[2])) for pod in oc_adm_top_pod_output}.items(),
-        key=lambda pod: pod[1],
-    )
-    return {
-        "virt_api_pod": virt_api_with_highest_memory_usage[0],
-        "memory_usage": virt_api_with_highest_memory_usage[1],
-    }
-
-
-@pytest.fixture(scope="class")
-def virt_api_requested_memory(hco_namespace, admin_client, highest_memory_usage_virt_api_pod):
-    return float(
-        bitmath.parse_string_unsafe(
-            get_pod_by_name_prefix(
-                dyn_client=admin_client,
-                pod_prefix=highest_memory_usage_virt_api_pod["virt_api_pod"],
-                namespace=hco_namespace.name,
-            )
-            .instance.spec.containers[0]
-            .resources.requests.memory
-        )
-    )
-
-
-@pytest.fixture()
-def virt_api_rss_memory(admin_client, hco_namespace, highest_memory_usage_virt_api_pod):
-    return int(
-        bitmath.Byte(
-            int(
-                get_pod_by_name_prefix(
-                    dyn_client=admin_client,
-                    pod_prefix=highest_memory_usage_virt_api_pod["virt_api_pod"],
-                    namespace=hco_namespace.name,
-                ).execute(command=RSS_MEMORY_COMMAND)
-            )
-        ).MiB
-    )
-
-
 @pytest.fixture()
 def vm_memory_working_set_bytes(vm_for_test, virt_launcher_pod_metrics_resource_exists):
     samples = TimeoutSampler(
diff --git a/tests/observability/metrics/constants.py b/tests/observability/metrics/constants.py
@@ -1,3 +1,5 @@
+import shlex
+
 from ocp_resources.resource import Resource
 
 from utilities.constants import (
@@ -80,3 +82,7 @@
 KUBEVIRT_VMSNAPSHOT_PERSISTENTVOLUMECLAIM_LABELS = (
     "kubevirt_vmsnapshot_persistentvolumeclaim_labels{{vm_name='{vm_name}'}}"
 )
+KUBEVIRT_VMI_MIGRATION_DATA_TOTAL_BYTES = "kubevirt_vmi_migration_data_total_bytes{{name='{vm_name}'}}"
+BINDING_NAME = "binding_name"
+BINDING_TYPE = "binding_type"
+RSS_MEMORY_COMMAND = shlex.split("bash -c \"cat /sys/fs/cgroup/memory.stat | grep '^anon ' | awk '{print $2}'\"")
diff --git a/tests/observability/metrics/test_metrics.py b/tests/observability/metrics/test_metrics.py
@@ -1,4 +1,3 @@
-import bitmath
 import pytest
 
 from tests.observability.metrics.constants import (
@@ -14,7 +13,7 @@
     assert_vmi_dommemstat_with_metric_value,
     compare_kubevirt_vmi_info_metric_with_vm_info,
     get_vm_metrics,
-    validate_metric_value_within_range,
+    validate_memory_delta_metrics_value_within_range,
 )
 from tests.observability.utils import validate_metrics_value
 from utilities.constants import KUBEVIRT_HCO_HYPERCONVERGED_CR_EXISTS, VIRT_API, VIRT_HANDLER
@@ -177,48 +176,44 @@ def test_kubevirt_vmi_info(self, prometheus, single_metric_vm, single_metric_vmi
 
 
 class TestMemoryDeltaFromRequestedBytes:
-    @pytest.mark.polarion("CNV-11632")
-    def test_metric_kubevirt_memory_delta_from_requested_bytes_working_set(
-        self, prometheus, highest_memory_usage_virt_api_pod, virt_api_requested_memory
-    ):
-        validate_metric_value_within_range(
-            prometheus=prometheus,
-            metric_name=f"kubevirt_memory_delta_from_requested_bytes{{container='{VIRT_API}', "
-            f"reason='memory_working_set_delta_from_request'}}",
-            expected_value=float(
-                bitmath.MiB(highest_memory_usage_virt_api_pod["memory_usage"] - virt_api_requested_memory).Byte
+    @pytest.mark.parametrize(
+        "metric, rss",
+        [
+            pytest.param(
+                f"kubevirt_memory_delta_from_requested_bytes{{container='{VIRT_API}', "
+                f"reason='memory_working_set_delta_from_request'}}",
+                False,
+                marks=pytest.mark.polarion("CNV-11632"),
+                id="test_metric_kubevirt_memory_delta_from_requested_bytes_working_set",
             ),
-        )
-
-    @pytest.mark.polarion("CNV-11633")
-    def test_metric_kubevirt_memory_delta_from_requested_bytes_rss(
-        self, prometheus, virt_api_rss_memory, virt_api_requested_memory
-    ):
-        validate_metric_value_within_range(
-            prometheus=prometheus,
-            metric_name=f"kubevirt_memory_delta_from_requested_bytes{{container='{VIRT_API}', "
-            f"reason='memory_rss_delta_from_request'}}",
-            expected_value=float(bitmath.MiB(virt_api_rss_memory - virt_api_requested_memory).Byte),
-        )
-
-    @pytest.mark.polarion("CNV-11690")
-    def test_metric_cnv_abnormal_working_set(
-        self, prometheus, highest_memory_usage_virt_api_pod, virt_api_requested_memory
-    ):
-        validate_metric_value_within_range(
-            prometheus=prometheus,
-            metric_name=f"cnv_abnormal{{container='{VIRT_API}', reason='memory_working_set_delta_from_request'}}",
-            expected_value=float(
-                bitmath.MiB(highest_memory_usage_virt_api_pod["memory_usage"] - virt_api_requested_memory).Byte
+            pytest.param(
+                f"kubevirt_memory_delta_from_requested_bytes{{container='{VIRT_API}', "
+                f"reason='memory_rss_delta_from_request'}}",
+                True,
+                marks=pytest.mark.polarion("CNV-11633"),
+                id="test_metric_kubevirt_memory_delta_from_requested_bytes_rss",
             ),
-        )
-
-    @pytest.mark.polarion("CNV-11691")
-    def test_metric_cnv_abnormal_rss(self, prometheus, virt_api_rss_memory, virt_api_requested_memory):
-        validate_metric_value_within_range(
+            pytest.param(
+                f"cnv_abnormal{{container='{VIRT_API}', reason='memory_working_set_delta_from_request'}}",
+                False,
+                marks=pytest.mark.polarion("CNV-11690"),
+                id="test_metric_cnv_abnormal_working_set",
+            ),
+            pytest.param(
+                f"cnv_abnormal{{container='{VIRT_API}', reason='memory_rss_delta_from_request'}}",
+                True,
+                marks=pytest.mark.polarion("CNV-11691"),
+                id="test_metric_cnv_abnormal_rss",
+            ),
+        ],
+    )
+    def test_memory_delta_from_requested_bytes(self, prometheus, admin_client, hco_namespace, metric, rss):
+        validate_memory_delta_metrics_value_within_range(
             prometheus=prometheus,
-            metric_name=f"cnv_abnormal{{container='{VIRT_API}', reason='memory_rss_delta_from_request'}}",
-            expected_value=float(bitmath.MiB(virt_api_rss_memory - virt_api_requested_memory).Byte),
+            metric_name=metric,
+            rss=rss,
+            admin_client=admin_client,
+            hco_namespace=hco_namespace.name,
         )
 
 
diff --git a/tests/observability/metrics/utils.py b/tests/observability/metrics/utils.py
@@ -1,10 +1,11 @@
 import logging
+import math
 import re
 import shlex
 import urllib
 from collections import Counter
 from datetime import datetime, timezone
-from typing import Any, Optional, Union
+from typing import Any, Optional
 
 import bitmath
 import pytest
@@ -26,6 +27,7 @@
     KUBEVIRT_VMI_FILESYSTEM_BYTES,
     KUBEVIRT_VMI_FILESYSTEM_BYTES_WITH_MOUNT_POINT,
     METRIC_SUM_QUERY,
+    RSS_MEMORY_COMMAND,
 )
 from tests.observability.utils import validate_metrics_value
 from utilities.constants import (
@@ -903,7 +905,7 @@ def validate_metric_value_within_range(
         prometheus=prometheus,
         metrics_name=metric_name,
     )
-    sample: Union[int, float] = 0
+    sample: int | float = 0
     try:
         for sample in samples:
             if sample:
@@ -1221,7 +1223,7 @@ def validate_metric_value_with_round_down(
         prometheus=prometheus,
         metrics_name=metric_name,
     )
-    sample: Union[int, float] = 0
+    sample: int | float = 0
     try:
         for sample in samples:
             sample = round(float(sample))
@@ -1230,3 +1232,131 @@ def validate_metric_value_with_round_down(
     except TimeoutExpiredError:
         LOGGER.info(f"Metric int value of: {metric_name} is: {sample}, expected value:{expected_value}")
         raise
+
+
+def get_pod_memory_stats(admin_client: DynamicClient, hco_namespace: str, pod_prefix: str) -> float:
+    return float(
+        bitmath.Byte(
+            float(
+                get_pod_by_name_prefix(
+                    dyn_client=admin_client,
+                    pod_prefix=pod_prefix,
+                    namespace=hco_namespace,
+                )
+                .execute(command=RSS_MEMORY_COMMAND)
+                .strip()
+            )
+        )
+    )
+
+
+def get_highest_memory_usage_virt_api_pod_tuple(hco_namespace: str) -> tuple[str, int]:
+    """
+    This function returns pod name and memory value tuple of virt-api pod with the highest memory usage.
+        Args:
+        hco_namespace: Hco namespacem
+    Returns:
+        tuple: containing the name of the virt-api pod with the highest memory usage and value of the memory.
+    """
+    virt_api_with_highest_memory_usage = (
+        run_command(
+            command=shlex.split(
+                f"bash -c 'oc adm top pod -n {hco_namespace} --sort-by memory "
+                f"--no-headers -l kubevirt.io=virt-api | head -n 1'"
+            ),
+        )[1]
+        .strip()
+        .split()
+    )
+    return (
+        virt_api_with_highest_memory_usage[0],
+        int(bitmath.parse_string_unsafe(virt_api_with_highest_memory_usage[2]).Byte),
+    )
+
+
+def get_pod_requested_memory(hco_namespace: str, admin_client: DynamicClient, pod_prefix: str) -> float:
+    """
+    Get the requested memory for a pod.
+
+    Args:
+        hco_namespace: Hco namespace
+        admin_client: The Kubernetes admin client
+        pod_prefix: Prefix of the pod name to get requested memory from
+
+    Returns:
+        float: Requested memory in bytes
+    """
+    return float(
+        bitmath.parse_string_unsafe(
+            get_pod_by_name_prefix(
+                dyn_client=admin_client,
+                pod_prefix=pod_prefix,
+                namespace=hco_namespace,
+            )
+            .instance.spec.containers[0]
+            .resources.requests.memory
+        ).Byte
+    )
+
+
+def expected_kubevirt_memory_delta_from_requested_bytes(
+    hco_namespace: str, admin_client: DynamicClient, rss: bool
+) -> int:
+    """
+    Calculate the expected memory delta between actual and requested memory.
+
+    Args:
+        hco_namespace: The namespace where virt-api pods are running
+        admin_client: The Kubernetes admin client
+        rss: If True, use RSS memory, otherwise use total memory usage
+
+    Returns:
+        int: The memory delta in bytes
+    """
+    pod_name, pod_memory = get_highest_memory_usage_virt_api_pod_tuple(hco_namespace=hco_namespace)
+    virt_api_requested_memory = get_pod_requested_memory(
+        hco_namespace=hco_namespace,
+        admin_client=admin_client,
+        pod_prefix=pod_name,
+    )
+    if rss:
+        virt_api_rss_memory = get_pod_memory_stats(
+            admin_client=admin_client,
+            hco_namespace=hco_namespace,
+            pod_prefix=pod_name,
+        )
+        return int(virt_api_rss_memory - virt_api_requested_memory)
+    return int(pod_memory - virt_api_requested_memory)
+
+
+def validate_memory_delta_metrics_value_within_range(
+    prometheus: Prometheus,
+    metric_name: str,
+    rss: bool,
+    admin_client: DynamicClient,
+    hco_namespace: str,
+    timeout: int = TIMEOUT_4MIN,
+) -> None:
+    samples = TimeoutSampler(
+        wait_timeout=timeout,
+        sleep=TIMEOUT_15SEC,
+        func=get_metrics_value,
+        prometheus=prometheus,
+        metrics_name=metric_name,
+    )
+    sample: int | float = 0
+    expected_value = None
+    try:
+        for sample in samples:
+            if sample:
+                sample = abs(float(sample))
+                expected_value = abs(
+                    expected_kubevirt_memory_delta_from_requested_bytes(
+                        admin_client=admin_client, hco_namespace=hco_namespace, rss=rss
+                    )
+                )
+                if math.isclose(sample, expected_value, rel_tol=0.05):
+                    return
+    except TimeoutExpiredError:
+        LOGGER.error(f"{sample} should be within 5% of {expected_value}")
+        raise
diff --git a/utilities/monitoring.py b/utilities/monitoring.py
@@ -167,9 +167,9 @@ def get_all_firing_alerts(prometheus):
 
 
 def get_metrics_value(prometheus, metrics_name):
-    metric_results = prometheus.query(query=metrics_name)["data"]["result"]
-    if metric_results:
-        metric_values_list = [value for metric_val in metric_results for value in metric_val.get("value")]
+    metric_results = prometheus.query(query=metrics_name).get("data", {})
+    if metric_results and (metric_res := metric_results["result"]):
+        metric_values_list = [value for metric_val in metric_res for value in metric_val.get("value")]
         return metric_values_list[1]
     LOGGER.warning(f"For Query {metrics_name}, empty results found.")
     return 0
