From 1baed28fb2b609bea9ea394b8515b8a634384de6 Mon Sep 17 00:00:00 2001 From: Shubham Gupta <69793468+shubham-cmyk@users.noreply.github.com> Date: Sun, 31 Mar 2024 23:19:36 +0530 Subject: [PATCH] test: Add unit test (#856) * test: Add unit test Signed-off-by: Shubham Gupta * test: Cover TestCheckRedisCluster Signed-off-by: Shubham Gupta * test: fix coverage Signed-off-by: Shubham Gupta * fix: coverage Signed-off-by: Shubham Gupta * fix: remove print statement Signed-off-by: Shubham Gupta --------- Signed-off-by: Shubham Gupta --- k8sutils/redis-cluster.go | 1 - k8sutils/redis.go | 30 +++--- k8sutils/redis_test.go | 188 ++++++++++++++++++++++++++++++++++++-- k8sutils/secrets.go | 2 + mocks/utils/utils.go | 17 +++- 5 files changed, 210 insertions(+), 28 deletions(-) diff --git a/k8sutils/redis-cluster.go b/k8sutils/redis-cluster.go index 9eba2827a..99ca5a5ba 100644 --- a/k8sutils/redis-cluster.go +++ b/k8sutils/redis-cluster.go @@ -1,7 +1,6 @@ package k8sutils import ( - "fmt" "strconv" "strings" diff --git a/k8sutils/redis.go b/k8sutils/redis.go index bb983084c..525677a2e 100644 --- a/k8sutils/redis.go +++ b/k8sutils/redis.go @@ -165,8 +165,9 @@ func createRedisReplicationCommand(client kubernetes.Interface, logger logr.Logg pass, err := getRedisPassword(client, logger, cr.Namespace, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name, *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Key) if err != nil { logger.Error(err, "Failed to retrieve Redis password", "Secret", *cr.Spec.KubernetesConfig.ExistingPasswordSecret.Name) + } else { + cmd = append(cmd, "-a", pass) } - cmd = append(cmd, "-a", pass) } cmd = append(cmd, getRedisTLSArgs(cr.Spec.TLS, leaderPod.PodName)...) @@ -185,7 +186,10 @@ func ExecuteRedisReplicationCommand(ctx context.Context, client kubernetes.Inter leaderCounts := cr.Spec.GetReplicaCounts("leader") followerPerLeader := followerCounts / leaderCounts - nodes := checkRedisCluster(ctx, client, logger, cr) + redisClient := configureRedisClient(client, logger, cr, cr.ObjectMeta.Name+"-leader-0") + defer redisClient.Close() + + nodes := checkRedisCluster(ctx, redisClient, logger) for followerIdx := 0; followerIdx <= int(followerCounts)-1; { for i := 0; i < int(followerPerLeader) && followerIdx <= int(followerCounts)-1; i++ { followerPod := RedisDetails{ @@ -222,18 +226,10 @@ func ExecuteRedisReplicationCommand(ctx context.Context, client kubernetes.Inter } // checkRedisCluster will check the redis cluster have sufficient nodes or not -func checkRedisCluster(ctx context.Context, client kubernetes.Interface, logger logr.Logger, cr *redisv1beta2.RedisCluster) [][]string { - redisClient := configureRedisClient(client, logger, cr, cr.ObjectMeta.Name+"-leader-0") - defer redisClient.Close() - cmd := redis.NewStringCmd(ctx, "cluster", "nodes") - err := redisClient.Process(ctx, cmd) +func checkRedisCluster(ctx context.Context, redisClient *redis.Client, logger logr.Logger) [][]string { + output, err := redisClient.ClusterNodes(ctx).Result() if err != nil { - logger.Error(err, "Redis command failed with this error") - } - - output, err := cmd.Result() - if err != nil { - logger.Error(err, "Redis command failed with this error") + logger.Error(err, "Error in getting Redis cluster nodes") } logger.V(1).Info("Redis cluster nodes are listed", "Output", output) @@ -298,8 +294,10 @@ func executeFailoverCommand(ctx context.Context, client kubernetes.Interface, lo // CheckRedisNodeCount will check the count of redis nodes func CheckRedisNodeCount(ctx context.Context, client kubernetes.Interface, logger logr.Logger, cr *redisv1beta2.RedisCluster, nodeType string) int32 { + redisClient := configureRedisClient(client, logger, cr, cr.ObjectMeta.Name+"-leader-0") + defer redisClient.Close() var redisNodeType string - clusterNodes := checkRedisCluster(ctx, client, logger, cr) + clusterNodes := checkRedisCluster(ctx, redisClient, logger) count := len(clusterNodes) switch nodeType { @@ -326,7 +324,9 @@ func CheckRedisNodeCount(ctx context.Context, client kubernetes.Interface, logge // CheckRedisClusterState will check the redis cluster state func CheckRedisClusterState(ctx context.Context, client kubernetes.Interface, logger logr.Logger, cr *redisv1beta2.RedisCluster) int { - clusterNodes := checkRedisCluster(ctx, client, logger, cr) + redisClient := configureRedisClient(client, logger, cr, cr.ObjectMeta.Name+"-leader-0") + defer redisClient.Close() + clusterNodes := checkRedisCluster(ctx, redisClient, logger) count := 0 for _, node := range clusterNodes { if strings.Contains(node[2], "fail") || strings.Contains(node[7], "disconnected") { diff --git a/k8sutils/redis_test.go b/k8sutils/redis_test.go index 9b5adfecf..941005582 100644 --- a/k8sutils/redis_test.go +++ b/k8sutils/redis_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/OT-CONTAINER-KIT/redis-operator/api" redisv1beta2 "github.com/OT-CONTAINER-KIT/redis-operator/api/v1beta2" mock_utils "github.com/OT-CONTAINER-KIT/redis-operator/mocks/utils" "github.com/go-logr/logr" @@ -16,6 +17,8 @@ import ( "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/kubernetes/fake" k8sClientFake "k8s.io/client-go/kubernetes/fake" "k8s.io/utils/ptr" ) @@ -349,22 +352,84 @@ func TestGetRedisTLSArgs(t *testing.T) { } func TestCreateRedisReplicationCommand(t *testing.T) { + logger := logr.Discard() + type secret struct { + name string + namespace string + key string + } tests := []struct { - name string - redisCluster *redisv1beta2.RedisCluster + name string + redisCluster *redisv1beta2.RedisCluster + secret leaderPod RedisDetails followerPod RedisDetails expectedCommand []string }{ { name: "Test case with cluster version v7", + secret: secret{ + name: "redis-password-secret", + namespace: "default", + key: "password", + }, redisCluster: &redisv1beta2.RedisCluster{ ObjectMeta: metav1.ObjectMeta{ Name: "redis-cluster", Namespace: "default", }, Spec: redisv1beta2.RedisClusterSpec{ - Size: ptr.To(int32(3)), + Size: ptr.To(int32(3)), + KubernetesConfig: redisv1beta2.KubernetesConfig{ + KubernetesConfig: api.KubernetesConfig{ + ExistingPasswordSecret: &api.ExistingPasswordSecret{ + Name: ptr.To("redis-password-secret"), + Key: ptr.To("password"), + }, + }, + }, + ClusterVersion: ptr.To("v7"), + Port: ptr.To(6379), + }, + }, + leaderPod: RedisDetails{ + PodName: "redis-cluster-leader-0", + Namespace: "default", + }, + followerPod: RedisDetails{ + PodName: "redis-cluster-follower-0", + Namespace: "default", + }, + expectedCommand: []string{ + "redis-cli", "--cluster", "add-node", + "redis-cluster-follower-0.redis-cluster-follower-headless.default.svc:6379", + "redis-cluster-leader-0.redis-cluster-leader-headless.default.svc:6379", + "--cluster-slave", + "-a", "password", + }, + }, + { + name: "Test case with cluster version v7 failed to get password", + secret: secret{ + name: "wrong-name", + namespace: "default", + key: "password", + }, + redisCluster: &redisv1beta2.RedisCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "redis-cluster", + Namespace: "default", + }, + Spec: redisv1beta2.RedisClusterSpec{ + Size: ptr.To(int32(3)), + KubernetesConfig: redisv1beta2.KubernetesConfig{ + KubernetesConfig: api.KubernetesConfig{ + ExistingPasswordSecret: &api.ExistingPasswordSecret{ + Name: ptr.To("redis-password-secret"), + Key: ptr.To("password"), + }, + }, + }, ClusterVersion: ptr.To("v7"), Port: ptr.To(6379), }, @@ -415,9 +480,17 @@ func TestCreateRedisReplicationCommand(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - client := mock_utils.CreateFakeClientWithPodIPs(tt.redisCluster) - logger := testr.New(t) + pods := mock_utils.CreateFakeObjectWithPodIPs(tt.redisCluster) + var secret []runtime.Object + if tt.redisCluster.Spec.KubernetesConfig.ExistingPasswordSecret != nil { + secret = mock_utils.CreateFakeObjectWithSecret(tt.secret.name, tt.secret.namespace, tt.secret.key) + } + + var objects []runtime.Object + objects = append(objects, pods...) + objects = append(objects, secret...) + client := fake.NewSimpleClientset(objects...) cmd := createRedisReplicationCommand(client, logger, tt.redisCluster, tt.leaderPod, tt.followerPod) // Assert the command is as expected using testify @@ -568,6 +641,22 @@ func Test_checkAttachedSlave(t *testing.T) { infoErr: redis.ErrClosed, expectedSlaveCount: -1, }, + { + name: "field not found", + podName: "pod2", + infoReturn: "# Replication\r\n" + + "role:master\r\n" + + "master_failover_state:no-failover\r\n" + + "master_replid:7b634a76ebb7d5f07007f1d5aec8abff8200704e\r\n" + + "master_replid2:0000000000000000000000000000000000000000\r\n" + + "master_repl_offset:0\r\n" + + "second_repl_offset:-1\r\n" + + "repl_backlog_active:0\r\n" + + "repl_backlog_size:1048576\r\n" + + "repl_backlog_first_byte_offset:0\r\n" + + "repl_backlog_histlen:0\r\n", + expectedSlaveCount: 0, + }, } for _, tt := range tests { @@ -598,6 +687,7 @@ func Test_checkRedisServerRole(t *testing.T) { podName string infoReturn string infoErr error + shouldFail bool expectedResult string }{ { @@ -635,10 +725,26 @@ func Test_checkRedisServerRole(t *testing.T) { expectedResult: "slave", }, { - name: "error fetching role info", - podName: "pod3", - infoErr: redis.ErrClosed, - expectedResult: "", + name: "error fetching role info", + podName: "pod3", + infoErr: redis.ErrClosed, + shouldFail: true, + }, + { + name: "field not found", + podName: "pod2", + infoReturn: "# Replication\r\n" + + "connected_slaves:0\r\n" + + "master_failover_state:no-failover\r\n" + + "master_replid:7b634a76ebb7d5f07007f1d5aec8abff8200704e\r\n" + + "master_replid2:0000000000000000000000000000000000000000\r\n" + + "master_repl_offset:0\r\n" + + "second_repl_offset:-1\r\n" + + "repl_backlog_active:0\r\n" + + "repl_backlog_size:1048576\r\n" + + "repl_backlog_first_byte_offset:0\r\n" + + "repl_backlog_histlen:0\r\n", + shouldFail: true, }, } @@ -654,10 +760,72 @@ func Test_checkRedisServerRole(t *testing.T) { } role := checkRedisServerRole(ctx, client, logger, tt.podName) - assert.Equal(t, tt.expectedResult, role, "Test case: "+tt.name) + if tt.shouldFail { + assert.Empty(t, role, "Test case: "+tt.name) + } else { + assert.Equal(t, tt.expectedResult, role, "Test case: "+tt.name) + } if err := mock.ExpectationsWereMet(); err != nil { t.Errorf("there were unmet expectations: %s", err) } }) } } + +func TestCheckRedisCluster(t *testing.T) { + logger := logr.Discard() // Discard logs + + tests := []struct { + name string + expectError error + clusterNodesOutput string + expectedResult [][]string + }{ + { + name: "Detailed cluster nodes output", + clusterNodesOutput: `07c37dfeb235213a872192d90877d0cd55635b91 127.0.0.1:30004@31004,hostname4 slave e7d1eecce10fd6bb5eb35b9f99a514335d9ba9ca 0 1426238317239 4 connected +67ed2db8d677e59ec4a4cefb06858cf2a1a89fa1 127.0.0.1:30002@31002,hostname2 master - 0 1426238316232 2 connected 5461-10922 +292f8b365bb7edb5e285caf0b7e6ddc7265d2f4f 127.0.0.1:30003@31003,hostname3 master - 0 1426238318243 3 connected 10923-16383 +6ec23923021cf3ffec47632106199cb7f496ce01 127.0.0.1:30005@31005,hostname5 slave 67ed2db8d677e59ec4a4cefb06858cf2a1a89fa1 0 1426238316232 5 connected +824fe116063bc5fcf9f4ffd895bc17aee7731ac3 127.0.0.1:30006@31006,hostname6 slave 292f8b365bb7edb5e285caf0b7e6ddc7265d2f4f 0 1426238317741 6 connected +e7d1eecce10fd6bb5eb35b9f99a514335d9ba9ca 127.0.0.1:30001@31001,hostname1 myself,master - 0 0 1 connected 0-5460`, + expectedResult: [][]string{ + {"07c37dfeb235213a872192d90877d0cd55635b91", "127.0.0.1:30004@31004,hostname4", "slave", "e7d1eecce10fd6bb5eb35b9f99a514335d9ba9ca", "0", "1426238317239", "4", "connected"}, + {"67ed2db8d677e59ec4a4cefb06858cf2a1a89fa1", "127.0.0.1:30002@31002,hostname2", "master", "-", "0", "1426238316232", "2", "connected", "5461-10922"}, + {"292f8b365bb7edb5e285caf0b7e6ddc7265d2f4f", "127.0.0.1:30003@31003,hostname3", "master", "-", "0", "1426238318243", "3", "connected", "10923-16383"}, + {"6ec23923021cf3ffec47632106199cb7f496ce01", "127.0.0.1:30005@31005,hostname5", "slave", "67ed2db8d677e59ec4a4cefb06858cf2a1a89fa1", "0", "1426238316232", "5", "connected"}, + {"824fe116063bc5fcf9f4ffd895bc17aee7731ac3", "127.0.0.1:30006@31006,hostname6", "slave", "292f8b365bb7edb5e285caf0b7e6ddc7265d2f4f", "0", "1426238317741", "6", "connected"}, + {"e7d1eecce10fd6bb5eb35b9f99a514335d9ba9ca", "127.0.0.1:30001@31001,hostname1", "myself,master", "-", "0", "0", "1", "connected", "0-5460"}, + }, + }, + { + name: "Error from ClusterNodes", + expectError: redis.ErrClosed, + expectedResult: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + db, mock := redismock.NewClientMock() + + if tc.expectError != nil { + mock.ExpectClusterNodes().SetErr(tc.expectError) + } else { + mock.ExpectClusterNodes().SetVal(tc.clusterNodesOutput) + } + result := checkRedisCluster(context.TODO(), db, logger) + + if tc.expectError != nil { + assert.Nil(t, result) + } else { + assert.ElementsMatch(t, tc.expectedResult, result) + } + + // Ensure all expectations are met + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("there were unfulfilled expectations: %s", err) + } + }) + } +} diff --git a/k8sutils/secrets.go b/k8sutils/secrets.go index 40f3853fe..ab86a5960 100644 --- a/k8sutils/secrets.go +++ b/k8sutils/secrets.go @@ -25,9 +25,11 @@ func getRedisPassword(client kubernetes.Interface, logger logr.Logger, namespace } for key, value := range secretName.Data { if key == secretKey { + logger.V(1).Info("Secret key found in the secret", "secretKey", secretKey) return strings.TrimSpace(string(value)), nil } } + logger.Error(errors.New("secret key not found"), "Secret key not found in the secret") return "", nil } diff --git a/mocks/utils/utils.go b/mocks/utils/utils.go index 9c58952fc..b48dad6a2 100644 --- a/mocks/utils/utils.go +++ b/mocks/utils/utils.go @@ -30,7 +30,7 @@ func CreateFakeClientWithPodIPs_LeaderPods(cr *redisv1beta2.RedisCluster) *fake. return fake.NewSimpleClientset(pods...) } -func CreateFakeClientWithPodIPs(cr *redisv1beta2.RedisCluster) *fake.Clientset { +func CreateFakeObjectWithPodIPs(cr *redisv1beta2.RedisCluster) []runtime.Object { leaderReplicas := cr.Spec.GetReplicaCounts("leader") followerReplicas := cr.Spec.GetReplicaCounts("follower") pods := make([]runtime.Object, leaderReplicas+followerReplicas) @@ -60,7 +60,20 @@ func CreateFakeClientWithPodIPs(cr *redisv1beta2.RedisCluster) *fake.Clientset { } } - return fake.NewSimpleClientset(pods...) + return pods +} + +func CreateFakeObjectWithSecret(name, namespace, key string) []runtime.Object { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: map[string][]byte{ + key: []byte("password"), + }, + } + return []runtime.Object{secret} } func CreateFakeClientWithSecrets(cr *redisv1beta2.RedisCluster, secretName, secretKey, secretValue string) *fake.Clientset {