Reshape CredentialRotation Conditions to API conventions

Replace the three Conditions (Rotating, OldPasswordRetained, Ready) with
the convention-compliant trio RotationReady, DiscardReady, and
DualPassword. The old Rotating condition was a present-tense type
whose Reason encoded a workflow state machine, and identifiers such as
NotStarted / Completed carried different meanings across Rotating and
Ready — all explicitly flagged in the PR review as API convention
violations.

Each new Condition is an independent past-tense / adjective / noun
observation:
- RotationReady tracks observedRotationGeneration == spec.rotationGeneration.
- DiscardReady tracks observedDiscardGeneration == spec.discardGeneration.
- DualPassword tracks MySQL's dual-password state. Named to parallel
  Kubernetes conditions such as MemoryPressure, where True describes the
  situation rather than health — avoiding the past-tense surface-form
  mismatch that made the earlier OldPasswordRetained=True read as "retain
  completed" instead of "retain currently in effect."

The workflow step is no longer stored on the CR. A new helper
CredentialRotation.Step() derives it on the fly from the three Conditions
plus the generation comparisons, so callers (Reconciler dispatch,
ClusterManager dispatch, webhook IsIdle/IsAwaitingDiscard, kubectl-moco
CLI) only see a single function. observedRotationGeneration is now
promoted at the end of the rotation phase, so RotationReady can flip True
independently of DiscardReady — a kubectl wait on RotationReady no longer
blocks until discard completes.

The post-distribute StatefulSet rollout wait moves into ClusterManager's
DISCARD handler, eliminating the need for a separate RolloutSettled
Condition. To keep the Reconciler-side DiscardReady transition observable,
ClusterManager blocks on DiscardReady=False/Pending before touching
MySQL — the Reconciler always wins the initial state-set and emits the
DiscardStarted Event. Refused, Blocked, and Stale errors are surfaced via
the Reason field of RotationReady / DiscardReady (single meaning per
identifier across both Ready conditions). The dispatch handles recovery
from any of Initial / Refused / Blocked uniformly by detecting "not yet
DiscardReady=False/Pending".

The design doc is rewritten to match: it explains why three Conditions
satisfy the API conventions, walks through the derived Step matrix,
documents the Reconciler/ClusterManager handshake on DiscardReady, and
updates the crash-safety table for the new boundaries.

Signed-off-by: shunki-fujita <shunki-fujita@cybozu.co.jp>

Author: shunki-fujita

api/v1beta2/credentialrotation_helpers.go

