From 1cf27bbf089eece85d343de8e69c5a574d419b4d Mon Sep 17 00:00:00 2001 From: Jami Karvanen Date: Fri, 15 Mar 2024 07:37:58 +0200 Subject: [PATCH] fix: m/s replication when using ipv6 (#827) * Delete unnecessary brackets wrapping the IPv6 address Signed-off-by: Jami Karvanen * Extract redis address formatting into separate getRedisServerAddress function Signed-off-by: Jami Karvanen * Add tests for getRedisServerAddress Signed-off-by: Jami Karvanen * Use getRedisServerAddress also in cluster-scaling.go Signed-off-by: Jami Karvanen --------- Signed-off-by: Jami Karvanen --- k8sutils/cluster-scaling.go | 16 ++++----- k8sutils/redis.go | 27 ++++++++++----- k8sutils/redis_test.go | 67 ++++++++++++++++++++++++++++++++++++- 3 files changed, 93 insertions(+), 17 deletions(-) diff --git a/k8sutils/cluster-scaling.go b/k8sutils/cluster-scaling.go index 6e9303bc3..6699a5a75 100644 --- a/k8sutils/cluster-scaling.go +++ b/k8sutils/cluster-scaling.go @@ -35,7 +35,7 @@ func ReshardRedisCluster(client kubernetes.Interface, logger logr.Logger, cr *re if *cr.Spec.ClusterVersion == "v7" { cmd = append(cmd, getRedisHostname(transferPOD, cr, "leader")+fmt.Sprintf(":%d", *cr.Spec.Port)) } else { - cmd = append(cmd, getRedisServerIP(client, logger, transferPOD)+fmt.Sprintf(":%d", *cr.Spec.Port)) + cmd = append(cmd, getRedisServerAddress(client, logger, transferPOD, *cr.Spec.Port)) } if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil { @@ -157,7 +157,7 @@ func RebalanceRedisClusterEmptyMasters(client kubernetes.Interface, logger logr. if *cr.Spec.ClusterVersion == "v7" { cmd = append(cmd, getRedisHostname(pod, cr, "leader")+fmt.Sprintf(":%d", *cr.Spec.Port)) } else { - cmd = append(cmd, getRedisServerIP(client, logger, pod)+fmt.Sprintf(":%d", *cr.Spec.Port)) + cmd = append(cmd, getRedisServerAddress(client, logger, pod, *cr.Spec.Port)) } cmd = append(cmd, "--cluster-use-empty-masters") @@ -209,7 +209,7 @@ func RebalanceRedisCluster(client kubernetes.Interface, logger logr.Logger, cr * if *cr.Spec.ClusterVersion == "v7" { cmd = append(cmd, getRedisHostname(pod, cr, "leader")+fmt.Sprintf(":%d", *cr.Spec.Port)) } else { - cmd = append(cmd, getRedisServerIP(client, logger, pod)+fmt.Sprintf(":%d", *cr.Spec.Port)) + cmd = append(cmd, getRedisServerAddress(client, logger, pod, *cr.Spec.Port)) } if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil { @@ -247,8 +247,8 @@ func AddRedisNodeToCluster(ctx context.Context, client kubernetes.Interface, log cmd = append(cmd, getRedisHostname(newPod, cr, "leader")+fmt.Sprintf(":%d", *cr.Spec.Port)) cmd = append(cmd, getRedisHostname(existingPod, cr, "leader")+fmt.Sprintf(":%d", *cr.Spec.Port)) } else { - cmd = append(cmd, getRedisServerIP(client, logger, newPod)+fmt.Sprintf(":%d", *cr.Spec.Port)) - cmd = append(cmd, getRedisServerIP(client, logger, existingPod)+fmt.Sprintf(":%d", *cr.Spec.Port)) + cmd = append(cmd, getRedisServerAddress(client, logger, newPod, *cr.Spec.Port)) + cmd = append(cmd, getRedisServerAddress(client, logger, existingPod, *cr.Spec.Port)) } if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil { @@ -326,7 +326,7 @@ func RemoveRedisFollowerNodesFromCluster(ctx context.Context, client kubernetes. if *cr.Spec.ClusterVersion == "v7" { cmd = append(cmd, getRedisHostname(existingPod, cr, "leader")+fmt.Sprintf(":%d", *cr.Spec.Port)) } else { - cmd = append(cmd, getRedisServerIP(client, logger, existingPod)+fmt.Sprintf(":%d", *cr.Spec.Port)) + cmd = append(cmd, getRedisServerAddress(client, logger, existingPod, *cr.Spec.Port)) } for _, followerNodeID := range followerNodeIDs { @@ -356,7 +356,7 @@ func RemoveRedisNodeFromCluster(ctx context.Context, client kubernetes.Interface if *cr.Spec.ClusterVersion == "v7" { cmd = append(cmd, getRedisHostname(existingPod, cr, "leader")+fmt.Sprintf(":%d", *cr.Spec.Port)) } else { - cmd = append(cmd, getRedisServerIP(client, logger, existingPod)+fmt.Sprintf(":%d", *cr.Spec.Port)) + cmd = append(cmd, getRedisServerAddress(client, logger, existingPod, *cr.Spec.Port)) } removePodNodeID := getRedisNodeID(ctx, client, logger, cr, removePod) @@ -417,7 +417,7 @@ func ClusterFailover(ctx context.Context, client kubernetes.Interface, logger lo if *cr.Spec.ClusterVersion == "v7" { cmd = append(cmd, getRedisHostname(pod, cr, "leader")+fmt.Sprintf(":%d", *cr.Spec.Port)) } else { - cmd = append(cmd, getRedisServerIP(client, logger, pod)+fmt.Sprintf(":%d", *cr.Spec.Port)) + cmd = append(cmd, getRedisServerAddress(client, logger, pod, *cr.Spec.Port)) } if cr.Spec.KubernetesConfig.ExistingPasswordSecret != nil { diff --git a/k8sutils/redis.go b/k8sutils/redis.go index 3de6e2787..ee0db0a17 100644 --- a/k8sutils/redis.go +++ b/k8sutils/redis.go @@ -47,13 +47,24 @@ func getRedisServerIP(client kubernetes.Interface, logger logr.Logger, redisInfo // If we're NOT IPv4, assume we're IPv6.. if net.ParseIP(redisIP).To4() == nil { logger.V(1).Info("Redis is using IPv6", "ip", redisIP) - redisIP = fmt.Sprintf("[%s]", redisIP) } logger.V(1).Info("Successfully got the IP for Redis", "ip", redisIP) return redisIP } +func getRedisServerAddress(client kubernetes.Interface, logger logr.Logger, rd RedisDetails, port int) string { + ip := getRedisServerIP(client, logger, rd) + format := "%s:%d" + + // if ip is IPv6, wrap it in brackets + if net.ParseIP(ip).To4() == nil { + format = "[%s]:%d" + } + + return fmt.Sprintf(format, ip, port) +} + // getRedisHostname will return the complete FQDN for redis func getRedisHostname(redisInfo RedisDetails, cr *redisv1beta2.RedisCluster, role string) string { fqdn := fmt.Sprintf("%s.%s-%s-headless.%s.svc", redisInfo.PodName, cr.ObjectMeta.Name, role, cr.Namespace) @@ -85,7 +96,7 @@ func CreateMultipleLeaderRedisCommand(client kubernetes.Interface, logger logr.L if cr.Spec.ClusterVersion != nil && *cr.Spec.ClusterVersion == "v7" { address = getRedisHostname(RedisDetails{PodName: podName, Namespace: cr.Namespace}, cr, "leader") + fmt.Sprintf(":%d", *cr.Spec.Port) } else { - address = getRedisServerIP(client, logger, RedisDetails{PodName: podName, Namespace: cr.Namespace}) + fmt.Sprintf(":%d", *cr.Spec.Port) + address = getRedisServerAddress(client, logger, RedisDetails{PodName: podName, Namespace: cr.Namespace}, *cr.Spec.Port) } cmd = append(cmd, address) } @@ -144,8 +155,8 @@ func createRedisReplicationCommand(client kubernetes.Interface, logger logr.Logg followerAddress = getRedisHostname(followerPod, cr, "follower") + fmt.Sprintf(":%d", *cr.Spec.Port) leaderAddress = getRedisHostname(leaderPod, cr, "leader") + fmt.Sprintf(":%d", *cr.Spec.Port) } else { - followerAddress = getRedisServerIP(client, logger, followerPod) + fmt.Sprintf(":%d", *cr.Spec.Port) - leaderAddress = getRedisServerIP(client, logger, leaderPod) + fmt.Sprintf(":%d", *cr.Spec.Port) + followerAddress = getRedisServerAddress(client, logger, followerPod, *cr.Spec.Port) + leaderAddress = getRedisServerAddress(client, logger, leaderPod, *cr.Spec.Port) } cmd = append(cmd, followerAddress, leaderAddress, "--cluster-slave") @@ -340,14 +351,14 @@ func configureRedisClient(client kubernetes.Interface, logger logr.Logger, cr *r logger.Error(err, "Error in getting redis password") } redisClient = redis.NewClient(&redis.Options{ - Addr: getRedisServerIP(client, logger, redisInfo) + fmt.Sprintf(":%d", *cr.Spec.Port), + Addr: getRedisServerAddress(client, logger, redisInfo, *cr.Spec.Port), Password: pass, DB: 0, TLSConfig: getRedisTLSConfig(client, logger, cr, redisInfo), }) } else { redisClient = redis.NewClient(&redis.Options{ - Addr: getRedisServerIP(client, logger, redisInfo) + fmt.Sprintf(":%d", *cr.Spec.Port), + Addr: getRedisServerAddress(client, logger, redisInfo, *cr.Spec.Port), Password: "", DB: 0, TLSConfig: getRedisTLSConfig(client, logger, cr, redisInfo), @@ -459,14 +470,14 @@ func configureRedisReplicationClient(client kubernetes.Interface, logger logr.Lo logger.Error(err, "Error in getting redis password") } redisClient = redis.NewClient(&redis.Options{ - Addr: getRedisServerIP(client, logger, redisInfo) + ":6379", + Addr: getRedisServerAddress(client, logger, redisInfo, 6379), Password: pass, DB: 0, TLSConfig: getRedisReplicationTLSConfig(client, logger, cr, redisInfo), }) } else { redisClient = redis.NewClient(&redis.Options{ - Addr: getRedisServerIP(client, logger, redisInfo) + ":6379", + Addr: getRedisServerAddress(client, logger, redisInfo, 6379), Password: "", DB: 0, TLSConfig: getRedisReplicationTLSConfig(client, logger, cr, redisInfo), diff --git a/k8sutils/redis_test.go b/k8sutils/redis_test.go index 14c0b2717..732654592 100644 --- a/k8sutils/redis_test.go +++ b/k8sutils/redis_test.go @@ -90,7 +90,7 @@ func TestGetRedisServerIP(t *testing.T) { PodName: "redis-pod", Namespace: "default", }, - expectedIP: "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]", + expectedIP: "2001:0db8:85a3:0000:0000:8a2e:0370:7334", expectEmpty: false, }, { @@ -141,6 +141,71 @@ func TestGetRedisServerIP(t *testing.T) { } } +func TestGetRedisServerAddress(t *testing.T) { + tests := []struct { + name string + setup func() *k8sClientFake.Clientset + redisInfo RedisDetails + expectedAddr string + expectEmpty bool + }{ + { + name: "Successfully retrieve IPv4 URI", + setup: func() *k8sClientFake.Clientset { + return k8sClientFake.NewSimpleClientset(&corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "redis-pod", + Namespace: "default", + }, + Status: corev1.PodStatus{ + PodIP: "192.168.1.1", + }, + }) + }, + redisInfo: RedisDetails{ + PodName: "redis-pod", + Namespace: "default", + }, + expectedAddr: "192.168.1.1:6379", + expectEmpty: false, + }, + { + name: "Successfully retrieve IPv6 URI", + setup: func() *k8sClientFake.Clientset { + return k8sClientFake.NewSimpleClientset(&corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "redis-pod", + Namespace: "default", + }, + Status: corev1.PodStatus{ + PodIP: "2001:0db8:85a3:0000:0000:8a2e:0370:7334", + }, + }) + }, + redisInfo: RedisDetails{ + PodName: "redis-pod", + Namespace: "default", + }, + expectedAddr: "[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:6379", + expectEmpty: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := tt.setup() + logger := testr.New(t) + redisIP := getRedisServerAddress(client, logger, tt.redisInfo, 6379) + + if tt.expectEmpty { + assert.Empty(t, redisIP, "Expected an empty address") + } else { + assert.Equal(t, tt.expectedAddr, redisIP, "Expected and actual address do not match") + } + }) + } +} + func TestGetRedisHostname(t *testing.T) { tests := []struct { name string