No-quorum recovery can promote an arbitrary or second Redis master

[bcdonadio]

Affected version

powerhome/redis-operator:v4.5.0 (commit f39b4d433cbf8a32387c765941bdb11c0e7b648f).

Summary

No-quorum recovery can make an unsafe master choice. GetNumberMasters ignores per-node IsMaster errors, so a running master that cannot be inspected can be counted as absent. With a resulting count of zero and no Sentinel quorum, the operator calls SetOldestAsMaster, which selects by Kubernetes creation time rather than replication offset. Replica-reconfiguration failures are logged but can still leave the operation reporting success, allowing stale promotion or multiple masters.

What this blocks

• Claiming fail-closed or split-brain-safe recovery when Sentinel has no quorum.
• Claiming the newest Redis replica is selected after a cold/no-quorum event.
• Automatic recovery on incomplete or failed Redis inspection.
• Any zero-RPO or exactly-one-master guarantee for the affected recovery path.

Reproduction

In a disposable three-Redis/three-Sentinel environment:

1. Establish one master and two replicas with different replication offsets.
2. Remove Sentinel quorum.
3. Make INFO replication fail or time out only from the operator to the current master while leaving that Redis process running.
4. Observe the next reconciliation and query every Redis and Sentinel for the resulting master topology.

Evidence

• GetNumberMasters skips IsMaster errors and counts only successful positive results: check.go#L316-L343.
• Zero known masters plus no Sentinel quorum invokes SetOldestAsMaster: checker.go#L106-L151.
• Candidate selection sorts by Kubernetes CreationTimestamp: heal.go#L104-L130.
• Replica reconfiguration failures are logged but not returned: heal.go#L141-L158.
• IsMaster itself can return an error for an uninspectable running node: client.go#L222-L237.

Actual behavior

• Unknown nodes are treated as though they were known non-masters.
• The oldest pod can be promoted even when another replica contains newer data.
• An existing but uninspectable master can remain writable.
• Failed demotions can leave more than one master.
• The next reconciliation can stop with more than one master, fix manually after divergent writes are already possible.

Expected behavior

Inspection failure must be an unknown state and must fail closed without topology mutation. If an explicit no-quorum recovery mechanism remains, it must select from verified replication state, fence/demote every other node, and fail unless exactly one master is positively confirmed.

Requested fix

• Represent IsMaster error separately from not master.
• Require positive classification of every running Redis node before no-master recovery.
• Select candidates using verified replication offset, not pod age.
• Return failure if any demotion/reconfiguration fails.
• Add partition, stale-replica, no-quorum, auth-error, and transport-error integration tests.

Acceptance criteria

• Any per-node inspection error leaves topology unchanged and raises an observable condition.
• No candidate is selected solely by creation timestamp.
• A recovery is successful only when every other running node is confirmed as a replica of the selected master.
• Tests prove exactly one master after recovery, or a failed state requiring manual intervention.
• Normal quorum-backed Sentinel failover remains covered.

Cross-check roster

The investigation and issue framing were cross-checked using GPT-5.6-Sol, Claude Opus 5, Grok 4.6, and GLM-5.3. Not every model independently executed every reproduction.