Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ping instead of checking pod readiness #1038

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/common_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ type KubernetesConfig struct {
Service *ServiceConfig `json:"service,omitempty"`
IgnoreAnnotations []string `json:"ignoreAnnotations,omitempty"`
MinReadySeconds *int32 `json:"minReadySeconds,omitempty"`
PodManagementPolicy appsv1.PodManagementPolicyType `json:"podManagementPolicy,omitempty"`
}

// ServiceConfig define the type of service to be created and its annotations
Expand Down
8 changes: 8 additions & 0 deletions config/crd/bases/redis.redis.opstreelabs.in_redis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1110,6 +1110,10 @@ spec:
minReadySeconds:
format: int32
type: integer
podManagementPolicy:
description: PodManagementPolicyType defines the policy for creating
pods under a stateful set.
type: string
redisSecret:
description: ExistingPasswordSecret is the struct to access the
existing secret
Expand Down Expand Up @@ -6139,6 +6143,10 @@ spec:
minReadySeconds:
format: int32
type: integer
podManagementPolicy:
description: PodManagementPolicyType defines the policy for creating
pods under a stateful set.
type: string
redisSecret:
description: ExistingPasswordSecret is the struct to access the
existing secret
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ spec:
minReadySeconds:
format: int32
type: integer
podManagementPolicy:
description: PodManagementPolicyType defines the policy for creating
pods under a stateful set.
type: string
redisSecret:
description: ExistingPasswordSecret is the struct to access the
existing secret
Expand Down Expand Up @@ -6755,6 +6759,10 @@ spec:
minReadySeconds:
format: int32
type: integer
podManagementPolicy:
description: PodManagementPolicyType defines the policy for creating
pods under a stateful set.
type: string
redisSecret:
description: ExistingPasswordSecret is the struct to access the
existing secret
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,10 @@ spec:
minReadySeconds:
format: int32
type: integer
podManagementPolicy:
description: PodManagementPolicyType defines the policy for creating
pods under a stateful set.
type: string
redisSecret:
description: ExistingPasswordSecret is the struct to access the
existing secret
Expand Down Expand Up @@ -6147,6 +6151,10 @@ spec:
minReadySeconds:
format: int32
type: integer
podManagementPolicy:
description: PodManagementPolicyType defines the policy for creating
pods under a stateful set.
type: string
redisSecret:
description: ExistingPasswordSecret is the struct to access the
existing secret
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1113,6 +1113,10 @@ spec:
minReadySeconds:
format: int32
type: integer
podManagementPolicy:
description: PodManagementPolicyType defines the policy for creating
pods under a stateful set.
type: string
redisSecret:
description: ExistingPasswordSecret is the struct to access the
existing secret
Expand Down Expand Up @@ -3601,6 +3605,10 @@ spec:
minReadySeconds:
format: int32
type: integer
podManagementPolicy:
description: PodManagementPolicyType defines the policy for creating
pods under a stateful set.
type: string
redisSecret:
description: ExistingPasswordSecret is the struct to access the
existing secret
Expand Down
6 changes: 3 additions & 3 deletions controllers/rediscluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (r *RedisClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request
return intctrlutil.RequeueWithError(err, reqLogger, "")
}

