Skip to content

Commit e5c21fb

Browse files
Damian Seredyndrivebyer
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](OT-CONTAINER-KIT#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 e23140a commit e5c21fb

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
@@ -206,7 +206,12 @@ func CreateMultipleLeaderRedisCommand(ctx context.Context, client kubernetes.Int
206206
PodName: cr.Name + "-leader-" + strconv.Itoa(podCount),
207207
Namespace: cr.Namespace,
208208
}
209-
cmd.AddFlag(getEndpoint(ctx, client, cr, rd))
209+
endpoint := getEndpoint(ctx, client, cr, rd)
210+
if endpoint == "" {
211+
log.FromContext(ctx).Error(errors.New("failed to get endpoint"), "Unable to resolve endpoint for pod", "Pod", rd.PodName)
212+
continue
213+
}
214+
cmd.AddFlag(endpoint)
210215
}
211216
cmd.AddFlag("--cluster-yes")
212217
return cmd
@@ -271,10 +276,19 @@ func getRedisTLSArgs(tlsConfig *commonapi.TLSConfig, clientHost string) []string
271276
}
272277

273278
// createRedisReplicationCommand will create redis replication creation command
274-
func createRedisReplicationCommand(ctx context.Context, client kubernetes.Interface, cr *rcvb2.RedisCluster, leaderPod RedisDetails, followerPod RedisDetails) []string {
279+
func createRedisReplicationCommand(ctx context.Context, client kubernetes.Interface, cr *rcvb2.RedisCluster, leaderPod RedisDetails, followerPod RedisDetails) ([]string, error) {
280+
followerEndpoint := getEndpoint(ctx, client, cr, followerPod)
281+
if followerEndpoint == "" {
282+
return nil, fmt.Errorf("failed to get endpoint for follower pod %s", followerPod.PodName)
283+
}
284+
leaderEndpoint := getEndpoint(ctx, client, cr, leaderPod)
285+
if leaderEndpoint == "" {
286+
return nil, fmt.Errorf("failed to get endpoint for leader pod %s", leaderPod.PodName)
287+
}
288+
275289
cmd := []string{"redis-cli", "--cluster", "add-node"}
276-
cmd = append(cmd, getEndpoint(ctx, client, cr, followerPod))
277-
cmd = append(cmd, getEndpoint(ctx, client, cr, leaderPod))
290+
cmd = append(cmd, followerEndpoint)
291+
cmd = append(cmd, leaderEndpoint)
278292
cmd = append(cmd, "--cluster-slave")
279293
if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil {
280294
pass, err := getRedisPassword(ctx, client, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key)
@@ -285,7 +299,7 @@ func createRedisReplicationCommand(ctx context.Context, client kubernetes.Interf
285299
}
286300
}
287301
cmd = append(cmd, getRedisTLSArgs(cr.Spec.TLS, leaderPod.PodName)...)
288-
return cmd
302+
return cmd, nil
289303
}
290304

291305
// ExecuteRedisReplicationCommand will execute the replication command
@@ -315,7 +329,12 @@ func ExecuteRedisReplicationCommand(ctx context.Context, client kubernetes.Inter
315329
podIP = getRedisServerIP(ctx, client, followerPod)
316330
if !checkRedisNodePresence(ctx, nodes, podIP) {
317331
log.FromContext(ctx).V(1).Info("Adding node to cluster.", "Node.IP", podIP, "Follower.Pod", followerPod)
318-
cmd := createRedisReplicationCommand(ctx, client, cr, leaderPod, followerPod)
332+
cmd, err := createRedisReplicationCommand(ctx, client, cr, leaderPod, followerPod)
333+
if err != nil {
334+
log.FromContext(ctx).Error(err, "Failed to create replication command")
335+
followerIdx++
336+
continue
337+
}
319338
redisClient := configureRedisClient(ctx, client, cr, followerPod.PodName)
320339
pong, err := redisClient.Ping(ctx).Result()
321340
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)