From 2838af46d20ea80f5e9f3f46b0ea43c9ecf6c981 Mon Sep 17 00:00:00 2001
From: yangw <yang.wu@daocloud.io>
Date: Sun, 16 Jun 2024 23:30:24 +0800
Subject: [PATCH] fix: ReadyReplicas need to be checked in `IsStatefulSetReady`
 (#993)

Signed-off-by: drivebyer <wuyangmuc@gmail.com>
---
 controllers/rediscluster_controller.go | 16 ++--------------
 k8sutils/statefulset.go                |  5 ++++-
 2 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/controllers/rediscluster_controller.go b/controllers/rediscluster_controller.go
index 6e4b9fa99..969d44291 100644
--- a/controllers/rediscluster_controller.go
+++ b/controllers/rediscluster_controller.go
@@ -116,18 +116,11 @@ func (r *RedisClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	if err != nil {
 		return intctrlutil.RequeueWithError(err, reqLogger, "")
 	}
-
 	err = k8sutils.ReconcileRedisPodDisruptionBudget(instance, "leader", instance.Spec.RedisLeader.PodDisruptionBudget, r.K8sClient)
 	if err != nil {
 		return intctrlutil.RequeueWithError(err, reqLogger, "")
 	}
 
-	// todo: remove me after watch statefulset in controller
-	redisLeaderInfo, err := k8sutils.GetStatefulSet(r.K8sClient, r.Log, instance.GetNamespace(), instance.GetName()+"-leader")
-	if err != nil {
-		return intctrlutil.RequeueWithError(err, reqLogger, "")
-	}
-
 	if r.IsStatefulSetReady(ctx, instance.Namespace, instance.Name+"-leader") {
 		// Mark the cluster status as initializing if there are no follower nodes
 		if (instance.Status.ReadyLeaderReplicas == 0 && instance.Status.ReadyFollowerReplicas == 0) ||
@@ -153,11 +146,6 @@ func (r *RedisClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request
 			return intctrlutil.RequeueWithError(err, reqLogger, "")
 		}
 	}
-	// todo: remove me after watch statefulset in controller
-	redisFollowerInfo, err := k8sutils.GetStatefulSet(r.K8sClient, r.Log, instance.GetNamespace(), instance.GetName()+"-follower")
-	if err != nil {
-		return intctrlutil.RequeueWithError(err, reqLogger, "")
-	}
 
 	if !(r.IsStatefulSetReady(ctx, instance.Namespace, instance.Name+"-leader") && r.IsStatefulSetReady(ctx, instance.Namespace, instance.Name+"-follower")) {
 		return intctrlutil.Reconciled()
@@ -172,7 +160,7 @@ func (r *RedisClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	}
 
 	if nc := k8sutils.CheckRedisNodeCount(ctx, r.K8sClient, r.Log, instance, ""); nc != totalReplicas {
-		reqLogger.Info("Creating redis cluster by executing cluster creation commands", "Leaders.Ready", redisLeaderInfo.Status.ReadyReplicas, "Followers.Ready", redisFollowerInfo.Status.ReadyReplicas)
+		reqLogger.Info("Creating redis cluster by executing cluster creation commands")
 		leaderCount := k8sutils.CheckRedisNodeCount(ctx, r.K8sClient, r.Log, instance, "leader")
 		if leaderCount != leaderReplicas {
 			reqLogger.Info("Not all leader are part of the cluster...", "Leaders.Count", leaderCount, "Instance.Size", leaderReplicas)
@@ -188,7 +176,7 @@ func (r *RedisClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request
 				}
 			}
 		} else {
-			if followerReplicas > 0 && redisFollowerInfo.Status.ReadyReplicas == followerReplicas {
+			if followerReplicas > 0 {
 				reqLogger.Info("All leader are part of the cluster, adding follower/replicas", "Leaders.Count", leaderCount, "Instance.Size", leaderReplicas, "Follower.Replicas", followerReplicas)
 				k8sutils.ExecuteRedisReplicationCommand(ctx, r.K8sClient, r.Log, instance)
 			} else {
diff --git a/k8sutils/statefulset.go b/k8sutils/statefulset.go
index b742b9864..10d5f0288 100644
--- a/k8sutils/statefulset.go
+++ b/k8sutils/statefulset.go
@@ -74,7 +74,10 @@ func (s *StatefulSetService) IsStatefulSetReady(ctx context.Context, namespace,
 		logger.V(1).Info("StatefulSet is not ready", "Status.ObservedGeneration", sts.Status.ObservedGeneration, "ObjectMeta.Generation", sts.ObjectMeta.Generation)
 		return false
 	}
-
+	if int(sts.Status.ReadyReplicas) != replicas {
+		logger.V(1).Info("StatefulSet is not ready", "Status.ReadyReplicas", sts.Status.ReadyReplicas, "Replicas", replicas)
+		return false
+	}
 	return true
 }