Repository navigation
Conversation
With componentsDeployment.enabled=true and no metrics store configured, dashboard-service never turns ready and `helm --wait` times out. At startup the data engine looks up the `clickhouse` Component and then the `bigquery` Component, but the dashboard-service-components-reader Role only grants get on `clickhouse`. isComponentExpected counts the Forbidden error as a configured store, so the dashboard picks BigQuery, cannot build it, and /readyz returns 503 forever. Grant get on `bigquery` too. The lookup now returns NotFound and the dashboard starts without a chart data engine. TAG=agy CONV=b6f14b72-3ac1-4d84-ae7e-f90e012997d5
There was a problem hiding this comment.
Code Review
This pull request adds the 'bigquery' resource to the dashboard service's ServiceAccount RBAC rules to allow the data engine to probe the BigQuery component at startup, preventing readiness issues. The associated Helm tests have been updated to assert this change. The reviewer noted that the newly added test 'test_components_rbac_grants_bigquery_component_get' is redundant with an existing test since both use identical input parameters and assertions, and recommended removing the redundant test while merging its descriptive docstring into the existing one.
| def test_components_rbac_grants_bigquery_component_get(self) -> None: | ||
| """The Role also covers the bigquery Component the data engine probes at startup. | ||
|
|
||
| The dashboard counts a forbidden Component lookup as a configured store, so without this | ||
| grant a release with neither ClickHouse nor BigQuery picks BigQuery and never turns ready. | ||
| """ | ||
| manifest = render_template( | ||
| DASHBOARD_SERVICEACCOUNT, | ||
| { | ||
| "dashboardServiceDeployment.enabled": "true", | ||
| "componentsDeployment.enabled": "true", | ||
| "metrics.enabled": "false", | ||
| }, | ||
| ) | ||
| self.assertIn('resourceNames: ["clickhouse", "bigquery"]', manifest) | ||
|
|
There was a problem hiding this comment.
The new test test_components_rbac_grants_bigquery_component_get is redundant with test_components_rbac_rendered_when_components_enabled_even_without_metrics (defined at lines 262-275). Both tests render the template with the exact same input parameters and assert the same resourceNames list containing "clickhouse" and "bigquery". To maintain a clean and DRY test suite, consider removing this redundant test. The detailed explanation of why the bigquery grant is necessary can be incorporated into the docstring of the existing test.
Problem
With
componentsDeployment.enabled=trueand no metrics store configured,dashboard-servicenever turns Ready, sohelm upgrade --install --waittimes out.At startup the dashboard's data engine (
initDataEngineWithGetterincmd/dashboard_service/main.go) looks up theclickhouseComponent and then thebigqueryComponent. Thedashboard-service-components-readerRole only grantsgetonclickhouse, so thebigquerylookup fails with Forbidden.isComponentExpectedcounts any error other than NotFound as a configured store, so the dashboard picks the BigQuery engine, can never build it, and/readyzreturns 503 forever:Existing environments miss this. The KinD e2e setup prunes
dashboard-service, and releases with a metrics store configured never reach the BigQuery lookup because the ClickHouse check succeeds first. I hit it while running the full e2e suite on an AKS island-mode deployment with both components anddashboard-serviceenabled.Fix
Add
bigqueryto the Role'sresourceNames. The lookup now returns NotFound, the dashboard logsClickHouse is not configured, starting without a chart data engine, and the pod turns Ready.isComponentExpectedstill treats Forbidden as configured. Failing closed on API errors is a reasonable default, and this change removes the Forbidden path for the chart's own deployment.Testing
helm lint deploy/helmpython3 -m unittest discover --start-directory test/helm --pattern '*_test.py': 77 tests pass. This updates the threeresourceNamesassertions indashboard_service_clickhouse_config_test.pyand addstest_components_rbac_grants_bigquery_component_get./readyzwent from 503 to ready andhelm --waitcompleted.TAG=agy
CONV=b6f14b72-3ac1-4d84-ae7e-f90e012997d5