Skip to content

Commit 104bc28

Browse files
kolluriakolluria
committed
Updated reconciler to use status subresource update
Updated reconciler to use controller utils to add and remove finalizer
1 parent 624dee6 commit 104bc28

File tree

2 files changed

+67
-58
lines changed

2 files changed

+67
-58
lines changed

‎pkg/kubernetes/kubernetes.go‎

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import (
5151
ccV1beta1 "sigs.k8s.io/cluster-api/api/v1beta1"
5252
"sigs.k8s.io/controller-runtime/pkg/client"
5353
apiutils "sigs.k8s.io/controller-runtime/pkg/client/apiutil"
54+
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
5455
cr_log "sigs.k8s.io/controller-runtime/pkg/log"
5556

5657
"github.com/go-logr/zapr"
@@ -798,14 +799,48 @@ func DeletePersistentVolume(ctx context.Context, k8sClient clientset.Interface,
798799
// If the object is a Custom Resource, make sure that the `subresources` field in the
799800
// CustomResourceDefinition includes `status` to enable status subresource updates.
800801
func UpdateStatus(ctx context.Context, c client.Client, obj client.Object) error {
801-
log := logger.GetLogger(ctx)
802+
log := logger.GetLogger(ctx).
803+
With("kind", obj.GetObjectKind().GroupVersionKind().Kind).
804+
With("name", obj.GetNamespace()+"/"+obj.GetName())
802805
if err := c.Status().Update(ctx, obj); err != nil {
803-
log.Errorf("Failed to update status for %s %s/%s: %v", obj.GetObjectKind().GroupVersionKind().Kind,
804-
obj.GetNamespace(), obj.GetName(), err)
806+
log.Errorf("Failed to update status. err: %v", err)
805807
return err
806808
}
807809

808-
log.Infof("Successfully updated status for %s %s/%s", obj.GetObjectKind().GroupVersionKind().Kind,
809-
obj.GetNamespace(), obj.GetName())
810+
log.Debug("Successfully updated status.")
810811
return nil
811812
}
813+
814+
// AddFinalizer adds the specified finalizer to the given Kubernetes object if it is not already present.
815+
// It updates the object in the Kubernetes cluster to persist the change.
816+
func AddFinalizer(ctx context.Context, c client.Client, obj client.Object, finalizer string) error {
817+
log := logger.GetLogger(ctx).
818+
With("finalizer", finalizer).
819+
With("kind", obj.GetObjectKind().GroupVersionKind().Kind).
820+
With("name", obj.GetNamespace()+"/"+obj.GetName())
821+
822+
if !controllerutil.AddFinalizer(obj, finalizer) {
823+
log.Debug("Finalizer already present. No update needed.")
824+
return nil
825+
}
826+
827+
log.Info("Adding finalizer to object.")
828+
return c.Update(ctx, obj)
829+
}
830+
831+
// RemoveFinalizer removes the specified finalizer from the given Kubernetes object if it is present.
832+
// It updates the object in the Kubernetes cluster to persist the change.
833+
func RemoveFinalizer(ctx context.Context, c client.Client, obj client.Object, finalizer string) error {
834+
log := logger.GetLogger(ctx).
835+
With("finalizer", finalizer).
836+
With("kind", obj.GetObjectKind().GroupVersionKind().Kind).
837+
With("name", obj.GetNamespace()+"/"+obj.GetName())
838+
839+
if !controllerutil.RemoveFinalizer(obj, finalizer) {
840+
log.Debug("Finalizer not present. No update needed.")
841+
return nil
842+
}
843+
844+
log.Info("Removing finalizer from object.")
845+
return c.Update(ctx, obj)
846+
}

‎pkg/syncer/cnsoperator/controller/cnsfileaccessconfig/cnsfileaccessconfig_controller.go‎

Lines changed: 27 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ import (
4545
"sigs.k8s.io/controller-runtime/pkg/reconcile"
4646
"sigs.k8s.io/controller-runtime/pkg/source"
4747
cnsoperatorapis "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator"
48-
cnsfileaccessconfigv1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/cnsfileaccessconfig/v1alpha1"
48+
v1a1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/cnsfileaccessconfig/v1alpha1"
4949
volumes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/volume"
5050
commonconfig "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config"
5151
"sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common"
@@ -191,8 +191,8 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
191191
// Watch for changes to primary resource CnsFileAccessConfig.
192192
err = c.Watch(source.Kind(
193193
mgr.GetCache(),
194-
&cnsfileaccessconfigv1alpha1.CnsFileAccessConfig{},
195-
&handler.TypedEnqueueRequestForObject[*cnsfileaccessconfigv1alpha1.CnsFileAccessConfig]{},
194+
&v1a1.CnsFileAccessConfig{},
195+
&handler.TypedEnqueueRequestForObject[*v1a1.CnsFileAccessConfig]{},
196196
))
197197
if err != nil {
198198
log.Errorf("Failed to watch for changes to CnsFileAccessConfig resource with error: %+v", err)
@@ -228,7 +228,7 @@ func (r *ReconcileCnsFileAccessConfig) Reconcile(ctx context.Context,
228228
request reconcile.Request) (reconcile.Result, error) {
229229
log := logger.GetLogger(ctx)
230230
// Fetch the CnsFileAccessConfig instance.
231-
instance := &cnsfileaccessconfigv1alpha1.CnsFileAccessConfig{}
231+
instance := &v1a1.CnsFileAccessConfig{}
232232
err := r.client.Get(ctx, request.NamespacedName, instance)
233233
if err != nil {
234234
if apierrors.IsNotFound(err) {
@@ -297,11 +297,10 @@ func (r *ReconcileCnsFileAccessConfig) Reconcile(ctx context.Context,
297297
}
298298

299299
// Remove finalizer from CnsFileAccessConfig CRD
300-
removeFinalizerFromCRDInstance(ctx, instance)
301-
err = updateCnsFileAccessConfig(ctx, r.client, instance)
300+
err = k8s.RemoveFinalizer(ctx, r.client, instance, cnsoperatortypes.CNSFinalizer)
302301
if err != nil {
303-
msg := fmt.Sprintf("failed to update CnsFileAccessConfig instance: %q on namespace: %q. Error: %+v",
304-
instance.Name, instance.Namespace, err)
302+
msg := fmt.Sprintf("failed to remove finalizer from CnsFileAccessConfig "+
303+
"instance: %q on namespace: %q. Error: %+v", instance.Name, instance.Namespace, err)
305304
recordEvent(ctx, r, instance, v1.EventTypeWarning, msg)
306305
return reconcile.Result{RequeueAfter: timeout}, nil
307306
}
@@ -370,8 +369,7 @@ func (r *ReconcileCnsFileAccessConfig) Reconcile(ctx context.Context,
370369
return reconcile.Result{RequeueAfter: timeout}, nil
371370
}
372371
}
373-
removeFinalizerFromCRDInstance(ctx, instance)
374-
err = updateCnsFileAccessConfig(ctx, r.client, instance)
372+
err = k8s.RemoveFinalizer(ctx, r.client, instance, cnsoperatortypes.CNSFinalizer)
375373
if err != nil {
376374
msg := fmt.Sprintf("failed to update CnsFileAccessConfig instance: %q on namespace: %q. Error: %+v",
377375
instance.Name, instance.Namespace, err)
@@ -396,24 +394,14 @@ func (r *ReconcileCnsFileAccessConfig) Reconcile(ctx context.Context,
396394
backOffDurationMapMutex.Unlock()
397395
return reconcile.Result{}, nil
398396
}
399-
cnsFinalizerExists := false
400-
// Check if finalizer already exists.
401-
for _, finalizer := range instance.Finalizers {
402-
if finalizer == cnsoperatortypes.CNSFinalizer {
403-
cnsFinalizerExists = true
404-
break
405-
}
406-
}
407-
if !cnsFinalizerExists {
408-
// Add finalizer.
409-
instance.Finalizers = append(instance.Finalizers, cnsoperatortypes.CNSFinalizer)
410-
err = updateCnsFileAccessConfig(ctx, r.client, instance)
411-
if err != nil {
412-
msg := fmt.Sprintf("failed to update CnsFileAccessConfig instance: %q on namespace: %q. Error: %+v",
413-
instance.Name, instance.Namespace, err)
414-
recordEvent(ctx, r, instance, v1.EventTypeWarning, msg)
415-
return reconcile.Result{RequeueAfter: timeout}, nil
416-
}
397+
398+
// Add finalizer to CnsFileAccessConfig instance if it does not already exist.
399+
err = k8s.AddFinalizer(ctx, r.client, instance, cnsoperatortypes.CNSFinalizer)
400+
if err != nil {
401+
msg := fmt.Sprintf("failed to add finalizer on CnsFileAccessConfig instance: %q on namespace: %q. Error: %+v",
402+
instance.Name, instance.Namespace, err)
403+
recordEvent(ctx, r, instance, v1.EventTypeWarning, msg)
404+
return reconcile.Result{RequeueAfter: timeout}, nil
417405
}
418406

419407
vmOwnerRefExists := false
@@ -528,7 +516,7 @@ func (r *ReconcileCnsFileAccessConfig) Reconcile(ctx context.Context,
528516
// addPvcFinalizer checks if CnsPvcFinalizer exists on PVC.
529517
// If it does not exist, it updates the PVC with it.
530518
func addPvcFinalizer(ctx context.Context,
531-
instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig, client client.Client) error {
519+
instance *v1a1.CnsFileAccessConfig, client client.Client) error {
532520
log := logger.GetLogger(ctx)
533521

534522
if !commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.FileVolumesWithVmService) {
@@ -566,7 +554,7 @@ func addPvcFinalizer(ctx context.Context,
566554

567555
// isPvcInUse returns true if there is at least 1 VM which is using the given PVC.
568556
func isPvcInUse(ctx context.Context, pvcName string,
569-
instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig) (bool, error) {
557+
instance *v1a1.CnsFileAccessConfig) (bool, error) {
570558
log := logger.GetLogger(ctx)
571559

572560
cnsFileVolumeClientInstance, err := cnsfilevolumeclient.GetFileVolumeClientInstance(ctx)
@@ -581,7 +569,7 @@ func isPvcInUse(ctx context.Context, pvcName string,
581569
// removeFinalizerFromPVC will remove the CNS Finalizer, cns.vmware.com/pvc-protection,
582570
// from a given PersistentVolumeClaim.
583571
func removeFinalizerFromPVC(ctx context.Context, client client.Client,
584-
instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig) error {
572+
instance *v1a1.CnsFileAccessConfig) error {
585573
log := logger.GetLogger(ctx)
586574

587575
if !commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.FileVolumesWithVmService) {
@@ -640,7 +628,7 @@ func removeFinalizerFromPVC(ctx context.Context, client client.Client,
640628
// This method is used when we don't have VM instance. It fetches the VM IP from CNSFileVolumeClient
641629
// instance for the VM name associated with CnsFileAccessConfig.
642630
func (r *ReconcileCnsFileAccessConfig) removePermissionsForFileVolume(ctx context.Context, volumeID string,
643-
instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig, skipConfigureVolumeACL bool) error {
631+
instance *v1a1.CnsFileAccessConfig, skipConfigureVolumeACL bool) error {
644632
log := logger.GetLogger(ctx)
645633
volumePermissionLock, _ := volumePermissionLockMap.LoadOrStore(volumeID, &sync.Mutex{})
646634
instanceLock, _ := volumePermissionLock.(*sync.Mutex)
@@ -685,7 +673,7 @@ func (r *ReconcileCnsFileAccessConfig) removePermissionsForFileVolume(ctx contex
685673
// permissions by setting the parameter removePermission to true or false
686674
// respectively. Returns error if any operation fails.
687675
func (r *ReconcileCnsFileAccessConfig) configureNetPermissionsForFileVolume(ctx context.Context,
688-
volumeID string, vm *vmoperatortypes.VirtualMachine, instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig,
676+
volumeID string, vm *vmoperatortypes.VirtualMachine, instance *v1a1.CnsFileAccessConfig,
689677
removePermission bool) error {
690678
log := logger.GetLogger(ctx)
691679
volumePermissionLock, _ := volumePermissionLockMap.LoadOrStore(volumeID, &sync.Mutex{})
@@ -865,10 +853,10 @@ func validateVmAndPvc(ctx context.Context, instanceLabels map[string]string, ins
865853
// setInstanceSuccess sets instance to success and records an event on the
866854
// CnsFileAccessConfig instance.
867855
func setInstanceSuccess(ctx context.Context, r *ReconcileCnsFileAccessConfig,
868-
instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig, msg string) error {
856+
instance *v1a1.CnsFileAccessConfig, msg string) error {
869857
instance.Status.Done = true
870858
instance.Status.Error = ""
871-
err := updateCnsFileAccessConfig(ctx, r.client, instance)
859+
err := k8s.UpdateStatus(ctx, r.client, instance)
872860
if err != nil {
873861
return err
874862
}
@@ -879,18 +867,18 @@ func setInstanceSuccess(ctx context.Context, r *ReconcileCnsFileAccessConfig,
879867
// setInstanceError sets error and records an event on the CnsFileAccessConfig
880868
// instance.
881869
func setInstanceError(ctx context.Context, r *ReconcileCnsFileAccessConfig,
882-
instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig, errMsg string) {
870+
instance *v1a1.CnsFileAccessConfig, errMsg string) {
883871
log := logger.GetLogger(ctx)
884872
instance.Status.Error = errMsg
885-
err := updateCnsFileAccessConfig(ctx, r.client, instance)
873+
err := k8s.UpdateStatus(ctx, r.client, instance)
886874
if err != nil {
887875
log.Errorf("updateCnsFileAccessConfig failed. err: %v", err)
888876
}
889877
recordEvent(ctx, r, instance, v1.EventTypeWarning, errMsg)
890878
}
891879

892880
func updateCnsFileAccessConfig(ctx context.Context, client client.Client,
893-
instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig) error {
881+
instance *v1a1.CnsFileAccessConfig) error {
894882
log := logger.GetLogger(ctx)
895883
err := client.Update(ctx, instance)
896884
if err != nil {
@@ -904,7 +892,7 @@ func updateCnsFileAccessConfig(ctx context.Context, client client.Client,
904892
// appropriately and logs the message.
905893
// backOffDuration is reset to 1 second on success and doubled on failure.
906894
func recordEvent(ctx context.Context, r *ReconcileCnsFileAccessConfig,
907-
instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig, eventtype string, msg string) {
895+
instance *v1a1.CnsFileAccessConfig, eventtype string, msg string) {
908896
log := logger.GetLogger(ctx)
909897
log.Debugf("Event type is %s", eventtype)
910898
namespacedName := types.NamespacedName{
@@ -927,17 +915,3 @@ func recordEvent(ctx context.Context, r *ReconcileCnsFileAccessConfig,
927915
backOffDurationMapMutex.Unlock()
928916
}
929917
}
930-
931-
// removeFinalizerFromCRDInstance will remove the CNS Finalizer = cns.vmware.com,
932-
// from a given CnsFileAccessConfig instance.
933-
func removeFinalizerFromCRDInstance(ctx context.Context, instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig) {
934-
log := logger.GetLogger(ctx)
935-
for i, finalizer := range instance.Finalizers {
936-
if finalizer == cnsoperatortypes.CNSFinalizer {
937-
log.Debugf("Removing %q finalizer from CnsFileAccessConfig instance with name: %q on namespace: %q",
938-
cnsoperatortypes.CNSFinalizer, instance.Name, instance.Namespace)
939-
instance.Finalizers = append(instance.Finalizers[:i], instance.Finalizers[i+1:]...)
940-
break
941-
}
942-
}
943-
}

0 commit comments

Comments
 (0)