Skip to content

chore(ops): centralize OpsRequest condition reason literals and fix message typos#10194

Closed
weicao wants to merge 1 commit into
mainfrom
feat/ops-s1-slice-a-reason-constants
Closed

chore(ops): centralize OpsRequest condition reason literals and fix message typos#10194
weicao wants to merge 1 commit into
mainfrom
feat/ops-s1-slice-a-reason-constants

Conversation

@weicao
Copy link
Copy Markdown
Contributor

@weicao weicao commented May 5, 2026

Summary

No behavior change. Centralizes the inline Reason string literals in apis/operations/v1alpha1/opsrequest_conditions.go into named constants in the same file, and corrects three user-visible message typos in pkg/operations.

This is the first slice of the OpsRequest status reason model work; it intentionally adds nothing else (no new vocabulary, no catalog/registry, no phase changes, no evidence backfill). Subsequent slices will introduce the structured anchor reasons and the catalog on top of these constants.

What changed

Reason constants centralized

The NewXxxCondition constructors previously set Condition.Reason to inline string literals (e.g. "VolumeExpansionStarted"). The literal values are unchanged; they are now declared once in the existing constants block and referenced from each constructor. 16 new exported constants:

Constant Value
ReasonValidateOpsRequestPassed ValidateOpsRequestPassed
ReasonOpsRequestProcessedSuccessfully OpsRequestProcessedSuccessfully
ReasonRestartStarted RestartStarted
ReasonStartToRebuildInstances StartToRebuildInstances
ReasonSwitchoverStarted SwitchoverStarted
ReasonVerticalScalingStarted VerticalScalingStarted
ReasonHorizontalScalingStarted HorizontalScalingStarted
ReasonVolumeExpansionStarted VolumeExpansionStarted
ReasonExposeStarted ExposeStarted
ReasonVersionUpgradeStarted VersionUpgradeStarted
ReasonStopStarted StopStarted
ReasonStartCluster StartCluster
ReasonReconfigureStarted ReconfigureStarted
ReasonReconfigureFailed ReconfigureFailed
ReasonBackupStarted BackupStarted
ReasonRestoreStarted RestoreStarted

Typo fixes (English grammar in user-visible failure messages)

File:Line Before After
pkg/operations/rebuild_instance.go:112 instance "%s" is availabled, can not rebuild it instance "%s" is available, can not rebuild it
pkg/operations/rebuild_instance.go:491 the replicas of the component "%s" has been modifeied by another operation the replicas of the component "%s" has been modified by another operation
pkg/operations/custom.go:305 cannot found the component "%s" in cluster "%s" cannot find the component "%s" in cluster "%s"

These messages are surfaced via intctrlutil.NewFatalError and end up in OpsRequest.status and reconciler logs, so they are externally visible.

New tests

  • TestReasonConstantsStringDrift pins the literal value of every Reason constant. A silent rename or string change fails the test, defending consumers that match on the Reason string value rather than the Go identifier.
  • TestNewConditionReasonsMatchConstants verifies that each NewXxxCondition constructor returns a Condition whose Reason equals the centralized constant. Defends against an accidental in-place edit that re-introduces an inline string literal.

Test plan

  • go test ./apis/operations/v1alpha1/... (4 tests, including the two new ones) — passes
  • go test ./pkg/operations/util/... — passes (8.8s)
  • go test ./pkg/operations/... (full Ginkgo suite, envtest 1.26.1) — passes (28.8s)
  • go test ./controllers/operations/... — passes (17.9s)
  • go vet ./apis/operations/v1alpha1/... ./pkg/operations/... — clean
  • Repo-wide search for the three typo tokens (modifeied, availabled, cannot found) returns zero matches after this change.

Out of scope (subsequent slices)

  • Adding new structured anchor reasons (e.g. for queue-blocked, precheck-failed, pod/job-failed, kbagent-action-failed, dependent-operation-failed, timeout-waiting-for-ready). These are introduced in a follow-up alongside a new reason catalog/registry file.
  • Phase change for queue-full handling (currently Failed; tracked separately).
  • Evidence backfill at terse-message paths (backup.go, restore.go, rebuild_instance_inplace.go, ops_manager.go).
  • A make verify lint analyzer enforcing registered reasons and required evidence.

