Skip to content

monitor: retain rule storage version across initializer upgrades - #7138

Closed
csuzhangxc wants to merge 1 commit into
pingcap:release-1.xfrom
csuzhangxc:codex/preserve-monitor-rule-version
Closed

csuzhangxc wants to merge 1 commit into
pingcap:release-1.xfrom
csuzhangxc:codex/preserve-monitor-rule-version

Conversation

@csuzhangxc

@csuzhangxc csuzhangxc commented Nov 5, 2026 •

Copy link
Copy Markdown
Member

Updating a TidbMonitor initializer image tag currently also changes the default alert-rule persistence directory. For example, saved rules under tidb:v8.5.7 stop being restored after an initializer-only security tag change, even though their files remain on the PVC.

Keep the effective rules storage version independent of initializer images. Before migrating or rolling out workloads, record it in a controller-owned TidbMonitor annotation. For existing monitors, recover the exact reloader --sub-store-path from owned StatefulSets or legacy Deployments through the live API; for new monitors, initialize it from the first initializer version. The record survives workload replacement and is shared by TiDB initialization, DM initialization and the reloader.

spec.alertManagerRulesVersion remains authoritative. An explicit selection updates the record; clearing the field afterwards retains the last selected directory. No new CRD field or schema upgrade is needed. Update the API description, generated reference and advanced example, and document upgrade behavior in docs/monitor-rule-version.md.

If owned shards disagree, a custom command hides the version, or prior workload status exists but the workload is gone, stop and require an explicit rules version. API read failures or a resourceVersion-guarded annotation write failure prevent workload rollout. Reading the live API avoids treating an informer cache miss as a new deployment.

Related: pingcap/monitoring#1244. Upgrade this Operator before changing initializer tags, while existing workloads are available for version recovery. Older Operators still require manually pinning alertManagerRulesVersion. If an older controller has already switched directories, explicitly select the old version to recover those rules; this change does not scan PVC contents or merge rule files.

Validation:

  • The image-upgrade/recreation regression fails with the original directory-selection logic and passes with this change.
  • Full Monitor reconcile tests verify the new initializer image uses the old rules directory and failed recording prevents StatefulSet creation. Coverage includes legacy Deployment recovery, cache misses, foreign owners, shard agreement/conflicts, TiDB/DM/reloader path consistency, explicit set/clear, missing prior workloads, flag forms and current resourceVersion use after finalizer updates.
  • go test -race ./pkg/monitor/monitor ./pkg/controller/tidbmonitor, matching go vet, and Linux amd64 controller-manager build pass.
  • Live kind v1.31.2 API smoke test with real TidbMonitor, StatefulSet and Deployment objects: recovered tidb:v8.5.7 after an image-tag update, retained it after workload deletion, honored explicit selection/clearing, and rejected stale annotation writes with HTTP 409 while preserving other annotations. This tests the reconciliation helper against the API; a full controller/Prometheus deployment was not run for this PR.
  • OpenAPI and API reference generation, new-file boilerplate, formatting/diff checks and independent review pass. The temporary kind cluster was removed.

Summary by CodeRabbit

  • New Features
    • Alert-rule persistence versions are now independent of initializer image versions. When no version is specified, existing monitors retain the version used by their workloads, while new monitors use the initializer version.
    • Explicitly selecting a version changes the rules directory; removing the selection retains the last recorded version.
    • Reconciliation stops when the version cannot be reliably determined or recorded, rather than guessing.
  • Documentation
    • Added guidance on version selection, recovery scenarios, and migration considerations.

@ti-chi-bot

ti-chi-bot Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign qiffang for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot
ti-chi-bot Bot requested a review from shonge November 5, 2026 03:58
@ti-chi-bot ti-chi-bot Bot added the size/XL label Nov 5, 2026
@coderabbitai

coderabbitai Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Explain Change →

Caution

Review failed

The pull request is closed.

Walkthrough

The change records the effective alert-rule persistence version on a TidbMonitor. The operator can recover it from owned workloads, retain it across initializer image updates, or use an explicitly configured value. Reconciliation records the version before syncing the StatefulSet.

Changes: Monitor Rules Version Persistence

Layer / File(s) Summary
Rules-version contract
pkg/apis/label/label.go, pkg/apis/pingcap/v1alpha1/tidbmonitor_types.go, pkg/apis/pingcap/v1alpha1/openapi_generated.go, examples/advanced/tidb-monitor.yaml, docs/api-references/docs.md, docs/monitor-rule-version.md
The API descriptions and guide document explicit version selection, preservation of the recorded version when unset, and the initializer version used for a new monitor. The annotation key identifies the recorded version.
Version selection and recovery
pkg/monitor/monitor/util.go, pkg/monitor/monitor/rules_version.go, pkg/monitor/monitor/rules_version_test.go
Version selection uses the explicit field first, then the annotation, then the initializer version. When both the field and annotation are unset, recovery reads owned workloads and parses the reloader rules directory. Tests cover selection, recovery, and error cases.
Reconciliation and annotation recording
pkg/monitor/monitor/monitor_manager.go, pkg/monitor/monitor/monitor_manager_test.go, pkg/monitor/monitor/rules_version_test.go
Reconciliation records the selected version before StatefulSet synchronization and stops if recording fails. The finalizer update refreshes the in-memory resource version. Tests cover recording before rollout and patching with the current resource version.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant SyncMonitor
  participant syncMonitorRulesVersion
  participant KubernetesAPI
  participant StatefulSetSync
  SyncMonitor->>syncMonitorRulesVersion: select and record rules version
  syncMonitorRulesVersion->>KubernetesAPI: list workloads when recovery is needed
  KubernetesAPI-->>syncMonitorRulesVersion: return owned workload versions
  syncMonitorRulesVersion->>KubernetesAPI: patch annotation with resource version
  syncMonitorRulesVersion-->>SyncMonitor: return selected version or error
  SyncMonitor->>StatefulSetSync: sync after successful recording
Loading

Suggested reviewers: fgksgf

Merge Risk: Low · up to 4c179

The version-retention flow has bounded risks involving initializer overrides, failure visibility, and admission-mutated settings. These warrant owner awareness or localized fixes; no broad rollout or data-loss failure is established.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

ast-grep (0.51.1)

pkg/apis/pingcap/v1alpha1/openapi_generated.go

ast-grep timed out on this file

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

A rabbit checks the rules at dawn,
And marks the version to carry on.
Through image changes, tags may roam,
The saved rules path still guides them home.
I thump my paws and nibble greens,
While monitors keep their versions keen.

@csuzhangxc csuzhangxc closed this Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant