fix: controller fixes (slot guard, Pending-recovery, NodePort webhook, User phase key) - #101
Open
chideat wants to merge 4 commits into
Open
fix: controller fixes (slot guard, Pending-recovery, NodePort webhook, User phase key)#101chideat wants to merge 4 commits into
chideat wants to merge 4 commits into
Conversation
The engine judged shard master/slot presence via the IsJoined-gated accessors shard.Master()/shard.Slots(), which return nil/empty when a node can't currently see a peer. After a full restart a master that still locally owns its slots but can't reach its peers was therefore treated as masterless/slotless, triggering destructive EnsureSlots/Rebalance (ADDSLOTS, FAILOVER, SETSLOT) that created epoch/slot conflicts and prevented reconvergence (risking split-brain). Add ungated predicates (ShardAssignedSlots, ShardHasMaster, IsDisconnectedButIntact) in internal/ops/cluster/predicate.go that judge ownership from the master node's real local slots regardless of peer connectivity, and base the reassignment decision on actual ownership: a disconnected-but-intact shard waits to re-MEET instead of reassigning slots it already holds. Gated in engine check_4 and the ensure_slots / join_node / rebalance actors. Ported from redis-operator 7e6f6910 (RedisRole->NodeRole, type name adaptation).
…wnscale An instance created with oversized resources (all pods Pending) never recovered after patching spec.resources down: with Pending pods the engine loops in HealPod / HealMonitor and never reaches EnsureResource (the only actor that updates the StatefulSet), so the desired resources never reach the template and the pods stay Pending forever. - cluster HealPod: always fall through to CommandEnsureResource, and delete stale-revision Pending pods so the StatefulSet recreates them from the updated template. valkey's cluster StatefulSet uses ParallelPodManagement + RollingUpdate, so the comment reflects that (not redis's OrderedReady premise). - failover HealMonitor: return CommandEnsureResource instead of idle-waiting when no usable master candidate exists (every node not-ready). The failover StatefulSet uses ParallelPodManagement, so refreshing the template reschedules the Pending pods. Covered by actor_heal_monitor_test.go. Ported from redis-operator d77f64b8 (cluster) and 9ae8f422 (failover).
A second instance requesting a NodePort already in use by another instance was admitted, then silently failed to reconcile (the API server rejects the duplicate NodePort Service) while the CR stayed Initializing rather than surfacing a clear error. The Valkey validating webhook now checks requested NodePorts (data + sentinel Access.Ports) against existing NodePort Services cluster-wide and denies the request when one is already allocated by a different instance. NodePorts are cluster-scoped, so it lists Services across all namespaces via an uncached reader (mgr.GetAPIReader); Services belonging to the same instance (same namespace + instance-name label) are excluded so updating an existing NodePort instance is not rejected by its own Services. ValidateUpdate delegates to ValidateCreate, so the check covers create and update. Covered by valkey_webhook_nodeport_test.go. Ported from redis-operator 067e1954 (AccessPort/NodePortSequence -> Access.Ports).
…N key UserStatus.Phase used the JSON tag `Phase` (capital P), so `.status.Phase` was invisible to clients/tooling reading the conventional `.status.phase` (kubectl, printer columns, scripts). Retag the canonical field `json:"phase"` and keep the legacy capital key as a new `OldPhase` field tagged `json:"Phase"` for backward compatibility with stored resources and old clients. The controller writes both in lockstep via UserStatus.SetPhase (replacing the direct Status.Phase assignments), and the printer column now reads `.status.phase`. Regenerated the CRD. Ported from redis-operator 79e49056.
chideat
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports four controller/ops fixes from the Alauda redis-operator into valkey-operator. Each commit cites its upstream redis commit for provenance.
Fixes
fix(cluster): don't reassign slots a disconnected-but-intact shard owns (redis7e6f6910)After a full restart, a master that still locally owns its slots but can't yet reach its peers was judged masterless/slotless (the
IsJoined-gatedshard.Master()/shard.Slots()), triggering destructiveEnsureSlots/Rebalancethat created epoch/slot conflicts and risked split-brain. Adds ungated predicates (ShardAssignedSlots,ShardHasMaster,IsDisconnectedButIntact) ininternal/ops/cluster/predicate.go; a disconnected-but-intact shard now waits to re-MEET. Gated in enginecheck_4+ theensure_slots/join_node/rebalanceactors. Unit-tested.fix(cluster,failover): recover a Pending instance after a resource downscale (redisd77f64b8,9ae8f422)An instance created with oversized resources (all pods Pending) never recovered after patching
spec.resourcesdown: the engine looped inHealPod/HealMonitorand never reachedEnsureResource, so the template was never rewritten. ClusterHealPodnow always ensures resources and deletes stale-revision Pending pods; failoverHealMonitorensures resources instead of idle-waiting when no master candidate exists. Rationale comments reflect valkey'sParallelPodManagement+RollingUpdateStatefulSets. Unit-tested (failover).fix(webhook): reject a Valkey whose NodePort is already allocated (redis067e1954)A second instance requesting an in-use NodePort was admitted, then silently failed to reconcile. The validating webhook now checks requested NodePorts (data + sentinel
Access.Ports) against existing NodePort Services cluster-wide via an uncached reader, excluding the instance's own Services. Covers create and update. Unit-tested (6 cases).fix(api): expose User status phase under the conventionalphaseJSON key (redis79e49056)UserStatus.Phaseusedjson:"Phase", invisible to.status.phasereaders (kubectl, printer columns). Retags the canonical fieldjson:"phase", keeps a legacyOldPhase json:"Phase"for backward compat, writes both viaSetPhase, fixes the printer column, regenerates the CRD.Notes
68dca51c("redisuser: don't take ownership of user-provided password secrets") — it was reverted upstream onaa-integrate(6943ad3) because removing ownership broke theOwns(&Secret{})password-rotation reconcile trigger. valkey already owns the secret (the reverted-to state) and142ffebrefines its finalizer timing.Testing
make test(full envtest suite) passes, incl.internal/controller,internal/controller/rds,internal/webhook/rds/v1alpha1(96.3%),internal/webhook/v1alpha1(88.5%).go build ./...,go vet, and the new unit tests (predicate_test.go,actor_heal_monitor_test.go,valkey_webhook_nodeport_test.go) all pass.🤖 Generated with Claude Code