Skip to content

Commit e5f1ede

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 566db6f commit e5f1ede

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

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

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