monitoring/ceph-mixin: scope test queries per dashboard

get_dashboards_data() keeps every query in a single map keyed by
"<panel title>-<legendFormat>", but that id is not unique: several
dashboards have a panel titled "IOPS", "OSDs" or "Throughput". The
dashboard read last overwrites the earlier entry, so one query shadows
another and never gets tested. glob() walks the files in filesystem
order, so which query survives, and whether the test passes, depends on
readdir.

run-tox-promql-query-test hit this in "Test IOPS Read"
(ceph-cluster.feature): the scenario feeds ceph_osd_op_r but the id
resolved to a CephFS pool panel.

    FAILED:
      expr: "sum(rate(ceph_pool_rd{cluster=~"mycluster|",  pool_id=~"UNSET VARIABLE"}[1m]))", time: 1m,
          exp:"{} 2.5E+01"
          got:"nil"
    HOOK-ERROR in after_scenario: AssertionError:

pool_id is "UNSET VARIABLE" because the scenario does not set $mdatapool,
and no ceph_pool_rd series were fed, so the query returns nil. It only
fails when readdir returns cephfsdashboard after ceph-cluster-advanced,
so the earlier change that walked nested rows and made the winner
deterministic did not fix it; it fixed which query wins, not that the
wrong one can win.

Key the queries per dashboard (data['queries'][<dashboard>][<id>]) and
have each .feature name its dashboard in a Background step. The lookup is
confined to that dashboard, so a title/legend used elsewhere no longer
shadows it. A duplicate within one dashboard is still an error, unless it
is in a collapsed row.

ceph-cluster.feature covers ceph-cluster-advanced; the rest map to one
dashboard each.

Signed-off-by: Kefu Chai <k.chai@proxmox.com>

diff --git a/monitoring/ceph-mixin/tests_dashboards/features/ceph-cluster.feature b/monitoring/ceph-mixin/tests_dashboards/features/ceph-cluster.feature
index c144e52664d04ffd07b01d204dc0cc5adeb78221..e852212865310a2e8c2a21e4edda49fac6d3eefe 100644 (file)
--- a/monitoring/ceph-mixin/tests_dashboards/features/ceph-cluster.feature
+++ b/monitoring/ceph-mixin/tests_dashboards/features/ceph-cluster.feature
@@ -1,5 +1,8 @@
 Feature: Ceph Cluster Dashboard
 
+  Background:
+    Given the dashboard is `ceph-cluster-advanced`
+
   Scenario: "Test cluster health"
   Given the following series:
     | metrics                  | values |

diff --git a/monitoring/ceph-mixin/tests_dashboards/features/environment.py b/monitoring/ceph-mixin/tests_dashboards/features/environment.py
index 921474015c8f2e2943eaca6ffe6d6b8284151791..4d710c75e70c964ae2a88968ad641f69d26be893 100644 (file)
--- a/monitoring/ceph-mixin/tests_dashboards/features/environment.py
+++ b/monitoring/ceph-mixin/tests_dashboards/features/environment.py
@@ -15,6 +15,7 @@ class GlobalContext:
         self.promql_expr_test = None
         self.data = get_dashboards_data()
         self.query_map = self.data['queries']
+        self.dashboard = None
 
     def reset_promql_test(self):
         self.promql_expr_test = PromqlTest()
@@ -53,6 +54,7 @@ global_context = GlobalContext()
 
 def before_scenario(context, scenario):
     global_context.reset_promql_test()
+    global_context.dashboard = None
 
 
 def after_scenario(context, scenario):
@@ -63,6 +65,14 @@ def after_all(context):
     global_context.print_query_stats()
 
 
+@given('the dashboard is `{dashboard}`')
+def step_impl(context, dashboard):
+    if dashboard not in global_context.query_map:
+        raise KeyError(f'Unknown dashboard "{dashboard}". Known: '
+                       f'{sorted(global_context.query_map)}')
+    global_context.dashboard = dashboard
+
+
 @given("the following series")
 def step_impl(context):
     for row in context.table:
