diff --git a/mocks/operator/redisfailover/service/RedisFailoverCheck.go b/mocks/operator/redisfailover/service/RedisFailoverCheck.go index b0fb5212b..4dfccdd7a 100644 --- a/mocks/operator/redisfailover/service/RedisFailoverCheck.go +++ b/mocks/operator/redisfailover/service/RedisFailoverCheck.go @@ -428,6 +428,20 @@ func (_m *RedisFailoverCheck) IsRedisRunning(rFailover *v1.RedisFailover) bool { return r0 } +// IsRedisRunningQuorum provides a mock function with given fields: rFailover +func (_m *RedisFailoverCheck) IsRedisRunningQuorum(rFailover *v1.RedisFailover) bool { + ret := _m.Called(rFailover) + + var r0 bool + if rf, ok := ret.Get(0).(func(*v1.RedisFailover) bool); ok { + r0 = rf(rFailover) + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + // IsSentinelRunning provides a mock function with given fields: rFailover func (_m *RedisFailoverCheck) IsSentinelRunning(rFailover *v1.RedisFailover) bool { ret := _m.Called(rFailover) @@ -442,6 +456,20 @@ func (_m *RedisFailoverCheck) IsSentinelRunning(rFailover *v1.RedisFailover) boo return r0 } +// IsSentinelRunningQuorum provides a mock function with given fields: rFailover +func (_m *RedisFailoverCheck) IsSentinelRunningQuorum(rFailover *v1.RedisFailover) bool { + ret := _m.Called(rFailover) + + var r0 bool + if rf, ok := ret.Get(0).(func(*v1.RedisFailover) bool); ok { + r0 = rf(rFailover) + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} + type mockConstructorTestingTNewRedisFailoverCheck interface { mock.TestingT Cleanup(func()) diff --git a/operator/redisfailover/checker.go b/operator/redisfailover/checker.go index c2e6c8f59..6fc44e1e0 100644 --- a/operator/redisfailover/checker.go +++ b/operator/redisfailover/checker.go @@ -116,15 +116,19 @@ func (r *RedisFailoverHandler) CheckAndHeal(rf *redisfailoverv1.RedisFailover) e // Sentinel has not death nodes // Sentinel knows the correct slave number - if !r.rfChecker.IsRedisRunning(rf) { - setRedisCheckerMetrics(r.mClient, "redis", rf.Namespace, rf.Name, metrics.REDIS_REPLICA_MISMATCH, metrics.NOT_APPLICABLE, errors.New("not all replicas running")) - r.logger.WithField("redisfailover", rf.ObjectMeta.Name).WithField("namespace", rf.ObjectMeta.Namespace).Debugf("Number of redis mismatch, waiting for redis statefulset reconcile") + // Heal as long as a quorum (majority) of pods is running rather than requiring + // the full set. A single Pending pod (unschedulable affinity, AZ loss) must not + // block master election and sentinel reconfiguration for the survivors; the + // downstream heal logic already operates only on the running/reachable pods. + if !r.rfChecker.IsRedisRunningQuorum(rf) { + setRedisCheckerMetrics(r.mClient, "redis", rf.Namespace, rf.Name, metrics.REDIS_REPLICA_MISMATCH, metrics.NOT_APPLICABLE, errors.New("redis quorum not running")) + r.logger.WithField("redisfailover", rf.ObjectMeta.Name).WithField("namespace", rf.ObjectMeta.Namespace).Debugf("Redis quorum not running, waiting for redis statefulset reconcile") return nil } - if !r.rfChecker.IsSentinelRunning(rf) { - setRedisCheckerMetrics(r.mClient, "sentinel", rf.Namespace, rf.Name, metrics.SENTINEL_REPLICA_MISMATCH, metrics.NOT_APPLICABLE, errors.New("not all replicas running")) - r.logger.WithField("redisfailover", rf.ObjectMeta.Name).WithField("namespace", rf.ObjectMeta.Namespace).Debugf("Number of sentinel mismatch, waiting for sentinel deployment reconcile") + if !r.rfChecker.IsSentinelRunningQuorum(rf) { + setRedisCheckerMetrics(r.mClient, "sentinel", rf.Namespace, rf.Name, metrics.SENTINEL_REPLICA_MISMATCH, metrics.NOT_APPLICABLE, errors.New("sentinel quorum not running")) + r.logger.WithField("redisfailover", rf.ObjectMeta.Name).WithField("namespace", rf.ObjectMeta.Namespace).Debugf("Sentinel quorum not running, waiting for sentinel deployment reconcile") return nil } diff --git a/operator/redisfailover/checker_test.go b/operator/redisfailover/checker_test.go index 1e27a53d3..397b04dfa 100644 --- a/operator/redisfailover/checker_test.go +++ b/operator/redisfailover/checker_test.go @@ -301,15 +301,24 @@ func TestCheckAndHeal(t *testing.T) { mrfc := &mRFService.RedisFailoverCheck{} mrfh := &mRFService.RedisFailoverHeal{} + // Normal CheckAndHeal gates on a quorum; bootstrap mode still gates on + // the full set, so route the mock to whichever the code under test calls. + redisRunningMethod := "IsRedisRunningQuorum" + sentinelRunningMethod := "IsSentinelRunningQuorum" + if bootstrappingTests { + redisRunningMethod = "IsRedisRunning" + sentinelRunningMethod = "IsSentinelRunning" + } + if test.redisCheckNumberOK { - mrfc.On("IsRedisRunning", rf).Once().Return(true) + mrfc.On(redisRunningMethod, rf).Once().Return(true) } else { continueTests = false - mrfc.On("IsRedisRunning", rf).Once().Return(false) + mrfc.On(redisRunningMethod, rf).Once().Return(false) } if allowSentinels { - mrfc.On("IsSentinelRunning", rf).Once().Return(true) + mrfc.On(sentinelRunningMethod, rf).Once().Return(true) } if bootstrappingTests && continueTests { diff --git a/operator/redisfailover/service/check.go b/operator/redisfailover/service/check.go index 8c40b7519..7da54db53 100644 --- a/operator/redisfailover/service/check.go +++ b/operator/redisfailover/service/check.go @@ -38,7 +38,9 @@ type RedisFailoverCheck interface { GetRedisRevisionHash(podName string, rFailover *redisfailoverv1.RedisFailover) (string, error) CheckRedisSlavesReady(slaveIP string, rFailover *redisfailoverv1.RedisFailover) (bool, error) IsRedisRunning(rFailover *redisfailoverv1.RedisFailover) bool + IsRedisRunningQuorum(rFailover *redisfailoverv1.RedisFailover) bool IsSentinelRunning(rFailover *redisfailoverv1.RedisFailover) bool + IsSentinelRunningQuorum(rFailover *redisfailoverv1.RedisFailover) bool IsClusterRunning(rFailover *redisfailoverv1.RedisFailover) bool } @@ -516,12 +518,28 @@ func (r *RedisFailoverChecker) IsRedisRunning(rFailover *redisfailoverv1.RedisFa return err == nil && len(dp.Items) > int(rFailover.Spec.Redis.Replicas-1) && AreAllRunning(dp, int(rFailover.Spec.Redis.Replicas)) } +// IsRedisRunningQuorum returns true when at least a majority (quorum) of the +// redis pods are Running. Unlike IsRedisRunning it does not require the full set, +// so healing can still proceed while a minority of pods are stuck Pending. +func (r *RedisFailoverChecker) IsRedisRunningQuorum(rFailover *redisfailoverv1.RedisFailover) bool { + dp, err := r.k8sService.GetStatefulSetPods(rFailover.Namespace, GetRedisName(rFailover)) + return err == nil && AreQuorumRunning(dp, int(rFailover.Spec.Redis.Replicas)) +} + // IsSentinelRunning returns true if all the pods are Running func (r *RedisFailoverChecker) IsSentinelRunning(rFailover *redisfailoverv1.RedisFailover) bool { dp, err := r.k8sService.GetDeploymentPods(rFailover.Namespace, GetSentinelName(rFailover)) return err == nil && len(dp.Items) > int(rFailover.Spec.Sentinel.Replicas-1) && AreAllRunning(dp, int(rFailover.Spec.Sentinel.Replicas)) } +// IsSentinelRunningQuorum returns true when at least a majority (quorum) of the +// sentinel pods are Running, so the operator can reconfigure the surviving +// sentinels even while a minority are stuck Pending. +func (r *RedisFailoverChecker) IsSentinelRunningQuorum(rFailover *redisfailoverv1.RedisFailover) bool { + dp, err := r.k8sService.GetDeploymentPods(rFailover.Namespace, GetSentinelName(rFailover)) + return err == nil && AreQuorumRunning(dp, int(rFailover.Spec.Sentinel.Replicas)) +} + // IsClusterRunning returns true if all the pods in the given redisfailover are Running func (r *RedisFailoverChecker) IsClusterRunning(rFailover *redisfailoverv1.RedisFailover) bool { return r.IsSentinelRunning(rFailover) && r.IsRedisRunning(rFailover) @@ -544,3 +562,19 @@ func AreAllRunning(pods *corev1.PodList, expectedRunningPods int) bool { } return runningPods >= expectedRunningPods } + +// AreQuorumRunning reports whether at least a majority (quorum) of the expected +// pods are Running. Scheduling and terminal pods are not counted, but a minority +// of Pending pods no longer blocks the result, so the operator can keep healing +// the surviving pods after a partial outage instead of waiting for the full set. +func AreQuorumRunning(pods *corev1.PodList, expectedReplicas int) bool { + var runningPods int + for i := range pods.Items { + pod := &pods.Items[i] + if util.PodIsScheduling(pod) || util.PodIsTerminal(pod) { + continue + } + runningPods++ + } + return runningPods >= expectedReplicas/2+1 +} diff --git a/operator/redisfailover/service/quorum_running_test.go b/operator/redisfailover/service/quorum_running_test.go new file mode 100644 index 000000000..1e10c52c5 --- /dev/null +++ b/operator/redisfailover/service/quorum_running_test.go @@ -0,0 +1,45 @@ +package service_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + + rfservice "github.com/dnse-tech/redis-operator/operator/redisfailover/service" +) + +func podsWithPhases(phases ...corev1.PodPhase) *corev1.PodList { + pods := &corev1.PodList{} + for _, p := range phases { + pods.Items = append(pods.Items, corev1.Pod{Status: corev1.PodStatus{Phase: p}}) + } + return pods +} + +func TestAreQuorumRunning(t *testing.T) { + run, pend, fail := corev1.PodRunning, corev1.PodPending, corev1.PodFailed + + tests := []struct { + name string + phases []corev1.PodPhase + replicas int + expected bool + }{ + {"all three running", []corev1.PodPhase{run, run, run}, 3, true}, + {"two of three running is quorum", []corev1.PodPhase{run, run, pend}, 3, true}, + {"one of three running is below quorum", []corev1.PodPhase{run, pend, pend}, 3, false}, + {"failed pod does not count", []corev1.PodPhase{run, run, fail}, 3, true}, + {"single replica running", []corev1.PodPhase{run}, 1, true}, + {"single replica pending", []corev1.PodPhase{pend}, 1, false}, + {"five replicas three running is quorum", []corev1.PodPhase{run, run, run, pend, pend}, 5, true}, + {"five replicas two running below quorum", []corev1.PodPhase{run, run, pend, pend, pend}, 5, false}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := rfservice.AreQuorumRunning(podsWithPhases(test.phases...), test.replicas) + assert.Equal(t, test.expected, got) + }) + } +}