Repository: red-hat-storage/ocs-ci
Issue: Align metrics tests with change in behaviour in measure_stop_ceph_mon fixture

Since recent change done in https://github.com/red-hat-storage/ocs-ci/pull/1690/, `measure_stop_ceph_mon`  fixture no longer guarantees that the problem is happening during whole duration of the fixture, as reported via it's `start` and `stop` timestamps.

This makes test case `test_monitoring_shows_mon_down` fail, as it expects that the problem is happening during whole time range communicated by the fixture.

Based on CI analysis.

## Proposed solution

Fixture `test_monitoring_shows_mon_down` should communicate min. duration of the problem (for how long the test using it's data should expect the mon to be down), and the test then needs to use this information to run more specific checks.



Changed files:
ocs_ci/utility/tests/test_prometheus.py, tests/manage/monitoring/conftest.py, ocs_ci/utility/prometheus.py, tests/manage/monitoring/prometheusmetrics/test_monitoring_negative.py

diff --git a/ocs_ci/utility/prometheus.py b/ocs_ci/utility/prometheus.py
index 75b94b1a..3456794f 100644
--- a/ocs_ci/utility/prometheus.py
+++ b/ocs_ci/utility/prometheus.py
@@ -85,6 +85,7 @@ def check_query_range_result(
     bad_values=(),
     exp_metric_num=None,
     exp_delay=None,
+    exp_good_time=None,
 ):
     """
     Check that result of range query matches given expectations. Useful
@@ -106,6 +107,10 @@ def check_query_range_result(
             time range for which we should tolerate bad values. This is
             useful if you change cluster state and processing of this
             change is expected to take some time.
+        exp_good_time (int): Number of seconds during which we should see
+            good values in the metrics data. When this time passess values
+            can go bad (but can't be invalid). If not specified, good values
+            should be presend during the whole time.
 
     Returns:
         bool: True if result matches given expectations, False otherwise
@@ -145,6 +150,10 @@ def check_query_range_result(
                 if exp_delay is not None and delta.seconds < exp_delay:
                     logger.info(
                         msg + f" but within expected {exp_delay}s delay")
+                elif (exp_good_time is not None
+                        and delta.seconds >= exp_good_time):
+                    logger.info(
+                        msg + f" but after {exp_good_time}s already passed")
                 else:
                     logger.error(msg)
                     bad_value_timestamps.append(dt)
diff --git a/ocs_ci/utility/tests/test_prometheus.py b/ocs_ci/utility/tests/test_prometheus.py
index 65fab4d9..3dbc176c 100644
--- a/ocs_ci/utility/tests/test_prometheus.py
+++ b/ocs_ci/utility/tests/test_prometheus.py
@@ -105,6 +105,21 @@ def query_range_result_delay_60s(query_range_result_ok):
     return query_range_data
 
 
+@pytest.fixture
+def query_range_result_bad_last_90s(query_range_result_ok):
+    """
+    Range query data with bad values in last 6 values (last 90s of the
+    measurement).
+    """
+    query_range_data = query_range_result_ok
+    # insert bad value "0" for last 90s in both metrics
+    for i in range(2):
+        for j in range(10, 16):
+            value_tuple = query_range_data[i]['values'][j]
+            value_tuple[1] = "0"
+    return query_range_data
+
+
 def test_check_query_range_result_null():
     """
     The function does't throw any exception and returns true when executed
@@ -176,3 +191,21 @@ def test_check_query_range_result_exp_delay(query_range_result_delay_60s):
         bad_values=[0],
         exp_delay=60)
     assert result2, "taking exp_delay into account, validation should pass"
+
+
+def test_check_query_range_result_exp_good_time(query_range_result_bad_last_90s):
+    """
+    Check that exp_good_time is taken into account, so that initial bad values
+    are ignored if appear after the good time passess.
+    """
+    result1 = check_query_range_result(
+        query_range_result_bad_last_90s,
+        good_values=[1],
+        bad_values=[0])
+    assert not result1, "without exp_good_time validation should fail"
+    result2 = check_query_range_result(
+        query_range_result_bad_last_90s,
+        good_values=[1],
+        bad_values=[0],
+        exp_good_time=150)
+    assert result2, "taking exp_good_time into account, validation should pass"
diff --git a/tests/manage/monitoring/conftest.py b/tests/manage/monitoring/conftest.py
index 44eda21b..b847955a 100644
--- a/tests/manage/monitoring/conftest.py
+++ b/tests/manage/monitoring/conftest.py
@@ -123,6 +123,9 @@ def measure_stop_ceph_mon(measurement_dir):
     logger.info(f"Monitors to stop: {mons_to_stop}")
     logger.info(f"Monitors left to run: {mons[:split_index]}")
 
