Skip to content

Commit 81296c7

Browse files
drivebyerdrivebyer
authored
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 <wuyangmuc@gmail.com> * remove which can ensure by chainsaw test `setup` Signed-off-by: drivebyer <wuyangmuc@gmail.com> --------- Signed-off-by: drivebyer <wuyangmuc@gmail.com>
1 parent 6ee6417 commit 81296c7

File tree

7 files changed

+51
-923
lines changed

7 files changed

+51
-923
lines changed

‎main.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,8 @@ func main() {
119119
}
120120

121121
if err = (&redis.Reconciler{
122-
Client: mgr.GetClient(),
123-
K8sClient: k8sclient,
124-
Dk8sClient: dk8sClient,
122+
Client: mgr.GetClient(),
123+
K8sClient: k8sclient,
125124
}).SetupWithManager(mgr); err != nil {
126125
setupLog.Error(err, "unable to create controller", "controller", "Redis")
127126
os.Exit(1)

‎pkg/controllers/redis/redis_controller.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
redisv1beta2 "github.com/OT-CONTAINER-KIT/redis-operator/api/v1beta2"
2424
intctrlutil "github.com/OT-CONTAINER-KIT/redis-operator/pkg/controllerutil"
2525
"github.com/OT-CONTAINER-KIT/redis-operator/pkg/k8sutils"
26-
"k8s.io/client-go/dynamic"
2726
"k8s.io/client-go/kubernetes"
2827
ctrl "sigs.k8s.io/controller-runtime"
2928
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -32,8 +31,7 @@ import (
3231
// Reconciler reconciles a Redis object
3332
type Reconciler struct {
3433
client.Client
35-
K8sClient kubernetes.Interface
36-
Dk8sClient dynamic.Interface
34+
K8sClient kubernetes.Interface
3735
}
3836

3937
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
4442
return intctrlutil.RequeueWithErrorChecking(ctx, err, "failed to get redis instance")
4543
}
4644
if instance.ObjectMeta.GetDeletionTimestamp() != nil {
47-
if err = k8sutils.HandleRedisFinalizer(ctx, r.Client, r.K8sClient, instance); err != nil {
45+
if err = k8sutils.HandleRedisFinalizer(ctx, r.Client, instance); err != nil {
4846
return intctrlutil.RequeueWithError(ctx, err, "failed to handle redis finalizer")
4947
}
5048
return intctrlutil.Reconciled()

‎pkg/controllers/redis/redis_controller_suite_test.go‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import (
2626
. "github.com/onsi/ginkgo/v2"
2727
. "github.com/onsi/gomega"
2828
"github.com/onsi/gomega/gexec"
29-
"k8s.io/client-go/dynamic"
3029
"k8s.io/client-go/kubernetes"
3130
"k8s.io/client-go/kubernetes/scheme"
3231
ctrl "sigs.k8s.io/controller-runtime"
@@ -96,13 +95,9 @@ var _ = BeforeSuite(func() {
9695
k8sClient, err := kubernetes.NewForConfig(cfg)
9796
Expect(err).ToNot(HaveOccurred())
9897

99-
dk8sClient, err := dynamic.NewForConfig(cfg)
100-
Expect(err).ToNot(HaveOccurred())
101-
10298
err = (&Reconciler{
103-
Client: k8sManager.GetClient(),
104-
K8sClient: k8sClient,
105-
Dk8sClient: dk8sClient,
99+
Client: k8sManager.GetClient(),
100+
K8sClient: k8sClient,
106101
}).SetupWithManager(k8sManager)
107102
Expect(err).ToNot(HaveOccurred())
108103

‎pkg/controllers/rediscluster/rediscluster_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
5555
return intctrlutil.RequeueWithErrorChecking(ctx, err, "failed to get redis cluster instance")
5656
}
5757
if instance.ObjectMeta.GetDeletionTimestamp() != nil {
58-
if err = k8sutils.HandleRedisClusterFinalizer(ctx, r.Client, r.K8sClient, instance); err != nil {
58+
if err = k8sutils.HandleRedisClusterFinalizer(ctx, r.Client, instance); err != nil {
5959
return intctrlutil.RequeueWithError(ctx, err, "failed to handle redis cluster finalizer")
6060
}
6161
return intctrlutil.Reconciled()

‎pkg/controllers/redisreplication/redisreplication_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ type reconciler struct {
112112

113113
func (r *Reconciler) reconcileFinalizer(ctx context.Context, instance *redisv1beta2.RedisReplication) (ctrl.Result, error) {
114114
if k8sutils.IsDeleted(instance) {
115-
if err := k8sutils.HandleRedisReplicationFinalizer(ctx, r.Client, r.K8sClient, instance); err != nil {
115+
if err := k8sutils.HandleRedisReplicationFinalizer(ctx, r.Client, instance); err != nil {
116116
return intctrlutil.RequeueWithError(ctx, err, "")
117117
}
118118
return intctrlutil.Reconciled()

‎pkg/k8sutils/finalizer.go‎

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import (
55
"fmt"
66

77
redisv1beta2 "github.com/OT-CONTAINER-KIT/redis-operator/api/v1beta2"
8+
corev1 "k8s.io/api/core/v1"
89
"k8s.io/apimachinery/pkg/api/errors"
910
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
10-
"k8s.io/client-go/kubernetes"
1111
"k8s.io/utils/env"
1212
"sigs.k8s.io/controller-runtime/pkg/client"
1313
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -22,16 +22,16 @@ const (
2222
)
2323

2424
// HandleRedisFinalizer finalize resource if instance is marked to be deleted
25-
func HandleRedisFinalizer(ctx context.Context, ctrlclient client.Client, k8sClient kubernetes.Interface, cr *redisv1beta2.Redis) error {
25+
func HandleRedisFinalizer(ctx context.Context, ctrlclient client.Client, cr *redisv1beta2.Redis) error {
2626
if cr.GetDeletionTimestamp() != nil {
2727
if controllerutil.ContainsFinalizer(cr, RedisFinalizer) {
2828
if cr.Spec.Storage != nil && !cr.Spec.Storage.KeepAfterDelete {
29-
if err := finalizeRedisPVC(ctx, k8sClient, cr); err != nil {
29+
if err := finalizeRedisPVC(ctx, ctrlclient, cr); err != nil {
3030
return err
3131
}
3232
}
3333
controllerutil.RemoveFinalizer(cr, RedisFinalizer)
34-
if err := ctrlclient.Update(context.TODO(), cr); err != nil {
34+
if err := ctrlclient.Update(ctx, cr); err != nil {
3535
log.FromContext(ctx).Error(err, "Could not remove finalizer", "finalizer", RedisFinalizer)
3636
return err
3737
}
@@ -41,16 +41,16 @@ func HandleRedisFinalizer(ctx context.Context, ctrlclient client.Client, k8sClie
4141
}
4242

4343
// HandleRedisClusterFinalizer finalize resource if instance is marked to be deleted
44-
func HandleRedisClusterFinalizer(ctx context.Context, ctrlclient client.Client, k8sClient kubernetes.Interface, cr *redisv1beta2.RedisCluster) error {
44+
func HandleRedisClusterFinalizer(ctx context.Context, ctrlclient client.Client, cr *redisv1beta2.RedisCluster) error {
4545
if cr.GetDeletionTimestamp() != nil {
4646
if controllerutil.ContainsFinalizer(cr, RedisClusterFinalizer) {
4747
if cr.Spec.Storage != nil && !cr.Spec.Storage.KeepAfterDelete {
48-
if err := finalizeRedisClusterPVC(ctx, k8sClient, cr); err != nil {
48+
if err := finalizeRedisClusterPVC(ctx, ctrlclient, cr); err != nil {
4949
return err
5050
}
5151
}
5252
controllerutil.RemoveFinalizer(cr, RedisClusterFinalizer)
53-
if err := ctrlclient.Update(context.TODO(), cr); err != nil {
53+
if err := ctrlclient.Update(ctx, cr); err != nil {
5454
log.FromContext(ctx).Error(err, "Could not remove finalizer "+RedisClusterFinalizer)
5555
return err
5656
}
@@ -60,16 +60,16 @@ func HandleRedisClusterFinalizer(ctx context.Context, ctrlclient client.Client,
6060
}
6161

6262
// Handle RedisReplicationFinalizer finalize resource if instance is marked to be deleted
63-
func HandleRedisReplicationFinalizer(ctx context.Context, ctrlclient client.Client, k8sClient kubernetes.Interface, cr *redisv1beta2.RedisReplication) error {
63+
func HandleRedisReplicationFinalizer(ctx context.Context, ctrlclient client.Client, cr *redisv1beta2.RedisReplication) error {
6464
if cr.GetDeletionTimestamp() != nil {
6565
if controllerutil.ContainsFinalizer(cr, RedisReplicationFinalizer) {
6666
if cr.Spec.Storage != nil && !cr.Spec.Storage.KeepAfterDelete {
67-
if err := finalizeRedisReplicationPVC(ctx, k8sClient, cr); err != nil {
67+
if err := finalizeRedisReplicationPVC(ctx, ctrlclient, cr); err != nil {
6868
return err
6969
}
7070
}
7171
controllerutil.RemoveFinalizer(cr, RedisReplicationFinalizer)
72-
if err := ctrlclient.Update(context.TODO(), cr); err != nil {
72+
if err := ctrlclient.Update(ctx, cr); err != nil {
7373
log.FromContext(ctx).Error(err, "Could not remove finalizer "+RedisReplicationFinalizer)
7474
return err
7575
}
@@ -83,7 +83,7 @@ func HandleRedisSentinelFinalizer(ctx context.Context, ctrlclient client.Client,
8383
if cr.GetDeletionTimestamp() != nil {
8484
if controllerutil.ContainsFinalizer(cr, RedisSentinelFinalizer) {
8585
controllerutil.RemoveFinalizer(cr, RedisSentinelFinalizer)
86-
if err := ctrlclient.Update(context.TODO(), cr); err != nil {
86+
if err := ctrlclient.Update(ctx, cr); err != nil {
8787
log.FromContext(ctx).Error(err, "Could not remove finalizer "+RedisSentinelFinalizer)
8888
return err
8989
}
@@ -96,16 +96,22 @@ func HandleRedisSentinelFinalizer(ctx context.Context, ctrlclient client.Client,
9696
func AddFinalizer(ctx context.Context, cr client.Object, finalizer string, cl client.Client) error {
9797
if !controllerutil.ContainsFinalizer(cr, finalizer) {
9898
controllerutil.AddFinalizer(cr, finalizer)
99-
return cl.Update(context.TODO(), cr)
99+
return cl.Update(ctx, cr)
100100
}
101101
return nil
102102
}
103103

104104
// finalizeRedisPVC delete PVC
105-
func finalizeRedisPVC(ctx context.Context, client kubernetes.Interface, cr *redisv1beta2.Redis) error {
105+
func finalizeRedisPVC(ctx context.Context, client client.Client, cr *redisv1beta2.Redis) error {
106106
pvcTemplateName := env.GetString(EnvOperatorSTSPVCTemplateName, cr.Name)
107107
PVCName := fmt.Sprintf("%s-%s-0", pvcTemplateName, cr.Name)
108-
err := client.CoreV1().PersistentVolumeClaims(cr.Namespace).Delete(context.TODO(), PVCName, metav1.DeleteOptions{})
108+
pvc := &corev1.PersistentVolumeClaim{
109+
ObjectMeta: metav1.ObjectMeta{
110+
Namespace: cr.Namespace,
111+
Name: PVCName,
112+
},
113+
}
114+
err := client.Delete(ctx, pvc)
109115
if err != nil && !errors.IsNotFound(err) {
110116
log.FromContext(ctx).Error(err, "Could not delete Persistent Volume Claim", "PVCName", PVCName)
111117
return err
@@ -114,12 +120,18 @@ func finalizeRedisPVC(ctx context.Context, client kubernetes.Interface, cr *redi
114120
}
115121

116122
// finalizeRedisClusterPVC delete PVCs
117-
func finalizeRedisClusterPVC(ctx context.Context, client kubernetes.Interface, cr *redisv1beta2.RedisCluster) error {
123+
func finalizeRedisClusterPVC(ctx context.Context, client client.Client, cr *redisv1beta2.RedisCluster) error {
118124
for _, role := range []string{"leader", "follower"} {
119125
for i := 0; i < int(cr.Spec.GetReplicaCounts(role)); i++ {
120126
pvcTemplateName := env.GetString(EnvOperatorSTSPVCTemplateName, cr.Name+"-"+role)
121127
PVCName := fmt.Sprintf("%s-%s-%s-%d", pvcTemplateName, cr.Name, role, i)
122-
err := client.CoreV1().PersistentVolumeClaims(cr.Namespace).Delete(context.TODO(), PVCName, metav1.DeleteOptions{})
128+
pvc := &corev1.PersistentVolumeClaim{
129+
ObjectMeta: metav1.ObjectMeta{
130+
Namespace: cr.Namespace,
131+
Name: PVCName,
132+
},
133+
}
134+
err := client.Delete(ctx, pvc)
123135
if err != nil && !errors.IsNotFound(err) {
124136
log.FromContext(ctx).Error(err, "Could not delete Persistent Volume Claim "+PVCName)
125137
return err
@@ -128,7 +140,13 @@ func finalizeRedisClusterPVC(ctx context.Context, client kubernetes.Interface, c
128140
if cr.Spec.Storage.NodeConfVolume {
129141
for i := 0; i < int(cr.Spec.GetReplicaCounts(role)); i++ {
130142
PVCName := fmt.Sprintf("%s-%s-%s-%d", "node-conf", cr.Name, role, i)
131-
err := client.CoreV1().PersistentVolumeClaims(cr.Namespace).Delete(context.TODO(), PVCName, metav1.DeleteOptions{})
143+
pvc := &corev1.PersistentVolumeClaim{
144+
ObjectMeta: metav1.ObjectMeta{
145+
Namespace: cr.Namespace,
146+
Name: PVCName,
147+
},
148+
}
149+
err := client.Delete(ctx, pvc)
132150
if err != nil && !errors.IsNotFound(err) {
133151
log.FromContext(ctx).Error(err, "Could not delete Persistent Volume Claim "+PVCName)
134152
return err
@@ -140,11 +158,17 @@ func finalizeRedisClusterPVC(ctx context.Context, client kubernetes.Interface, c
140158
}
141159

142160
// finalizeRedisReplicationPVC delete PVCs
143-
func finalizeRedisReplicationPVC(ctx context.Context, client kubernetes.Interface, cr *redisv1beta2.RedisReplication) error {
161+
func finalizeRedisReplicationPVC(ctx context.Context, client client.Client, cr *redisv1beta2.RedisReplication) error {
144162
for i := 0; i < int(cr.Spec.GetReplicationCounts("replication")); i++ {
145163
pvcTemplateName := env.GetString(EnvOperatorSTSPVCTemplateName, cr.Name)
146164
PVCName := fmt.Sprintf("%s-%s-%d", pvcTemplateName, cr.Name, i)
147-
err := client.CoreV1().PersistentVolumeClaims(cr.Namespace).Delete(context.TODO(), PVCName, metav1.DeleteOptions{})
165+
pvc := &corev1.PersistentVolumeClaim{
166+
ObjectMeta: metav1.ObjectMeta{
167+
Namespace: cr.Namespace,
168+
Name: PVCName,
169+
},
170+
}
171+
err := client.Delete(ctx, pvc)
148172
if err != nil && !errors.IsNotFound(err) {
149173
log.FromContext(ctx).Error(err, "Could not delete Persistent Volume Claim "+PVCName)
150174
return err

0 commit comments

Comments
 (0)