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 <[email protected]> * remove which can ensure by chainsaw test `setup` Signed-off-by: drivebyer <[email protected]> --------- Signed-off-by: drivebyer <[email protected]>
OT-CONTAINER-KIT · Dec 22, 2024 · 81296c7 · 81296c7
1 parent 6ee6417
commit 81296c7
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 923 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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