Skip to content

Commit 341c351

Browse files
Damian SeredynDamian Seredyn
authored andcommitted
fix: Add endpoint validation in cluster creation commands
Redis cluster creation was failing silently due to empty endpoint values being added to Redis CLI commands after [PR 1568](#1568) merge. Scope: - Validate getEndpoint() returns non-empty values before using them - Return errors from createRedisReplicationCommand when endpoints fail - Prevents silent failures in cluster bootstrap process Signed-off-by: Damian Seredyn <[email protected]>
1 parent 1cc6cf1 commit 341c351

File tree

2 files changed

+28
-7
lines changed

2 files changed

+28
-7
lines changed

internal/k8sutils/redis.go

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,12 @@ func CreateMultipleLeaderRedisCommand(ctx context.Context, client kubernetes.Int
204204
PodName: cr.Name + "-leader-" + strconv.Itoa(podCount),
205205
Namespace: cr.Namespace,
206206
}
207-
cmd.AddFlag(getEndpoint(ctx, client, cr, rd))
207+
endpoint := getEndpoint(ctx, client, cr, rd)
208+
if endpoint == "" {
209+
log.FromContext(ctx).Error(errors.New("failed to get endpoint"), "Unable to resolve endpoint for pod", "Pod", rd.PodName)
210+
continue
211+
}
212+
cmd.AddFlag(endpoint)
208213
}
209214
cmd.AddFlag("--cluster-yes")
210215
return cmd
@@ -269,10 +274,19 @@ func getRedisTLSArgs(tlsConfig *commonapi.TLSConfig, clientHost string) []string
269274
}
270275

271276
// createRedisReplicationCommand will create redis replication creation command
272-
func createRedisReplicationCommand(ctx context.Context, client kubernetes.Interface, cr *rcvb2.RedisCluster, leaderPod RedisDetails, followerPod RedisDetails) []string {
277+
func createRedisReplicationCommand(ctx context.Context, client kubernetes.Interface, cr *rcvb2.RedisCluster, leaderPod RedisDetails, followerPod RedisDetails) ([]string, error) {
278+
followerEndpoint := getEndpoint(ctx, client, cr, followerPod)
279+
if followerEndpoint == "" {
280+
return nil, fmt.Errorf("failed to get endpoint for follower pod %s", followerPod.PodName)
281+
}
282+
leaderEndpoint := getEndpoint(ctx, client, cr, leaderPod)
283+
if leaderEndpoint == "" {
284+
return nil, fmt.Errorf("failed to get endpoint for leader pod %s", leaderPod.PodName)
285+
}
286+
273287
cmd := []string{"redis-cli", "--cluster", "add-node"}
274-
cmd = append(cmd, getEndpoint(ctx, client, cr, followerPod))
275-
cmd = append(cmd, getEndpoint(ctx, client, cr, leaderPod))
288+
cmd = append(cmd, followerEndpoint)
289+
cmd = append(cmd, leaderEndpoint)
276290
cmd = append(cmd, "--cluster-slave")
277291
if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
278292
pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
@@ -283,7 +297,7 @@ func createRedisReplicationCommand(ctx context.Context, client kubernetes.Interf
283297
}
284298
}
285299
cmd = append(cmd, getRedisTLSArgs(cr.Spec.TLS, leaderPod.PodName)...)
286-
return cmd
300+
return cmd, nil
287301
}
288302

289303
// ExecuteRedisReplicationCommand will execute the replication command
@@ -313,7 +327,12 @@ func ExecuteRedisReplicationCommand(ctx context.Context, client kubernetes.Inter
313327
podIP = getRedisServerIP(ctx, client, followerPod)
314328
if !checkRedisNodePresence(ctx, nodes, podIP) {
315329
log.FromContext(ctx).V(1).Info("Adding node to cluster.", "Node.IP", podIP, "Follower.Pod", followerPod)
316-
cmd := createRedisReplicationCommand(ctx, client, cr, leaderPod, followerPod)
330+
cmd, err := createRedisReplicationCommand(ctx, client, cr, leaderPod, followerPod)
331+
if err != nil {
332+
log.FromContext(ctx).Error(err, "Failed to create replication command")
333+
followerIdx++
334+
continue
335+
}
317336
redisClient := configureRedisClient(ctx, client, cr, followerPod.PodName)
318337
pong, err := redisClient.Ping(ctx).Result()
319338
redisClient.Close()

internal/k8sutils/redis_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -542,8 +542,10 @@ func TestCreateRedisReplicationCommand(t *testing.T) {
542542
objects = append(objects, secret...)
543543

544544
client := k8sClientFake.NewSimpleClientset(objects...)
545-
cmd := createRedisReplicationCommand(context.TODO(), client, tt.redisCluster, tt.leaderPod, tt.followerPod)
545+
cmd, err := createRedisReplicationCommand(context.TODO(), client, tt.redisCluster, tt.leaderPod, tt.followerPod)
546546

547+
// Assert no error occurred
548+
assert.NoError(t, err)
547549
// Assert the command is as expected using testify
548550
assert.Equal(t, tt.expectedCommand, cmd)
549551
})

0 commit comments

Comments
 (0)