Skip to content

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

Merged
weicao merged 1 commit into
mainfrom
feature/ops-s1-slice-a-reason-constants
May 9, 2026
Merged

chore(ops): centralize OpsRequest condition reason literals and fix message typos#10195
weicao merged 1 commit into
mainfrom
feature/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.

Note: this PR replaces an earlier draft (#10194) that was closed when the source branch was renamed to satisfy the repository's branch-name pre-check (the original branch used feat/ instead of the required feature/ prefix). The code diff and earlier review evidence are unchanged. After reopening, commit metadata was amended to use a human CLA author; the current head is ac84e2257.

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.

@weicao weicao requested review from a team and wangyelei as code owners May 5, 2026 09:19
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 5, 2026

CLA assistant check
All committers have signed the CLA.

@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
@weicao
Copy link
Copy Markdown
Contributor Author

weicao commented May 5, 2026

QA review pass carried over from #10194.

This PR uses the accepted branch prefix (feature/...) with the same commit 80381b019; I rechecked that the Slice A scope remains unchanged: no behavior change, Reason string values pinned by tests, constructor reasons point at constants, and the three typo fixes are user-visible wording only.

Original QA review trail: #10194 (comment)

@weicao
Copy link
Copy Markdown
Contributor Author

weicao commented May 5, 2026

Review trail (carried over from closed PR #10194)

This PR is the same commit (80381b019) as #10194, re-opened from a feature/ branch to satisfy the repository's branch-name pre-check. The earlier review evidence on #10194 still applies:

Local validation evidence

  • go vet ./apis/operations/v1alpha1/... ./pkg/operations/... — clean
  • go test ./apis/operations/v1alpha1/... — 4 tests including 2 new drift guards, all pass
  • go test ./pkg/operations/... — full Ginkgo suite with envtest 1.26.1, 28.8s, all pass
  • go test ./pkg/operations/util/... — 8.8s, pass
  • go test ./controllers/operations/... — 17.9s, pass
  • Repo-wide grep for the three typo tokens (modifeied, availabled, cannot found) — 0 matches

The only outstanding gate on #10194 was the branch-name pre-check; that is resolved by this re-opened PR.

@weicao
Copy link
Copy Markdown
Contributor Author

weicao commented May 5, 2026

@CLAassistant recheck

@weicao
Copy link
Copy Markdown
Contributor Author

weicao commented May 5, 2026

recheckcla

@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

❌ Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.73%. Comparing base (8d4e577) to head (ac84e22).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
pkg/operations/custom.go 0.00% 1 Missing ⚠️
pkg/operations/rebuild_instance.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10195      +/-   ##
==========================================
+ Coverage   52.27%   52.73%   +0.45%     
==========================================
  Files         531      531              
  Lines       60705    61062     +357     
==========================================
+ Hits        31736    32198     +462     
+ Misses      25748    25646     -102     
+ Partials     3221     3218       -3     
Flag Coverage Δ
unittests 52.73% <89.47%> (+0.45%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@weicao
Copy link
Copy Markdown
Contributor Author

weicao commented May 5, 2026

@CLAassistant recheck

@apecloud-bot apecloud-bot added the approved PR Approved Test label May 6, 2026
@cjc7373 cjc7373 added the pick-1.1 Auto cherry-pick to release-1.1 when PR merged label May 7, 2026
@cjc7373
Copy link
Copy Markdown
Contributor

cjc7373 commented May 7, 2026

/recheck-cla

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026

Recheck CLA triggered.

…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.

Prepared with Ava assistance.
@weicao weicao force-pushed the feature/ops-s1-slice-a-reason-constants branch from 80381b0 to ac84e22 Compare May 8, 2026 15:26
@apecloud-bot apecloud-bot removed the approved PR Approved Test label May 8, 2026
@wangyelei
Copy link
Copy Markdown
Contributor

/approved

@apecloud-bot apecloud-bot added the approved PR Approved Test label May 9, 2026
@weicao weicao merged commit fdb94ca into main May 9, 2026
37 checks passed
@weicao weicao deleted the feature/ops-s1-slice-a-reason-constants branch May 9, 2026 03:13
@github-actions github-actions Bot added this to the Release 1.2.0 milestone May 9, 2026
@apecloud-bot
Copy link
Copy Markdown
Collaborator

/cherry-pick release-1.1

@apecloud-bot
Copy link
Copy Markdown
Collaborator

🤖 says: Error cherry-picking.

Auto-merging pkg/operations/custom.go
Auto-merging pkg/operations/rebuild_instance.go
CONFLICT (content): Merge conflict in pkg/operations/rebuild_instance.go
error: could not apply fdb94ca... chore(ops): centralize OpsRequest condition reason literals and fix message typos (#10195)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
hint: Disable this message with "git config set advice.mergeConflict false"

@apecloud-bot
Copy link
Copy Markdown
Collaborator

🤖 says: ‼️ cherry pick action failed.
See: https://github.com/apecloud/kubeblocks/actions/runs/25590152362

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

Labels

approved PR Approved Test pick-1.1 Auto cherry-pick to release-1.1 when PR merged size/L Denotes a PR that changes 100-499 lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants