Repository navigation
Add dedicated alert for HostedClusters stuck deleting - #7767
Chai-bot (redhat-chai-bot) wants to merge 4 commits into
Conversation
|
Pipeline controller notification This PR uses the pipeline controller for second-stage tests. Selection and triggering follow the repository configuration. Use |
openshift-ci
Bot
added
area/docs
There was a problem hiding this comment.
🔵 Needs a closer look
Human validation is still needed for Azure acceptance of the 48-hour timeout and the metric identity labels collected in the deployed environment.
No open issues 🎉
What changed in this PR
Adds a dedicated severity 4 alert for HostedClusters continuously observed deleting for 48 hours, without changing existing KAS rules.
Changes:
- Adds the deletion metric shared with #7237, deduplicates scrape series while preserving cluster identity, and registers the alert.
- Corrects Azure duration conversion to emit
PT48Hand adds duration and alert regression tests. - Regenerates deployment artifacts and Helm fixtures, and documents observation semantics and triage.
| File | Description |
|---|---|
tooling/prometheus-rules/internal/generator.go |
Converts durations into Azure-compatible hours, minutes, and seconds. |
tooling/prometheus-rules/internal/generator_test.go |
Covers long, compound, and fractional durations. |
observability/prometheus/values-mgmt.yaml |
Declares the HostedCluster deletion Info metric. |
observability/prometheus/testdata/zz_fixture_TestHelmTemplate_helmtest_mgmt_resources.yaml |
Updates the rendered metric fixture. |
observability/prometheus/testdata/zz_fixture_TestHelmTemplate_helmtest_mgmt_resources_unset.yaml |
Updates the unset-values metric fixture. |
observability/alerts/hosted-cluster-deletion-stuck-prometheusRule.yaml |
Defines the per-cluster, 48-hour deletion alert. |
observability/alerts/hosted-cluster-deletion-stuck-prometheusRule_test.yaml |
Tests timing, identity, replica churn, and telemetry gaps. |
observability/alerts-rp-hcps.yaml |
Registers the alert for RP HCP generation. |
docs/prometheus-rules.md |
Links to the deletion alert documentation. |
docs/hosted-cluster-deletion-stuck.md |
Explains semantics, triage, and rollout checks. |
dev-infrastructure/zz_fixture_TestHelmTemplate_dev_westus3_mgmt_1_arohcp_monitor.yaml |
Updates the development monitoring fixture. |
dev-infrastructure/modules/metrics/rules/generatedRPHCPPrometheusAlertingRules.bicep |
Adds the generated Azure alert with PT48H. |
Review effort: Balanced
|
The AI-generated. Review for accuracy. |
openshift-ci
Bot
added
the
area/test
|
Pushed the fixture correction in Validation: uncached AI-generated. Review for accuracy. |
|
Scheduling tests matching the |
| labels: | ||
| component: cluster-deletion | ||
| rules: | ||
| - alert: HostedClusterDeletionStuck |
There was a problem hiding this comment.
RP-lane alerts need the userJourney prefix, so this should be named something like userJourneyClusterDeletionStuck to sit alongside userJourneyClusterDeletionStuckOperation.
There was a problem hiding this comment.
Addressed in 5baf037: renamed the alert to userJourneyClusterDeletionStuck and regenerated the rule output, tests, and alert-diagnostics fixture.
AI-generated. Review for accuracy.
| max by (name, namespace, _id, subscription_id, resource_id, cluster, region) ( | ||
| hostedClusterAPI_deleting{deletion_timestamp!=""} | ||
| ) == 1 | ||
| for: 48h |
There was a problem hiding this comment.
for: 48h counts how long Prometheus has continuously seen the series rather than how long the cluster has been deleting, so any scrape gap or rule redeploy restarts the clock and clusters already stuck at rollout wait another two days; adding a Gauge on [metadata, deletionTimestamp] with nilIsZero: true to the HostedCluster config (KSM v2.20 parses RFC3339 to unix seconds) and alerting on time() - (hostedClusterAPI_deletion_timestamp_seconds > 0) > 172800 with a short for such as 15m measures real deletion age and removes the need for PT48H.
There was a problem hiding this comment.
Addressed in 5baf037: replaced the copied Info declaration with hostedClusterAPI_deletion_timestamp_seconds, a Gauge from metadata.deletionTimestamp with nilIsZero: true. The rule now checks actual age >172800 seconds, with for: 15m, preserving timestamp values through the positive filter and deduplicating replicas by HostedCluster identity. The PR-only duration-converter changes have been removed; those files now match the base.
Ten committed alert scenarios passed. A separate executable probe loading the rendered ConfigMap through upstream KSM v2.20.0 configuration/factory code passed seven cases, including RFC3339 conversion and absent/null timestamps. That probe is outside repository CI and does not verify the deployed image or live metric identity; live verification remains outstanding.
AI-generated. Review for accuracy.
| labels: | ||
| severity: "4" | ||
| annotations: | ||
| summary: "HostedCluster {{ $labels.cluster }}/{{ $labels.namespace }}/{{ $labels.name }} continuously observed deleting for 48h ({{ $labels._id }}, {{ $labels.subscription_id }}, {{ $labels.resource_id }})" |
There was a problem hiding this comment.
The summary should drop {{ $labels.cluster }}, since IcM already adds the cluster to the title.
There was a problem hiding this comment.
Addressed in 5baf037: removed the management-cluster label from the summary. Resource identity remains available for diagnosis.
AI-generated. Review for accuracy.
| annotations: | ||
| summary: "HostedCluster {{ $labels.cluster }}/{{ $labels.namespace }}/{{ $labels.name }} continuously observed deleting for 48h ({{ $labels._id }}, {{ $labels.subscription_id }}, {{ $labels.resource_id }})" | ||
| description: "Resource {{ $labels.resource_id }} (cluster ID {{ $labels._id }}, subscription {{ $labels.subscription_id }}) has been continuously observed with a non-empty deletion timestamp for 48h. This measures observation time, not timestamp age. Check deletion progress and telemetry before concluding deletion succeeded." | ||
| runbook_url: https://aka.ms/cluster-deletion-tsg |
There was a problem hiding this comment.
This alert needs its own entry in the cluster-deletion-tsg Alert Triage table with diagnosis steps and the escalation path, since docs/hosted-cluster-deletion-stuck.md in the repo isn't what on-call opens from the IcM.
There was a problem hiding this comment.
Still outstanding: the external cluster-deletion TSG Alert Triage table has not been updated. The repository documentation now describes the actual-age Gauge behavior, but that is not a substitute for the requested on-call guide entry. Kept this explicitly listed as a remaining dependency in the PR description.
AI-generated. Review for accuracy.
| summary: "HostedCluster {{ $labels.cluster }}/{{ $labels.namespace }}/{{ $labels.name }} continuously observed deleting for 48h ({{ $labels._id }}, {{ $labels.subscription_id }}, {{ $labels.resource_id }})" | ||
| description: "Resource {{ $labels.resource_id }} (cluster ID {{ $labels._id }}, subscription {{ $labels.subscription_id }}) has been continuously observed with a non-empty deletion timestamp for 48h. This measures observation time, not timestamp age. Check deletion progress and telemetry before concluding deletion succeeded." | ||
| runbook_url: https://aka.ms/cluster-deletion-tsg | ||
| correlationId: "HostedClusterDeletionStuck/{{ $labels.cluster }}/{{ $labels.namespace }}/{{ $labels.name }}/{{ $labels._id }}/{{ $labels.subscription_id }}/{{ $labels.resource_id }}" |
There was a problem hiding this comment.
The correlationId should drop resource_id and match the sibling stuck-operation alert with {alertName}/{{ $labels.cluster }}/{{ $labels.subscription_id }}, which keeps hcp-suppress scoped to one customer.
There was a problem hiding this comment.
Addressed in 5baf037: correlation is now userJourneyClusterDeletionStuck/{{ $labels.cluster }}/{{ $labels.subscription_id }}. Removed resource_id from the correlation key while retaining it in per-HostedCluster grouping and diagnostic details. Generated output and expected annotations were updated and validated.
AI-generated. Review for accuracy.
| - alert: HostedClusterDeletionStuck | ||
| expr: | | ||
| max by (name, namespace, _id, subscription_id, resource_id, cluster, region) ( | ||
| hostedClusterAPI_deleting{deletion_timestamp!=""} |
There was a problem hiding this comment.
There's no alert-tester report on this yet, and since the series doesn't exist in any environment, this needs an alert-tester run (docs/alert-verification.md) against int or stage once the KSM change rolls out, showing the new series with the expected identity labels and how many HostedClusters per region would fire, so we know the alert isn't silent and what Sev4 volume to expect in prod.
There was a problem hiding this comment.
Still outstanding: no int/stage deployment or live alert-tester run has been performed. Local rules, generated templates, and the KSM conversion probe passed, but they do not prove live series availability, identity labels, or matching counts per region. The PR description explicitly requires that report after an authorized metric rollout, distinguishing missing metrics from zero matching stuck clusters.
AI-generated. Review for accuracy.
| path: [status, conditions, "[type=ValidAzureKMSConfig]", status] | ||
| labelName: status | ||
| list: ["True", "False", "Unknown"] | ||
| - name: "deleting" |
There was a problem hiding this comment.
Updated this PR only in 5baf037: the copied deleting Info declaration is replaced by the numeric timestamp Gauge. #7237 is unchanged and will need its deletion-suppression consumers updated to use the new metric. The PR description retains the rollout dependency: dedicated stuck-deletion detection must be deployed and verified before relying on KAS suppression. No environment deployment was performed.
AI-generated. Review for accuracy.
|
Thanks for the review. The proposed timestamp Gauge changes the behavior currently documented in this PR: actual deletion age over 48 hours with a short pending period, rather than 48 hours of continuous observation of the Info metric. I’m pausing implementation pending confirmation of that semantic change; I have not switched the metric or threshold behavior yet. The remaining review items are recorded for the same batch:
The existing e2e-parallel failure is still unresolved; its relationship to this change has not been established. AI-generated. Review for accuracy. |
003fbd9 to
d603770
Compare
|
Published the validated rebase onto main ( Resolved duration-converter conflicts while retaining upstream validation fixes and regression coverage. All alert queues, generator tests/lint, chart checks, Bicep validation, and uncached alert-diagnostics corpus tests passed locally. Generated KAS rules match the new base unchanged. The current Info metric / continuous 48-hour observation behavior remains unchanged. The proposed actual-deletion-age change and other review items remain outstanding, as does live environment validation. New-head CI results have not yet been verified. AI-generated. Review for accuracy. |
|
Investigated the new This PR does not modify the affected module manifests or the templatize build system. The evidence supports a dependency-download failure, not a reported configuration mismatch; I have not made speculative code changes or confirmed that the download issue has recovered. A targeted rerun of this check is the next verification step. The e2e failure listed in the same bot comment belongs to the older AI-generated. Review for accuracy. |
Add an independent severity 4 alert after 48 hours of observed deleting state, preserving HostedCluster identity across scrape replicas. Reuse the Info metric contract proposed in public PR Azure#7237 without changing KAS alerts or SLOs. Encode multi-day Prometheus durations as valid ISO time durations for Azure rule groups and cover observation boundaries, telemetry gaps, replica churn, and independent cluster identities.
d603770 to
de0b957
Compare
|
Published the rebase onto The only conflict was the generated alert-diagnostics corpus fixture. Regenerated it through its supported consumer, preserving upstream alerts and adding only the expected 37 lines for this alert. Post-rebase diff review, uncached corpus tests, scoped lint, Helm fixtures/scheduling, formatting, and Bicep build/lint passed. Generated KAS rules match the new base unchanged. The Info metric, severity 4, and continuous 48-hour observation behavior remain unchanged. Review items remain outstanding; this rebase does not implement the proposed actual-deletion-age change. New-head CI and live Azure acceptance have not yet been verified. AI-generated. Review for accuracy. |
There was a problem hiding this comment.
🔵 Needs a closer look
Monitoring-owner validation is needed for Azure’s acceptance of the 48-hour firing timeout and the collected metric’s identity labels, neither of which local tests or Bicep compilation verify.
No open issues 🎉
Review effort: Balanced
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: redhat-chai-bot The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| time() - max by (name, namespace, _id, subscription_id, resource_id, cluster, region) ( | ||
| hostedClusterAPI_deletion_timestamp_seconds > 0 | ||
| ) > 172800 | ||
| for: 15m |
There was a problem hiding this comment.
Rechecked the live PR description: it now specifies hostedClusterAPI_deletion_timestamp_seconds, actual deletion age >48 hours, for: 15m, and the compiled PT15M artifact. The validation section also distinguishes local checks from the still-outstanding external TSG update and live int/stage verification. The description was updated alongside the Gauge publication; no further code change is needed for this mismatch.
AI-generated. Review for accuracy.
|
Scheduling tests matching the |
|
Chai-bot (@redhat-chai-bot): The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Investigated the e2e-parallel failure on
This PR does not change ClusterService application or chart files. However, the available artifacts do not establish why its pods were not ready, so I cannot yet classify this as a transient flake or definitively exclude an indirect relationship. The top-level and provisioning build logs have been removed, and the remaining artifacts lack the relevant application pod logs/events. Next diagnostic evidence needed: clusters-service startup/readiness logs and Kubernetes events for the svc cluster around 03:17–03:28 UTC on October 29. No code changes or retest triggered. AI-generated. Review for accuracy. |
Summary
userJourneyClusterDeletionStuck, a severity 4 alert for HostedClusters whose deletion timestamp is more than 48 hours old, with a 15-minute pending period. Detection is independent of delete-operation records.hostedClusterAPI_deletion_timestamp_secondsas a Gauge frommetadata.deletionTimestampwithnilIsZero: true, replacing the copieddeletingInfo declaration.Alert semantics
The rule uses
for: 15m, notfor: 48h. An already-old deletion must satisfy only the short pending period. Missing telemetry can interrupt pending state and must not be interpreted as successful deletion.Validation
PT15M, severity 4, and correlation ID. Full correction and cumulative diffs reviewed; KAS content unchanged.Remaining rollout and review dependencies
AI-generated. Review for accuracy.
Haoran Wang (@wanghaoran1988) requested from Slack