chore(monitoring): remove dead Prometheus rule ConfigMaps - #2092
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
Included review availability: 2 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughThe Helm chart version changed from ChangesMonitoring chart update
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR removes unused Prometheus rule ConfigMap rendering without changing the active alerting configuration, so no actionable merge-blocking risk remains after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
|
":exclamation: THIS PULL REQUEST WILL NEED TO BE DELIVERED MANUALLY ❗
|
|
":exclamation: THIS PULL REQUEST WILL NEED TO BE DELIVERED MANUALLY ❗
|
450cf5d to
655fa5e
Compare
|
":exclamation: THIS PULL REQUEST WILL NEED TO BE DELIVERED MANUALLY ❗
|
|
":exclamation: THIS PULL REQUEST WILL NEED TO BE DELIVERED MANUALLY ❗
|
create-staging-pr and create-manual-merge-prod-pr both depend only on
build-helmreleases, so they run concurrently, and both derived
branch_name="${repo_name}/${GITHUB_SHA::8}" -- the is_staging branch and
the else branch of prepare-pr-metadata were identical. Both jobs then
raced to push the same ref into gitops-deployments and the loser failed
with "cannot lock ref ...: reference already exists", taking the gate
with it.
This only fires when a helm chart changes: with charts == '[]',
helm-charts-build skips, build-helmreleases skips, and both PR jobs skip
on build-helmreleases.result == 'success'. The last change under
k8s/charts/ landed in 2021, four years before this pipeline existed, so
the pair had never both run until PR gooddata#2092 bumped lcm-bricks to 2.0.9 and
the gate failed twice in the merge queue.
Suffix the manual-merge prod branch with _prod, following the convention
the auto-merge variant already uses (_${cluster}_automerge), and say
"for all production clusters" in the commit message and PR title so the
staging and prod PRs are distinguishable. No consumer matches on the
title: revert-staging-pr keys off commit_hash and create-pr-comment off
pr_url.
JIRA: GRIF-518
|
":exclamation: THIS PULL REQUEST WILL NEED TO BE DELIVERED MANUALLY ❗ |
These ConfigMaps are not read by anything. The last Prometheus able to load ConfigMap-based rules was deleted in 2023 (INFRA-2151). Alerting for this app is evaluated centrally by the Mimir ruler from PrometheusRule CRs in k8s-infra crs/kustomizations/mimir-rules-global/bear/. JIRA: GRIF-518 risk: low
655fa5e to
11e5a57
Compare
|
":exclamation: THIS PULL REQUEST WILL NEED TO BE DELIVERED MANUALLY ❗ |
|
":exclamation: THIS PULL REQUEST WILL NEED TO BE DELIVERED MANUALLY ❗ |
build-helmreleases drops a cluster from its output when the target
HelmRelease already matches the built chart ("There is nothing to update
for chart lcm-bricks in cluster 62, skipping..."). create-staging-pr and
create-manual-merge-prod-pr ran anyway, produced an empty commit
("Working tree clean. Nothing to commit.", committed: false) and then died
in gh pr create:
pull request create failed: GraphQL: ... No commits between master and
gooddata-ruby/091e94de, Head ref must be a branch
This is reachable whenever a previous attempt already delivered part of the
change. It happened on PR gooddata#2092: the 11:00Z attempt created and merged its
staging PR (gitops-deployments#26557) before the run stalled on runner
availability and the merge queue ejected the PR, so the retry found stg1
already on 2.0.9 and could not open a staging PR at all.
Gate the PR-creating steps on the commit actually having been made. A
push that fails still fails the step, so committed == 'true' only filters
the genuine no-op. Emit a step summary in that case rather than passing
silently.
Two downstream consumers had to follow, since an empty output is now
reachable where it previously was not:
- revert-staging-pr would run `git revert --no-commit ""` on an empty
commit_hash if e2e failed after a no-op staging step; require a
non-empty commit_hash.
- create-pr-comment advertised "PR for all clusters -> " with a blank URL;
say no delivery is needed instead. Also quote the assignment.
JIRA: GRIF-518
wait-for-deployment builds its matrix from build-helmreleases.outputs.staging_helmreleases, and create-auto-merge-prod-pr from updated_prod_cluster_names. Neither checked the value for '[]' first, unlike every other matrix job in this file (services-build and helm-charts-build both guard on != '[]' && != ''). A matrix that expands to zero combinations cannot produce a job, so the reusable-workflow call reports failure. On PR gooddata#2092 that surfaced as a gate failure with no failing job anywhere: 15 of the pipeline's 16 jobs were success or skipped, wait-for-deployment had no check run at all, and alls-green received gooddata-ruby-lcm-promote-pipeline result=failure with empty outputs. The trigger was gooddata#2094. Before it, create-staging-pr failed outright when there was nothing to deploy, so wait-for-deployment's !contains(needs.*.result, 'failure') was false and the job was skipped before its matrix was ever evaluated -- it showed up as 'skipped' in run 32728855771. Once create-staging-pr correctly succeeded on a no-op, the condition passed, the matrix was evaluated for the first time in this state, and it was empty. gooddata#2094 did not introduce this, it removed the failure that was masking it. staging_helmreleases is empty whenever stg1 already runs the built chart, which is the normal state after a previous attempt delivered its staging PR -- gitops-deployments#26557 in this case. create-auto-merge-prod-pr carries the same defect. It is latent here because the gate sets AUTO_MERGE: false, but it is live for any consumer that auto-merges into an already up-to-date production cluster. JIRA: GRIF-518
3 participants
Closes work for GRIF-518.
These
monitoring-rules.yaml/alertingRules.yamlConfigMaps are not read by anything:PrometheusRuleCRD. The kube-prometheus-stack ink8s-infrahas noruleFiles, noadditionalRulesConfigMapsand no rules sidecar, and Prometheus Operator never reads arbitrary ConfigMaps.configmap-reload+/etc/prometheus-rules) was deleted back in 2023 by INFRA-2151.monitoring-namespace PrometheusRules. Nothing from the app namespaces, nothing ConfigMap-sourced.Alerting for these apps is evaluated centrally by the Mimir ruler from
k8s-infra/crs/kustomizations/mimir-rules-global/bear/(thegeneric-<team>-rules.yamlfiles plus the app-specific ones).Removed
k8s/charts/lcm-bricks/templates/prometheus/alertingRules.yamlOther changes
k8s/charts/lcm-bricks/Chart.yamlChart versions are bumped so the CD pipeline accepts the rebuild. Values keys that existed only to feed the rules are dropped;
clusterIdis kept wherever another template still uses it.No alerting change.
k8s-infrais untouched, so the ruler's rule set is identical before and after.🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Monitoring