From 81296c7a50d91ab420a2f9ee2ac8ccda5d84b033 Mon Sep 17 00:00:00 2001 From: yangw Date: Sun, 22 Dec 2024 17:39:01 +0800 Subject: [PATCH] feat: use client.Client instead of K8sClient in finalizer. (#1183) * feat: use client.Client instead of K8sClient in finalizer - Eliminated the K8sClient field from the Reconciler struct in Redis-related controller files, including redis_controller.go, redis_controller_suite_test.go, and redisreplication_controller.go, as it was not utilized. - Updated finalizer handling functions to remove K8sClient parameters, simplifying the function signatures and improving code clarity. - Adjusted related test cases to reflect these changes, ensuring consistency across the codebase. This refactor enhances maintainability by streamlining the code and removing unnecessary dependencies. Signed-off-by: drivebyer * remove which can ensure by chainsaw test `setup` Signed-off-by: drivebyer --------- Signed-off-by: drivebyer --- main.go | 5 +- pkg/controllers/redis/redis_controller.go | 6 +- .../redis/redis_controller_suite_test.go | 9 +- .../rediscluster/rediscluster_controller.go | 2 +- .../redisreplication_controller.go | 2 +- pkg/k8sutils/finalizer.go | 62 +- pkg/k8sutils/finalizer_test.go | 888 ------------------ 7 files changed, 51 insertions(+), 923 deletions(-) delete mode 100644 pkg/k8sutils/finalizer_test.go diff --git a/main.go b/main.go index 354ba1ccb..900b121c7 100644 --- a/main.go +++ b/main.go @@ -119,9 +119,8 @@ func main() { } if err = (&redis.Reconciler{ - Client: mgr.GetClient(), - K8sClient: k8sclient, - Dk8sClient: dk8sClient, + Client: mgr.GetClient(), + K8sClient: k8sclient, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Redis") os.Exit(1) diff --git a/pkg/controllers/redis/redis_controller.go b/pkg/controllers/redis/redis_controller.go index 9fb9ae5d5..199483527 100644 --- a/pkg/controllers/redis/redis_controller.go +++ b/pkg/controllers/redis/redis_controller.go @@ -23,7 +23,6 @@ import ( redisv1beta2 "github.com/OT-CONTAINER-KIT/redis-operator/api/v1beta2" intctrlutil "github.com/OT-CONTAINER-KIT/redis-operator/pkg/controllerutil" "github.com/OT-CONTAINER-KIT/redis-operator/pkg/k8sutils" - "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -32,8 +31,7 @@ import ( // Reconciler reconciles a Redis object type Reconciler struct { client.Client - K8sClient kubernetes.Interface - Dk8sClient dynamic.Interface + K8sClient kubernetes.Interface } func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { @@ -44,7 +42,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return intctrlutil.RequeueWithErrorChecking(ctx, err, "failed to get redis instance") } if instance.ObjectMeta.GetDeletionTimestamp() != nil { - if err = k8sutils.HandleRedisFinalizer(ctx, r.Client, r.K8sClient, instance); err != nil { + if err = k8sutils.HandleRedisFinalizer(ctx, r.Client, instance); err != nil { return intctrlutil.RequeueWithError(ctx, err, "failed to handle redis finalizer") } return intctrlutil.Reconciled() diff --git a/pkg/controllers/redis/redis_controller_suite_test.go b/pkg/controllers/redis/redis_controller_suite_test.go index 68a75c2a8..86cd0ad59 100644 --- a/pkg/controllers/redis/redis_controller_suite_test.go +++ b/pkg/controllers/redis/redis_controller_suite_test.go @@ -26,7 +26,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "github.com/onsi/gomega/gexec" - "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" @@ -96,13 +95,9 @@ var _ = BeforeSuite(func() { k8sClient, err := kubernetes.NewForConfig(cfg) Expect(err).ToNot(HaveOccurred()) - dk8sClient, err := dynamic.NewForConfig(cfg) - Expect(err).ToNot(HaveOccurred()) - err = (&Reconciler{ - Client: k8sManager.GetClient(), - K8sClient: k8sClient, - Dk8sClient: dk8sClient, + Client: k8sManager.GetClient(), + K8sClient: k8sClient, }).SetupWithManager(k8sManager) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/controllers/rediscluster/rediscluster_controller.go b/pkg/controllers/rediscluster/rediscluster_controller.go index e06ccd48d..cba1f6291 100644 --- a/pkg/controllers/rediscluster/rediscluster_controller.go +++ b/pkg/controllers/rediscluster/rediscluster_controller.go @@ -55,7 +55,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return intctrlutil.RequeueWithErrorChecking(ctx, err, "failed to get redis cluster instance") } if instance.ObjectMeta.GetDeletionTimestamp() != nil { - if err = k8sutils.HandleRedisClusterFinalizer(ctx, r.Client, r.K8sClient, instance); err != nil { + if err = k8sutils.HandleRedisClusterFinalizer(ctx, r.Client, instance); err != nil { return intctrlutil.RequeueWithError(ctx, err, "failed to handle redis cluster finalizer") } return intctrlutil.Reconciled() diff --git a/pkg/controllers/redisreplication/redisreplication_controller.go b/pkg/controllers/redisreplication/redisreplication_controller.go index b205468b3..bc53305f8 100644 --- a/pkg/controllers/redisreplication/redisreplication_controller.go +++ b/pkg/controllers/redisreplication/redisreplication_controller.go @@ -112,7 +112,7 @@ type reconciler struct { func (r *Reconciler) reconcileFinalizer(ctx context.Context, instance *redisv1beta2.RedisReplication) (ctrl.Result, error) { if k8sutils.IsDeleted(instance) { - if err := k8sutils.HandleRedisReplicationFinalizer(ctx, r.Client, r.K8sClient, instance); err != nil { + if err := k8sutils.HandleRedisReplicationFinalizer(ctx, r.Client, instance); err != nil { return intctrlutil.RequeueWithError(ctx, err, "") } return intctrlutil.Reconciled() diff --git a/pkg/k8sutils/finalizer.go b/pkg/k8sutils/finalizer.go index 6331fd319..91572cc1a 100644 --- a/pkg/k8sutils/finalizer.go +++ b/pkg/k8sutils/finalizer.go @@ -5,9 +5,9 @@ import ( "fmt" redisv1beta2 "github.com/OT-CONTAINER-KIT/redis-operator/api/v1beta2" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" "k8s.io/utils/env" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -22,16 +22,16 @@ const ( ) // HandleRedisFinalizer finalize resource if instance is marked to be deleted -func HandleRedisFinalizer(ctx context.Context, ctrlclient client.Client, k8sClient kubernetes.Interface, cr *redisv1beta2.Redis) error { +func HandleRedisFinalizer(ctx context.Context, ctrlclient client.Client, cr *redisv1beta2.Redis) error { if cr.GetDeletionTimestamp() != nil { if controllerutil.ContainsFinalizer(cr, RedisFinalizer) { if cr.Spec.Storage != nil && !cr.Spec.Storage.KeepAfterDelete { - if err := finalizeRedisPVC(ctx, k8sClient, cr); err != nil { + if err := finalizeRedisPVC(ctx, ctrlclient, cr); err != nil { return err } } controllerutil.RemoveFinalizer(cr, RedisFinalizer) - if err := ctrlclient.Update(context.TODO(), cr); err != nil { + if err := ctrlclient.Update(ctx, cr); err != nil { log.FromContext(ctx).Error(err, "Could not remove finalizer", "finalizer", RedisFinalizer) return err } @@ -41,16 +41,16 @@ func HandleRedisFinalizer(ctx context.Context, ctrlclient client.Client, k8sClie } // HandleRedisClusterFinalizer finalize resource if instance is marked to be deleted -func HandleRedisClusterFinalizer(ctx context.Context, ctrlclient client.Client, k8sClient kubernetes.Interface, cr *redisv1beta2.RedisCluster) error { +func HandleRedisClusterFinalizer(ctx context.Context, ctrlclient client.Client, cr *redisv1beta2.RedisCluster) error { if cr.GetDeletionTimestamp() != nil { if controllerutil.ContainsFinalizer(cr, RedisClusterFinalizer) { if cr.Spec.Storage != nil && !cr.Spec.Storage.KeepAfterDelete { - if err := finalizeRedisClusterPVC(ctx, k8sClient, cr); err != nil { + if err := finalizeRedisClusterPVC(ctx, ctrlclient, cr); err != nil { return err } } controllerutil.RemoveFinalizer(cr, RedisClusterFinalizer) - if err := ctrlclient.Update(context.TODO(), cr); err != nil { + if err := ctrlclient.Update(ctx, cr); err != nil { log.FromContext(ctx).Error(err, "Could not remove finalizer "+RedisClusterFinalizer) return err } @@ -60,16 +60,16 @@ func HandleRedisClusterFinalizer(ctx context.Context, ctrlclient client.Client, } // Handle RedisReplicationFinalizer finalize resource if instance is marked to be deleted -func HandleRedisReplicationFinalizer(ctx context.Context, ctrlclient client.Client, k8sClient kubernetes.Interface, cr *redisv1beta2.RedisReplication) error { +func HandleRedisReplicationFinalizer(ctx context.Context, ctrlclient client.Client, cr *redisv1beta2.RedisReplication) error { if cr.GetDeletionTimestamp() != nil { if controllerutil.ContainsFinalizer(cr, RedisReplicationFinalizer) { if cr.Spec.Storage != nil && !cr.Spec.Storage.KeepAfterDelete { - if err := finalizeRedisReplicationPVC(ctx, k8sClient, cr); err != nil { + if err := finalizeRedisReplicationPVC(ctx, ctrlclient, cr); err != nil { return err } } controllerutil.RemoveFinalizer(cr, RedisReplicationFinalizer) - if err := ctrlclient.Update(context.TODO(), cr); err != nil { + if err := ctrlclient.Update(ctx, cr); err != nil { log.FromContext(ctx).Error(err, "Could not remove finalizer "+RedisReplicationFinalizer) return err } @@ -83,7 +83,7 @@ func HandleRedisSentinelFinalizer(ctx context.Context, ctrlclient client.Client, if cr.GetDeletionTimestamp() != nil { if controllerutil.ContainsFinalizer(cr, RedisSentinelFinalizer) { controllerutil.RemoveFinalizer(cr, RedisSentinelFinalizer) - if err := ctrlclient.Update(context.TODO(), cr); err != nil { + if err := ctrlclient.Update(ctx, cr); err != nil { log.FromContext(ctx).Error(err, "Could not remove finalizer "+RedisSentinelFinalizer) return err } @@ -96,16 +96,22 @@ func HandleRedisSentinelFinalizer(ctx context.Context, ctrlclient client.Client, func AddFinalizer(ctx context.Context, cr client.Object, finalizer string, cl client.Client) error { if !controllerutil.ContainsFinalizer(cr, finalizer) { controllerutil.AddFinalizer(cr, finalizer) - return cl.Update(context.TODO(), cr) + return cl.Update(ctx, cr) } return nil } // finalizeRedisPVC delete PVC -func finalizeRedisPVC(ctx context.Context, client kubernetes.Interface, cr *redisv1beta2.Redis) error { +func finalizeRedisPVC(ctx context.Context, client client.Client, cr *redisv1beta2.Redis) error { pvcTemplateName := env.GetString(EnvOperatorSTSPVCTemplateName, cr.Name) PVCName := fmt.Sprintf("%s-%s-0", pvcTemplateName, cr.Name) - err := client.CoreV1().PersistentVolumeClaims(cr.Namespace).Delete(context.TODO(), PVCName, metav1.DeleteOptions{}) + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cr.Namespace, + Name: PVCName, + }, + } + err := client.Delete(ctx, pvc) if err != nil && !errors.IsNotFound(err) { log.FromContext(ctx).Error(err, "Could not delete Persistent Volume Claim", "PVCName", PVCName) return err @@ -114,12 +120,18 @@ func finalizeRedisPVC(ctx context.Context, client kubernetes.Interface, cr *redi } // finalizeRedisClusterPVC delete PVCs -func finalizeRedisClusterPVC(ctx context.Context, client kubernetes.Interface, cr *redisv1beta2.RedisCluster) error { +func finalizeRedisClusterPVC(ctx context.Context, client client.Client, cr *redisv1beta2.RedisCluster) error { for _, role := range []string{"leader", "follower"} { for i := 0; i < int(cr.Spec.GetReplicaCounts(role)); i++ { pvcTemplateName := env.GetString(EnvOperatorSTSPVCTemplateName, cr.Name+"-"+role) PVCName := fmt.Sprintf("%s-%s-%s-%d", pvcTemplateName, cr.Name, role, i) - err := client.CoreV1().PersistentVolumeClaims(cr.Namespace).Delete(context.TODO(), PVCName, metav1.DeleteOptions{}) + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cr.Namespace, + Name: PVCName, + }, + } + err := client.Delete(ctx, pvc) if err != nil && !errors.IsNotFound(err) { log.FromContext(ctx).Error(err, "Could not delete Persistent Volume Claim "+PVCName) return err @@ -128,7 +140,13 @@ func finalizeRedisClusterPVC(ctx context.Context, client kubernetes.Interface, c if cr.Spec.Storage.NodeConfVolume { for i := 0; i < int(cr.Spec.GetReplicaCounts(role)); i++ { PVCName := fmt.Sprintf("%s-%s-%s-%d", "node-conf", cr.Name, role, i) - err := client.CoreV1().PersistentVolumeClaims(cr.Namespace).Delete(context.TODO(), PVCName, metav1.DeleteOptions{}) + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cr.Namespace, + Name: PVCName, + }, + } + err := client.Delete(ctx, pvc) if err != nil && !errors.IsNotFound(err) { log.FromContext(ctx).Error(err, "Could not delete Persistent Volume Claim "+PVCName) return err @@ -140,11 +158,17 @@ func finalizeRedisClusterPVC(ctx context.Context, client kubernetes.Interface, c } // finalizeRedisReplicationPVC delete PVCs -func finalizeRedisReplicationPVC(ctx context.Context, client kubernetes.Interface, cr *redisv1beta2.RedisReplication) error { +func finalizeRedisReplicationPVC(ctx context.Context, client client.Client, cr *redisv1beta2.RedisReplication) error { for i := 0; i < int(cr.Spec.GetReplicationCounts("replication")); i++ { pvcTemplateName := env.GetString(EnvOperatorSTSPVCTemplateName, cr.Name) PVCName := fmt.Sprintf("%s-%s-%d", pvcTemplateName, cr.Name, i) - err := client.CoreV1().PersistentVolumeClaims(cr.Namespace).Delete(context.TODO(), PVCName, metav1.DeleteOptions{}) + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: cr.Namespace, + Name: PVCName, + }, + } + err := client.Delete(ctx, pvc) if err != nil && !errors.IsNotFound(err) { log.FromContext(ctx).Error(err, "Could not delete Persistent Volume Claim "+PVCName) return err diff --git a/pkg/k8sutils/finalizer_test.go b/pkg/k8sutils/finalizer_test.go deleted file mode 100644 index 4f46d8f7f..000000000 --- a/pkg/k8sutils/finalizer_test.go +++ /dev/null @@ -1,888 +0,0 @@ -package k8sutils - -import ( - "context" - "fmt" - "testing" - "time" - - "github.com/OT-CONTAINER-KIT/redis-operator/api" - "github.com/OT-CONTAINER-KIT/redis-operator/api/v1beta2" - mockClient "github.com/OT-CONTAINER-KIT/redis-operator/mocks/client" - "github.com/stretchr/testify/assert" - corev1 "k8s.io/api/core/v1" - k8serrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - k8sClientFake "k8s.io/client-go/kubernetes/fake" - "k8s.io/utils/ptr" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -func TestHandleRedisFinalizer(t *testing.T) { - tests := []struct { - name string - mockClient *mockClient.MockClient - hasFinalizers bool - cr *v1beta2.Redis - existingPVC *corev1.PersistentVolumeClaim - expectError bool - }{ - { - name: "Redis CR without finalizer", - mockClient: &mockClient.MockClient{ - UpdateFn: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - hasFinalizers: false, - cr: &v1beta2.Redis{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-standalone", - Namespace: "default", - Finalizers: []string{}, - DeletionTimestamp: &metav1.Time{ - Time: time.Now(), - }, - }, - Spec: v1beta2.RedisSpec{ - Storage: &v1beta2.Storage{ - Storage: api.Storage{ - KeepAfterDelete: false, - }, - }, - }, - }, - existingPVC: &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-standalone-redis-standalone-0", - Namespace: "default", - }, - }, - expectError: false, - }, - { - name: "Redis CR with finalizer", - mockClient: &mockClient.MockClient{ - UpdateFn: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - hasFinalizers: true, - cr: &v1beta2.Redis{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-standalone", - Namespace: "default", - Finalizers: []string{RedisFinalizer}, - DeletionTimestamp: &metav1.Time{ - Time: time.Now(), - }, - }, - Spec: v1beta2.RedisSpec{ - Storage: &v1beta2.Storage{ - Storage: api.Storage{ - KeepAfterDelete: false, - }, - }, - }, - }, - existingPVC: &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-standalone-redis-standalone-0", - Namespace: "default", - }, - }, - expectError: false, - }, - { - name: "Redis CR with finalizer and keepAfterDelete true", - mockClient: &mockClient.MockClient{ - UpdateFn: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - hasFinalizers: true, - cr: &v1beta2.Redis{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-standalone", - Namespace: "default", - Finalizers: []string{RedisFinalizer}, - DeletionTimestamp: &metav1.Time{ - Time: time.Now(), - }, - }, - Spec: v1beta2.RedisSpec{ - Storage: &v1beta2.Storage{ - Storage: api.Storage{ - KeepAfterDelete: true, - }, - }, - }, - }, - existingPVC: &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-standalone-redis-standalone-0", - Namespace: "default", - }, - }, - expectError: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - var k8sClient *k8sClientFake.Clientset - if tc.existingPVC != nil { - k8sClient = k8sClientFake.NewSimpleClientset(tc.existingPVC.DeepCopyObject()) - } else { - k8sClient = k8sClientFake.NewSimpleClientset() - } - - // Verify that the PVC was created - if tc.existingPVC != nil { - pvcName := fmt.Sprintf("%s-%s-0", tc.cr.Name, tc.cr.Name) - _, err := k8sClient.CoreV1().PersistentVolumeClaims(tc.cr.Namespace).Get(context.TODO(), pvcName, metav1.GetOptions{}) - assert.NoError(t, err) - } - - err := HandleRedisFinalizer(context.TODO(), tc.mockClient, k8sClient, tc.cr) - if tc.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Empty(t, tc.cr.GetFinalizers()) - } - - // Verify that the PVC is not found in case of success or non-existent PVC - if !tc.expectError && tc.cr.DeletionTimestamp != nil && tc.hasFinalizers { - pvcName := fmt.Sprintf("%s-%s-0", tc.cr.GetName(), tc.cr.GetName()) - _, err = k8sClient.CoreV1().PersistentVolumeClaims(tc.cr.GetNamespace()).Get(context.TODO(), pvcName, metav1.GetOptions{}) - if tc.cr.Spec.Storage.KeepAfterDelete { - assert.NoError(t, err) - } else { - assert.True(t, k8serrors.IsNotFound(err)) - } - } - }) - } -} - -func TestHandleRedisClusterFinalizer(t *testing.T) { - tests := []struct { - name string - mockClient *mockClient.MockClient - hasFinalizers bool - cr *v1beta2.RedisCluster - existingPVC []*corev1.PersistentVolumeClaim - expectError bool - }{ - { - name: "Redis Cluster CR without finalizer", - mockClient: &mockClient.MockClient{ - UpdateFn: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - hasFinalizers: false, - cr: &v1beta2.RedisCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-cluster", - Namespace: "default", - Finalizers: []string{}, - DeletionTimestamp: &metav1.Time{ - Time: time.Now(), - }, - }, - }, - existingPVC: helperRedisClusterPVCs("redis-cluster", "default"), - expectError: false, - }, - { - name: "Redis Cluster CR with finalizer", - mockClient: &mockClient.MockClient{ - UpdateFn: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - hasFinalizers: true, - cr: &v1beta2.RedisCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-cluster", - Namespace: "default", - Finalizers: []string{RedisClusterFinalizer}, - DeletionTimestamp: &metav1.Time{ - Time: time.Now(), - }, - }, - Spec: v1beta2.RedisClusterSpec{ - Size: ptr.To(int32(3)), - Storage: &v1beta2.ClusterStorage{ - Storage: api.Storage{ - KeepAfterDelete: false, - }, - NodeConfVolume: true, - }, - }, - }, - existingPVC: helperRedisClusterPVCs("redis-cluster", "default"), - expectError: false, - }, - { - name: "Redis Cluster CR with finalizer and keepAfterDelete true", - mockClient: &mockClient.MockClient{ - UpdateFn: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - hasFinalizers: true, - cr: &v1beta2.RedisCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-cluster", - Namespace: "default", - Finalizers: []string{RedisClusterFinalizer}, - DeletionTimestamp: &metav1.Time{ - Time: time.Now(), - }, - }, - Spec: v1beta2.RedisClusterSpec{ - Size: ptr.To(int32(3)), - Storage: &v1beta2.ClusterStorage{ - Storage: api.Storage{ - KeepAfterDelete: true, - }, - }, - }, - }, - existingPVC: helperRedisClusterPVCs("redis-cluster", "default"), - expectError: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - var k8sClient *k8sClientFake.Clientset - if tc.existingPVC != nil { - k8sClient = k8sClientFake.NewSimpleClientset(helperToRuntimeObjects(tc.existingPVC)...) - } else { - k8sClient = k8sClientFake.NewSimpleClientset() - } - - // Verify that the PVC was created - if len(tc.existingPVC) != 0 { - for _, pvc := range tc.existingPVC { - pvcName := pvc.GetName() - _, err := k8sClient.CoreV1().PersistentVolumeClaims(tc.cr.Namespace).Get(context.TODO(), pvcName, metav1.GetOptions{}) - assert.NoError(t, err) - } - } - - err := HandleRedisClusterFinalizer(context.TODO(), tc.mockClient, k8sClient, tc.cr) - if tc.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Empty(t, tc.cr.GetFinalizers()) - } - - // Verify that the PVC is not found in case of success or non-existent PVC - if !tc.expectError && tc.cr.DeletionTimestamp != nil && tc.hasFinalizers { - for _, pvc := range tc.existingPVC { - pvcName := pvc.GetName() - t.Log(pvcName) - _, err := k8sClient.CoreV1().PersistentVolumeClaims(tc.cr.GetNamespace()).Get(context.TODO(), pvcName, metav1.GetOptions{}) - if tc.cr.Spec.Storage.KeepAfterDelete { - assert.NoError(t, err) - } else { - assert.True(t, k8serrors.IsNotFound(err)) - } - } - } - }) - } -} - -func TestHandleRedisReplicationFinalizer(t *testing.T) { - tests := []struct { - name string - mockClient *mockClient.MockClient - hasFinalizers bool - cr *v1beta2.RedisReplication - existingPVC []*corev1.PersistentVolumeClaim - expectError bool - }{ - { - name: "Redis Replication CR without finalizer", - mockClient: &mockClient.MockClient{ - UpdateFn: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - hasFinalizers: false, - cr: &v1beta2.RedisReplication{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication", - Namespace: "redis", - Finalizers: []string{}, - DeletionTimestamp: &metav1.Time{ - Time: time.Now(), - }, - }, - Spec: v1beta2.RedisReplicationSpec{ - Size: ptr.To(int32(3)), - Storage: &v1beta2.Storage{ - Storage: api.Storage{ - KeepAfterDelete: false, - }, - }, - }, - }, - existingPVC: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication-redis-replication-0", - Namespace: "redis", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication-redis-replication-1", - Namespace: "redis", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication-redis-replication-2", - Namespace: "redis", - }, - }, - }, - expectError: false, - }, - { - name: "Redis Replication CR with finalizer", - mockClient: &mockClient.MockClient{ - UpdateFn: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - hasFinalizers: false, - cr: &v1beta2.RedisReplication{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication", - Namespace: "redis", - Finalizers: []string{RedisReplicationFinalizer}, - DeletionTimestamp: &metav1.Time{ - Time: time.Now(), - }, - }, - Spec: v1beta2.RedisReplicationSpec{ - Size: ptr.To(int32(3)), - Storage: &v1beta2.Storage{ - Storage: api.Storage{ - KeepAfterDelete: false, - }, - }, - }, - }, - existingPVC: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication-redis-replication-0", - Namespace: "redis", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication-redis-replication-1", - Namespace: "redis", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication-redis-replication-2", - Namespace: "redis", - }, - }, - }, - expectError: false, - }, - { - name: "Redis Replication CR with finalizer and keepAfterDelete true", - mockClient: &mockClient.MockClient{ - UpdateFn: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - hasFinalizers: false, - cr: &v1beta2.RedisReplication{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication", - Namespace: "redis", - Finalizers: []string{RedisReplicationFinalizer}, - DeletionTimestamp: &metav1.Time{ - Time: time.Now(), - }, - }, - Spec: v1beta2.RedisReplicationSpec{ - Size: ptr.To(int32(3)), - Storage: &v1beta2.Storage{ - Storage: api.Storage{ - KeepAfterDelete: true, - }, - }, - }, - }, - existingPVC: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication-redis-replication-0", - Namespace: "redis", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication-redis-replication-1", - Namespace: "redis", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication-redis-replication-2", - Namespace: "redis", - }, - }, - }, - expectError: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - var k8sClient *k8sClientFake.Clientset - if tc.existingPVC != nil { - k8sClient = k8sClientFake.NewSimpleClientset(helperToRuntimeObjects(tc.existingPVC)...) - } else { - k8sClient = k8sClientFake.NewSimpleClientset() - } - - // Verify that the PVC was created - if len(tc.existingPVC) != 0 { - for _, pvc := range tc.existingPVC { - pvcName := pvc.GetName() - _, err := k8sClient.CoreV1().PersistentVolumeClaims(tc.cr.Namespace).Get(context.TODO(), pvcName, metav1.GetOptions{}) - assert.NoError(t, err) - } - } - - err := HandleRedisReplicationFinalizer(context.TODO(), tc.mockClient, k8sClient, tc.cr) - if tc.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Empty(t, tc.cr.GetFinalizers()) - } - - // Verify that the PVC is not found in case of success or non-existent PVC - if !tc.expectError && tc.cr.DeletionTimestamp != nil && tc.hasFinalizers { - for _, pvc := range tc.existingPVC { - pvcName := pvc.GetName() - t.Log(pvcName) - _, err := k8sClient.CoreV1().PersistentVolumeClaims(tc.cr.GetNamespace()).Get(context.TODO(), pvcName, metav1.GetOptions{}) - if tc.cr.Spec.Storage.KeepAfterDelete { - assert.NoError(t, err) - } else { - assert.True(t, k8serrors.IsNotFound(err)) - } - } - } - }) - } -} - -func TestHandleRedisSentinelFinalizer(t *testing.T) { - tests := []struct { - name string - mockClient *mockClient.MockClient - hasFinalizers bool - cr *v1beta2.RedisSentinel - expectError bool - }{ - { - name: "Redis Sentinel CR without finalizer", - mockClient: &mockClient.MockClient{ - UpdateFn: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - hasFinalizers: false, - cr: &v1beta2.RedisSentinel{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-sentinel", - Namespace: "default", - Finalizers: []string{}, - DeletionTimestamp: &metav1.Time{ - Time: time.Now(), - }, - }, - Spec: v1beta2.RedisSentinelSpec{}, - }, - expectError: false, - }, - { - name: "Redis Sentinel CR with finalizer", - mockClient: &mockClient.MockClient{ - UpdateFn: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - }, - hasFinalizers: false, - cr: &v1beta2.RedisSentinel{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-sentinel", - Namespace: "default", - Finalizers: []string{RedisSentinelFinalizer}, - DeletionTimestamp: &metav1.Time{ - Time: time.Now(), - }, - }, - Spec: v1beta2.RedisSentinelSpec{}, - }, - expectError: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - err := HandleRedisSentinelFinalizer(context.TODO(), tc.mockClient, tc.cr) - if tc.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Empty(t, tc.cr.GetFinalizers()) - } - }) - } -} - -func TestFinalizeRedisPVC(t *testing.T) { - tests := []struct { - name string - existingPVC *corev1.PersistentVolumeClaim - expectError bool - errorExpected error - }{ - { - name: "PVC exists and is deleted successfully", - existingPVC: &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-redis-test-redis-0", - Namespace: "default", - }, - }, - expectError: false, - errorExpected: nil, - }, - { - name: "PVC does not exist and no error should be returned", - existingPVC: nil, - expectError: false, - errorExpected: nil, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - cr := &v1beta2.Redis{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-redis", - Namespace: "default", - }, - } - var k8sClient *k8sClientFake.Clientset - if tc.existingPVC != nil { - k8sClient = k8sClientFake.NewSimpleClientset(tc.existingPVC.DeepCopyObject()) - } else { - k8sClient = k8sClientFake.NewSimpleClientset() - } - - // Verify that the PVC was created - if tc.existingPVC != nil { - pvcName := fmt.Sprintf("%s-%s-0", cr.Name, cr.Name) - _, err := k8sClient.CoreV1().PersistentVolumeClaims(cr.Namespace).Get(context.TODO(), pvcName, metav1.GetOptions{}) - assert.NoError(t, err) - } - - err := finalizeRedisPVC(context.TODO(), k8sClient, cr) - if tc.expectError { - assert.Error(t, err) - assert.Equal(t, tc.errorExpected, err) - } else { - assert.NoError(t, err) - } - - // Verify that the PVC is not found in case of success or non-existent PVC - if !tc.expectError { - pvcName := fmt.Sprintf("%s-%s-0", cr.Name, cr.Name) - _, err = k8sClient.CoreV1().PersistentVolumeClaims(cr.Namespace).Get(context.TODO(), pvcName, metav1.GetOptions{}) - assert.True(t, k8serrors.IsNotFound(err)) - } - }) - } -} - -func TestFinalizeRedisReplicationPVC(t *testing.T) { - tests := []struct { - name string - existingPVCs []*corev1.PersistentVolumeClaim - redisReplication *v1beta2.RedisReplication - expectError bool - }{ - { - name: "Successful deletion of Redis Replication PVCs", - redisReplication: &v1beta2.RedisReplication{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication", - Namespace: "redis", - }, - Spec: v1beta2.RedisReplicationSpec{ - Size: ptr.To(int32(3)), - }, - }, - existingPVCs: []*corev1.PersistentVolumeClaim{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication-redis-replication-0", - Namespace: "redis", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication-redis-replication-1", - Namespace: "redis", - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication-redis-replication-2", - Namespace: "redis", - }, - }, - }, - expectError: false, - }, - { - name: "PVC does not exist and no error should be returned", - existingPVCs: nil, - redisReplication: &v1beta2.RedisReplication{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-replication", - Namespace: "redis", - }, - Spec: v1beta2.RedisReplicationSpec{ - Size: ptr.To(int32(3)), - }, - }, - expectError: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - var k8sClient *k8sClientFake.Clientset - if tc.existingPVCs != nil { - k8sClient = k8sClientFake.NewSimpleClientset(helperToRuntimeObjects(tc.existingPVCs)...) - } else { - k8sClient = k8sClientFake.NewSimpleClientset() - } - - err := finalizeRedisReplicationPVC(context.TODO(), k8sClient, tc.redisReplication) - if tc.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - - // Verify PVCs are deleted - if !tc.expectError { - for _, pvc := range tc.existingPVCs { - _, err := k8sClient.CoreV1().PersistentVolumeClaims(pvc.Namespace).Get(context.TODO(), pvc.Name, metav1.GetOptions{}) - assert.True(t, k8serrors.IsNotFound(err)) - } - } - }) - } -} - -func TestFinalizeRedisClusterPVC(t *testing.T) { - tests := []struct { - name string - existingPVCs []*corev1.PersistentVolumeClaim - redisCluster *v1beta2.RedisCluster - expectError bool - }{ - { - name: "Successful deletion of Redis Cluster PVCs", - redisCluster: &v1beta2.RedisCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-cluster", - Namespace: "redis", - }, - Spec: v1beta2.RedisClusterSpec{ - Size: ptr.To(int32(3)), - Storage: &v1beta2.ClusterStorage{ - NodeConfVolume: true, - }, - }, - }, - existingPVCs: helperRedisClusterPVCs("redis-cluster", "redis"), - expectError: false, - }, - { - name: "PVC does not exist and no error should be returned", - existingPVCs: nil, - redisCluster: &v1beta2.RedisCluster{ - ObjectMeta: metav1.ObjectMeta{ - Name: "redis-cluster", - Namespace: "redis", - }, - Spec: v1beta2.RedisClusterSpec{ - Size: ptr.To(int32(3)), - Storage: &v1beta2.ClusterStorage{ - NodeConfVolume: false, - }, - }, - }, - expectError: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - var k8sClient *k8sClientFake.Clientset - if tc.existingPVCs != nil { - k8sClient = k8sClientFake.NewSimpleClientset(helperToRuntimeObjects(tc.existingPVCs)...) - } else { - k8sClient = k8sClientFake.NewSimpleClientset() - } - - err := finalizeRedisClusterPVC(context.TODO(), k8sClient, tc.redisCluster) - if tc.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - - // Verify PVCs are deleted - if !tc.expectError { - for _, pvc := range tc.existingPVCs { - _, err := k8sClient.CoreV1().PersistentVolumeClaims(pvc.Namespace).Get(context.TODO(), pvc.Name, metav1.GetOptions{}) - assert.True(t, k8serrors.IsNotFound(err)) - } - } - }) - } -} - -func helperToRuntimeObjects(pvcs []*corev1.PersistentVolumeClaim) []runtime.Object { - objs := make([]runtime.Object, len(pvcs)) - for i, pvc := range pvcs { - objs[i] = pvc.DeepCopyObject() - } - return objs -} - -func helperRedisClusterPVCs(clusterName string, namespace string) []*corev1.PersistentVolumeClaim { - var pvcs []*corev1.PersistentVolumeClaim - roles := []string{"leader", "follower"} - for _, role := range roles { - for i := 0; i < 3; i++ { - clusterPVCName := fmt.Sprintf("%s-%s-%s-%s-%d", clusterName, role, clusterName, role, i) - clusterPVC := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: clusterPVCName, - Namespace: namespace, - }, - } - pvcs = append(pvcs, clusterPVC) - nodeConfPVCName := fmt.Sprintf("node-conf-%s-%s-%d", clusterName, role, i) - nodeConfPVC := &corev1.PersistentVolumeClaim{ - ObjectMeta: metav1.ObjectMeta{ - Name: nodeConfPVCName, - Namespace: namespace, - }, - } - pvcs = append(pvcs, nodeConfPVC) - } - } - return pvcs -} - -func TestAddFinalizer(t *testing.T) { - type args struct { - cr *v1beta2.Redis - finalizer string - } - tests := []struct { - name string - args args - want *v1beta2.Redis - wantErr bool - }{ - { - name: "CR without finalizer", - args: args{ - cr: &v1beta2.Redis{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-redis", - Namespace: "default", - Finalizers: []string{}, - }, - }, - finalizer: RedisFinalizer, - }, - want: &v1beta2.Redis{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-redis", - Namespace: "default", - Finalizers: []string{RedisFinalizer}, - }, - }, - wantErr: false, - }, - { - name: "CR with finalizer", - args: args{ - cr: &v1beta2.Redis{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-redis", - Namespace: "default", - Finalizers: []string{RedisFinalizer}, - }, - }, - finalizer: RedisFinalizer, - }, - want: &v1beta2.Redis{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-redis", - Namespace: "default", - Finalizers: []string{RedisFinalizer}, - }, - }, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - mc := &mockClient.MockClient{ - UpdateFn: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return nil - }, - } - err := AddFinalizer(context.TODO(), tt.args.cr, tt.args.finalizer, mc) - if (err != nil) != tt.wantErr { - t.Errorf("AddFinalizer() error = %v, wantErr %v", err, tt.wantErr) - } - assert.Equal(t, tt.want.ObjectMeta.Finalizers, tt.args.cr.ObjectMeta.Finalizers) - }) - } -}