…essage typos

No behavior change. The Reason string values emitted at runtime are
preserved exactly; this commit only replaces the inline string literals
in apis/operations/v1alpha1/opsrequest_conditions.go with named
constants in the same file, and corrects three message typos in
pkg/operations.

Why
- Console, integration tests, and an upcoming reason catalog/registry
  match on the Reason string value rather than the Go identifier. A
  silent rename would break consumers without a compile-time signal.
- The same Reason values are referenced in multiple files, so a single
  authoritative declaration prevents drift.

Reason constants centralized
- ReasonValidateOpsRequestPassed = "ValidateOpsRequestPassed"
- ReasonOpsRequestProcessedSuccessfully = "OpsRequestProcessedSuccessfully"
- ReasonRestartStarted = "RestartStarted"
- ReasonStartToRebuildInstances = "StartToRebuildInstances"
- ReasonSwitchoverStarted = "SwitchoverStarted"
- ReasonVerticalScalingStarted = "VerticalScalingStarted"
- ReasonHorizontalScalingStarted = "HorizontalScalingStarted"
- ReasonVolumeExpansionStarted = "VolumeExpansionStarted"
- ReasonExposeStarted = "ExposeStarted"
- ReasonVersionUpgradeStarted = "VersionUpgradeStarted"
- ReasonStopStarted = "StopStarted"
- ReasonStartCluster = "StartCluster"
- ReasonReconfigureStarted = "ReconfigureStarted"
- ReasonReconfigureFailed = "ReconfigureFailed"
- ReasonBackupStarted = "BackupStarted"
- ReasonRestoreStarted = "RestoreStarted"

Typo fixes (user-visible failure messages only; English grammar)
- pkg/operations/rebuild_instance.go:112 "is availabled" -> "is available"
- pkg/operations/rebuild_instance.go:491 "has been modifeied" -> "has been modified"
- pkg/operations/custom.go:305 "cannot found the component" -> "cannot find the component"

Tests
- New TestReasonConstantsStringDrift pins the literal value of every
  Reason constant; a future rename or string change will fail the test.
- New TestNewConditionReasonsMatchConstants verifies each NewXxxCondition
  constructor sets Condition.Reason to the centralized constant.
- Existing TestNewAllCondition and TestSetStatusCondition continue to
  pass.
- pkg/operations and controllers/operations Ginkgo suites run green
  with envtest assets.
@weicao weicao requested review from a team and wangyelei as code owners May 5, 2026 09:14
@apecloud-bot
Copy link
Copy Markdown
Collaborator

Auto Cherry-pick Instructions

Usage:
  - /nopick: Not auto cherry-pick when PR merged.
  - /pick: release-x.x [release-x.x]: Auto cherry-pick to the specified branch when PR merged.

Example:
  - /nopick
  - /pick release-1.1

@github-actions github-actions Bot added the size/L Denotes a PR that changes 100-499 lines. label May 5, 2026
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Ava seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@apecloud-bot
Copy link
Copy Markdown
Collaborator

This branch name is not following the standards: feature/|bugfix/|release/|hotfix/|support/|releasing/|dependabot/

@weicao weicao closed this May 5, 2026
@weicao weicao deleted the feat/ops-s1-slice-a-reason-constants branch May 5, 2026 09:17
@github-actions github-actions Bot added this to the Release 1.2.0 milestone May 5, 2026
@weicao
Copy link
Copy Markdown
Contributor Author

weicao commented May 5, 2026

QA review pass for Slice A.

Checked scope against OPS-S1 v1 boundary:

  • Reason values are centralized without string drift; constructors now point at constants.
  • Added drift guards cover both constant literal values and constructor Reason values.
  • Typo fixes are user-visible message cleanup only; no phase/control-flow/evidence behavior change.
  • custom.go:305 wording cannot find the component is acceptable and clearer than a mechanical not found replacement.

Local spot validation:

  • go test ./apis/operations/v1alpha1/... passes.
  • rg "modifeied|availabled|cannot found" . returns no matches.

Only remaining blocker I see is the GitHub branch-name gate (feat/... should become feature/...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Denotes a PR that changes 100-499 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants