diff --git a/manifests/supervisorcluster/1.29/cns-csi.yaml b/manifests/supervisorcluster/1.29/cns-csi.yaml index 4cf45d6172..2a10eb02c5 100644 --- a/manifests/supervisorcluster/1.29/cns-csi.yaml +++ b/manifests/supervisorcluster/1.29/cns-csi.yaml @@ -46,7 +46,7 @@ rules: resources: ["volumeattachments/status"] verbs: ["patch"] - apiGroups: ["cns.vmware.com"] - resources: ["cnsvolumemetadatas", "cnsfileaccessconfigs"] + resources: ["cnsvolumemetadatas", "cnsfileaccessconfigs", "cnsfileaccessconfigs/status"] verbs: ["get", "list", "watch", "update"] - apiGroups: ["cns.vmware.com"] resources: ["cnsnodevmattachments", "cnsnodevmbatchattachments", "cnsnodevmbatchattachments/status", "cnsnodevmattachments/status"] diff --git a/manifests/supervisorcluster/1.30/cns-csi.yaml b/manifests/supervisorcluster/1.30/cns-csi.yaml index 7f7479040f..00d9696ea7 100644 --- a/manifests/supervisorcluster/1.30/cns-csi.yaml +++ b/manifests/supervisorcluster/1.30/cns-csi.yaml @@ -46,7 +46,7 @@ rules: resources: ["volumeattachments/status"] verbs: ["patch"] - apiGroups: ["cns.vmware.com"] - resources: ["cnsvolumemetadatas", "cnsfileaccessconfigs"] + resources: ["cnsvolumemetadatas", "cnsfileaccessconfigs", "cnsfileaccessconfigs/status"] verbs: ["get", "list", "watch", "update"] - apiGroups: ["cns.vmware.com"] resources: ["cnsnodevmattachments", "cnsnodevmbatchattachments", "cnsnodevmbatchattachments/status", "cnsnodevmattachments/status"] diff --git a/manifests/supervisorcluster/1.31/cns-csi.yaml b/manifests/supervisorcluster/1.31/cns-csi.yaml index efc2a1d331..77df9dde97 100644 --- a/manifests/supervisorcluster/1.31/cns-csi.yaml +++ b/manifests/supervisorcluster/1.31/cns-csi.yaml @@ -46,7 +46,7 @@ rules: resources: ["volumeattachments/status"] verbs: ["patch"] - apiGroups: ["cns.vmware.com"] - resources: ["cnsvolumemetadatas", "cnsfileaccessconfigs"] + resources: ["cnsvolumemetadatas", "cnsfileaccessconfigs", "cnsfileaccessconfigs/status"] verbs: ["get", "list", "watch", "update"] - apiGroups: ["cns.vmware.com"] resources: ["cnsnodevmattachments", "cnsnodevmbatchattachments", "cnsnodevmbatchattachments/status", "cnsnodevmattachments/status"] diff --git a/manifests/supervisorcluster/1.32/cns-csi.yaml b/manifests/supervisorcluster/1.32/cns-csi.yaml index 7f7479040f..00d9696ea7 100644 --- a/manifests/supervisorcluster/1.32/cns-csi.yaml +++ b/manifests/supervisorcluster/1.32/cns-csi.yaml @@ -46,7 +46,7 @@ rules: resources: ["volumeattachments/status"] verbs: ["patch"] - apiGroups: ["cns.vmware.com"] - resources: ["cnsvolumemetadatas", "cnsfileaccessconfigs"] + resources: ["cnsvolumemetadatas", "cnsfileaccessconfigs", "cnsfileaccessconfigs/status"] verbs: ["get", "list", "watch", "update"] - apiGroups: ["cns.vmware.com"] resources: ["cnsnodevmattachments", "cnsnodevmbatchattachments", "cnsnodevmbatchattachments/status", "cnsnodevmattachments/status"] diff --git a/pkg/apis/cnsoperator/config/cnsfileaccessconfig_crd.yaml b/pkg/apis/cnsoperator/config/cnsfileaccessconfig_crd.yaml index 9529a7202f..193036c765 100644 --- a/pkg/apis/cnsoperator/config/cnsfileaccessconfig_crd.yaml +++ b/pkg/apis/cnsoperator/config/cnsfileaccessconfig_crd.yaml @@ -74,6 +74,8 @@ spec: type: object served: true storage: true + subresources: + status: {} status: acceptedNames: kind: "" diff --git a/pkg/kubernetes/kubernetes.go b/pkg/kubernetes/kubernetes.go index f710598ec7..c07f9ac13a 100644 --- a/pkg/kubernetes/kubernetes.go +++ b/pkg/kubernetes/kubernetes.go @@ -51,6 +51,7 @@ import ( ccV1beta1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" apiutils "sigs.k8s.io/controller-runtime/pkg/client/apiutil" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" cr_log "sigs.k8s.io/controller-runtime/pkg/log" "github.com/go-logr/zapr" @@ -798,14 +799,48 @@ func DeletePersistentVolume(ctx context.Context, k8sClient clientset.Interface, // If the object is a Custom Resource, make sure that the `subresources` field in the // CustomResourceDefinition includes `status` to enable status subresource updates. func UpdateStatus(ctx context.Context, c client.Client, obj client.Object) error { - log := logger.GetLogger(ctx) + log := logger.GetLogger(ctx). + With("kind", obj.GetObjectKind().GroupVersionKind().Kind). + With("name", obj.GetNamespace()+"/"+obj.GetName()) if err := c.Status().Update(ctx, obj); err != nil { - log.Errorf("Failed to update status for %s %s/%s: %v", obj.GetObjectKind().GroupVersionKind().Kind, - obj.GetNamespace(), obj.GetName(), err) + log.Errorf("Failed to update status. err: %v", err) return err } - log.Infof("Successfully updated status for %s %s/%s", obj.GetObjectKind().GroupVersionKind().Kind, - obj.GetNamespace(), obj.GetName()) + log.Debug("Successfully updated status.") return nil } + +// AddFinalizer adds the specified finalizer to the given Kubernetes object if it is not already present. +// It updates the object in the Kubernetes cluster to persist the change. +func AddFinalizer(ctx context.Context, c client.Client, obj client.Object, finalizer string) error { + log := logger.GetLogger(ctx). + With("finalizer", finalizer). + With("kind", obj.GetObjectKind().GroupVersionKind().Kind). + With("name", obj.GetNamespace()+"/"+obj.GetName()) + + if !controllerutil.AddFinalizer(obj, finalizer) { + log.Debug("Finalizer already present. No update needed.") + return nil + } + + log.Info("Adding finalizer to object.") + return c.Update(ctx, obj) +} + +// RemoveFinalizer removes the specified finalizer from the given Kubernetes object if it is present. +// It updates the object in the Kubernetes cluster to persist the change. +func RemoveFinalizer(ctx context.Context, c client.Client, obj client.Object, finalizer string) error { + log := logger.GetLogger(ctx). + With("finalizer", finalizer). + With("kind", obj.GetObjectKind().GroupVersionKind().Kind). + With("name", obj.GetNamespace()+"/"+obj.GetName()) + + if !controllerutil.RemoveFinalizer(obj, finalizer) { + log.Debug("Finalizer not present. No update needed.") + return nil + } + + log.Info("Removing finalizer from object.") + return c.Update(ctx, obj) +} diff --git a/pkg/syncer/cnsoperator/controller/cnsfileaccessconfig/cnsfileaccessconfig_controller.go b/pkg/syncer/cnsoperator/controller/cnsfileaccessconfig/cnsfileaccessconfig_controller.go index e7fdf39ebf..ddffd14413 100644 --- a/pkg/syncer/cnsoperator/controller/cnsfileaccessconfig/cnsfileaccessconfig_controller.go +++ b/pkg/syncer/cnsoperator/controller/cnsfileaccessconfig/cnsfileaccessconfig_controller.go @@ -36,16 +36,16 @@ import ( "k8s.io/client-go/kubernetes/scheme" typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/config" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" - "sigs.k8s.io/controller-runtime/pkg/source" cnsoperatorapis "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator" - cnsfileaccessconfigv1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/cnsfileaccessconfig/v1alpha1" + v1a1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/cnsfileaccessconfig/v1alpha1" volumes "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/cns-lib/volume" commonconfig "sigs.k8s.io/vsphere-csi-driver/v3/pkg/common/config" "sigs.k8s.io/vsphere-csi-driver/v3/pkg/csi/service/common" @@ -179,25 +179,17 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error { maxWorkerThreads := util.GetMaxWorkerThreads(ctx, workerThreadsEnvVar, defaultMaxWorkerThreads) // Create a new controller. - c, err := controller.New("cnsfileaccessconfig-controller", mgr, - controller.Options{Reconciler: r, MaxConcurrentReconciles: maxWorkerThreads}) + err := ctrl.NewControllerManagedBy(mgr).Named("cnsfileaccessconfig-controller"). + For(&v1a1.CnsFileAccessConfig{}). + WithEventFilter(predicate.GenerationChangedPredicate{}). + WithOptions(controller.Options{MaxConcurrentReconciles: maxWorkerThreads}). + Complete(r) if err != nil { - log.Errorf("Failed to create new CnsFileAccessConfig controller with error: %+v", err) + log.Errorf("Failed to build application controller. Err: %v", err) return err } backOffDuration = make(map[types.NamespacedName]time.Duration) - - // Watch for changes to primary resource CnsFileAccessConfig. - err = c.Watch(source.Kind( - mgr.GetCache(), - &cnsfileaccessconfigv1alpha1.CnsFileAccessConfig{}, - &handler.TypedEnqueueRequestForObject[*cnsfileaccessconfigv1alpha1.CnsFileAccessConfig]{}, - )) - if err != nil { - log.Errorf("Failed to watch for changes to CnsFileAccessConfig resource with error: %+v", err) - return err - } return nil } @@ -228,7 +220,7 @@ func (r *ReconcileCnsFileAccessConfig) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { log := logger.GetLogger(ctx) // Fetch the CnsFileAccessConfig instance. - instance := &cnsfileaccessconfigv1alpha1.CnsFileAccessConfig{} + instance := &v1a1.CnsFileAccessConfig{} err := r.client.Get(ctx, request.NamespacedName, instance) if err != nil { if apierrors.IsNotFound(err) { @@ -297,11 +289,10 @@ func (r *ReconcileCnsFileAccessConfig) Reconcile(ctx context.Context, } // Remove finalizer from CnsFileAccessConfig CRD - removeFinalizerFromCRDInstance(ctx, instance) - err = updateCnsFileAccessConfig(ctx, r.client, instance) + err = k8s.RemoveFinalizer(ctx, r.client, instance, cnsoperatortypes.CNSFinalizer) if err != nil { - msg := fmt.Sprintf("failed to update CnsFileAccessConfig instance: %q on namespace: %q. Error: %+v", - instance.Name, instance.Namespace, err) + msg := fmt.Sprintf("failed to remove finalizer from CnsFileAccessConfig "+ + "instance: %q on namespace: %q. Error: %+v", instance.Name, instance.Namespace, err) recordEvent(ctx, r, instance, v1.EventTypeWarning, msg) return reconcile.Result{RequeueAfter: timeout}, nil } @@ -370,8 +361,7 @@ func (r *ReconcileCnsFileAccessConfig) Reconcile(ctx context.Context, return reconcile.Result{RequeueAfter: timeout}, nil } } - removeFinalizerFromCRDInstance(ctx, instance) - err = updateCnsFileAccessConfig(ctx, r.client, instance) + err = k8s.RemoveFinalizer(ctx, r.client, instance, cnsoperatortypes.CNSFinalizer) if err != nil { msg := fmt.Sprintf("failed to update CnsFileAccessConfig instance: %q on namespace: %q. Error: %+v", instance.Name, instance.Namespace, err) @@ -396,24 +386,14 @@ func (r *ReconcileCnsFileAccessConfig) Reconcile(ctx context.Context, backOffDurationMapMutex.Unlock() return reconcile.Result{}, nil } - cnsFinalizerExists := false - // Check if finalizer already exists. - for _, finalizer := range instance.Finalizers { - if finalizer == cnsoperatortypes.CNSFinalizer { - cnsFinalizerExists = true - break - } - } - if !cnsFinalizerExists { - // Add finalizer. - instance.Finalizers = append(instance.Finalizers, cnsoperatortypes.CNSFinalizer) - err = updateCnsFileAccessConfig(ctx, r.client, instance) - if err != nil { - msg := fmt.Sprintf("failed to update CnsFileAccessConfig instance: %q on namespace: %q. Error: %+v", - instance.Name, instance.Namespace, err) - recordEvent(ctx, r, instance, v1.EventTypeWarning, msg) - return reconcile.Result{RequeueAfter: timeout}, nil - } + + // Add finalizer to CnsFileAccessConfig instance if it does not already exist. + err = k8s.AddFinalizer(ctx, r.client, instance, cnsoperatortypes.CNSFinalizer) + if err != nil { + msg := fmt.Sprintf("failed to add finalizer on CnsFileAccessConfig instance: %q on namespace: %q. Error: %+v", + instance.Name, instance.Namespace, err) + recordEvent(ctx, r, instance, v1.EventTypeWarning, msg) + return reconcile.Result{RequeueAfter: timeout}, nil } vmOwnerRefExists := false @@ -528,7 +508,7 @@ func (r *ReconcileCnsFileAccessConfig) Reconcile(ctx context.Context, // addPvcFinalizer checks if CnsPvcFinalizer exists on PVC. // If it does not exist, it updates the PVC with it. func addPvcFinalizer(ctx context.Context, - instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig, client client.Client) error { + instance *v1a1.CnsFileAccessConfig, client client.Client) error { log := logger.GetLogger(ctx) if !commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.FileVolumesWithVmService) { @@ -566,7 +546,7 @@ func addPvcFinalizer(ctx context.Context, // isPvcInUse returns true if there is at least 1 VM which is using the given PVC. func isPvcInUse(ctx context.Context, pvcName string, - instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig) (bool, error) { + instance *v1a1.CnsFileAccessConfig) (bool, error) { log := logger.GetLogger(ctx) cnsFileVolumeClientInstance, err := cnsfilevolumeclient.GetFileVolumeClientInstance(ctx) @@ -581,7 +561,7 @@ func isPvcInUse(ctx context.Context, pvcName string, // removeFinalizerFromPVC will remove the CNS Finalizer, cns.vmware.com/pvc-protection, // from a given PersistentVolumeClaim. func removeFinalizerFromPVC(ctx context.Context, client client.Client, - instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig) error { + instance *v1a1.CnsFileAccessConfig) error { log := logger.GetLogger(ctx) if !commonco.ContainerOrchestratorUtility.IsFSSEnabled(ctx, common.FileVolumesWithVmService) { @@ -640,7 +620,7 @@ func removeFinalizerFromPVC(ctx context.Context, client client.Client, // This method is used when we don't have VM instance. It fetches the VM IP from CNSFileVolumeClient // instance for the VM name associated with CnsFileAccessConfig. func (r *ReconcileCnsFileAccessConfig) removePermissionsForFileVolume(ctx context.Context, volumeID string, - instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig, skipConfigureVolumeACL bool) error { + instance *v1a1.CnsFileAccessConfig, skipConfigureVolumeACL bool) error { log := logger.GetLogger(ctx) volumePermissionLock, _ := volumePermissionLockMap.LoadOrStore(volumeID, &sync.Mutex{}) instanceLock, _ := volumePermissionLock.(*sync.Mutex) @@ -685,7 +665,7 @@ func (r *ReconcileCnsFileAccessConfig) removePermissionsForFileVolume(ctx contex // permissions by setting the parameter removePermission to true or false // respectively. Returns error if any operation fails. func (r *ReconcileCnsFileAccessConfig) configureNetPermissionsForFileVolume(ctx context.Context, - volumeID string, vm *vmoperatortypes.VirtualMachine, instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig, + volumeID string, vm *vmoperatortypes.VirtualMachine, instance *v1a1.CnsFileAccessConfig, removePermission bool) error { log := logger.GetLogger(ctx) volumePermissionLock, _ := volumePermissionLockMap.LoadOrStore(volumeID, &sync.Mutex{}) @@ -865,10 +845,10 @@ func validateVmAndPvc(ctx context.Context, instanceLabels map[string]string, ins // setInstanceSuccess sets instance to success and records an event on the // CnsFileAccessConfig instance. func setInstanceSuccess(ctx context.Context, r *ReconcileCnsFileAccessConfig, - instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig, msg string) error { + instance *v1a1.CnsFileAccessConfig, msg string) error { instance.Status.Done = true instance.Status.Error = "" - err := updateCnsFileAccessConfig(ctx, r.client, instance) + err := k8s.UpdateStatus(ctx, r.client, instance) if err != nil { return err } @@ -879,10 +859,10 @@ func setInstanceSuccess(ctx context.Context, r *ReconcileCnsFileAccessConfig, // setInstanceError sets error and records an event on the CnsFileAccessConfig // instance. func setInstanceError(ctx context.Context, r *ReconcileCnsFileAccessConfig, - instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig, errMsg string) { + instance *v1a1.CnsFileAccessConfig, errMsg string) { log := logger.GetLogger(ctx) instance.Status.Error = errMsg - err := updateCnsFileAccessConfig(ctx, r.client, instance) + err := k8s.UpdateStatus(ctx, r.client, instance) if err != nil { log.Errorf("updateCnsFileAccessConfig failed. err: %v", err) } @@ -890,7 +870,7 @@ func setInstanceError(ctx context.Context, r *ReconcileCnsFileAccessConfig, } func updateCnsFileAccessConfig(ctx context.Context, client client.Client, - instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig) error { + instance *v1a1.CnsFileAccessConfig) error { log := logger.GetLogger(ctx) err := client.Update(ctx, instance) if err != nil { @@ -904,7 +884,7 @@ func updateCnsFileAccessConfig(ctx context.Context, client client.Client, // appropriately and logs the message. // backOffDuration is reset to 1 second on success and doubled on failure. func recordEvent(ctx context.Context, r *ReconcileCnsFileAccessConfig, - instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig, eventtype string, msg string) { + instance *v1a1.CnsFileAccessConfig, eventtype string, msg string) { log := logger.GetLogger(ctx) log.Debugf("Event type is %s", eventtype) namespacedName := types.NamespacedName{ @@ -927,17 +907,3 @@ func recordEvent(ctx context.Context, r *ReconcileCnsFileAccessConfig, backOffDurationMapMutex.Unlock() } } - -// removeFinalizerFromCRDInstance will remove the CNS Finalizer = cns.vmware.com, -// from a given CnsFileAccessConfig instance. -func removeFinalizerFromCRDInstance(ctx context.Context, instance *cnsfileaccessconfigv1alpha1.CnsFileAccessConfig) { - log := logger.GetLogger(ctx) - for i, finalizer := range instance.Finalizers { - if finalizer == cnsoperatortypes.CNSFinalizer { - log.Debugf("Removing %q finalizer from CnsFileAccessConfig instance with name: %q on namespace: %q", - cnsoperatortypes.CNSFinalizer, instance.Name, instance.Namespace) - instance.Finalizers = append(instance.Finalizers[:i], instance.Finalizers[i+1:]...) - break - } - } -}