if r.IsStatefulSetReady(ctx, instance.Namespace, instance.Name+"-leader") {
if r.IsStatefulSetReady(ctx, r.K8sClient, instance, 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) ||
instance.Status.ReadyFollowerReplicas != followerReplicas {
Expand All @@ -147,8 +147,8 @@ func (r *RedisClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request
}
}

if !(r.IsStatefulSetReady(ctx, instance.Namespace, instance.Name+"-leader") && r.IsStatefulSetReady(ctx, instance.Namespace, instance.Name+"-follower")) {
return intctrlutil.Reconciled()
if !(r.IsStatefulSetReady(ctx, r.K8sClient, instance, instance.Namespace, instance.Name+"-leader") && r.IsStatefulSetReady(ctx, r.K8sClient, instance, instance.Namespace, instance.Name+"-follower")) {
return intctrlutil.RequeueAfter(reqLogger, time.Second*10, "Redis cluster is not ready")
}

// Mark the cluster status as bootstrapping if all the leader and follower nodes are ready
Expand Down
4 changes: 2 additions & 2 deletions controllers/redisreplication_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ func (r *RedisReplicationReconciler) Reconcile(ctx context.Context, req ctrl.Req
if err != nil {
return intctrlutil.RequeueWithError(err, reqLogger, "")
}
if !r.IsStatefulSetReady(ctx, instance.Namespace, instance.Name) {
return intctrlutil.Reconciled()
if !r.IsStatefulSetReady(ctx, r.K8sClient, instance, instance.Namespace, instance.Name) {
return intctrlutil.RequeueAfter(reqLogger, time.Second*10, "")
}

var realMaster string
Expand Down
1 change: 1 addition & 0 deletions docs/content/en/docs/CRD Reference/Redis API/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ _Appears in:_
| `redisSecret` _[ExistingPasswordSecret](#existingpasswordsecret)_ | |
| `imagePullSecrets` _[LocalObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#localobjectreference-v1-core)_ | |
| `updateStrategy` _[StatefulSetUpdateStrategy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#statefulsetupdatestrategy-v1-apps)_ | |
| `podManagementPolicy` _[PodManagementPolicyType](https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#pod-management-policies)_ | |

#### VolumeMount

Expand Down
26 changes: 26 additions & 0 deletions k8sutils/cluster-scaling.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
redisv1beta2 "github.com/OT-CONTAINER-KIT/redis-operator/api/v1beta2"
"github.com/go-logr/logr"
redis "github.com/redis/go-redis/v9"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes"
)

Expand Down Expand Up @@ -106,6 +107,31 @@
return strconv.Itoa(totalSlots)
}

// pingRedisNode will ping the redis node to check if it is up and running
func pingRedisNode(ctx context.Context, client kubernetes.Interface, logger logr.Logger, cr runtime.Object, pod RedisDetails) bool {
var redisClient *redis.Client

Check warning on line 112 in k8sutils/cluster-scaling.go

View check run for this annotation

Codecov / codecov/patch

k8sutils/cluster-scaling.go#L111-L112

Added lines #L111 - L112 were not covered by tests

switch cr := cr.(type) {
case *redisv1beta2.RedisCluster:
redisClient = configureRedisClient(client, logger, cr, pod.PodName)
case *redisv1beta2.RedisReplication:
redisClient = configureRedisReplicationClient(client, logger, cr, pod.PodName)
default:
logger.Error(nil, "Unknown CR type")
return false

Check warning on line 121 in k8sutils/cluster-scaling.go

View check run for this annotation

Codecov / codecov/patch

k8sutils/cluster-scaling.go#L114-L121

Added lines #L114 - L121 were not covered by tests
}

defer redisClient.Close()

Check warning on line 124 in k8sutils/cluster-scaling.go

View check run for this annotation

Codecov / codecov/patch

k8sutils/cluster-scaling.go#L124

Added line #L124 was not covered by tests

pong, err := redisClient.Ping(ctx).Result()
if err != nil || pong != "PONG" {
logger.Error(err, "Failed to ping Redis server")
return false

Check warning on line 129 in k8sutils/cluster-scaling.go

View check run for this annotation

Codecov / codecov/patch

k8sutils/cluster-scaling.go#L126-L129

Added lines #L126 - L129 were not covered by tests
}

return true

Check warning on line 132 in k8sutils/cluster-scaling.go

View check run for this annotation

Codecov / codecov/patch

k8sutils/cluster-scaling.go#L132

Added line #L132 was not covered by tests
}

// getRedisNodeID would return nodeID of a redis node by passing pod
func getRedisNodeID(ctx context.Context, client kubernetes.Interface, logger logr.Logger, cr *redisv1beta2.RedisCluster, pod RedisDetails) string {
redisClient := configureRedisClient(client, logger, cr, pod.PodName)
Expand Down
1 change: 1 addition & 0 deletions k8sutils/redis-cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func generateRedisClusterParams(cr *redisv1beta2.RedisCluster, replicas int32, e
IgnoreAnnotations: cr.Spec.KubernetesConfig.IgnoreAnnotations,
HostNetwork: cr.Spec.HostNetwork,
MinReadySeconds: minreadyseconds,
PodManagementPolicy: cr.Spec.KubernetesConfig.PodManagementPolicy,
}
if cr.Spec.RedisExporter != nil {
res.EnableMetrics = cr.Spec.RedisExporter.Enabled
Expand Down
1 change: 1 addition & 0 deletions k8sutils/redis-replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ func generateRedisReplicationParams(cr *redisv1beta2.RedisReplication) statefulS
UpdateStrategy: cr.Spec.KubernetesConfig.UpdateStrategy,
IgnoreAnnotations: cr.Spec.KubernetesConfig.IgnoreAnnotations,
MinReadySeconds: minreadyseconds,
PodManagementPolicy: cr.Spec.KubernetesConfig.PodManagementPolicy,
}
if cr.Spec.KubernetesConfig.ImagePullSecrets != nil {
res.ImagePullSecrets = cr.Spec.KubernetesConfig.ImagePullSecrets
Expand Down
1 change: 1 addition & 0 deletions k8sutils/redis-sentinel.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ func generateRedisSentinelParams(cr *redisv1beta2.RedisSentinel, replicas int32,
UpdateStrategy: cr.Spec.KubernetesConfig.UpdateStrategy,
IgnoreAnnotations: cr.Spec.KubernetesConfig.IgnoreAnnotations,
MinReadySeconds: minreadyseconds,
PodManagementPolicy: cr.Spec.KubernetesConfig.PodManagementPolicy,
}

if cr.Spec.KubernetesConfig.ImagePullSecrets != nil {
Expand Down
1 change: 1 addition & 0 deletions k8sutils/redis-standalone.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ func generateRedisStandaloneParams(cr *redisv1beta2.Redis) statefulSetParameters
UpdateStrategy: cr.Spec.KubernetesConfig.UpdateStrategy,
IgnoreAnnotations: cr.Spec.KubernetesConfig.IgnoreAnnotations,
MinReadySeconds: minreadyseconds,
PodManagementPolicy: cr.Spec.KubernetesConfig.PodManagementPolicy,
}
if cr.Spec.KubernetesConfig.ImagePullSecrets != nil {
res.ImagePullSecrets = cr.Spec.KubernetesConfig.ImagePullSecrets
Expand Down
1 change: 1 addition & 0 deletions k8sutils/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func generateServiceDef(serviceMeta metav1.ObjectMeta, epp exporterPortProvider,
}
if headless {
service.Spec.ClusterIP = "None"
service.Spec.PublishNotReadyAddresses = true
}
if exporterPort, ok := epp(); ok {
redisExporterService := enableMetricsPort(exporterPort)
Expand Down
14 changes: 8 additions & 6 deletions k8sutils/services_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,10 @@ func TestGenerateServiceDef(t *testing.T) {
Protocol: corev1.ProtocolTCP,
},
},
Selector: map[string]string{"role": "sentinel"},
ClusterIP: "None",
Type: corev1.ServiceTypeClusterIP,
Selector: map[string]string{"role": "sentinel"},
ClusterIP: "None",
Type: corev1.ServiceTypeClusterIP,
PublishNotReadyAddresses: true,
},
},
},
Expand Down Expand Up @@ -184,9 +185,10 @@ func TestGenerateServiceDef(t *testing.T) {
Protocol: corev1.ProtocolTCP,
},
},
Selector: map[string]string{"role": "redis"},
ClusterIP: "None",
Type: corev1.ServiceTypeClusterIP,
Selector: map[string]string{"role": "redis"},
ClusterIP: "None",
Type: corev1.ServiceTypeClusterIP,
PublishNotReadyAddresses: true,
},
},
},
Expand Down
32 changes: 22 additions & 10 deletions k8sutils/statefulset.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes"
"k8s.io/utils/env"
"k8s.io/utils/ptr"
)

type StatefulSet interface {
IsStatefulSetReady(ctx context.Context, namespace, name string) bool
IsStatefulSetReady(ctx context.Context, client kubernetes.Interface, cr runtime.Object, namespace, name string) bool
}

type StatefulSetService struct {
Expand All @@ -41,7 +42,7 @@
}
}