@@ -5,100 +5,201 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// CurrentStep returns the Reason of the Rotating condition if it is True,
-// otherwise the empty string. The empty string indicates the CR is not in
-// an in-flight sub-step (idle, completed, refused, or condition absent).
-func (cr *CredentialRotation) CurrentStep() string {
-	cond := apimeta.FindStatusCondition(cr.Status.Conditions, ConditionRotating)
-	if cond == nil || cond.Status != metav1.ConditionTrue {
-		return ""
+// RotationStep represents the internal workflow step of a rotation cycle.
+// It is derived on the fly from the combination of conditions and the
+// rotationGeneration / discardGeneration comparisons; it is NOT stored on
+// the CR. Persisting a phase enum violates the Kubernetes API
+// conventions, which is why the workflow is represented as a small set
+// of orthogonal Conditions instead.
+type RotationStep string
+
+const (
+	// StepIdle means no work is in flight and no new spec change has
+	// been issued. The CR is either freshly created with conditions not
+	// yet initialised, or the most recent cycle has fully completed.
+	StepIdle RotationStep = "Idle"
+
+	// StepApplyingRetain means ClusterManager is executing or about to
+	// execute ALTER USER ... RETAIN CURRENT PASSWORD on all instances.
+	// Detected when newRotation is true and DualPassword is False
+	// (RETAIN has not yet succeeded for this cycle).
+	StepApplyingRetain RotationStep = "ApplyingRetain"
+
+	// StepDistributingPassword means the Reconciler should distribute
+	// pending passwords to per-namespace Secrets and trigger the
+	// rolling restart. Detected when newRotation is true and
+	// DualPassword is True (RETAIN succeeded; the Reconciler now needs
+	// to publish the new password).
+	StepDistributingPassword RotationStep = "DistributingPassword"
+
+	// StepAwaitingDiscard means the rotation phase is complete and the
+	// CR is waiting for the operator to bump discardGeneration.
+	StepAwaitingDiscard RotationStep = "AwaitingDiscard"
+
+	// StepApplyingDiscard means the discard phase is in flight.
+	// Subsumes the post-distribute StatefulSet rollout wait:
+	// ClusterManager checks the StatefulSet rollout state itself and
+	// defers DISCARD until the rollout has completed.
+	StepApplyingDiscard RotationStep = "ApplyingDiscard"
+
+	// StepFinalizing means DISCARD is complete and the Reconciler
+	// should promote the pending passwords to current in the source
+	// Secret. Detected when newDiscard is true and DualPassword has
+	// flipped back to False.
+	StepFinalizing RotationStep = "Finalizing"
+
+	// StepRotationRefused means the rotation could not start (e.g.
+	// MySQLCluster has 0 replicas). Nothing has been mutated yet, so
+	// the CR is still effectively idle for the purposes of accepting a
+	// new rotationGeneration.
+	StepRotationRefused RotationStep = "RotationRefused"
+
+	// StepRotationBlocked means the rotation phase started but cannot
+	// progress (e.g. MySQLCluster scaled to 0 mid-RETAIN, after pending
+	// passwords were written to the source Secret).
+	StepRotationBlocked RotationStep = "RotationBlocked"
+
+	// StepDiscardRefused means the discard phase could not start (e.g.
+	// MySQLCluster scaled to 0 between AwaitingDiscard and the first
+	// DISCARD). Dual passwords are still held; manual recovery is
+	// required if the operator wants to abandon the cycle.
+	StepDiscardRefused RotationStep = "DiscardRefused"
+
+	// StepDiscardBlocked means the discard phase started but cannot
+	// progress (e.g. MySQLCluster scaled to 0 mid-DISCARD). Partial
+	// DISCARD may have completed; manual recovery is required.
+	StepDiscardBlocked RotationStep = "DiscardBlocked"
+
+	// StepStalePending means the source Secret is in an inconsistent
+	// state; manual recovery is required.
+	StepStalePending RotationStep = "StalePending"
+)
+
+// Step derives the current internal workflow step from spec, status, and
+// conditions. The result is purely a function of the CR's persisted
+// state, so callers can rely on it being stable across reconciles when
+// nothing has changed.
+func (cr *CredentialRotation) Step() RotationStep {
+	// Stale states are stuck and take priority — neither generation
+	// comparison nor sub-step conditions are meaningful while the
+	// source Secret is inconsistent.
+	if ConditionFalseWithReason(cr, ConditionRotationReady, ReasonStale) ||
+		ConditionFalseWithReason(cr, ConditionDiscardReady, ReasonStale) {
+		return StepStalePending
 	}
-	return cond.Reason
-}
 
-// RotatingReason returns the current Reason of the Rotating condition
-// regardless of its Status (or the empty string if the condition is absent).
-// Use this when you need to distinguish terminal Status=False reasons such
-// as Completed or RotationRefused from the absence of the condition; for
-// "is the CR in an active sub-step?" prefer CurrentStep.
-func (cr *CredentialRotation) RotatingReason() string {
-	cond := apimeta.FindStatusCondition(cr.Status.Conditions, ConditionRotating)
-	if cond == nil {
-		return ""
+	// Conditions have never been written → the controller hasn't
+	// reconciled yet. Treat as idle so the very first reconcile can
+	// initialise the cycle via handleStartRotation.
+	if apimeta.FindStatusCondition(cr.Status.Conditions, ConditionRotationReady) == nil {
+		return StepIdle
 	}
-	return cond.Reason
-}
 
-// IsIdle reports whether the CR is in a state that permits starting a new
-// rotation cycle. Idle means the Rotating condition is absent, or has
-// Status=False with one of the terminal idle reasons.
-func (cr *CredentialRotation) IsIdle() bool {
-	cond := apimeta.FindStatusCondition(cr.Status.Conditions, ConditionRotating)
-	if cond == nil {
-		return true
+	newRotation := cr.Spec.RotationGeneration > cr.Status.ObservedRotationGeneration
+	newDiscard := cr.Spec.DiscardGeneration > cr.Status.ObservedDiscardGeneration
+	dualPassword := apimeta.IsStatusConditionTrue(cr.Status.Conditions, ConditionDualPassword)
+
+	if newRotation {
+		if ConditionFalseWithReason(cr, ConditionRotationReady, ReasonRefused) {
+			return StepRotationRefused
+		}
+		if ConditionFalseWithReason(cr, ConditionRotationReady, ReasonBlocked) {
+			return StepRotationBlocked
+		}
+		if !dualPassword {
+			// RETAIN has not yet succeeded for this cycle.
+			return StepApplyingRetain
+		}
+		// RETAIN succeeded; pending passwords need to be distributed.
+		return StepDistributingPassword
 	}
-	if cond.Status != metav1.ConditionFalse {
-		return false
+
+	if newDiscard {
+		if ConditionFalseWithReason(cr, ConditionDiscardReady, ReasonRefused) {
+			return StepDiscardRefused
+		}
+		if ConditionFalseWithReason(cr, ConditionDiscardReady, ReasonBlocked) {
+			return StepDiscardBlocked
+		}
+		if dualPassword {
+			// DISCARD has not yet succeeded; ClusterManager will run
+			// it once the post-distribute rollout has settled.
+			return StepApplyingDiscard
+		}
+		// DISCARD succeeded (DualPassword flipped back to False);
+		// the Reconciler now promotes the pending passwords.
+		return StepFinalizing
+	}
+
+	// Generations match. We are either fully idle, or sitting in the
+	// awaiting-discard steady state (rotation phase done, dual passwords
+	// still held).
+	if dualPassword {
+		return StepAwaitingDiscard
 	}
-	switch cond.Reason {
-	case ReasonNotStarted, ReasonCompleted, ReasonRotationRefused:
+	return StepIdle
+}
+
+// IsIdle reports whether the CR is in a state that permits the operator
+// to start a new rotation cycle by bumping spec.rotationGeneration. This
+// is true when the previous cycle has fully completed (rotation +
+// discard), or when the previous rotation request was refused without
+// any mutations, or when no cycle has been reconciled yet.
+func (cr *CredentialRotation) IsIdle() bool {
+	switch cr.Step() {
+	case StepIdle, StepRotationRefused:
 		return true
 	default:
 		return false
 	}
 }
 
-// IsDeletable reports whether the CR may be deleted without abandoning an
-// actively progressing rotation. The CR is deletable when idle, or when
-// stuck in a state that the documented recovery procedure resolves by
-// deleting the CR (RotationBlocked or StalePending).
+// IsAwaitingDiscard reports whether the CR is in the steady state that
+// follows a completed rotation phase, where the operator may bump
+// spec.discardGeneration to trigger the discard phase.
+func (cr *CredentialRotation) IsAwaitingDiscard() bool {
+	return cr.Step() == StepAwaitingDiscard
+}
+
+// IsDeletable reports whether the CR may be deleted without abandoning
+// an actively progressing cycle. The CR is deletable when:
+//   - idle (no mutations exist),
+//   - the most recent rotation request was Refused without mutations,
+//   - the cycle is stuck in a state that the documented recovery
+//     procedure resolves by deleting the CR (Blocked / Stale, either
+//     phase).
+//
+// AwaitingDiscard and DiscardRefused are NOT deletable: MySQL still
+// holds dual passwords, and a naïve deletion would leave behind state
+// that no controller can recover from. Operators must use the
+// documented recovery procedure (which scales the cluster down to
+// transition into Blocked first).
 func (cr *CredentialRotation) IsDeletable() bool {
-	if cr.IsIdle() {
-		return true
-	}
-	cond := apimeta.FindStatusCondition(cr.Status.Conditions, ConditionRotating)
-	if cond == nil || cond.Status != metav1.ConditionTrue {
-		return false
-	}
-	switch cond.Reason {
-	case ReasonRotationBlocked, ReasonStalePending:
+	switch cr.Step() {
+	case StepIdle,
+		StepRotationRefused,
+		StepRotationBlocked,
+		StepDiscardBlocked,
+		StepStalePending:
 		return true
 	default:
 		return false
 	}
 }
 
-// SetRotating sets or updates the Rotating condition.
-func (cr *CredentialRotation) SetRotating(status metav1.ConditionStatus, reason, message string) {
-	apimeta.SetStatusCondition(&cr.Status.Conditions, metav1.Condition{
-		Type:               ConditionRotating,
-		Status:             status,
-		Reason:             reason,
-		Message:            message,
-		ObservedGeneration: cr.Generation,
-	})
+// SetRotationReady sets or updates the RotationReady condition.
+func (cr *CredentialRotation) SetRotationReady(status metav1.ConditionStatus, reason, message string) {
+	cr.setCondition(ConditionRotationReady, status, reason, message)
 }
 
-// SetOldPasswordRetained sets or updates the OldPasswordRetained condition.
-func (cr *CredentialRotation) SetOldPasswordRetained(status metav1.ConditionStatus, reason, message string) {
-	apimeta.SetStatusCondition(&cr.Status.Conditions, metav1.Condition{
-		Type:               ConditionOldPasswordRetained,
-		Status:             status,
-		Reason:             reason,
-		Message:            message,
-		ObservedGeneration: cr.Generation,
-	})
+// SetDiscardReady sets or updates the DiscardReady condition.
+func (cr *CredentialRotation) SetDiscardReady(status metav1.ConditionStatus, reason, message string) {
+	cr.setCondition(ConditionDiscardReady, status, reason, message)
 }
 
-// SetReady sets or updates the Ready condition.
-func (cr *CredentialRotation) SetReady(status metav1.ConditionStatus, reason, message string) {
-	apimeta.SetStatusCondition(&cr.Status.Conditions, metav1.Condition{
-		Type:               ConditionReady,
-		Status:             status,
-		Reason:             reason,
-		Message:            message,
-		ObservedGeneration: cr.Generation,
-	})
+// SetDualPassword sets or updates the DualPassword condition.
+func (cr *CredentialRotation) SetDualPassword(status metav1.ConditionStatus, reason, message string) {
+	cr.setCondition(ConditionDualPassword, status, reason, message)
 }
 
 // StampObservedGeneration sets status.observedGeneration to the current
@@ -108,3 +209,25 @@ func (cr *CredentialRotation) SetReady(status metav1.ConditionStatus, reason, me
 func (cr *CredentialRotation) StampObservedGeneration() {
 	cr.Status.ObservedGeneration = cr.Generation
 }
+
+func (cr *CredentialRotation) setCondition(condType string, status metav1.ConditionStatus, reason, message string) {
+	apimeta.SetStatusCondition(&cr.Status.Conditions, metav1.Condition{
+		Type:               condType,
+		Status:             status,
+		Reason:             reason,
+		Message:            message,
+		ObservedGeneration: cr.Generation,
+	})
+}
+
+// ConditionFalseWithReason reports whether the named condition is present
+// with Status=False and the given Reason. The Stale / Refused / Blocked
+// reasons that Step() checks all live on Status=False, so this helper
+// captures that pattern.
+func ConditionFalseWithReason(cr *CredentialRotation, condType, reason string) bool {
+	cond := apimeta.FindStatusCondition(cr.Status.Conditions, condType)
+	if cond == nil {
+		return false
+	}
+	return cond.Status == metav1.ConditionFalse && cond.Reason == reason
+}