From ba5294f94c4edfb3f244c8fbbda934ca0b6e5ad7 Mon Sep 17 00:00:00 2001 From: Niraj Yadav Date: Mon, 15 Jul 2024 17:37:21 +0530 Subject: [PATCH] Modify pvc controller to reconcile encryptionkeyrotation Signed-off-by: Niraj Yadav --- .../persistentvolumeclaim_controller.go | 520 +++++++++++++----- .../persistentvolumeclaim_controller_test.go | 8 +- 2 files changed, 399 insertions(+), 129 deletions(-) diff --git a/internal/controller/csiaddons/persistentvolumeclaim_controller.go b/internal/controller/csiaddons/persistentvolumeclaim_controller.go index 00edebfc7..ecf7756df 100644 --- a/internal/controller/csiaddons/persistentvolumeclaim_controller.go +++ b/internal/controller/csiaddons/persistentvolumeclaim_controller.go @@ -56,9 +56,14 @@ type PersistentVolumeClaimReconciler struct { var ( rsCronJobScheduleTimeAnnotation = "reclaimspace." + csiaddonsv1alpha1.GroupVersion.Group + "/schedule" rsCronJobNameAnnotation = "reclaimspace." + csiaddonsv1alpha1.GroupVersion.Group + "/cronjob" - csiAddonsDriverAnnotation = "reclaimspace." + csiaddonsv1alpha1.GroupVersion.Group + "/drivers" - ErrConnNotFoundRequeueNeeded = errors.New("connection not found, requeue needed") - ErrScheduleNotFound = errors.New("schedule not found") + rsCSIAddonsDriverAnnotation = "reclaimspace." + csiaddonsv1alpha1.GroupVersion.Group + "/drivers" + + krcJobScheduleTimeAnnotation = "keyrotation." + csiaddonsv1alpha1.GroupVersion.Group + "/schedule" + krcJobNameAnnotation = "keyrotation." + csiaddonsv1alpha1.GroupVersion.Group + "/cronjob" + krCSIAddonsDriverAnnotation = "keyrotation." + csiaddonsv1alpha1.GroupVersion.Group + "/drivers" + + ErrConnNotFoundRequeueNeeded = errors.New("connection not found, requeue needed") + ErrScheduleNotFound = errors.New("schedule not found") ) const ( @@ -68,6 +73,7 @@ const ( //+kubebuilder:rbac:groups=core,resources=persistentvolumeclaims,verbs=get;list;watch;patch //+kubebuilder:rbac:groups=core,resources=persistentvolumeclaims/finalizers,verbs=update //+kubebuilder:rbac:groups=csiaddons.openshift.io,resources=reclaimspacecronjobs,verbs=get;list;watch;create;delete;update +//+kubebuilder:rbac:groups=csiaddons.openshift.io,resources=encryptionkeyrotationcronjobs,verbs=get;list;watch;create;delete;update //+kubebuilder:rbac:groups=core,resources=namespaces,verbs=get;list;watch //+kubebuilder:rbac:groups=storage.k8s.io,resources=storageclasses,verbs=get;list;watch @@ -114,105 +120,28 @@ func (r *PersistentVolumeClaimReconciler) Reconcile(ctx context.Context, req ctr return ctrl.Result{}, nil } - rsCronJob, err := r.findChildCronJob(ctx, &logger, &req) - if err != nil { - return ctrl.Result{}, err - } - if rsCronJob != nil { - logger = logger.WithValues("ReclaimSpaceCronJobName", rsCronJob.Name) - } - - schedule, err := r.determineScheduleAndRequeue(ctx, &logger, pvc, pv.Spec.CSI.Driver) - if errors.Is(err, ErrConnNotFoundRequeueNeeded) { - return ctrl.Result{Requeue: true}, nil + // reconcile key rotation + keyRotationErr := r.processKeyRotation(ctx, &logger, &req, pvc, pv) + if keyRotationErr != nil { + // Log and let the loop continue so that we could process reclaimspace + logger.Error(keyRotationErr, "reconcile loop failed for keyrotation for pvc", "PVCName", pvc.Name) } - if errors.Is(err, ErrScheduleNotFound) { - // if schedule is not found, - // delete cron job. - if rsCronJob != nil { - err = r.deleteChildCronJob(ctx, &logger, rsCronJob) - if err != nil { - return ctrl.Result{}, err - } - } - // delete name from annotation. - _, nameFound := pvc.Annotations[rsCronJobNameAnnotation] - if nameFound { - // remove name annotation by patching it to null. - patch := []byte(fmt.Sprintf(`{"metadata":{"annotations":{%q: null}}}`, rsCronJobNameAnnotation)) - err = r.Client.Patch(ctx, pvc, client.RawPatch(types.StrategicMergePatchType, patch)) - if err != nil { - logger.Error(err, "Failed to remove annotation") - return ctrl.Result{}, err - } - } - logger.Info("Annotation not set, exiting reconcile") - // no schedule annotation set, just dequeue. - return ctrl.Result{}, nil - } - if err != nil { - return ctrl.Result{}, err - } + // reconcile reclaim space + reclaimSpaceResult, reclaimSpaceErr := r.processReclaimSpace(ctx, &logger, &req, pvc, pv) - logger = logger.WithValues("Schedule", schedule) + // If any one of the above steps failed, we requeue + // Reclaim space takes precedence over key rotation + if reclaimSpaceErr != nil { + logger.Error(reclaimSpaceErr, "failed to reconcile reclaim space for pvc", "PVCName", pvc.Name) - if rsCronJob != nil { - newRSCronJob := constructRSCronJob(rsCronJob.Name, req.Namespace, schedule, pvc.Name) - if reflect.DeepEqual(newRSCronJob.Spec, rsCronJob.Spec) { - logger.Info("No change in reclaimSpaceCronJob.Spec, exiting reconcile") - - return ctrl.Result{}, nil - } - // update rsCronJob spec - rsCronJob.Spec = newRSCronJob.Spec - err = r.Client.Update(ctx, rsCronJob) - if err != nil { - logger.Error(err, "Failed to update reclaimSpaceCronJob") - - return ctrl.Result{}, err - } - logger.Info("Successfully updated reclaimSpaceCronJob") - - return ctrl.Result{}, nil + return reclaimSpaceResult, reclaimSpaceErr } - rsCronJobName := generateCronJobName(req.Name) - logger = logger.WithValues("ReclaimSpaceCronJobName", rsCronJobName) - // add cronjob name and schedule in annotations. - // adding annotation is required for the case when pvc does not have - // have schedule annotation but namespace has. - patch := []byte(fmt.Sprintf(`{"metadata":{"annotations":{%q:%q,%q:%q}}}`, - rsCronJobNameAnnotation, - rsCronJobName, - rsCronJobScheduleTimeAnnotation, - schedule, - )) - logger.Info("Adding annotation", "Annotation", string(patch)) - err = r.Client.Patch(ctx, pvc, client.RawPatch(types.StrategicMergePatchType, patch)) - if err != nil { - logger.Error(err, "Failed to update annotation") - - return ctrl.Result{}, err - } - - rsCronJob = constructRSCronJob(rsCronJobName, req.Namespace, schedule, pvc.Name) - err = ctrl.SetControllerReference(pvc, rsCronJob, r.Scheme) - if err != nil { - logger.Error(err, "Failed to set controllerReference") - - return ctrl.Result{}, err - } - - err = r.Client.Create(ctx, rsCronJob) - if err != nil { - logger.Error(err, "Failed to create reclaimSpaceCronJob") - - return ctrl.Result{}, err - } - logger.Info("Successfully created reclaimSpaceCronJob") - - return ctrl.Result{}, nil + // Otherwise, return the value of key rotation operation + // No need to log here as it is already logged at the + // time of occurrence. + return ctrl.Result{}, keyRotationErr } // checkDriverSupportReclaimsSpace checks if the driver supports space @@ -223,7 +152,7 @@ func (r *PersistentVolumeClaimReconciler) Reconcile(ctx context.Context, req ctr func (r *PersistentVolumeClaimReconciler) checkDriverSupportReclaimsSpace(logger *logr.Logger, annotations map[string]string, driver string) (bool, bool) { reclaimSpaceSupportedByDriver := false - if drivers, ok := annotations[csiAddonsDriverAnnotation]; ok && slices.Contains(strings.Split(drivers, ","), driver) { + if drivers, ok := annotations[rsCSIAddonsDriverAnnotation]; ok && slices.Contains(strings.Split(drivers, ","), driver) { reclaimSpaceSupportedByDriver = true } @@ -241,58 +170,95 @@ func (r *PersistentVolumeClaimReconciler) checkDriverSupportReclaimsSpace(logger return false, true } +// checkDriverSupportsEncryptionKeyRotate checks if the driver supports key +// rotation or not. If the driver does not support key rotation, it +// returns false and if the driver supports key rotation, it returns true. +// If the driver name is not registered in the connection pool, it returns +// false and requeues the request. +func (r *PersistentVolumeClaimReconciler) checkDriverSupportsEncryptionKeyRotate( + logger *logr.Logger, + annotations map[string]string, + driver string) (bool, bool) { + keyRotationSupportedByDriver := false + + if drivers, ok := annotations[krCSIAddonsDriverAnnotation]; ok && slices.Contains(strings.Split(drivers, ","), driver) { + keyRotationSupportedByDriver = true + } + + ok := r.supportsEncryptionKeyRotation(driver) + if keyRotationSupportedByDriver && !ok { + logger.Info("Driver supports key rotation but driver is not registered in the connection pool, Reqeueing request", "DriverName", driver) + return true, false + } + + if !ok { + logger.Info("Driver does not support encryptionkeyrotation, skip Requeue", "DriverName", driver) + return false, false + } + + return false, true +} + // determineScheduleAndRequeue determines the schedule using the following steps // - Check if the schedule is present in the PVC annotations. If yes, use that. // - Check if the schedule is present in the namespace annotations. If yes, // use that. -// - If schedule is not present in namespace annotations, return ErrorScheduleNotFound. -// - If schedule is present in namespace annotations, check for reclaimSpace -// support by the driver. -// - If driver supports reclaimSpace, use the schedule from namespace. -// - If driver does not support reclaimSpace, return ErrScheduleNotFound. -// Depending on requeue value, it will throw ErrorConnNotFoundRequeueNeeded. +// - Check if the schedule is present in the storageclass annotations. If yes, +// use that. func (r *PersistentVolumeClaimReconciler) determineScheduleAndRequeue( ctx context.Context, logger *logr.Logger, pvc *corev1.PersistentVolumeClaim, driverName string, + annotationKey string, ) (string, error) { annotations := pvc.GetAnnotations() - schedule, scheduleFound := getScheduleFromAnnotation(logger, annotations) + schedule, scheduleFound := getScheduleFromAnnotation(annotationKey, logger, annotations) if scheduleFound { return schedule, nil } // check for namespace schedule annotation. - // We cannot have a generic solution for all CSI drivers to get the driver - // name from PV and check if driver supports space reclamation or not and - // requeue the request if the driver is not registered in the connection - // pool. This can put the controller in a requeue loop. Hence we are - // reading the driver name from the namespace annotation and checking if - // the driver is registered in the connection pool and if not we are not - // requeuing the request. ns := &corev1.Namespace{} err := r.Client.Get(ctx, types.NamespacedName{Name: pvc.Namespace}, ns) if err != nil { logger.Error(err, "Failed to get Namespace", "Namespace", pvc.Namespace) return "", err } - schedule, scheduleFound = getScheduleFromAnnotation(logger, ns.Annotations) + schedule, scheduleFound = getScheduleFromAnnotation(annotationKey, logger, ns.Annotations) // If the schedule is found, check whether driver supports the // space reclamation using annotation on namespace and registered driver // capability for decision on requeue. if scheduleFound { - requeue, supportReclaimspace := r.checkDriverSupportReclaimsSpace(logger, ns.Annotations, driverName) - if supportReclaimspace { - // if driver supports space reclamation, - // return schedule from ns annotation. - return schedule, nil - } - if requeue { - // The request needs to be requeued for checking - // driver support again. - return "", ErrConnNotFoundRequeueNeeded + // We cannot have a generic solution for all CSI drivers to get the driver + // name from PV and check if driver supports reclaimspace/keyrotation or not and + // requeue the request if the driver is not registered in the connection + // pool. This can put the controller in a requeue loop. Hence we are + // reading the driver name from the namespace annotation and checking if + // the driver is registered in the connection pool and if not we are not + // requeuing the request. + // Depending on requeue value, it will return ErrorConnNotFoundRequeueNeeded. + if annotationKey == krcJobScheduleTimeAnnotation { + requeue, keyRotationSupported := r.checkDriverSupportsEncryptionKeyRotate(logger, ns.Annotations, driverName) + if keyRotationSupported { + return schedule, nil + } + if requeue { + return "", ErrConnNotFoundRequeueNeeded + } + } else if annotationKey == rsCronJobScheduleTimeAnnotation { + requeue, supportReclaimspace := r.checkDriverSupportReclaimsSpace(logger, ns.Annotations, driverName) + if supportReclaimspace { + // if driver supports space reclamation, + // return schedule from ns annotation. + return schedule, nil + } + if requeue { + // The request needs to be requeued for checking + // driver support again. + return "", ErrConnNotFoundRequeueNeeded + } } } @@ -314,7 +280,7 @@ func (r *PersistentVolumeClaimReconciler) determineScheduleAndRequeue( logger.Error(err, "Failed to get StorageClass", "StorageClass", *pvc.Spec.StorageClassName) return "", err } - schedule, scheduleFound = getScheduleFromAnnotation(logger, sc.Annotations) + schedule, scheduleFound = getScheduleFromAnnotation(annotationKey, logger, sc.Annotations) if scheduleFound { return schedule, nil } @@ -333,6 +299,29 @@ func (r *PersistentVolumeClaimReconciler) SetupWithManager(mgr ctrl.Manager, ctr return err } + err = mgr.GetFieldIndexer().IndexField( + context.Background(), + &csiaddonsv1alpha1.EncryptionKeyRotationCronJob{}, + jobOwnerKey, + func(rawObj client.Object) []string { + job, ok := rawObj.(*csiaddonsv1alpha1.EncryptionKeyRotationCronJob) + if !ok { + return nil + } + owner := metav1.GetControllerOf(job) + if owner == nil { + return nil + } + if owner.APIVersion != "v1" || owner.Kind != "PersistentVolumeClaim" { + return nil + } + + return []string{owner.Name} + }) + if err != nil { + return err + } + pred := predicate.Funcs{ UpdateFunc: func(e event.UpdateEvent) bool { if e.ObjectNew == nil || e.ObjectOld == nil { @@ -342,13 +331,17 @@ func (r *PersistentVolumeClaimReconciler) SetupWithManager(mgr ctrl.Manager, ctr oldSchdeule, oldOk := e.ObjectOld.GetAnnotations()[rsCronJobScheduleTimeAnnotation] newSchdeule, newOk := e.ObjectNew.GetAnnotations()[rsCronJobScheduleTimeAnnotation] - return oldOk != newOk || oldSchdeule != newSchdeule + krcOldSchdeule, krcOldOk := e.ObjectOld.GetAnnotations()[krcJobScheduleTimeAnnotation] + krcNewSchdeule, krcNewOk := e.ObjectNew.GetAnnotations()[krcJobScheduleTimeAnnotation] + + return (oldOk != newOk || oldSchdeule != newSchdeule) || (krcOldOk != krcNewOk || krcOldSchdeule != krcNewSchdeule) }, } return ctrl.NewControllerManagedBy(mgr). For(&corev1.PersistentVolumeClaim{}). Owns(&csiaddonsv1alpha1.ReclaimSpaceCronJob{}). + Owns(&csiaddonsv1alpha1.EncryptionKeyRotationCronJob{}). WithEventFilter(pred). WithOptions(ctrlOptions). Complete(r) @@ -409,9 +402,10 @@ func (r *PersistentVolumeClaimReconciler) deleteChildCronJob( // A error is logged and default schedule is returned if it // is not in cron format. func getScheduleFromAnnotation( + key string, logger *logr.Logger, annotations map[string]string) (string, bool) { - schedule, ok := annotations[rsCronJobScheduleTimeAnnotation] + schedule, ok := annotations[key] if !ok { return "", false } @@ -491,3 +485,279 @@ func (r PersistentVolumeClaimReconciler) supportsReclaimSpace(driverName string) return false } + +// supportsEncryptionKeyRotation checks if the CSI driver supports EncryptionKeyRotation. +func (r PersistentVolumeClaimReconciler) supportsEncryptionKeyRotation(driverName string) bool { + conns := r.ConnPool.GetByNodeID(driverName, "") + for _, v := range conns { + for _, cap := range v.Capabilities { + if cap.GetEncryptionKeyRotation() != nil { + return true + } + } + } + + return false +} + +// processReclaimSpace reconciles ReclaimSpace based on annotations +func (r *PersistentVolumeClaimReconciler) processReclaimSpace( + ctx context.Context, + logger *logr.Logger, + req *reconcile.Request, + pvc *corev1.PersistentVolumeClaim, + pv *corev1.PersistentVolume) (ctrl.Result, error) { + rsCronJob, err := r.findChildCronJob(ctx, logger, req) + if err != nil { + return ctrl.Result{}, err + } + if rsCronJob != nil { + *logger = logger.WithValues("ReclaimSpaceCronJobName", rsCronJob.Name) + } + + schedule, err := r.determineScheduleAndRequeue(ctx, logger, pvc, pv.Spec.CSI.Driver, rsCronJobScheduleTimeAnnotation) + if errors.Is(err, ErrConnNotFoundRequeueNeeded) { + return ctrl.Result{Requeue: true}, nil + } + if errors.Is(err, ErrScheduleNotFound) { + // if schedule is not found, + // delete cron job. + if rsCronJob != nil { + err = r.deleteChildCronJob(ctx, logger, rsCronJob) + if err != nil { + return ctrl.Result{}, err + } + } + // delete name from annotation. + _, nameFound := pvc.Annotations[rsCronJobNameAnnotation] + if nameFound { + // remove name annotation by patching it to null. + patch := []byte(fmt.Sprintf(`{"metadata":{"annotations":{%q: null}}}`, rsCronJobNameAnnotation)) + err = r.Client.Patch(ctx, pvc, client.RawPatch(types.StrategicMergePatchType, patch)) + if err != nil { + logger.Error(err, "Failed to remove annotation") + + return ctrl.Result{}, err + } + } + logger.Info("Annotation not set, exiting reconcile") + // no schedule annotation set, just dequeue. + return ctrl.Result{}, nil + } + if err != nil { + return ctrl.Result{}, err + } + + *logger = logger.WithValues("Schedule", schedule) + + if rsCronJob != nil { + newRSCronJob := constructRSCronJob(rsCronJob.Name, req.Namespace, schedule, pvc.Name) + if reflect.DeepEqual(newRSCronJob.Spec, rsCronJob.Spec) { + logger.Info("No change in reclaimSpaceCronJob.Spec, exiting reconcile") + + return ctrl.Result{}, nil + } + // update rsCronJob spec + rsCronJob.Spec = newRSCronJob.Spec + err = r.Client.Update(ctx, rsCronJob) + if err != nil { + logger.Error(err, "Failed to update reclaimSpaceCronJob") + + return ctrl.Result{}, err + } + logger.Info("Successfully updated reclaimSpaceCronJob") + + return ctrl.Result{}, nil + } + + rsCronJobName := generateCronJobName(req.Name) + *logger = logger.WithValues("ReclaimSpaceCronJobName", rsCronJobName) + // add cronjob name and schedule in annotations. + // adding annotation is required for the case when pvc does not have + // have schedule annotation but namespace has. + patch := []byte(fmt.Sprintf(`{"metadata":{"annotations":{%q:%q,%q:%q}}}`, + rsCronJobNameAnnotation, + rsCronJobName, + rsCronJobScheduleTimeAnnotation, + schedule, + )) + logger.Info("Adding annotation", "Annotation", string(patch)) + err = r.Client.Patch(ctx, pvc, client.RawPatch(types.StrategicMergePatchType, patch)) + if err != nil { + logger.Error(err, "Failed to update annotation") + + return ctrl.Result{}, err + } + + rsCronJob = constructRSCronJob(rsCronJobName, req.Namespace, schedule, pvc.Name) + err = ctrl.SetControllerReference(pvc, rsCronJob, r.Scheme) + if err != nil { + logger.Error(err, "Failed to set controllerReference") + + return ctrl.Result{}, err + } + + err = r.Client.Create(ctx, rsCronJob) + if err != nil { + logger.Error(err, "Failed to create reclaimSpaceCronJob") + + return ctrl.Result{}, err + } + logger.Info("Successfully created reclaimSpaceCronJob") + + return ctrl.Result{}, nil +} + +// findChildEncryptionKeyRotationCronJob returns the active job from a list of +// EncryptionKeyRotationCronJobs in the request's namespace. +func (r *PersistentVolumeClaimReconciler) findChildEncryptionKeyRotationCronJob( + ctx context.Context, + logger *logr.Logger, + req *reconcile.Request) (*csiaddonsv1alpha1.EncryptionKeyRotationCronJob, error) { + var childJobs csiaddonsv1alpha1.EncryptionKeyRotationCronJobList + var activeJob *csiaddonsv1alpha1.EncryptionKeyRotationCronJob + + err := r.List(ctx, &childJobs, client.InNamespace(req.Namespace), client.MatchingFields{jobOwnerKey: req.Name}) + if err != nil { + logger.Error(err, "failed to list child encryptionkeyrotationcronjobs") + return activeJob, fmt.Errorf("failed to list encryptionkeyrotationcronjobs: %v", err) + } + + for i, job := range childJobs.Items { + // max allowed job is 1 + if i == 0 { + activeJob = &job + continue + } + + // delete the rest if found + if err = r.Delete(ctx, &job); err != nil { + if client.IgnoreNotFound(err) != nil { + logger.Error(err, "failed to delete extraneous child encryptionkeyrotationcronjob", "EncryptionKeyrotationCronJobName", &job.Name) + return nil, fmt.Errorf("failed to delete extraneous child encryptionkeyrotationcronjob: %w", err) + } + } + } + + return activeJob, nil +} + +// constructKRCronJob constructs an EncryptionKeyRotationCronJob object +func constructKRCronJob(name, namespace, schedule, pvcName string) *csiaddonsv1alpha1.EncryptionKeyRotationCronJob { + failedJobHistoryLimit := defaultFailedJobsHistoryLimit + successfulJobsHistoryLimit := defaultSuccessfulJobsHistoryLimit + + return &csiaddonsv1alpha1.EncryptionKeyRotationCronJob{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: csiaddonsv1alpha1.EncryptionKeyRotationCronJobSpec{ + Schedule: schedule, + JobSpec: csiaddonsv1alpha1.EncryptionKeyRotationJobTemplateSpec{ + Spec: csiaddonsv1alpha1.EncryptionKeyRotationJobSpec{ + Target: csiaddonsv1alpha1.TargetSpec{ + PersistentVolumeClaim: pvcName, + }, + BackoffLimit: defaultBackoffLimit, + RetryDeadlineSeconds: defaultRetryDeadlineSeconds, + }, + }, + FailedJobsHistoryLimit: &failedJobHistoryLimit, + SuccessfulJobsHistoryLimit: &successfulJobsHistoryLimit, + }, + } +} + +// processKeyRotation reconciles EncryptionKeyRotation based on annotations +func (r *PersistentVolumeClaimReconciler) processKeyRotation( + ctx context.Context, + logger *logr.Logger, + req *reconcile.Request, + pvc *corev1.PersistentVolumeClaim, + pv *corev1.PersistentVolume) error { + krcJob, err := r.findChildEncryptionKeyRotationCronJob(ctx, logger, req) + if err != nil { + return err + } + if krcJob != nil { + *logger = logger.WithValues("EncryptionKeyrotationCronJobName", krcJob.Name) + } + + // Determine schedule + sched, err := r.determineScheduleAndRequeue(ctx, logger, pvc, pv.Spec.CSI.Driver, krcJobScheduleTimeAnnotation) + if errors.Is(err, ErrScheduleNotFound) { + // No schedule, delete the job + if krcJob != nil { + err = r.Delete(ctx, krcJob) + if client.IgnoreNotFound(err) != nil { + logger.Error(err, "failed to delete child encryptionkeyrotationcronjob") + + return fmt.Errorf("failed to delete child encryptionkeyrotationcronjob %q:%w", krcJob.Name, err) + } + } + + logger.Info("annotation not found for key rotation, exiting reconcile") + return nil + } + if err != nil { + return err + } + + *logger = logger.WithValues("KeyRotationSchedule", sched) + + if krcJob != nil { + newKrcJob := constructKRCronJob(krcJob.Name, req.Namespace, sched, pvc.Name) + if reflect.DeepEqual(newKrcJob.Spec, krcJob.Spec) { + logger.Info("no change in encryptionkeyrotationjob spec, exiting reconcile") + return nil + } + + // Update the spec + krcJob.Spec = newKrcJob.Spec + err = r.Client.Update(ctx, krcJob) + if err != nil { + logger.Error(err, "failed to update encryptionkeyrotationcronjob") + return err //ctr.Result + } + + logger.Info("successfully updated encryptionkeyrotationcronjob") + return nil + } + + // Add the annotation to the pvc, this will help us optimize reconciles + krcJobName := generateCronJobName(req.Name) + patch := []byte(fmt.Sprintf(`{"metadata":{"annotations":{%q:%q,%q:%q}}}`, + krcJobNameAnnotation, + krcJobName, + krcJobScheduleTimeAnnotation, + sched, + )) + logger.Info("Adding keyrotation annotation to the pvc", "annotation", string(patch)) + err = r.Client.Patch(ctx, pvc, client.RawPatch(types.StrategicMergePatchType, patch)) + if err != nil { + logger.Error(err, "Failed to set annotation for keyrotation on the pvc") + + return err + } + + // Construct a new cron job + krcJob = constructKRCronJob(krcJobName, req.Namespace, sched, pvc.Name) + + // Set owner ref + err = ctrl.SetControllerReference(pvc, krcJob, r.Scheme) + if err != nil { + logger.Error(err, "failed to set controller ref for encryptionkeyrotationcronjob") + return err + } + + // Update the cluster with the new resource + err = r.Client.Create(ctx, krcJob) + if err != nil { + logger.Error(err, "failed to create new encryptionkeyrotationcronjob") + return err + } + + logger.Info("successfully created new encryptionkeyrotationcronjob") + return nil +} diff --git a/internal/controller/csiaddons/persistentvolumeclaim_controller_test.go b/internal/controller/csiaddons/persistentvolumeclaim_controller_test.go index 1aa25a487..3290b6351 100644 --- a/internal/controller/csiaddons/persistentvolumeclaim_controller_test.go +++ b/internal/controller/csiaddons/persistentvolumeclaim_controller_test.go @@ -246,7 +246,7 @@ func TestGetScheduleFromAnnotation(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, got1 := getScheduleFromAnnotation(tt.args.logger, tt.args.annotations) + got, got1 := getScheduleFromAnnotation(rsCronJobScheduleTimeAnnotation, tt.args.logger, tt.args.annotations) assert.Equal(t, tt.want, got) assert.Equal(t, tt.want1, got1) }) @@ -339,7 +339,7 @@ func TestDetermineScheduleAndRequeue(t *testing.T) { err = r.Client.Update(ctx, pvc) assert.NoError(t, err) - schedule, error := r.determineScheduleAndRequeue(ctx, &logger, pvc, driverName) + schedule, error := r.determineScheduleAndRequeue(ctx, &logger, pvc, driverName, rsCronJobScheduleTimeAnnotation) assert.NoError(t, error) assert.Equal(t, tt.want, schedule) }) @@ -349,7 +349,7 @@ func TestDetermineScheduleAndRequeue(t *testing.T) { emptyScName := "" pvc.Spec.StorageClassName = &emptyScName pvc.Annotations = nil - schedule, error := r.determineScheduleAndRequeue(ctx, &logger, pvc, driverName) + schedule, error := r.determineScheduleAndRequeue(ctx, &logger, pvc, driverName, rsCronJobScheduleTimeAnnotation) assert.ErrorIs(t, error, ErrScheduleNotFound) assert.Equal(t, "", schedule) }) @@ -359,7 +359,7 @@ func TestDetermineScheduleAndRequeue(t *testing.T) { sc.Name = "non-existent-sc" pvc.Spec.StorageClassName = &sc.Name pvc.Annotations = nil - schedule, error := r.determineScheduleAndRequeue(ctx, &logger, pvc, driverName) + schedule, error := r.determineScheduleAndRequeue(ctx, &logger, pvc, driverName, rsCronJobScheduleTimeAnnotation) assert.ErrorIs(t, error, ErrScheduleNotFound) assert.Equal(t, "", schedule) })