+    # run_time of operation
+    run_time = 60 * 14
+
     def stop_mon():
         """
         Downscale Ceph Monitor deployments for 14 minutes. First 15 minutes
@@ -137,8 +140,6 @@ def measure_stop_ceph_mon(measurement_dir):
         Returns:
             str: Names of downscaled deployments
         """
-        # run_time of operation
-        run_time = 60 * 14
         nonlocal oc
         nonlocal mons_to_stop
         for mon in mons_to_stop:
@@ -151,6 +152,9 @@ def measure_stop_ceph_mon(measurement_dir):
     test_file = os.path.join(measurement_dir, 'measure_stop_ceph_mon.json')
     measured_op = measure_operation(stop_mon, test_file)
 
+    # expected minimal downtime of a mon inflicted by this fixture
+    measured_op['min_downtime'] = run_time - (60 * 2)
+
     # get new list of monitors to make sure that new monitors were deployed
     mon_deployments = oc.get(selector=constants.MON_APP_LABEL)['items']
     mons = [
diff --git a/tests/manage/monitoring/prometheusmetrics/test_monitoring_negative.py b/tests/manage/monitoring/prometheusmetrics/test_monitoring_negative.py
index 9f7878ea..5df2ef17 100644
--- a/tests/manage/monitoring/prometheusmetrics/test_monitoring_negative.py
+++ b/tests/manage/monitoring/prometheusmetrics/test_monitoring_negative.py
@@ -23,6 +23,8 @@ def test_monitoring_shows_mon_down(measure_stop_ceph_mon):
     prometheus = PrometheusAPI()
     # time (in seconds) for monitoring to notice the change
     expected_delay = 60
+    # query resolution step used in this test case (number of seconds)
+    query_step = 15
 
     affected_mons = measure_stop_ceph_mon['result']
     # we asked to stop just a single mon ... make this assumption explicit
@@ -38,12 +40,13 @@ def test_monitoring_shows_mon_down(measure_stop_ceph_mon):
         query='ceph_health_status',
         start=measure_stop_ceph_mon['start'],
         end=measure_stop_ceph_mon['stop'],
-        step=15)
+        step=query_step)
     health_validation = check_query_range_result(
         result=health_result,
         good_values=[1],
         bad_values=[0],
         exp_metric_num=1,
+        exp_good_time=measure_stop_ceph_mon['min_downtime'],
         exp_delay=expected_delay)
     health_msg = "health status should be affected by missing mon"
     assert health_validation, health_msg
@@ -53,16 +56,26 @@ def test_monitoring_shows_mon_down(measure_stop_ceph_mon):
         query='ceph_mon_quorum_status{ceph_daemon="%s"}' % ceph_daemon,
         start=measure_stop_ceph_mon['start'],
         end=measure_stop_ceph_mon['stop'],
-        step=15)
+        step=query_step,
+        validate=False)
     mon_validation = check_query_range_result(
         result=mon_result,
         good_values=[0],
         bad_values=[1],
         exp_metric_num=1,
+        exp_good_time=measure_stop_ceph_mon['min_downtime'],
         exp_delay=expected_delay)
-    mon_msg = "ceph_osd_up value should be affected by missing osd"
+    mon_msg = "ceph_mon_quorum_status value should be affected by missing mon"
     assert mon_validation, mon_msg
 
+    # since we don't do strict result validation in the previous query, we
+    # are going to check the min. expected size of the reply explicitly, taking
+    # into account the min. expected downtime of the affected ceph mon
+    assert len(mon_result) == 1, "there should be one metric for one mon"
+    min_mon_samples = measure_stop_ceph_mon['min_downtime'] / query_step
+    mon_sample_size = len(mon_result[0]["values"])
+    assert mon_sample_size >= min_mon_samples
+
 
 @tier3
 @pytest.mark.polarion_id("OCS-1307")