fix: Refactor grafana provisioning#5843
Conversation
|
Skipping CI for Draft Pull Request. |
d09655b to
9c0960e
Compare
9c0960e to
a99db21
Compare
a99db21 to
c5803e3
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors Grafana provisioning in the dev-infrastructure pipelines by moving Grafana instance reconciliation out of Bicep and into a templatize pipeline step backed by grafanactl manage, while keeping Bicep responsible for role assignments and related outputs. It also bumps multiple tooling modules to a newer github.com/Azure/ARO-Tools revision that includes the new Grafana manage functionality.
Changes:
- Add a new templatize pipeline step (
GrafanaManageStep) and wire it into the pipeline runner. - Update global/stg global pipelines and add a dev-only Grafana pipeline plus supporting Bicep templates/params for Grafana role assignments.
- Update
output-global.bicepto read Grafana outputs from an existing Grafana resource; bumpARO-Toolsdeps across tooling/test modules.
Reviewed changes
Copilot reviewed 28 out of 38 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| topology.yaml | Registers a new dev-only Grafana serviceGroup pipeline. |
| tooling/yamlwrap/go.mod | Bumps ARO-Tools/tools/yamlwrap dependency revision. |
| tooling/yamlwrap/go.sum | Dependency hash updates for the yamlwrap tool module. |
| tooling/templatize/pkg/pipeline/run.go | Adds execution support for GrafanaManageStep. |
| tooling/templatize/pkg/pipeline/grafana_reconcile.go | Implements the Grafana manage/reconcile step via grafanactl reconcile options. |
| tooling/templatize/go.mod | Bumps ARO-Tools module revisions and adds indirect deps needed by new code paths. |
| tooling/templatize/go.sum | Dependency hash updates corresponding to the new ARO-Tools revision. |
| tooling/secret-sync/go.mod | Bumps ARO-Tools/tools/secret-sync and related indirect deps. |
| tooling/secret-sync/go.sum | Dependency hash updates for secret-sync module. |
| tooling/pipeline-documentation/go.mod | Bumps ARO-Tools/pipelines revision. |
| tooling/pipeline-documentation/go.sum | Dependency hash updates for pipeline-documentation module. |
| tooling/helmtest/go.mod | Bumps ARO-Tools deps used by helmtest. |
| tooling/helmtest/go.sum | Dependency hash updates for helmtest module. |
| tooling/hcpctl/go.mod | Updates cmdutils and armresourcegraph dependency versions. |
| tooling/hcpctl/go.sum | Dependency hash updates for hcpctl module. |
| tooling/grafanactl/main.go | Adds the manage subcommand to the grafanactl CLI wrapper. |
| tooling/grafanactl/go.mod | Bumps grafanactl wrapper deps and adds resourcegraph indirect dep. |
| tooling/grafanactl/go.sum | Dependency hash updates for grafanactl wrapper module. |
| tooling/aro-hcp-exporter/go.mod | Updates armresourcegraph dependency version. |
| tooling/aro-hcp-exporter/go.sum | Dependency hash updates for aro-hcp-exporter module. |
| test/go.mod | Bumps ARO-Tools deps and adds resourcegraph indirect dep for tests module. |
| test/go.sum | Dependency hash updates for test module. |
| mgmt-agent/go.mod | Bumps ARO-Tools/testutil dependency revision. |
| mgmt-agent/go.sum | Dependency hash updates for mgmt-agent module. |
| dev-infrastructure/templates/output-global.bicep | Changes Grafana outputs to read from an existing Grafana resource; still outputs global MSI id. |
| dev-infrastructure/templates/grafana-roles.bicep | New template to apply Grafana-related role assignments (and optional AFD permissions). |
| dev-infrastructure/templates/global-infra.bicep | Removes Grafana provisioning (now handled elsewhere) and adds globalMSIId output; currently contains unresolved merge markers. |
| dev-infrastructure/modules/grafana/integration-lookup.bicep | Removes the prior deployment-script based integration lookup module. |
| dev-infrastructure/modules/grafana/instance.bicep | Converts Grafana resource to existing and limits module responsibilities to RBAC assignments. |
| dev-infrastructure/modules/grafana/grafanaIntegrationsLookup.ps1 | Removes the prior PowerShell resource graph lookup script. |
| dev-infrastructure/grafana-pipeline.yaml | New dev-only pipeline for Grafana reconciliation and RBAC/monitoring-reader setup. |
| dev-infrastructure/global-pipeline.yaml | Adds Grafana manage + roles + monitoring-reader steps; currently contains unresolved merge markers. |
| dev-infrastructure/global-pipeline-stg.yaml | Adds Grafana manage + roles + monitoring-reader steps for STG; currently contains unresolved merge markers. |
| dev-infrastructure/configurations/grafana-roles.tmpl.bicepparam | New param file for grafana-roles template. |
| dev-infrastructure/configurations/grafana-roles-stg.tmpl.bicepparam | New STG param file variant for grafana-roles template. |
| dev-infrastructure/configurations/global-infra.tmpl.bicepparam | Removes Grafana-related params now that global-infra no longer provisions Grafana. |
| dev-infrastructure/configurations/global-infra-stg.tmpl.bicepparam | Removes Grafana-related params now that global-infra no longer provisions Grafana (STG variant). |
| config/config.yaml | Updates Grafana name/version values in defaults and dev cloud config (currently set to test/personal names). |
| - name: output | ||
| action: ARM | ||
| template: templates/output-global.bicep | ||
| parameters: configurations/output-global.tmpl.bicepparam | ||
| deploymentLevel: ResourceGroup | ||
| outputOnly: true | ||
| - name: grafana-reconcile |
|
This requires: Azure/ARO-Tools#260 |
daa0d03 to
3472c82
Compare
3472c82 to
cbb3f8f
Compare
| resource grafana 'Microsoft.Dashboard/grafana@2024-10-01' existing = { | ||
| name: grafanaName | ||
| } | ||
|
|
||
| output grafanaResourceId string = grafana.id | ||
| output grafanaPrincipalId string = grafana.identity.principalId |
This commit re-enables grafana management, previously disabled. With this change bicep steps to manage grafana are removed. This is moved over into grafanactl. The goal is to remove the need to provide input into bicep (AMW[].id, for creating integrations). Also adding a grafana pipeline that can be used to test out grafana changes in dev.
Also: update grafana version to version 12, since 11 is deprecated
b7d7c8f to
44295f2
Compare
| identityFrom: | ||
| resourceGroup: singleton | ||
| step: output | ||
| name: globalMSIId |
raelga
left a comment
There was a problem hiding this comment.
Nice refactor — grafanactl is clearly the right direction, and dropping the PowerShell AMW lookup is exactly what ARO-28038/SFI wants. Two things I think are correctness blockers once this lands on current main (which now includes #5847 / AROSLSRE-1362):
1. STG-global V2 reconcile pins the wrong version → regresses AROSLSRE-1362.
In global-pipeline-stg.yaml, the V2 grafana-reconcile step uses:
grafanaName: "{{ .stgGlobalV2.grafanaName }}" # V2 ✔
majorVersion: "{{ .monitoring.grafanaMajorVersion }}" # shared ✘ — should be stgGlobalV2#5847 added a V2-only stgGlobalV2.grafanaMajorVersion: "13" specifically because the new V2 workspace can't be born on the shared value (Azure retired v11 for new-workspace creation on 2026-06-15; only 12/13 accepted). Sourcing majorVersion from monitoring.grafanaMajorVersion here bypasses that pin — if the shared value resolves to 11 for stg, the new workspace hits the original GrafanaMajorVersionNotSupported again. Suggest pointing this step's majorVersion at {{ .stgGlobalV2.grafanaMajorVersion }}. (This is the re-point #5847's PR body flagged would be needed when creation moved to grafanactl.)
2. Dangling bicep param after rebase — and CI won't catch it.
This PR removes param grafanaMajorVersion from templates/global-infra.bicep, but #5847 (merged) left configurations/global-infra-stg.tmpl.bicepparam assigning param grafanaMajorVersion = '{{ .stgGlobalV2.grafanaMajorVersion }}'. Once rebased on main, that param file assigns a parameter the template no longer declares → the STG-global global deploy fails at rollout with an undeclared-parameter error.
Heads-up that ci/prow/bicep-lint will not flag this: it's az bicep lint --file over .bicep templates/modules only, it filters out *.tmpl.bicepparam, and nothing in the repo runs az bicep build-params, so param↔template drift isn't validated in CI — it only surfaces in the INT/STG EV2 rollout. Please also strip grafanaMajorVersion from global-infra-stg.tmpl.bicepparam and remove the now-orphaned stgGlobalV2.grafanaMajorVersion config field + its required entry in config.schema.json as part of this PR.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: janboll, raelga The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
https://redhat.atlassian.net/browse/ARO-28040
What
This change refactors grafana provisioning and management. Instead of bicep the lifecycle of grafana is managed via grafanactl.
Update to version 12, since version 11 is not available anymore and causes API errors.
Why
Ever since we can not use deployment scripts in bicep anymore, we can not run "side-effects", fetching input from outside the biceps managment scope. But since we for grafana need to lookup all the AMWs from all the subscriptions we need so programmatic way of doing this. Hence move this over to ARO-Tools/Grafanactl and manage the whole lifecycle using it. In that tool we can do the required API calls to fetch the AMWs.
Testing
This was manually tested using the also added Grafana pipeline.
Special notes for your reviewer
PR Checklist
If E2E tests are included:
demonstrate that the test is able to detect a defect/error and fail with
proper error message and logs which communicates nature of the problem.