From 89cb0fc152ba3c7719353f51d970ff2b8b910293 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Sat, 4 Jul 2026 08:54:17 +0800 Subject: [PATCH] monitoring/ceph-mixin: scope test queries per dashboard get_dashboards_data() keeps every query in a single map keyed by "-", 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'][][]) 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 --- .../features/ceph-cluster.feature | 3 +++ .../tests_dashboards/features/environment.py | 26 ++++++++++++++----- .../features/host-details.feature | 3 +++ .../features/hosts_overview.feature | 3 +++ .../features/osd-device-details.feature | 3 +++ .../features/osds-overview.feature | 3 +++ .../features/radosgw-detail.feature | 3 +++ .../features/radosgw_overview.feature | 3 +++ .../ceph-mixin/tests_dashboards/util.py | 26 +++++++++---------- 9 files changed, 53 insertions(+), 20 deletions(-) diff --git a/monitoring/ceph-mixin/tests_dashboards/features/ceph-cluster.feature b/monitoring/ceph-mixin/tests_dashboards/features/ceph-cluster.feature index c144e52664d..e8522128653 100644 --- 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 921474015c8..4d710c75e70 100644 --- 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 ' + '``" 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 e1a543dab34..ea2b51e2f9c 100644 --- 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 f2945d423dd..4f7e6cc5d45 100644 --- 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 f25167aaf66..09b57835c29 100644 --- 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 cb3bf876464..3c47f406e43 100644 --- 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 db5fb4e9017..89aa728ba7d 100644 --- 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 a34d5759437..697effb2d4a 100644 --- 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 071e7dc2b73..960780da6df 100644 --- 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'][]) 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 -- 2.47.3