@@ -111,19 +121,23 @@ def step_impl(context, panel_name, legend):
     """
     if legend == "EMPTY":
         legend = ''
+    if global_context.dashboard is None:
+        raise KeyError('No dashboard selected; add "Given the dashboard is '
+                       '`<name>`" to the feature Background')
     query_id = panel_name + '-' + legend
-    if query_id not in global_context.query_map:
-        print(f"QueryMap: {global_context.query_map}")
-        raise KeyError((f'Query with legend {legend} in panel "{panel_name}"'
-                           'couldn\'t be found'))
+    dashboard_queries = global_context.query_map[global_context.dashboard]
+    if query_id not in dashboard_queries:
+        raise KeyError(f'Query with legend {legend} in panel "{panel_name}" '
+                       f'couldn\'t be found in dashboard '
+                       f'"{global_context.dashboard}"')
 
-    expr = global_context.query_map[query_id]['query']
+    expr = dashboard_queries[query_id]['query']
     global_context.promql_expr_test.set_expression(expr)
     for row in context.table:
         metric = row['metrics']
         value = row['values']
         global_context.promql_expr_test.add_exp_samples(metric, float(value))
-    path = global_context.query_map[query_id]['path']
+    path = dashboard_queries[query_id]['path']
     global_context.data['stats'][path]['tested'] += 1
 
 

diff --git a/monitoring/ceph-mixin/tests_dashboards/features/host-details.feature b/monitoring/ceph-mixin/tests_dashboards/features/host-details.feature
index e1a543dab3465c69df93b846d219f711a60cae8f..ea2b51e2f9c78ad138e60ce109cbe0ae1baa18c3 100644 (file)
--- a/monitoring/ceph-mixin/tests_dashboards/features/host-details.feature
+++ b/monitoring/ceph-mixin/tests_dashboards/features/host-details.feature
@@ -1,5 +1,8 @@
 Feature: Host Details Dashboard
 
+  Background:
+    Given the dashboard is `host-details`
+
 Scenario: "Test OSD"
   Given the following series:
     | metrics | values |

diff --git a/monitoring/ceph-mixin/tests_dashboards/features/hosts_overview.feature b/monitoring/ceph-mixin/tests_dashboards/features/hosts_overview.feature
index f2945d423dde86cd4b3042bc25017f4eab1a0d4a..4f7e6cc5d45b522b708515292da1facb90ee8051 100644 (file)
--- a/monitoring/ceph-mixin/tests_dashboards/features/hosts_overview.feature
+++ b/monitoring/ceph-mixin/tests_dashboards/features/hosts_overview.feature
@@ -1,5 +1,8 @@
 Feature: Hosts Overview Dashboard
 
+  Background:
+    Given the dashboard is `hosts-overview`
+
 Scenario: "Test network load succeeds"
   Given the following series:
     | metrics | values |

diff --git a/monitoring/ceph-mixin/tests_dashboards/features/osd-device-details.feature b/monitoring/ceph-mixin/tests_dashboards/features/osd-device-details.feature
index f25167aaf66799d22acc4cb0234dcc76827eb9d4..09b57835c294f5d2093b0ade21db25b689f06bc6 100644 (file)
--- a/monitoring/ceph-mixin/tests_dashboards/features/osd-device-details.feature
+++ b/monitoring/ceph-mixin/tests_dashboards/features/osd-device-details.feature
@@ -1,5 +1,8 @@
 Feature: OSD device details
 
+  Background:
+    Given the dashboard is `osd-device-details`
+
 Scenario: "Test Physical Device Latency for $osd - Reads"
   Given the following series:
     | metrics | values |

diff --git a/monitoring/ceph-mixin/tests_dashboards/features/osds-overview.feature b/monitoring/ceph-mixin/tests_dashboards/features/osds-overview.feature
index cb3bf87646429e7cc15f33a77949380ff6f10b95..3c47f406e43359aa9ca01baabf8d39135ae6215c 100644 (file)
--- a/monitoring/ceph-mixin/tests_dashboards/features/osds-overview.feature
+++ b/monitoring/ceph-mixin/tests_dashboards/features/osds-overview.feature
@@ -1,5 +1,8 @@
 Feature: OSD Overview
 
+  Background:
+    Given the dashboard is `osds-overview`
+
 Scenario: "Test OSD onode Hits Ratio"
   Given the following series:
     | metrics | values |

diff --git a/monitoring/ceph-mixin/tests_dashboards/features/radosgw-detail.feature b/monitoring/ceph-mixin/tests_dashboards/features/radosgw-detail.feature
index db5fb4e901738c9d2bd91d109b4d53175ca872ba..89aa728ba7d7c06a73672b1aec41e566f853ee0a 100644 (file)
--- a/monitoring/ceph-mixin/tests_dashboards/features/radosgw-detail.feature
+++ b/monitoring/ceph-mixin/tests_dashboards/features/radosgw-detail.feature
@@ -1,5 +1,8 @@
 Feature: RGW Host Detail Dashboard
 
+  Background:
+    Given the dashboard is `radosgw-detail`
+
 Scenario: "Test $rgw_servers GET/PUT Latencies - GET"
   Given the following series:
     | metrics | values |

diff --git a/monitoring/ceph-mixin/tests_dashboards/features/radosgw_overview.feature b/monitoring/ceph-mixin/tests_dashboards/features/radosgw_overview.feature
index a34d5759437835c3a2d99eb3fb7814c2137c4b38..697effb2d4a879d8dc64e8282077b7dae8393e95 100644 (file)
--- a/monitoring/ceph-mixin/tests_dashboards/features/radosgw_overview.feature
+++ b/monitoring/ceph-mixin/tests_dashboards/features/radosgw_overview.feature
@@ -1,5 +1,8 @@
 Feature: RGW Overview Dashboard
 
+  Background:
+    Given the dashboard is `radosgw-overview`
+
 Scenario: "Test Average GET Latencies"
   Given the following series:
     | metrics | values |

diff --git a/monitoring/ceph-mixin/tests_dashboards/util.py b/monitoring/ceph-mixin/tests_dashboards/util.py
index 071e7dc2b73b38012b986f7910bcea3dde016874..960780da6df3975f98ba0e000e0886ee25155bba 100644 (file)
--- a/monitoring/ceph-mixin/tests_dashboards/util.py
+++ b/monitoring/ceph-mixin/tests_dashboards/util.py
@@ -23,8 +23,8 @@ def resolve_time_and_unit(time: str) -> Union[Tuple[int, str], Tuple[None, None]
 
 def get_dashboards_data() -> Dict[str, Any]:
     data: Dict[str, Any] = {'queries': {}, 'variables': {}, 'stats': {}}
-    for file in Path(__file__).parent.parent \
-                              .joinpath('dashboards_out').glob('*.json'):
+    for file in sorted(Path(__file__).parent.parent
+                       .joinpath('dashboards_out').glob('*.json')):
         with open(file, 'r') as f:
             dashboard_data = json.load(f)
             data['stats'][str(file)] = {'total': 0, 'tested': 0}
@@ -39,9 +39,16 @@ def add_dashboard_queries(data: Dict[str, Any], dashboard_data: Dict[str, Any],
     Grafana panels can have more than one target/query, in order to identify each
     query in the panel we append the "legendFormat" of the target to the panel name.
     format: panel_name-legendFormat
+
+    Queries are grouped per dashboard (data['queries'][<dashboard>]) so that
+    different dashboards reusing a panel title/legend, e.g. "IOPS-Read", stay in
+    separate namespaces. Each .feature picks its dashboard so the lookup is
+    unambiguous.
     """
     if 'panels' not in dashboard_data:
         return