func (s *StatefulSetService) IsStatefulSetReady(ctx context.Context, namespace, name string) bool {
func (s *StatefulSetService) IsStatefulSetReady(ctx context.Context, client kubernetes.Interface, cr runtime.Object, namespace, name string) bool {

Check warning on line 45 in k8sutils/statefulset.go

View check run for this annotation

Codecov / codecov/patch

k8sutils/statefulset.go#L45

Added line #L45 was not covered by tests
var (
partition = 0
replicas = 1
Expand Down Expand Up @@ -74,10 +75,19 @@
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

for i := 0; i < replicas; i++ {
pod := RedisDetails{
PodName: fmt.Sprintf("%s-%d", name, i),
Namespace: namespace,

Check warning on line 82 in k8sutils/statefulset.go

View check run for this annotation

Codecov / codecov/patch

k8sutils/statefulset.go#L79-L82

Added lines #L79 - L82 were not covered by tests
}

if !pingRedisNode(ctx, client, logger, cr, pod) {
logger.V(1).Info("StatefulSet is not ready", "PingRedisNode", pod.PodName)
return false

Check warning on line 87 in k8sutils/statefulset.go

View check run for this annotation

Codecov / codecov/patch

k8sutils/statefulset.go#L85-L87

Added lines #L85 - L87 were not covered by tests
}
}

return true
}

Expand Down Expand Up @@ -108,6 +118,7 @@
IgnoreAnnotations []string
HostNetwork bool
MinReadySeconds int32
PodManagementPolicy appsv1.PodManagementPolicyType
}

// containerParameters will define container input params
Expand Down Expand Up @@ -280,11 +291,12 @@
TypeMeta: generateMetaInformation("StatefulSet", "apps/v1"),
ObjectMeta: stsMeta,
Spec: appsv1.StatefulSetSpec{
Selector: LabelSelectors(stsMeta.GetLabels()),
ServiceName: fmt.Sprintf("%s-headless", stsMeta.Name),
Replicas: params.Replicas,
UpdateStrategy: params.UpdateStrategy,
MinReadySeconds: params.MinReadySeconds,
Selector: LabelSelectors(stsMeta.GetLabels()),
ServiceName: fmt.Sprintf("%s-headless", stsMeta.Name),
Replicas: params.Replicas,
UpdateStrategy: params.UpdateStrategy,
MinReadySeconds: params.MinReadySeconds,
PodManagementPolicy: params.PodManagementPolicy,
Template: corev1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: stsMeta.GetLabels(),
Expand Down
Loading