diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 3183d4bb1..d758d822b 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -91,6 +91,7 @@ func main() { flag.StringVar(&cfg.Namespace, "namespace", cfg.Namespace, "Namespace where the CSIAddons pod is deployed") flag.BoolVar(&enableAdmissionWebhooks, "enable-admission-webhooks", false, "[DEPRECATED] Enable the admission webhooks") flag.BoolVar(&showVersion, "version", false, "Print Version details") + flag.StringVar(&cfg.SchedulePrecedence, "schedule-precedence", "", "The order of precedence in which schedule of reclaimspace and keyrotation is considered. Possible values are sc-only") opts := zap.Options{ Development: true, TimeEncoder: zapcore.ISO8601TimeEncoder, @@ -180,9 +181,10 @@ func main() { os.Exit(1) } if err = (&controllers.PersistentVolumeClaimReconciler{ - Client: mgr.GetClient(), - Scheme: mgr.GetScheme(), - ConnPool: connPool, + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + ConnPool: connPool, + SchedulePrecedence: cfg.SchedulePrecedence, }).SetupWithManager(mgr, ctrlOptions); err != nil { setupLog.Error(err, "unable to create controller", "controller", "PersistentVolumeClaim") os.Exit(1) diff --git a/internal/controller/csiaddons/persistentvolumeclaim_controller.go b/internal/controller/csiaddons/persistentvolumeclaim_controller.go index d591ed8e3..59ba2f332 100644 --- a/internal/controller/csiaddons/persistentvolumeclaim_controller.go +++ b/internal/controller/csiaddons/persistentvolumeclaim_controller.go @@ -18,6 +18,7 @@ package controller import ( "context" + "encoding/json" "errors" "fmt" "reflect" @@ -27,6 +28,7 @@ import ( csiaddonsv1alpha1 "github.com/csi-addons/kubernetes-csi-addons/api/csiaddons/v1alpha1" "github.com/csi-addons/kubernetes-csi-addons/internal/connection" + "github.com/csi-addons/kubernetes-csi-addons/internal/util" "github.com/go-logr/logr" "github.com/robfig/cron/v3" @@ -52,7 +54,8 @@ type PersistentVolumeClaimReconciler struct { client.Client Scheme *runtime.Scheme // ConnectionPool consists of map of Connection objects. - ConnPool *connection.ConnectionPool + ConnPool *connection.ConnectionPool + SchedulePrecedence string } // Operation defines the sub operation to be performed @@ -63,10 +66,12 @@ var ( rsCronJobScheduleTimeAnnotation = "reclaimspace." + csiaddonsv1alpha1.GroupVersion.Group + "/schedule" rsCronJobNameAnnotation = "reclaimspace." + csiaddonsv1alpha1.GroupVersion.Group + "/cronjob" rsCSIAddonsDriverAnnotation = "reclaimspace." + csiaddonsv1alpha1.GroupVersion.Group + "/drivers" + rsCronJobExcludeAnnotation = "reclaimspace." + csiaddonsv1alpha1.GroupVersion.Group + "/exclude" krcJobScheduleTimeAnnotation = "keyrotation." + csiaddonsv1alpha1.GroupVersion.Group + "/schedule" krcJobNameAnnotation = "keyrotation." + csiaddonsv1alpha1.GroupVersion.Group + "/cronjob" krCSIAddonsDriverAnnotation = "keyrotation." + csiaddonsv1alpha1.GroupVersion.Group + "/drivers" + krcJobExcludeAnnotation = "keyrotation." + csiaddonsv1alpha1.GroupVersion.Group + "/exclude" ErrConnNotFoundRequeueNeeded = errors.New("connection not found, requeue needed") ErrScheduleNotFound = errors.New("schedule not found") @@ -210,12 +215,7 @@ func (r *PersistentVolumeClaimReconciler) checkDriverSupportCapability( return false, false } -// 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. -// - Check if the schedule is present in the storageclass annotations. If yes, -// use that. +// determineScheduleAndRequeue determines the schedule from annotations. func (r *PersistentVolumeClaimReconciler) determineScheduleAndRequeue( ctx context.Context, logger *logr.Logger, @@ -223,84 +223,39 @@ func (r *PersistentVolumeClaimReconciler) determineScheduleAndRequeue( driverName string, annotationKey string, ) (string, error) { - annotations := pvc.GetAnnotations() - schedule, scheduleFound := getScheduleFromAnnotation(annotationKey, logger, annotations) - if scheduleFound { - return schedule, nil - } + var schedule string + var err error - // check for namespace schedule annotation. - 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(annotationKey, logger, ns.Annotations) + logger.Info("Determining schedule using precedence", "SchedulePrecedence", r.SchedulePrecedence) - // 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 { - // 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. - switch annotationKey { - case krcJobScheduleTimeAnnotation: - requeue, keyRotationSupported := r.checkDriverSupportCapability(logger, ns.Annotations, driverName, keyRotationOp) - if keyRotationSupported { - return schedule, nil - } - if requeue { - return "", ErrConnNotFoundRequeueNeeded - } - case rsCronJobScheduleTimeAnnotation: - requeue, supportReclaimspace := r.checkDriverSupportCapability(logger, ns.Annotations, driverName, relciamSpaceOp) - 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 - } - default: - logger.Info("Unknown annotation key", "AnnotationKey", annotationKey) - return "", fmt.Errorf("unknown annotation key: %s", annotationKey) + if r.SchedulePrecedence == util.ScheduleSCOnly { + // DS flag, read only from the SC + if schedule = r.getScheduleFromSC(ctx, pvc, logger, annotationKey); schedule != "" { + return schedule, nil } - } - - // For static provisioned PVs, StorageClassName is nil or empty. - if pvc.Spec.StorageClassName == nil || len(*pvc.Spec.StorageClassName) == 0 { - logger.Info("StorageClassName is empty") return "", ErrScheduleNotFound } - storageClassName := *pvc.Spec.StorageClassName - // check for storageclass schedule annotation. - sc := &storagev1.StorageClass{} - err = r.Client.Get(ctx, types.NamespacedName{Name: storageClassName}, sc) - if err != nil { - if apierrors.IsNotFound(err) { - logger.Error(err, "StorageClass not found", "StorageClass", storageClassName) - return "", ErrScheduleNotFound - } + // Check on PVC + if schedule = r.getScheduleFromPVC(pvc, logger, annotationKey); schedule != "" { + return schedule, nil + } - logger.Error(err, "Failed to get StorageClass", "StorageClass", storageClassName) + // Check on NS, might get ErrConnNotFoundRequeueNeeded + // If so, return the error + if schedule, err = r.getScheduleFromNS(ctx, pvc, logger, driverName, annotationKey); schedule != "" { + return schedule, nil + } + if errors.Is(err, ErrConnNotFoundRequeueNeeded) { return "", err } - schedule, scheduleFound = getScheduleFromAnnotation(annotationKey, logger, sc.Annotations) - if scheduleFound { + + // Check SC + if schedule = r.getScheduleFromSC(ctx, pvc, logger, annotationKey); schedule != "" { return schedule, nil } + // If nothing matched, we did not find schedule return "", ErrScheduleNotFound } @@ -336,7 +291,7 @@ func (r *PersistentVolumeClaimReconciler) storageClassEventHandler() handler.Eve var requests []reconcile.Request for _, pvc := range pvcList.Items { - if annotationValueMissing(obj.GetAnnotations(), pvc.GetAnnotations(), annotationsToWatch) { + if annotationValueMissingOrDiff(obj.GetAnnotations(), pvc.GetAnnotations(), annotationsToWatch) { requests = append(requests, reconcile.Request{ NamespacedName: types.NamespacedName{ Name: pvc.Name, @@ -576,6 +531,46 @@ func generateCronJobName(parentName string) string { return fmt.Sprintf("%s-%d", parentName, time.Now().Unix()) } +// createPatchBytesForAnnotations creates JSON marshalled patch bytes for annotations. +func createPatchBytesForAnnotations(annotations map[string]string) ([]byte, error) { + patch := map[string]interface{}{ + "metadata": map[string]interface{}{ + "annotations": annotations, + }, + } + + patchBytes, err := json.Marshal(patch) + if err != nil { + return nil, err + } + + return patchBytes, nil +} + +// addAnnotationsToResource adds annotations to the specified resource's metadata. +func (r *PersistentVolumeClaimReconciler) addAnnotationsToResource( + ctx context.Context, + logger *logr.Logger, + annotations map[string]string, + resource client.Object) error { + patch, err := createPatchBytesForAnnotations(annotations) + if err != nil { + logger.Error(err, "Failed to create patch bytes for annotations") + + return err + } + logger.Info("Adding annotation", "Annotation", string(patch)) + + err = r.Client.Patch(ctx, resource, client.RawPatch(types.StrategicMergePatchType, patch)) + if err != nil { + logger.Error(err, "Failed to update annotation") + + return err + } + + return nil +} + // processReclaimSpace reconciles ReclaimSpace based on annotations func (r *PersistentVolumeClaimReconciler) processReclaimSpace( ctx context.Context, @@ -589,6 +584,10 @@ func (r *PersistentVolumeClaimReconciler) processReclaimSpace( } if rsCronJob != nil { *logger = logger.WithValues("ReclaimSpaceCronJobName", rsCronJob.Name) + if _, ok := rsCronJob.GetAnnotations()[rsCronJobExcludeAnnotation]; ok { + logger.Info("ReclaimSpaceCronJob has exclude annotation set, exiting reconcile") + return ctrl.Result{}, nil + } } schedule, err := r.determineScheduleAndRequeue(ctx, logger, pvc, pv.Spec.CSI.Driver, rsCronJobScheduleTimeAnnotation) @@ -643,6 +642,14 @@ func (r *PersistentVolumeClaimReconciler) processReclaimSpace( } logger.Info("Successfully updated reclaimSpaceCronJob") + // Update schedule on the pvc + err = r.addAnnotationsToResource(ctx, logger, map[string]string{ + rsCronJobScheduleTimeAnnotation: schedule, + }, pvc) + if err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil } @@ -651,17 +658,11 @@ func (r *PersistentVolumeClaimReconciler) processReclaimSpace( // 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)) + err = r.addAnnotationsToResource(ctx, logger, map[string]string{ + rsCronJobNameAnnotation: rsCronJobName, + rsCronJobScheduleTimeAnnotation: schedule, + }, pvc) if err != nil { - logger.Error(err, "Failed to update annotation") - return ctrl.Result{}, err } @@ -732,6 +733,10 @@ func (r *PersistentVolumeClaimReconciler) processKeyRotation( } if krcJob != nil { *logger = logger.WithValues("EncryptionKeyrotationCronJobName", krcJob.Name) + if _, ok := krcJob.GetAnnotations()[krcJobExcludeAnnotation]; ok { + logger.Info("EncryptionKeyRotationCronJob has exclude annotation set, exiting reconcile") + return nil + } } // Determine schedule @@ -770,24 +775,25 @@ func (r *PersistentVolumeClaimReconciler) processKeyRotation( logger.Error(err, "failed to update encryptionkeyrotationcronjob") return err // ctr.Result } - logger.Info("successfully updated encryptionkeyrotationcronjob") + + // update the schedule on the pvc + err = r.addAnnotationsToResource(ctx, logger, map[string]string{ + krcJobScheduleTimeAnnotation: sched, + }, pvc) + if err != nil { + return err + } 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)) + err = r.addAnnotationsToResource(ctx, logger, map[string]string{ + krcJobNameAnnotation: krcJobName, + krcJobScheduleTimeAnnotation: sched, + }, pvc) if err != nil { - logger.Error(err, "Failed to set annotation for keyrotation on the pvc") - return err } @@ -812,12 +818,12 @@ func (r *PersistentVolumeClaimReconciler) processKeyRotation( return nil } -// AnnotationValueMissing checks if any of the specified keys are missing -// from the PVC annotations when they are present in the StorageClass annotations. -func annotationValueMissing(scAnnotations, pvcAnnotations map[string]string, keys []string) bool { +// AnnotationValueMissingOrDiff checks if any of the specified keys are missing +// or differ from the PVC annotations when they are present in the StorageClass annotations. +func annotationValueMissingOrDiff(scAnnotations, pvcAnnotations map[string]string, keys []string) bool { for _, key := range keys { - if _, scHasAnnotation := scAnnotations[key]; scHasAnnotation { - if _, pvcHasAnnotation := pvcAnnotations[key]; !pvcHasAnnotation { + if scValue, scHasAnnotation := scAnnotations[key]; scHasAnnotation { + if pvcValue, pvcHasAnnotation := pvcAnnotations[key]; !pvcHasAnnotation || scValue != pvcValue { return true } } @@ -856,3 +862,102 @@ func createAnnotationPredicate(annotations ...string) predicate.Funcs { }, } } + +func (r *PersistentVolumeClaimReconciler) getScheduleFromSC( + ctx context.Context, + pvc *corev1.PersistentVolumeClaim, + logger *logr.Logger, + annotationKey string) string { + // For static provisioned PVs, StorageClassName is empty. + // Read SC schedule only when not statically provisioned. + if pvc.Spec.StorageClassName != nil && len(*pvc.Spec.StorageClassName) != 0 { + storageClassName := *pvc.Spec.StorageClassName + sc := &storagev1.StorageClass{} + err := r.Client.Get(ctx, types.NamespacedName{Name: storageClassName}, sc) + if err != nil { + if apierrors.IsNotFound(err) { + logger.Error(err, "StorageClass not found", "StorageClass", storageClassName) + return "" + } + + logger.Error(err, "Failed to get StorageClass", "StorageClass", storageClassName) + return "" + } + schedule, scheduleFound := getScheduleFromAnnotation(annotationKey, logger, sc.GetAnnotations()) + if scheduleFound { + return schedule + } + } + + return "" +} + +func (r *PersistentVolumeClaimReconciler) getScheduleFromNS( + ctx context.Context, + pvc *corev1.PersistentVolumeClaim, + logger *logr.Logger, + driverName string, + annotationKey string) (string, error) { + // check for namespace schedule annotation. + 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(annotationKey, logger, ns.GetAnnotations()) + + // 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 { + // 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. + switch annotationKey { + case krcJobScheduleTimeAnnotation: + requeue, keyRotationSupported := r.checkDriverSupportCapability(logger, ns.Annotations, driverName, keyRotationOp) + if keyRotationSupported { + return schedule, nil + } + if requeue { + return "", ErrConnNotFoundRequeueNeeded + } + case rsCronJobScheduleTimeAnnotation: + requeue, supportReclaimspace := r.checkDriverSupportCapability(logger, ns.Annotations, driverName, relciamSpaceOp) + 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 + } + default: + logger.Info("Unknown annotation key", "AnnotationKey", annotationKey) + return "", fmt.Errorf("unknown annotation key: %s", annotationKey) + } + } + + return "", ErrScheduleNotFound +} + +func (r *PersistentVolumeClaimReconciler) getScheduleFromPVC( + pvc *corev1.PersistentVolumeClaim, + logger *logr.Logger, + annotationKey string) string { + // Check for PVC annotation. + schedule, scheduleFound := getScheduleFromAnnotation(annotationKey, logger, pvc.GetAnnotations()) + if scheduleFound { + return schedule + } + + return "" +} diff --git a/internal/controller/csiaddons/persistentvolumeclaim_controller_test.go b/internal/controller/csiaddons/persistentvolumeclaim_controller_test.go index 9396447e7..76f5d25c6 100644 --- a/internal/controller/csiaddons/persistentvolumeclaim_controller_test.go +++ b/internal/controller/csiaddons/persistentvolumeclaim_controller_test.go @@ -273,8 +273,9 @@ func TestDetermineScheduleAndRequeue(t *testing.T) { } r := &PersistentVolumeClaimReconciler{ - Client: client, - ConnPool: connection.NewConnectionPool(), + Client: client, + ConnPool: connection.NewConnectionPool(), + SchedulePrecedence: "pvc-first", } // Create the namespace, storage class, and PVC @@ -333,68 +334,6 @@ func TestDetermineScheduleAndRequeue(t *testing.T) { }) } -func TestAnnotationValueMissing(t *testing.T) { - tests := []struct { - name string - scAnnotations map[string]string - pvcAnnotations map[string]string - keys []string - expected bool - }{ - { - name: "No annotations", - scAnnotations: map[string]string{}, - pvcAnnotations: map[string]string{}, - keys: []string{"key1", "key2"}, - expected: false, - }, - { - name: "SC has annotation, PVC doesn't", - scAnnotations: map[string]string{"key1": "value1"}, - pvcAnnotations: map[string]string{}, - keys: []string{"key1"}, - expected: true, - }, - { - name: "Both SC and PVC have annotation", - scAnnotations: map[string]string{"key1": "value1"}, - pvcAnnotations: map[string]string{"key1": "value1"}, - keys: []string{"key1"}, - expected: false, - }, - { - name: "SC has multiple annotations, PVC missing one", - scAnnotations: map[string]string{"key1": "value1", "key2": "value2"}, - pvcAnnotations: map[string]string{"key1": "value1"}, - keys: []string{"key1", "key2"}, - expected: true, - }, - { - name: "SC has annotation not in keys", - scAnnotations: map[string]string{"key3": "value3"}, - pvcAnnotations: map[string]string{}, - keys: []string{"key1", "key2"}, - expected: false, - }, - { - name: "Empty keys slice", - scAnnotations: map[string]string{"key1": "value1"}, - pvcAnnotations: map[string]string{}, - keys: []string{}, - expected: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := annotationValueMissing(tt.scAnnotations, tt.pvcAnnotations, tt.keys) - if result != tt.expected { - t.Errorf("AnnotationValueMissing() = %v, want %v", result, tt.expected) - } - }) - } -} - func TestAnnotationValueChanged(t *testing.T) { tests := []struct { name string @@ -732,3 +671,143 @@ func TestConstructRSCronJob(t *testing.T) { }) } } + +func TestCreatePatchBytes(t *testing.T) { + tests := []struct { + name string + annotations map[string]string + expected string + expectError bool + }{ + { + name: "Empty annotations", + annotations: map[string]string{}, + expected: `{"metadata":{"annotations":{}}}`, + expectError: false, + }, + { + name: "Single annotation", + annotations: map[string]string{ + "key1": "value1", + }, + expected: `{"metadata":{"annotations":{"key1":"value1"}}}`, + expectError: false, + }, + { + name: "Multiple annotations", + annotations: map[string]string{ + "key1": "value1", + "key2": "value2", + "key3": "value3", + }, + expected: `{"metadata":{"annotations":{"key1":"value1","key2":"value2","key3":"value3"}}}`, + expectError: false, + }, + { + name: "Annotations with special characters", + annotations: map[string]string{ + "key-with-dash": "value-with-dash", + "key_with_underscore": "value_with_underscore", + "key.with.dots": "value.with.dots", + }, + expected: `{"metadata":{"annotations":{"key-with-dash":"value-with-dash","key.with.dots":"value.with.dots","key_with_underscore":"value_with_underscore"}}}`, + expectError: false, + }, + { + name: "Annotations with empty values", + annotations: map[string]string{ + "empty1": "", + "empty2": "", + "key3": "value3", + }, + expected: `{"metadata":{"annotations":{"empty1":"","empty2":"","key3":"value3"}}}`, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + patch, err := createPatchBytesForAnnotations(tt.annotations) + + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.JSONEq(t, tt.expected, string(patch)) + } + }) + } +} +func TestAnnotationValueMissingOrDiff(t *testing.T) { + tests := []struct { + name string + scAnnotations map[string]string + pvcAnnotations map[string]string + keys []string + expected bool + }{ + { + name: "SC has annotation, PVC doesn't", + scAnnotations: map[string]string{"key1": "value1"}, + pvcAnnotations: map[string]string{}, + keys: []string{"key1"}, + expected: true, + }, + { + name: "SC and PVC have different values", + scAnnotations: map[string]string{"key1": "value1"}, + pvcAnnotations: map[string]string{"key1": "value2"}, + keys: []string{"key1"}, + expected: true, + }, + { + name: "SC and PVC have same values", + scAnnotations: map[string]string{"key1": "value1"}, + pvcAnnotations: map[string]string{"key1": "value1"}, + keys: []string{"key1"}, + expected: false, + }, + { + name: "SC doesn't have annotation", + scAnnotations: map[string]string{}, + pvcAnnotations: map[string]string{"key1": "value1"}, + keys: []string{"key1"}, + expected: false, + }, + { + name: "Multiple keys, one missing", + scAnnotations: map[string]string{"key1": "value1", "key2": "value2"}, + pvcAnnotations: map[string]string{"key1": "value1"}, + keys: []string{"key1", "key2"}, + expected: true, + }, + { + name: "Multiple keys, all present but one different", + scAnnotations: map[string]string{"key1": "value1", "key2": "value2"}, + pvcAnnotations: map[string]string{"key1": "value1", "key2": "differentValue"}, + keys: []string{"key1", "key2"}, + expected: true, + }, + { + name: "Empty keys slice", + scAnnotations: map[string]string{"key1": "value1"}, + pvcAnnotations: map[string]string{"key1": "value1"}, + keys: []string{}, + expected: false, + }, + { + name: "Key not in either annotation", + scAnnotations: map[string]string{"key1": "value1"}, + pvcAnnotations: map[string]string{"key1": "value1"}, + keys: []string{"key2"}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := annotationValueMissingOrDiff(tt.scAnnotations, tt.pvcAnnotations, tt.keys) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/internal/util/config.go b/internal/util/config.go index b8d2a9017..3e2be43aa 100644 --- a/internal/util/config.go +++ b/internal/util/config.go @@ -33,6 +33,7 @@ type Config struct { Namespace string ReclaimSpaceTimeout time.Duration MaxConcurrentReconciles int + SchedulePrecedence string } const ( @@ -42,6 +43,8 @@ const ( defaultNamespace = "csi-addons-system" defaultMaxConcurrentReconciles = 100 defaultReclaimSpaceTimeout = time.Minute * 3 + SchedulePrecedenceKey = "schedule-precedence" + ScheduleSCOnly = "sc-only" ) // NewConfig returns a new Config object with default values. @@ -50,6 +53,7 @@ func NewConfig() Config { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, + SchedulePrecedence: "", } } @@ -89,6 +93,12 @@ func (cfg *Config) readConfig(dataMap map[string]string) error { } cfg.MaxConcurrentReconciles = maxConcurrentReconciles + case SchedulePrecedenceKey: + if val != ScheduleSCOnly { + return fmt.Errorf("invalid value %q for key %q", val, SchedulePrecedenceKey) + } + cfg.SchedulePrecedence = val + default: return fmt.Errorf("unknown config key %q", key) } diff --git a/internal/util/config_test.go b/internal/util/config_test.go index 7d60325f4..a7dcacfb3 100644 --- a/internal/util/config_test.go +++ b/internal/util/config_test.go @@ -37,6 +37,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, + SchedulePrecedence: "", }, wantErr: false, }, @@ -47,6 +48,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, + SchedulePrecedence: "", }, wantErr: false, }, @@ -59,6 +61,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: time.Minute * 10, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, + SchedulePrecedence: "", }, wantErr: false, }, @@ -71,6 +74,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, + SchedulePrecedence: "", }, wantErr: true, }, @@ -83,6 +87,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: 1, + SchedulePrecedence: "", }, wantErr: false, }, @@ -95,6 +100,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, + SchedulePrecedence: "", }, wantErr: true, }, @@ -108,6 +114,7 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: time.Minute * 10, MaxConcurrentReconciles: 5, + SchedulePrecedence: "", }, wantErr: false, }, @@ -120,6 +127,46 @@ func TestConfigReadConfigFile(t *testing.T) { Namespace: defaultNamespace, ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, MaxConcurrentReconciles: defaultMaxConcurrentReconciles, + SchedulePrecedence: "", + }, + wantErr: true, + }, + { + name: "config file modifies schedule-precedence", + dataMap: map[string]string{ + "schedule-precedence": "sc-only", + }, + newConfig: Config{ + Namespace: defaultNamespace, + ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, + MaxConcurrentReconciles: defaultMaxConcurrentReconciles, + SchedulePrecedence: ScheduleSCOnly, + }, + wantErr: false, + }, + { + name: "config file has invalid schedule-precedence", + dataMap: map[string]string{ + "schedule-precedence": "invalid-precedence", + }, + newConfig: Config{ + Namespace: defaultNamespace, + ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, + MaxConcurrentReconciles: defaultMaxConcurrentReconciles, + SchedulePrecedence: "", + }, + wantErr: true, + }, + { + name: "config file has empty schedule-precedence", + dataMap: map[string]string{ + "schedule-precedence": "", + }, + newConfig: Config{ + Namespace: defaultNamespace, + ReclaimSpaceTimeout: defaultReclaimSpaceTimeout, + MaxConcurrentReconciles: defaultMaxConcurrentReconciles, + SchedulePrecedence: "", }, wantErr: true, },