+    dashboard = Path(path).stem
+    queries = data['queries'].setdefault(dashboard, {})
     error = 0
     panel_ids_in_file = set()
 
@@ -57,24 +64,15 @@ def add_dashboard_queries(data: Dict[str, Any], dashboard_data: Dict[str, Any],
                 title = panel['title']
                 legend_format = target['legendFormat'] if 'legendFormat' in target else ""
                 query_id = f'{title}-{legend_format}'
-                # Skip duplicates within collapsed rows
-                # but error on duplicates at top-level or across different contexts
+                # a repeated id within one dashboard is expected for a collapsed
+                # row; at top level it means a missing legendFormat
                 if query_id in panel_ids_in_file:
                     if legend_format != '__auto' and not in_row:
                         cprint((f'ERROR: Query in panel "{title}" with legend "{legend_format}"'
                                 f' already exists in the same file: "{path}"'), 'red')
                         error = 1
-                    # Skip adding duplicate
-                    continue
-                # Also check for duplicates across different files
-                # NOTE: This silently skips duplicate panel+legend combinations across dashboards,
-                # which means some queries never get tested. A better approach would be to include
-                # dashboard name in query_id (e.g., "dashboard:panel-legend"), but that requires
-                # updating all test .feature files to specify which dashboard they're testing.
-                if query_id in data['queries']:
-                    # Skip silently for duplicates across files (first one wins)
                     continue
-                data['queries'][query_id] = {'query': target['expr'], 'path': path}
+                queries[query_id] = {'query': target['expr'], 'path': path}
                 data['stats'][path]['total'] += 1
                 panel_ids_in_file.add(query_id)
         # Recurse into row panels